Remove plugin name and version from ProcessedAttributes response from… (

#183) Currently, the Verifier is expecting VerificationPlugin and VerificationPluginMinVersion extended attributes to be processed by a verification plugin, which just doesn't make sense. This PR will stop sending those two attributes to a plugin. Signed-off-by: rgnote <[email protected]>
notaryproject · Nov 4, 2022 · faeac45 · faeac45
1 parent 75b3248
commit faeac45
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 32 deletions.
diff --git a/verification/types.go b/verification/types.go
@@ -63,6 +63,12 @@ const (
 
 	TrustStorePrefixCA               TrustStorePrefix = "ca"
 	TrustStorePrefixSigningAuthority TrustStorePrefix = "signingAuthority"
+
+	// HeaderVerificationPlugin specifies the name of the verification plugin that should be used to verify the signature.
+	HeaderVerificationPlugin = "io.cncf.notary.verificationPlugin"
+
+	// HeaderVerificationPluginMinVersion specifies the minimum version of the verification plugin that should be used to verify the signature.
+	HeaderVerificationPluginMinVersion = "io.cncf.notary.verificationPluginMinVersion"
 )
 
 var (
@@ -135,6 +141,11 @@ var (
 		TrustStorePrefixCA,
 		TrustStorePrefixSigningAuthority,
 	}
+
+	VerificationPluginHeaders = []string{
+		HeaderVerificationPlugin,
+		HeaderVerificationPluginMinVersion,
+	}
 )
 
 // IsValidTrustStorePrefix returns true if the given string is a valid TrustStorePrefix, otherwise false.
@@ -229,11 +240,3 @@ func getPluginConfig(ctx context.Context) map[string]string {
 	}
 	return config
 }
