From fb368693a26f744668a93db10001845456f473eb Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Sat, 25 Sep 2021 07:46:54 -0700 Subject: [PATCH] Add digest resolution to `cosigned`. This change introduces a mutating webhook to complement our validating webhook. The validating webhook in #799 began rejecting tag reference because tags are mutable and can drift between validation and resolution by the kubelet. This change introduces a mutating webhook that resolves tags to digests as resources are created, so that users aren't necessarily forced to provide digests, but we get the benefits of them nonetheless. Fixes: https://github.com/sigstore/cosign/issues/784 Signed-off-by: Matt Moore --- cmd/cosign/webhook/main.go | 48 ++++-- config/200-clusterrole.yaml | 6 +- config/500-webhook-configuration.yaml | 21 +++ pkg/cosign/kubernetes/webhook/validator.go | 41 +++++ .../kubernetes/webhook/validator_test.go | 142 ++++++++++++++++++ test/e2e_test_cosigned.sh | 18 ++- 6 files changed, 261 insertions(+), 15 deletions(-) diff --git a/cmd/cosign/webhook/main.go b/cmd/cosign/webhook/main.go index ed2955a716c8..88b0ce6d0bde 100644 --- a/cmd/cosign/webhook/main.go +++ b/cmd/cosign/webhook/main.go @@ -31,6 +31,7 @@ import ( "knative.dev/pkg/webhook" "knative.dev/pkg/webhook/certificates" "knative.dev/pkg/webhook/resourcesemantics" + "knative.dev/pkg/webhook/resourcesemantics/defaulting" "knative.dev/pkg/webhook/resourcesemantics/validation" cwebhook "github.com/sigstore/cosign/pkg/cosign/kubernetes/webhook" @@ -58,9 +59,20 @@ func main() { sharedmain.MainWithContext(ctx, "cosigned", certificates.NewController, NewValidatingAdmissionController, + NewMutatingAdmissionController, ) } +var types = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{ + corev1.SchemeGroupVersion.WithKind("Pod"): &duckv1.Pod{}, + + appsv1.SchemeGroupVersion.WithKind("ReplicaSet"): &duckv1.WithPod{}, + appsv1.SchemeGroupVersion.WithKind("Deployment"): &duckv1.WithPod{}, + appsv1.SchemeGroupVersion.WithKind("StatefulSet"): &duckv1.WithPod{}, + appsv1.SchemeGroupVersion.WithKind("DaemonSet"): &duckv1.WithPod{}, + batchv1.SchemeGroupVersion.WithKind("Job"): &duckv1.WithPod{}, +} + func NewValidatingAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl { validator := cwebhook.NewValidator(ctx, *secretName) @@ -72,15 +84,7 @@ func NewValidatingAdmissionController(ctx context.Context, cmw configmap.Watcher "/validations", // The resources to validate. - map[schema.GroupVersionKind]resourcesemantics.GenericCRD{ - corev1.SchemeGroupVersion.WithKind("Pod"): &duckv1.Pod{}, - - appsv1.SchemeGroupVersion.WithKind("ReplicaSet"): &duckv1.WithPod{}, - appsv1.SchemeGroupVersion.WithKind("Deployment"): &duckv1.WithPod{}, - appsv1.SchemeGroupVersion.WithKind("StatefulSet"): &duckv1.WithPod{}, - appsv1.SchemeGroupVersion.WithKind("DaemonSet"): &duckv1.WithPod{}, - batchv1.SchemeGroupVersion.WithKind("Job"): &duckv1.WithPod{}, - }, + types, // A function that infuses the context passed to Validate/SetDefaults with custom metadata. func(ctx context.Context) context.Context { @@ -97,3 +101,29 @@ func NewValidatingAdmissionController(ctx context.Context, cmw configmap.Watcher nil, ) } + +func NewMutatingAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl { + validator := cwebhook.NewValidator(ctx, *secretName) + + return defaulting.NewAdmissionController(ctx, + // Name of the resource webhook. + webhookName, + + // The path on which to serve the webhook. + "/mutations", + + // The resources to validate. + types, + + // A function that infuses the context passed to Validate/SetDefaults with custom metadata. + func(ctx context.Context) context.Context { + ctx = duckv1.WithPodDefaulter(ctx, validator.ResolvePod) + ctx = duckv1.WithPodSpecDefaulter(ctx, validator.ResolvePodSpecable) + return ctx + }, + + // Whether to disallow unknown fields. + // We pass false because we're using partial schemas. + false, + ) +} diff --git a/config/200-clusterrole.yaml b/config/200-clusterrole.yaml index 794425aa4198..435f64758a04 100644 --- a/config/200-clusterrole.yaml +++ b/config/200-clusterrole.yaml @@ -21,12 +21,12 @@ rules: resources: ["events"] verbs: ["create"] - # Allow the reconciliation of exactly our validating webhook. + # Allow the reconciliation of exactly our validating and mutating webhooks. - apiGroups: ["admissionregistration.k8s.io"] - resources: ["validatingwebhookconfigurations"] + resources: ["validatingwebhookconfigurations", "mutatingwebhookconfigurations"] verbs: ["list", "watch"] - apiGroups: ["admissionregistration.k8s.io"] - resources: ["validatingwebhookconfigurations"] + resources: ["validatingwebhookconfigurations", "mutatingwebhookconfigurations"] verbs: ["get", "update"] resourceNames: ["cosigned.sigstore.dev"] diff --git a/config/500-webhook-configuration.yaml b/config/500-webhook-configuration.yaml index c697d9d1d5ef..8ad9e24ddfdf 100644 --- a/config/500-webhook-configuration.yaml +++ b/config/500-webhook-configuration.yaml @@ -32,6 +32,27 @@ webhooks: failurePolicy: Fail sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: cosigned.sigstore.dev +webhooks: +- name: cosigned.sigstore.dev + namespaceSelector: + # The webhook should only apply to things that opt-in + matchExpressions: + - key: cosigned.sigstore.dev/include + operator: In + values: ["true"] + admissionReviewVersions: [v1] + clientConfig: + service: + name: webhook + namespace: cosign-system + failurePolicy: Fail + sideEffects: None + --- apiVersion: v1 kind: Secret diff --git a/pkg/cosign/kubernetes/webhook/validator.go b/pkg/cosign/kubernetes/webhook/validator.go index bf7d9cd83e02..d63ad10ee55a 100644 --- a/pkg/cosign/kubernetes/webhook/validator.go +++ b/pkg/cosign/kubernetes/webhook/validator.go @@ -20,11 +20,13 @@ import ( "fmt" "github.com/google/go-containerregistry/pkg/name" + "github.com/sigstore/cosign/pkg/oci/remote" corev1 "k8s.io/api/core/v1" listersv1 "k8s.io/client-go/listers/core/v1" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" secretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret" + "knative.dev/pkg/logging" "knative.dev/pkg/system" ) @@ -91,3 +93,42 @@ func (v *Validator) validatePodSpec(ctx context.Context, ps *corev1.PodSpec) (er return errs } + +// ResolvePodSpecable implements duckv1.PodSpecValidator +func (v *Validator) ResolvePodSpecable(ctx context.Context, wp *duckv1.WithPod) { + v.resolvePodSpec(ctx, &wp.Spec.Template.Spec) +} + +// ResolvePod implements duckv1.PodValidator +func (v *Validator) ResolvePod(ctx context.Context, p *duckv1.Pod) { + v.resolvePodSpec(ctx, &p.Spec) +} + +// For testing +var remoteResolveDigest = remote.ResolveDigest + +func (v *Validator) resolvePodSpec(ctx context.Context, ps *corev1.PodSpec) { + resolveContainers := func(cs []corev1.Container) { + for i, c := range cs { + ref, err := name.ParseReference(c.Image) + if err != nil { + logging.FromContext(ctx).Debugf("Unable to parse reference: %v", err) + continue + } + + // If we are in the context of a mutating webhook, then resolve the tag to a digest. + switch { + case apis.IsInCreate(ctx), apis.IsInUpdate(ctx): + digest, err := remoteResolveDigest(ref) + if err != nil { + logging.FromContext(ctx).Debugf("Unable to resolve digest %q: %v", ref.String(), err) + continue + } + cs[i].Image = digest.String() + } + } + } + + resolveContainers(ps.InitContainers) + resolveContainers(ps.Containers) +} diff --git a/pkg/cosign/kubernetes/webhook/validator_test.go b/pkg/cosign/kubernetes/webhook/validator_test.go index f0b40367426d..f41073b166bd 100644 --- a/pkg/cosign/kubernetes/webhook/validator_test.go +++ b/pkg/cosign/kubernetes/webhook/validator_test.go @@ -20,9 +20,11 @@ import ( "errors" "testing" + "github.com/google/go-cmp/cmp" "github.com/google/go-containerregistry/pkg/name" "github.com/sigstore/cosign/pkg/cosign" "github.com/sigstore/cosign/pkg/oci" + "github.com/sigstore/cosign/pkg/oci/remote" "github.com/sigstore/cosign/pkg/oci/static" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -147,3 +149,143 @@ UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw== }) } } + +func TestResolvePodSpec(t *testing.T) { + tag := name.MustParseReference("gcr.io/distroless/static:nonroot") + // Resolved via crane digest on 2021/09/25 + digest := name.MustParseReference("gcr.io/distroless/static:nonroot@sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4") + + ctx, _ := rtesting.SetupFakeContext(t) + si := fakesecret.Get(ctx) + + secretName := "blah" + + si.Informer().GetIndexer().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: secretName, + }, + Data: map[string][]byte{ + // Random public key (cosign generate-key-pair) 2021-09-25 + "cosign.pub": []byte(`-----BEGIN PUBLIC KEY----- +MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEapTW568kniCbL0OXBFIhuhOboeox +UoJou2P8sbDxpLiE/v3yLw1/jyOrCPWYHWFXnyyeGlkgSVefG54tNoK7Uw== +-----END PUBLIC KEY----- +`), + }, + }) + + v := NewValidator(ctx, secretName) + + rrd := remoteResolveDigest + defer func() { + remoteResolveDigest = rrd + }() + resolve := func(ref name.Reference, opts ...remote.Option) (name.Digest, error) { + return digest.(name.Digest), nil + } + + tests := []struct { + name string + ps *corev1.PodSpec + want *corev1.PodSpec + wc func(context.Context) context.Context + rrd func(name.Reference, ...remote.Option) (name.Digest, error) + }{{ + name: "nothing changed (not the right update)", + ps: &corev1.PodSpec{ + InitContainers: []corev1.Container{{ + Name: "setup-stuff", + Image: tag.String(), + }}, + Containers: []corev1.Container{{ + Name: "user-container", + Image: tag.String(), + }}, + }, + want: &corev1.PodSpec{ + InitContainers: []corev1.Container{{ + Name: "setup-stuff", + Image: tag.String(), + }}, + Containers: []corev1.Container{{ + Name: "user-container", + Image: tag.String(), + }}, + }, + rrd: resolve, + }, { + name: "nothing changed (bad reference)", + ps: &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "user-container", + Image: "in@valid", + }}, + }, + want: &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "user-container", + Image: "in@valid", + }}, + }, + wc: apis.WithinCreate, + rrd: resolve, + }, { + name: "nothing changed (unable to resolve)", + ps: &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "user-container", + Image: tag.String(), + }}, + }, + want: &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "user-container", + Image: tag.String(), + }}, + }, + wc: apis.WithinCreate, + rrd: func(r name.Reference, o ...remote.Option) (name.Digest, error) { + return name.Digest{}, errors.New("boom") + }, + }, { + name: "digests resolve (in create)", + ps: &corev1.PodSpec{ + InitContainers: []corev1.Container{{ + Name: "setup-stuff", + Image: tag.String(), + }}, + Containers: []corev1.Container{{ + Name: "user-container", + Image: tag.String(), + }}, + }, + want: &corev1.PodSpec{ + InitContainers: []corev1.Container{{ + Name: "setup-stuff", + Image: digest.String(), + }}, + Containers: []corev1.Container{{ + Name: "user-container", + Image: digest.String(), + }}, + }, + wc: apis.WithinCreate, + rrd: resolve, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + remoteResolveDigest = test.rrd + got := test.ps.DeepCopy() + ctx := context.Background() + if test.wc != nil { + ctx = test.wc(context.Background()) + } + v.resolvePodSpec(ctx, got) + if !cmp.Equal(got, test.want) { + t.Errorf("resolvePodSpec = %s", cmp.Diff(got, test.want)) + } + }) + } +} diff --git a/test/e2e_test_cosigned.sh b/test/e2e_test_cosigned.sh index 44e248efab99..7278aded810d 100755 --- a/test/e2e_test_cosigned.sh +++ b/test/e2e_test_cosigned.sh @@ -18,7 +18,7 @@ set -ex echo '::group:: publish test image' -DIGEST=$(ko publish ./cmd/sample) +DIGEST=$(ko publish -t foo -B ./cmd/sample) cat > pod.yaml < job.yaml <