fix digest and verify logic (#5703)

* fix digest and verify logic

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* allow attestations with no attestors

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

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())
+			return iv.handleRegistryErrors(image, err), nil
-
-			// 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
 		}
-		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
+			for _, a := range attestor.Entries {
-			if len(entries) == 0 {
-				entries = []kyvernov1.Attestor{{}}
-			}
-
-			for _, a := range 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())
+					return iv.handleRegistryErrors(image, err), ""
-					// handle registry network errors as a rule error (instead of a policy failure)
+				}
-					var netErr *net.OpError
+
-					if errors.As(err, &netErr) {
+				if imageInfo.Digest == "" {
-						return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusError), ""
+					imageInfo.Digest = cosignResp.Digest
-					}
+					image = imageInfo.String()
-
-					return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail), ""
 				}
 
-				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("image attestations verification succeeded, verifiedCount: %v, requiredCount: %v", verifiedCount, requiredCount)
-					iv.logger.V(2).Info(msg)
+					break
-					return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusPass), ""
 				}
 			}
+
+			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 != nil {
-	if attestation.PredicateType != "" {
+		opts.PredicateType = attestation.PredicateType
 		opts.FetchAttestations = true
 	}
 

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)
+	engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext)
-	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1)
-	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message)
+	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)
+	engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext)
-	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1)
-	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message)
+	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)
+	engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext)
-	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1)
-	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message)
+	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)
+	engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext)
-	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1)
-	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message)
+	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)
+	engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext)
-	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1)
-	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusFail, err.PolicyResponse.Rules[0].Message)
+	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)
+	engineResp, _ := VerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), policyContext)
-	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1)
-	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusPass, err.PolicyResponse.Rules[0].Message)
+	assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status, response.RuleStatusPass, engineResp.PolicyResponse.Rules[0].Message)
 }
 
 func Test_SignaturesMultiKeyZeroGoodKey(t *testing.T) {