From 85bb5f32beaae2a5480f40a7c9701e955e7daa81 Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Fri, 16 Dec 2022 00:44:49 -0800 Subject: [PATCH] fix digest and verify logic (#5703) * fix digest and verify logic Signed-off-by: Jim Bugwadia * allow attestations with no attestors Signed-off-by: Jim Bugwadia Signed-off-by: Jim Bugwadia --- pkg/engine/imageVerify.go | 92 ++++++++++++++++++---------------- pkg/engine/imageVerify_test.go | 51 ++++++++++++------- 2 files changed, 82 insertions(+), 61 deletions(-) diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index a8f2bda398..c410092ef9 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -327,10 +327,18 @@ func (iv *imageVerifier) verifyImage( } if len(imageVerify.Attestors) > 0 { - ruleResp, _, _ := iv.verifyAttestors(ctx, imageVerify.Attestors, imageVerify, imageInfo, "") + ruleResp, cosignResp := iv.verifyAttestors(ctx, imageVerify.Attestors, imageVerify, imageInfo, "") if ruleResp.Status != response.RuleStatusPass { return ruleResp, "" } + + if len(imageVerify.Attestations) == 0 { + return ruleResp, cosignResp.Digest + } + + if imageInfo.Digest == "" { + imageInfo.Digest = cosignResp.Digest + } } return iv.verifyAttestations(ctx, imageVerify, imageInfo) @@ -342,9 +350,8 @@ func (iv *imageVerifier) verifyAttestors( imageVerify kyvernov1.ImageVerification, imageInfo apiutils.ImageInfo, predicateType string, -) (*response.RuleResponse, *cosign.Response, []kyvernov1.AttestorSet) { +) (*response.RuleResponse, *cosign.Response) { var cosignResponse *cosign.Response - var newAttestors []kyvernov1.AttestorSet image := imageInfo.String() for i, attestorSet := range attestors { @@ -354,25 +361,27 @@ func (iv *imageVerifier) verifyAttestors( cosignResponse, err = iv.verifyAttestorSet(ctx, attestorSet, imageVerify, imageInfo, path, predicateType) if err != nil { iv.logger.Error(err, "failed to verify image") - msg := fmt.Sprintf("failed to verify image %s: %s", image, err.Error()) - - // handle registry network errors as a rule error (instead of a policy failure) - var netErr *net.OpError - if errors.As(err, &netErr) { - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusError), nil, nil - } - - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail), nil, nil + return iv.handleRegistryErrors(image, err), nil } - newAttestors = append(newAttestors, attestors[i]) } if cosignResponse == nil { - return ruleError(iv.rule, response.ImageVerify, "invalid response", fmt.Errorf("nil")), nil, nil + return ruleError(iv.rule, response.ImageVerify, "invalid response", fmt.Errorf("nil")), nil } msg := fmt.Sprintf("verified image signatures for %s", image) - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusPass), cosignResponse, newAttestors + return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusPass), cosignResponse +} + +// handle registry network errors as a rule error (instead of a policy failure) +func (iv *imageVerifier) handleRegistryErrors(image string, err error) *response.RuleResponse { + msg := fmt.Sprintf("failed to verify image %s: %s", image, err.Error()) + var netErr *net.OpError + if errors.As(err, &netErr) { + return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusError) + } + + return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail) } func (iv *imageVerifier) verifyAttestations( @@ -385,58 +394,55 @@ func (iv *imageVerifier) verifyAttestations( var attestationError error path := fmt.Sprintf(".attestations[%d]", i) - attestors := attestation.Attestors if len(attestation.Attestors) == 0 { - attestors = []kyvernov1.AttestorSet{{}} + // add an empty attestor to allow fetching and checking attestations + attestation.Attestors = []kyvernov1.AttestorSet{{Entries: []kyvernov1.Attestor{{}}}} } - for j, attestor := range attestors { + for j, attestor := range attestation.Attestors { attestorPath := fmt.Sprintf("%s.attestors[%d]", path, j) - requiredCount := getRequiredCount(attestor) verifiedCount := 0 - entries := attestor.Entries - if len(entries) == 0 { - entries = []kyvernov1.Attestor{{}} - } - - for _, a := range entries { + for _, a := range attestor.Entries { entryPath := fmt.Sprintf("%s.entries[%d]", attestorPath, i) - opts, subPath := iv.buildOptionsAndPath(a, imageVerify, image, attestation) + opts, subPath := iv.buildOptionsAndPath(a, imageVerify, image, &imageVerify.Attestations[i]) cosignResp, err := cosign.FetchAttestations(ctx, iv.rclient, *opts) if err != nil { iv.logger.Error(err, "failed to fetch attestations") - msg := fmt.Sprintf("failed to fetch attestations %s: %s", image, err.Error()) - // handle registry network errors as a rule error (instead of a policy failure) - var netErr *net.OpError - if errors.As(err, &netErr) { - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusError), "" - } - - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail), "" + return iv.handleRegistryErrors(image, err), "" + } + + if imageInfo.Digest == "" { + imageInfo.Digest = cosignResp.Digest + image = imageInfo.String() } - verifiedCount++ attestationError = iv.verifyAttestation(cosignResp.Statements, attestation, imageInfo) if attestationError != nil { attestationError = errors.Wrapf(attestationError, entryPath+subPath) return ruleResponse(*iv.rule, response.ImageVerify, attestationError.Error(), response.RuleStatusFail), "" } + verifiedCount++ if verifiedCount >= requiredCount { - msg := fmt.Sprintf("image attestations verification succeeded, verifiedCount: %v, requiredCount: %v", verifiedCount, requiredCount) - iv.logger.V(2).Info(msg) - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusPass), "" + iv.logger.V(2).Info("image attestations verification succeeded, verifiedCount: %v, requiredCount: %v", verifiedCount, requiredCount) + break } } + + if verifiedCount < requiredCount { + msg := fmt.Sprintf("image attestations verification failed, verifiedCount: %v, requiredCount: %v", verifiedCount, requiredCount) + return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail), "" + } } + iv.logger.V(4).Info("attestation checks passed", "path", path, "image", imageInfo.String(), "predicateType", attestation.PredicateType) } msg := fmt.Sprintf("verified image attestations for %s", image) iv.logger.V(2).Info(msg) - return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusPass), "" + return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusPass), imageInfo.Digest } func (iv *imageVerifier) verifyAttestorSet( @@ -468,7 +474,7 @@ func (iv *imageVerifier) verifyAttestorSet( cosignResp, entryError = iv.verifyAttestorSet(ctx, *nestedAttestorSet, imageVerify, imageInfo, attestorPath, predicateType) } } else { - opts, subPath := iv.buildOptionsAndPath(a, imageVerify, image, kyvernov1.Attestation{PredicateType: predicateType}) + opts, subPath := iv.buildOptionsAndPath(a, imageVerify, image, nil) cosignResp, entryError = cosign.VerifySignature(ctx, iv.rclient, *opts) if entryError != nil { entryError = errors.Wrapf(entryError, attestorPath+subPath) @@ -543,7 +549,7 @@ func getRequiredCount(as kyvernov1.AttestorSet) int { return *as.Count } -func (iv *imageVerifier) buildOptionsAndPath(attestor kyvernov1.Attestor, imageVerify kyvernov1.ImageVerification, image string, attestation kyvernov1.Attestation) (*cosign.Options, string) { +func (iv *imageVerifier) buildOptionsAndPath(attestor kyvernov1.Attestor, imageVerify kyvernov1.ImageVerification, image string, attestation *kyvernov1.Attestation) (*cosign.Options, string) { path := "" opts := &cosign.Options{ ImageRef: image, @@ -555,8 +561,8 @@ func (iv *imageVerifier) buildOptionsAndPath(attestor kyvernov1.Attestor, imageV opts.Roots = imageVerify.Roots } - opts.PredicateType = attestation.PredicateType - if attestation.PredicateType != "" { + if attestation != nil { + opts.PredicateType = attestation.PredicateType opts.FetchAttestations = true } diff --git a/pkg/engine/imageVerify_test.go b/pkg/engine/imageVerify_test.go index b1f4823e78..e901b84794 100644 --- a/pkg/engine/imageVerify_test.go +++ b/pkg/engine/imageVerify_test.go @@ -45,6 +45,17 @@ var testPolicyGood = `{ "attestations": [ { "predicateType": "https://example.com/CodeReview/v1", + "attestors": [ + { + "entries": [ + { + "keys": { + "publicKeys": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEHMmDjK65krAyDaGaeyWNzgvIu155JI50B2vezCw8+3CVeE0lJTL5dbL3OP98Za0oAEBJcOxky8Riy/XcmfKZbw==\n-----END PUBLIC KEY-----" + } + } + ] + } + ], "conditions": [ { "all": [ @@ -433,28 +444,32 @@ func Test_ConfigMapMissingFailure(t *testing.T) { func Test_SignatureGoodSigned(t *testing.T) { policyContext := buildContext(t, testSampleSingleKeyPolicy, testSampleResource, "") + policyContext.policy.GetSpec().Rules[0].VerifyImages[0].MutateDigest = true cosign.ClearMock() - err, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) - assert.Equal(t, len(err.PolicyResponse.Rules), 1) - assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message) + engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) + assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1) + assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status, response.RuleStatusPass, engineResp.PolicyResponse.Rules[0].Message) + assert.Equal(t, len(engineResp.PolicyResponse.Rules[0].Patches), 1) + patch := engineResp.PolicyResponse.Rules[0].Patches[0] + assert.Equal(t, string(patch), "{\"op\":\"replace\",\"path\":\"/spec/containers/0/image\",\"value\":\"ghcr.io/kyverno/test-verify-image:signed@sha256:b31bfb4d0213f254d361e0079deaaebefa4f82ba7aa76ef82e90b4935ad5b105\"}") } func Test_SignatureUnsigned(t *testing.T) { cosign.ClearMock() unsigned := strings.Replace(testSampleResource, ":signed", ":unsigned", -1) policyContext := buildContext(t, testSampleSingleKeyPolicy, unsigned, "") - err, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) - assert.Equal(t, len(err.PolicyResponse.Rules), 1) - assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message) + engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) + assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1) + assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status, response.RuleStatusFail, engineResp.PolicyResponse.Rules[0].Message) } func Test_SignatureWrongKey(t *testing.T) { cosign.ClearMock() otherKey := strings.Replace(testSampleResource, ":signed", ":signed-by-someone-else", -1) policyContext := buildContext(t, testSampleSingleKeyPolicy, otherKey, "") - err, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) - assert.Equal(t, len(err.PolicyResponse.Rules), 1) - assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message) + engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) + assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1) + assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status, response.RuleStatusFail, engineResp.PolicyResponse.Rules[0].Message) } func Test_SignaturesMultiKey(t *testing.T) { @@ -463,9 +478,9 @@ func Test_SignaturesMultiKey(t *testing.T) { policy = strings.Replace(policy, "KEY2", testVerifyImageKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) - assert.Equal(t, len(err.PolicyResponse.Rules), 1) - assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message) + engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) + assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1) + assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status, response.RuleStatusPass, engineResp.PolicyResponse.Rules[0].Message) } func Test_SignaturesMultiKeyFail(t *testing.T) { @@ -473,9 +488,9 @@ func Test_SignaturesMultiKeyFail(t *testing.T) { policy := strings.Replace(testSampleMultipleKeyPolicy, "KEY1", testVerifyImageKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) - assert.Equal(t, len(err.PolicyResponse.Rules), 1) - assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message) + engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) + assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1) + assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status, response.RuleStatusFail, engineResp.PolicyResponse.Rules[0].Message) } func Test_SignaturesMultiKeyOneGoodKey(t *testing.T) { @@ -484,9 +499,9 @@ func Test_SignaturesMultiKeyOneGoodKey(t *testing.T) { policy = strings.Replace(policy, "KEY2", testOtherKey, -1) policy = strings.Replace(policy, "COUNT", "1", -1) policyContext := buildContext(t, policy, testSampleResource, "") - err, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) - assert.Equal(t, len(err.PolicyResponse.Rules), 1) - assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message) + engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext) + assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1) + assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status, response.RuleStatusPass, engineResp.PolicyResponse.Rules[0].Message) } func Test_SignaturesMultiKeyZeroGoodKey(t *testing.T) {