From faeac4574069951e5b5d4e5f8ea4662ddd8527d3 Mon Sep 17 00:00:00 2001 From: Rakesh Gariganti <5878554+rgnote@users.noreply.github.com> Date: Thu, 3 Nov 2022 22:25:23 -0700 Subject: [PATCH] =?UTF-8?q?Remove=20plugin=20name=20and=20version=20from?= =?UTF-8?q?=20ProcessedAttributes=20response=20from=E2=80=A6=20(#183)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 <5878554+rgnote@users.noreply.github.com> --- verification/types.go | 19 +++++++++++-------- verification/verifier.go | 11 +++++------ verification/verifier_helpers.go | 31 ++++++++++++++++++++----------- verification/verifier_test.go | 14 +++++++------- 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/verification/types.go b/verification/types.go index 49bb58bd..6c018fd2 100644 --- 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 index 74d0f922..860b4b98 100644 --- 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 index e1bc8f99..90b8e2fd 100644 --- 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 index 4c5c4ef0..b2b31879 100644 --- 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{