feat: added tag reference log for notation.Sign and notation.Verify (#…

…223)

This PR adds tag reference logs in notation.Sign and notation.Verify.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>

notation.go

@@ -13,9 +13,11 @@ import (
 	"time"
 
 	"github.com/notaryproject/notation-core-go/signature"
+	"github.com/notaryproject/notation-go/log"
 	"github.com/notaryproject/notation-go/registry"
 	"github.com/notaryproject/notation-go/verifier/trustpolicy"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	orasRegistry "oras.land/oras-go/v2/registry"
 )
 
 const annotationX509ChainThumbprint = "io.cncf.notary.x509chain.thumbprint#S256"
@@ -52,10 +54,26 @@ type Signer interface {
 // remote.
 // The descriptor of the sign content is returned upon sucessful signing.
 func Sign(ctx context.Context, signer Signer, repo registry.Repository, opts SignOptions) (ocispec.Descriptor, error) {
-	targetDesc, err := repo.Resolve(ctx, opts.ArtifactReference)
+	logger := log.GetLogger(ctx)
+
+	artifactRef := opts.ArtifactReference
+	ref, err := orasRegistry.ParseReference(artifactRef)
+	if err != nil {
+		return ocispec.Descriptor{}, err
+	}
+	if ref.Reference == "" {
+		return ocispec.Descriptor{}, errors.New("reference is missing digest or tag")
+	}
+	targetDesc, err := repo.Resolve(ctx, artifactRef)
 	if err != nil {
 		return ocispec.Descriptor{}, err
 	}
+	if ref.ValidateReferenceAsDigest() != nil {
+		// artifactRef is not a digest reference
+		logger.Warnf("Always sign the artifact using digest(`@sha256:...`) rather than a tag(`:%s`) because tags are mutable and a tag reference can point to a different artifact than the one signed", ref.Reference)
+		logger.Infof("Resolved artifact tag `%s` to digest `%s` before signing", ref.Reference, targetDesc.Digest.String())
+	}
+
 	sig, signerInfo, err := signer.Sign(ctx, targetDesc, opts)
 	if err != nil {
 		return ocispec.Descriptor{}, err
@@ -155,6 +173,8 @@ type RemoteVerifyOptions struct {
 // For more details on signature verification, see
 // https://github.com/notaryproject/notaryproject/blob/main/specs/trust-store-trust-policy.md#signature-verification
 func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, remoteOpts RemoteVerifyOptions) (ocispec.Descriptor, []*VerificationOutcome, error) {
+	logger := log.GetLogger(ctx)
+
 	// opts to be passed in verifier.Verify()
 	opts := VerifyOptions{
 		ArtifactReference: remoteOpts.ArtifactReference,
@@ -176,16 +196,30 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, re
 		return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("verifyOptions.MaxSignatureAttempts expects a positive number, got %d", remoteOpts.MaxSignatureAttempts)}
 	}
 
-	// get signature manifests
+	// get artifact descriptor
 	artifactRef := remoteOpts.ArtifactReference
+	ref, err := orasRegistry.ParseReference(artifactRef)
+	if err != nil {
+		return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: err.Error()}
+	}
+	if ref.Reference == "" {
+		return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: "reference is missing digest or tag"}
+	}
 	artifactDescriptor, err := repo.Resolve(ctx, artifactRef)
 	if err != nil {
 		return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: err.Error()}
 	}
+	if ref.ValidateReferenceAsDigest() != nil {
+		// artifactRef is not a digest reference
+		logger.Infof("Resolved artifact tag `%s` to digest `%s` before verification", ref.Reference, artifactDescriptor.Digest.String())
+		logger.Warn("The resolved digest may not point to the same signed artifact, since tags are mutable")
+	}
 
 	var verificationOutcomes []*VerificationOutcome
 	errExceededMaxVerificationLimit := ErrorVerificationFailed{Msg: fmt.Sprintf("total number of signatures associated with an artifact should be less than: %d", remoteOpts.MaxSignatureAttempts)}
 	numOfSignatureProcessed := 0