-
-const (
-	// VerificationPlugin specifies the name of the verification plugin that should be used to verify the signature.
-	VerificationPlugin = "io.cncf.notary.verificationPlugin"
-
-	// VerificationPluginMinVersion specifies the minimum version of the verification plugin that should be used to verify the signature.
-	VerificationPluginMinVersion = "io.cncf.notary.verificationPluginMinVersion"
-)
diff --git a/verification/verifier.go b/verification/verifier.go
@@ -162,7 +162,7 @@ func (v *Verifier) processSignature(ctx context.Context, sigBlob []byte, sigMani
 		if _, err := getVerificationPluginMinVersion(&outcome.EnvelopeContent.SignerInfo); err != nil && err != errExtendedAttributeNotExist {
 			return ErrorVerificationInconclusive{msg: fmt.Sprintf("error while getting plugin minimum version, error: %s", err)}
 		}
-		// TODO verify the plugin's version is equal to or greater than `outcome.SignerInfo.SignedAttributes.VerificationPluginMinVersion`
+		// TODO verify the plugin's version is equal to or greater than `outcome.SignerInfo.SignedAttributes.HeaderVerificationPluginMinVersion`
 		// https://github.com/notaryproject/notation-go/issues/102
 
 		// filter the "verification" capabilities supported by the installed plugin
@@ -242,12 +242,11 @@ func (v *Verifier) processPluginResponse(capabilitiesToVerify []plugin.Verificat
 	if err != nil {
 		return err
 	}
+
 	// verify all extended critical attributes are processed by the plugin
-	for _, attr := range outcome.EnvelopeContent.SignerInfo.SignedAttributes.ExtendedAttributes {
-		if attr.Critical {
-			if !isPresentAny(attr.Key, response.ProcessedAttributes) {
-				return fmt.Errorf("extended critical attribute %q was not processed by the verification plugin %q (all extended critical attributes must be processed by the verification plugin)", attr.Key, verificationPluginName)
-			}
+	for _, attr := range getNonPluginExtendedCriticalAttributes(&outcome.EnvelopeContent.SignerInfo) {
+		if !isPresentAny(attr.Key, response.ProcessedAttributes) {
+			return fmt.Errorf("extended critical attribute %q was not processed by the verification plugin %q (all extended critical attributes must be processed by the verification plugin)", attr.Key, verificationPluginName)
 		}
 	}
 

diff --git a/verification/verifier_helpers.go b/verification/verifier_helpers.go
@@ -256,12 +256,9 @@ func (v *Verifier) executePlugin(ctx context.Context, trustPolicy *TrustPolicy,
 	var attributesToProcess []interface{}
 	extendedAttributes := make(map[interface{}]interface{})
 
-	// pass extended critical attributes to the plugin's verify-signature command
-	for _, attr := range signerInfo.SignedAttributes.ExtendedAttributes {
-		if attr.Critical {
-			extendedAttributes[attr.Key] = attr.Value
-			attributesToProcess = append(attributesToProcess, attr.Key)
-		}
+	for _, attr := range getNonPluginExtendedCriticalAttributes(signerInfo) {
+		extendedAttributes[attr.Key] = attr.Value
+		attributesToProcess = append(attributesToProcess, attr.Key)
 	}
 
 	var certChain [][]byte
@@ -315,6 +312,18 @@ func (v *Verifier) executePlugin(ctx context.Context, trustPolicy *TrustPolicy,
 	return response, nil
 }
 
+func getNonPluginExtendedCriticalAttributes(signerInfo *signature.SignerInfo) []signature.Attribute {
+	var criticalExtendedAttrs []signature.Attribute
+	for _, attr := range signerInfo.SignedAttributes.ExtendedAttributes {
+		attrStrKey, ok := attr.Key.(string)
+		if ok && isPresent(attrStrKey, VerificationPluginHeaders) { // filter the plugin extended attributes
+			continue
+		}
+		criticalExtendedAttrs = append(criticalExtendedAttrs, attr)
+	}
+	return criticalExtendedAttrs
+}
+
 // extractCriticalStringExtendedAttribute extracts a critical string Extended attribute from a signer.
 func extractCriticalStringExtendedAttribute(signerInfo *signature.SignerInfo, key string) (string, error) {
 	attr, err := signerInfo.ExtendedAttribute(key)
@@ -336,29 +345,29 @@ func extractCriticalStringExtendedAttribute(signerInfo *signature.SignerInfo, ke
 
 // getVerificationPlugin get plugin name from the Extended attributes.
 func getVerificationPlugin(signerInfo *signature.SignerInfo) (string, error) {
-	name, err := extractCriticalStringExtendedAttribute(signerInfo, VerificationPlugin)
+	name, err := extractCriticalStringExtendedAttribute(signerInfo, HeaderVerificationPlugin)
 	if err != nil {
 		return "", err
 	}
 	// not an empty string
 	if strings.TrimSpace(name) == "" {
-		return "", fmt.Errorf("%v from extended attribute is an empty string", VerificationPlugin)
+		return "", fmt.Errorf("%v from extended attribute is an empty string", HeaderVerificationPlugin)
 	}
 	return name, nil
 }
 
 // getVerificationPlugin get plugin version from the Extended attributes.
 func getVerificationPluginMinVersion(signerInfo *signature.SignerInfo) (string, error) {
-	version, err := extractCriticalStringExtendedAttribute(signerInfo, VerificationPluginMinVersion)
+	version, err := extractCriticalStringExtendedAttribute(signerInfo, HeaderVerificationPluginMinVersion)
 	if err != nil {
 		return "", err
 	}
 	// empty version
 	if strings.TrimSpace(version) == "" {
-		return "", fmt.Errorf("%v from extended attribute is an empty string", VerificationPluginMinVersion)
+		return "", fmt.Errorf("%v from extended attribute is an empty string", HeaderVerificationPluginMinVersion)
 	}
 	if !semVerRegEx.MatchString(version) {
-		return "", fmt.Errorf("%v from extended attribute is not a valid SemVer", VerificationPluginMinVersion)
+		return "", fmt.Errorf("%v from extended attribute is not a valid SemVer", HeaderVerificationPluginMinVersion)
 	}
 	return version, nil
 }
diff --git a/verification/verifier_test.go b/verification/verifier_test.go
@@ -436,7 +436,7 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {
 				Success: true,
 			},
 		},
-		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key, VerificationPlugin},
+		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key},
 	}
 
 	verifier = Verifier{
@@ -459,7 +459,7 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {
 				Reason:  "i feel like failing today",
 			},
 		},
-		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key, VerificationPlugin},
+		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key},
 	}
 
 	verifier = Verifier{
@@ -481,7 +481,7 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {
 				Success: true,
 			},
 		},
-		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key, VerificationPlugin},
+		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key},
 	}
 
 	verifier = Verifier{
@@ -504,7 +504,7 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {
 				Reason:  "i feel like failing today",
 			},
 		},
-		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key, VerificationPlugin},
+		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key},
 	}
 
 	verifier = Verifier{
@@ -529,7 +529,7 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {
 				Success: true,
 			},
 		},
-		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key, VerificationPlugin},
+		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key},
 	}
 
 	verifier = Verifier{
@@ -582,7 +582,7 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {
 				Success: true,
 			},
 		},
-		ProcessedAttributes: []interface{}{VerificationPlugin}, // exclude the critical attribute
+		ProcessedAttributes: []interface{}{}, // exclude the critical attribute
 	}
 
 	verifier = Verifier{
@@ -600,7 +600,7 @@ func assertPluginVerification(scheme signature.SigningScheme, t *testing.T) {
 	pluginManager.PluginCapabilities = []plugin.Capability{plugin.CapabilityTrustedIdentityVerifier}
 	pluginManager.PluginRunnerExecuteResponse = &plugin.VerifySignatureResponse{
 		VerificationResults: map[plugin.VerificationCapability]*plugin.VerificationResult{},
-		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key, VerificationPlugin},
+		ProcessedAttributes: []interface{}{mock.PluginExtendedCriticalAttribute.Key},
 	}
 
 	verifier = Verifier{