fix: Appends annotations returned by plugin to signature manifest's a…

…nnotations (#262)

Another way to implement #253 as per suggestion
from @patrickzheng200

-------

As per
spec([link](https://github.com/notaryproject/notaryproject/blob/main/specs/plugin-extensibility.md#generate-envelope)) annotations returned by plugin should be appened to signature manifest's annotations and this PR introduces aforementioned behavior

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>

notation.go

@@ -53,6 +53,12 @@ type Signer interface {
 	Sign(ctx context.Context, desc ocispec.Descriptor, opts SignOptions) ([]byte, *signature.SignerInfo, error)
 }
 
+// signerAnnotation facilitates return of manifest annotations by signers
+type signerAnnotation interface {
+	// PluginAnnotations returns signature manifest annotations returned from plugin
+	PluginAnnotations() map[string]string
+}
+
 // Sign signs the artifact in the remote registry and push the signature to the
 // remote.
 // The descriptor of the sign content is returned upon sucessful signing.
@@ -89,8 +95,14 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, opts Sig
 	if err != nil {
 		return ocispec.Descriptor{}, err
 	}
+
+	var pluginAnnotations map[string]string
+	if signerAnts, ok := signer.(signerAnnotation); ok {
+		pluginAnnotations = signerAnts.PluginAnnotations()
+	}
+
 	logger.Debug("Generating annotation")
-	annotations, err := generateAnnotations(signerInfo)
+	annotations, err := generateAnnotations(signerInfo, pluginAnnotations)
 	if err != nil {
 		return ocispec.Descriptor{}, err
 	}
@@ -310,7 +322,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, re
 	return artifactDescriptor, verificationOutcomes, nil
 }
 
-func generateAnnotations(signerInfo *signature.SignerInfo) (map[string]string, error) {
+func generateAnnotations(signerInfo *signature.SignerInfo, annotations map[string]string) (map[string]string, error) {
 	var thumbprints []string
 	for _, cert := range signerInfo.CertificateChain {
 		checkSum := sha256.Sum256(cert.Raw)
@@ -321,7 +333,10 @@ func generateAnnotations(signerInfo *signature.SignerInfo) (map[string]string, e
 		return nil, err
 	}
 
-	return map[string]string{
-		annotationX509ChainThumbprint: string(val),
-	}, nil
-}
+	if annotations == nil {
+		annotations = make(map[string]string)
+	}
+
+	annotations[annotationX509ChainThumbprint] = string(val)
+	return annotations, nil
+}

notation_test.go

@@ -38,6 +38,18 @@ func TestSignSuccess(t *testing.T) {
 	}
 }
 
+func TestSignWithAnnotationsSuccess(t *testing.T) {
+	repo := mock.NewRepository()
+
+	opts := SignOptions{
+		ArtifactReference: mock.SampleArtifactUri,
+	}
+	_, err := Sign(context.Background(), &dummyPluginSigner{}, repo, opts)
+	if err != nil {
+		t.Fatalf("Sign failed with error: %v", err)
+	}
+}
+
 func TestSignWithInvalidExpiry(t *testing.T) {
 	repo := mock.NewRepository()
 	testCases := []struct {
@@ -223,6 +235,16 @@ func (s *dummySigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts Si
 	return []byte("ABC"), &signature.SignerInfo{}, nil
 }
 
+type dummyPluginSigner struct{}
+
+func (s *dummyPluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts SignOptions) ([]byte, *signature.SignerInfo, error) {
+	return []byte("ABC"), &signature.SignerInfo{}, nil
+}
+
+func (s *dummyPluginSigner) PluginAnnotations() map[string]string {
+	return map[string]string{"hi": "hola"}
+}
+
 type dummyVerifier struct {
 	TrustPolicyDoc    *trustpolicy.Document
 	PluginManager     plugin.Manager

signer/plugin.go

@@ -21,12 +21,13 @@ import (
 // pluginSigner signs artifacts and generates signatures.
 // It implements notation.Signer
 type pluginSigner struct {
-	plugin       plugin.SignPlugin
-	keyID        string
-	pluginConfig map[string]string
+	plugin              plugin.SignPlugin
+	keyID               string
+	pluginConfig        map[string]string
+	manifestAnnotations map[string]string
 }
 
-// NewSignerPlugin creates a notation.Signer that signs artifacts and generates
+// NewFromPlugin creates a notation.Signer that signs artifacts and generates
 // signatures by delegating the one or more operations to the named plugin,
 // as defined in https://github.com/notaryproject/notaryproject/blob/main/specs/plugin-extensibility.md#signing-interfaces.
 func NewFromPlugin(plugin plugin.Plugin, keyID string, pluginConfig map[string]string) (notation.Signer, error) {
@@ -44,6 +45,11 @@ func NewFromPlugin(plugin plugin.Plugin, keyID string, pluginConfig map[string]s
 	}, nil
 }
 
+// PluginAnnotations returns signature manifest annotations returned from plugin
+func (s *pluginSigner) PluginAnnotations() map[string]string {
+	return s.manifestAnnotations
+}
+
 // Sign signs the artifact described by its descriptor and returns the
 // marshalled envelope.
 func (s *pluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignOptions) ([]byte, *signature.SignerInfo, error) {
@@ -157,6 +163,7 @@ func (s *pluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocisp
 		return nil, nil, fmt.Errorf("during signing, following unknown attributes were added to subject descriptor: %+q", unknownAttributes)
 	}
 
+	s.manifestAnnotations = resp.Annotations
 	return resp.SignatureEnvelope, &envContent.SignerInfo, nil
 }
 

signer/plugin_test.go

@@ -51,6 +51,7 @@ type mockPlugin struct {
 	invalidSig        bool
 	invalidCertChain  bool
 	invalidDescriptor bool
+	annotations       map[string]string
 	key               crypto.PrivateKey
 	certs             []*x509.Certificate
 	keySpec           signature.KeySpec
@@ -185,6 +186,7 @@ func (p *mockPlugin) GenerateEnvelope(ctx context.Context, req *proto.GenerateEn
 		return &proto.GenerateEnvelopeResponse{
 			SignatureEnvelope:     data,
 			SignatureEnvelopeType: req.SignatureEnvelopeType,
+			Annotations: p.annotations,
 		}, nil
 	}
 	return &proto.GenerateEnvelopeResponse{}, nil
@@ -320,6 +322,31 @@ func TestPluginSigner_SignEnvelope_Valid(t *testing.T) {
 	}
 }
 
+func TestPluginSigner_SignWithAnnotations_Valid(t *testing.T) {
+	for _, envelopeType := range signature.RegisteredEnvelopeTypes() {
+		for _, keyCert := range keyCertPairCollections {
+			t.Run(fmt.Sprintf("external plugin,envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) {
+				keySpec, _ := proto.DecodeKeySpec(proto.KeySpec(keyCert.keySpecName))
+				annts := map[string]string{"key": "value"}
+				pluginSigner := pluginSigner{
+					plugin: &mockPlugin{
+						key:         keyCert.key,
+						certs:       keyCert.certs,
+						keySpec:     keySpec,
+						annotations:  map[string]string{"key": "value"},
+						wantEnvelope: true,
+					},
+				}
+				basicSignTest(t, &pluginSigner, envelopeType, &validMetadata)
+				if !reflect.DeepEqual(pluginSigner.PluginAnnotations(), annts) {
+					fmt.Println(pluginSigner.PluginAnnotations())
+					t.Errorf("mismatch in annotations returned from PluginAnnotations()")
+				}
+			})
+		}
+	}
+}
+
 func testSignerError(t *testing.T, signer pluginSigner, wantEr string, opts notation.SignOptions) {
 	t.Helper()
 	_, _, err := signer.Sign(context.Background(), ocispec.Descriptor{}, opts)