+
+	// get signature manifests
 	err = repo.ListSignatures(ctx, artifactDescriptor, func(signatureManifests []ocispec.Descriptor) error {
 		// process signatures
 		for _, sigManifestDesc := range signatureManifests {

notation_test.go

@@ -30,6 +30,38 @@ func TestRegistryResolveError(t *testing.T) {
 	}
 }
 
+func TestVerifyEmptyReference(t *testing.T) {
+	policyDocument := dummyPolicyDocument()
+	repo := mock.NewRepository()
+	verifier := dummyVerifier{&policyDocument, mock.PluginManager{}, false, *trustpolicy.LevelStrict}
+
+	errorMessage := "reference is missing digest or tag"
+	expectedErr := ErrorSignatureRetrievalFailed{Msg: errorMessage}
+
+	// mock the repository
+	opts := RemoteVerifyOptions{ArtifactReference: "localhost/test", MaxSignatureAttempts: 50}
+	_, _, err := Verify(context.Background(), &verifier, repo, opts)
+	if err == nil || !errors.Is(err, expectedErr) {
+		t.Fatalf("VerifyTagReference expected: %v got: %v", expectedErr, err)
+	}
+}
+
+func TestVerifyTagReferenceFailed(t *testing.T) {
+	policyDocument := dummyPolicyDocument()
+	repo := mock.NewRepository()
+	verifier := dummyVerifier{&policyDocument, mock.PluginManager{}, false, *trustpolicy.LevelStrict}
+
+	errorMessage := "invalid reference: invalid repository"
+	expectedErr := ErrorSignatureRetrievalFailed{Msg: errorMessage}
+
+	// mock the repository
+	opts := RemoteVerifyOptions{ArtifactReference: "localhost/UPPERCASE/test", MaxSignatureAttempts: 50}
+	_, _, err := Verify(context.Background(), &verifier, repo, opts)
+	if err == nil || !errors.Is(err, expectedErr) {
+		t.Fatalf("VerifyTagReference expected: %v got: %v", expectedErr, err)
+	}
+}
+
 func TestSkippedSignatureVerification(t *testing.T) {
 	policyDocument := dummyPolicyDocument()
 	repo := mock.NewRepository()

signer/plugin_test.go

@@ -13,6 +13,8 @@ import (
 	"time"
 
 	"github.com/notaryproject/notation-core-go/signature"
+	_ "github.com/notaryproject/notation-core-go/signature/cose"
+	_ "github.com/notaryproject/notation-core-go/signature/jws"
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/internal/envelope"
 	"github.com/notaryproject/notation-go/plugin"
@@ -80,8 +82,9 @@ func (p *mockPlugin) GetMetadata(ctx context.Context, req *proto.GetMetadataRequ
 
 // DescribeKey returns the KeySpec of a key.
 func (p *mockPlugin) DescribeKey(ctx context.Context, req *proto.DescribeKeyRequest) (*proto.DescribeKeyResponse, error) {
+	ks, _ := proto.EncodeKeySpec(p.keySpec)
 	return &proto.DescribeKeyResponse{
-		KeySpec: proto.KeySpecRSA2048,
+		KeySpec: ks,
 	}, nil
 }
 
@@ -152,7 +155,7 @@ func TestNewFromPluginFailed(t *testing.T) {
 
 func TestSigner_Sign_EnvelopeNotSupported(t *testing.T) {
 	signer := pluginSigner{
-		plugin: newMockPlugin(false, false, false, false, nil, nil, signature.KeySpec{}),
+		plugin: newMockPlugin(false, false, false, false, nil, nil, signature.KeySpec{Type: signature.KeyTypeRSA, Size: 2048}),
 	}
 	opts := notation.SignOptions{SignatureMediaType: "unsupported"}
 	testSignerError(t, signer, fmt.Sprintf("signature envelope format with media type %q is not supported", opts.SignatureMediaType), opts)
@@ -204,7 +207,7 @@ func TestPluginSigner_Sign_SignatureVerifyError(t *testing.T) {
 			signer := pluginSigner{
 				plugin: newMockPlugin(false, false, true, false, defaultKeyCert.key, defaultKeyCert.certs, defaultKeySpec),
 			}
-			testSignerError(t, signer, "signature returned by generateSignature cannot be verified", notation.SignOptions{SignatureMediaType: envelopeType})
+			testSignerError(t, signer, "signature is invalid", notation.SignOptions{SignatureMediaType: envelopeType})
 		})
 	}
 }
@@ -233,7 +236,7 @@ func TestPluginSigner_SignEnvelope_RunFailed(t *testing.T) {
 			signer := pluginSigner{
 				plugin: p,
 			}
-			testSignerError(t, signer, "generate-envelope command failed: failed GenerateEnvelope", notation.SignOptions{SignatureMediaType: envelopeType})
+			testSignerError(t, signer, "failed GenerateEnvelope", notation.SignOptions{SignatureMediaType: envelopeType})
 		})
 	}
 }

signer/signer_test.go

@@ -17,6 +17,8 @@ import (
 	"time"
 
 	"github.com/notaryproject/notation-core-go/signature"
+	_ "github.com/notaryproject/notation-core-go/signature/cose"
+	_ "github.com/notaryproject/notation-core-go/signature/jws"
 	"github.com/notaryproject/notation-core-go/testhelper"
 	"github.com/notaryproject/notation-core-go/timestamp/timestamptest"
 	"github.com/notaryproject/notation-go"