Merge branch 'main' into expiry

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"
@@ -62,10 +64,25 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, opts Sig
 		return ocispec.Descriptor{}, fmt.Errorf("expiry duration supports minimum granularity of seconds")
 	}
 
-	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
@@ -165,6 +182,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,
@@ -186,16 +205,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

@@ -71,6 +71,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()