From aeea2a7d98cb41be4704c5c099362ac8982a8a2e Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Mon, 19 Aug 2024 08:58:56 +0000 Subject: [PATCH] fix: Enforce validation on notation signature blob number --- pkg/referrerstore/oras/mocks/oras_storage.go | 8 + pkg/referrerstore/oras/oras.go | 17 +- pkg/referrerstore/oras/oras_test.go | 170 +++++++++++++++++-- pkg/verifier/notation/notation.go | 37 ++-- pkg/verifier/notation/notation_test.go | 20 +++ 5 files changed, 215 insertions(+), 37 deletions(-) diff --git a/pkg/referrerstore/oras/mocks/oras_storage.go b/pkg/referrerstore/oras/mocks/oras_storage.go index a02e6cc3f..b2bffa4d4 100644 --- a/pkg/referrerstore/oras/mocks/oras_storage.go +++ b/pkg/referrerstore/oras/mocks/oras_storage.go @@ -28,9 +28,14 @@ import ( type TestStorage struct { content.Storage ExistsMap map[digest.Digest]io.Reader + ExistsErr error + FetchErr error } func (s TestStorage) Exists(_ context.Context, target oci.Descriptor) (bool, error) { + if s.ExistsErr != nil { + return false, s.ExistsErr + } if _, ok := s.ExistsMap[target.Digest]; ok { return true, nil } @@ -43,6 +48,9 @@ func (s TestStorage) Push(_ context.Context, expected oci.Descriptor, content io } func (s TestStorage) Fetch(_ context.Context, target oci.Descriptor) (io.ReadCloser, error) { + if s.FetchErr != nil { + return nil, s.FetchErr + } if reader, ok := s.ExistsMap[target.Digest]; ok { return io.NopCloser(reader), nil } diff --git a/pkg/referrerstore/oras/oras.go b/pkg/referrerstore/oras/oras.go index 5a4942bf3..ef1116464 100644 --- a/pkg/referrerstore/oras/oras.go +++ b/pkg/referrerstore/oras/oras.go @@ -305,10 +305,18 @@ func (store *orasStore) GetReferenceManifest(ctx context.Context, subjectReferen // check if manifest exists in local ORAS cache isCached, err := store.localCache.Exists(ctx, referenceDesc.Descriptor) if err != nil { - return ocispecs.ReferenceManifest{}, err + logger.GetLogger(ctx, logOpt).Warnf("failed to check if manifest [%s] exists in cache: %v", referenceDesc.Descriptor.Digest, err) } metrics.ReportBlobCacheCount(ctx, isCached) + if isCached { + manifestBytes, err = store.getRawContentFromCache(ctx, referenceDesc.Descriptor) + if err != nil { + isCached = false + logger.GetLogger(ctx, logOpt).Warnf("failed to get manifest [%s] from cache: %v", referenceDesc.Descriptor.Digest, err) + } + } + if !isCached { // fetch manifest content from repository manifestReader, err := repository.Fetch(ctx, referenceDesc.Descriptor) @@ -326,12 +334,7 @@ func (store *orasStore) GetReferenceManifest(ctx context.Context, subjectReferen orasExistsExpectedError := fmt.Errorf("%s: %s: %w", referenceDesc.Descriptor.Digest, referenceDesc.Descriptor.MediaType, errdef.ErrAlreadyExists) err = store.localCache.Push(ctx, referenceDesc.Descriptor, bytes.NewReader(manifestBytes)) if err != nil && err.Error() != orasExistsExpectedError.Error() { - return ocispecs.ReferenceManifest{}, err - } - } else { - manifestBytes, err = store.getRawContentFromCache(ctx, referenceDesc.Descriptor) - if err != nil { - return ocispecs.ReferenceManifest{}, err + logger.GetLogger(ctx, logOpt).Warnf("failed to save manifest [%s] in cache: %v", referenceDesc.Descriptor.Digest, err) } } diff --git a/pkg/referrerstore/oras/oras_test.go b/pkg/referrerstore/oras/oras_test.go index 677eaa4e0..818d9c76c 100644 --- a/pkg/referrerstore/oras/oras_test.go +++ b/pkg/referrerstore/oras/oras_test.go @@ -40,7 +40,11 @@ import ( "oras.land/oras-go/v2/registry/remote/errcode" ) -const inputOriginalPath = "localhost:5000/net-monitor:v0" +const ( + inputOriginalPath = "localhost:5000/net-monitor:v0" + wrongReferenceMediatype = "application/vnd.oci.image.manifest.wrong.v1+json" + validReferenceMediatype = "application/vnd.oci.image.manifest.right.v1+json" +) // TestORASName tests the Name method of the oras store. func TestORASName(t *testing.T) { @@ -197,14 +201,12 @@ func TestORASGetReferenceManifest_CachedDesc(t *testing.T) { ctx := context.Background() firstDigest := digest.FromString("testDigest") artifactDigest := digest.FromString("testArtifactDigest") - expectedReferenceMediatype := "application/vnd.oci.image.manifest.right.v1+json" - wrongReferenceMediatype := "application/vnd.oci.image.manifest.wrong.v1+json" store, err := createBaseStore("1.0.0", conf) if err != nil { t.Fatalf("failed to create oras store: %v", err) } manifestCached := oci.Manifest{ - MediaType: expectedReferenceMediatype, + MediaType: validReferenceMediatype, Config: oci.Descriptor{}, Layers: []oci.Descriptor{}, } @@ -247,8 +249,8 @@ func TestORASGetReferenceManifest_CachedDesc(t *testing.T) { if err != nil { t.Fatalf("failed to get reference manifest: %v", err) } - if manifest.MediaType != expectedReferenceMediatype { - t.Fatalf("expected media type %s, got %s", expectedReferenceMediatype, manifest.MediaType) + if manifest.MediaType != validReferenceMediatype { + t.Fatalf("expected media type %s, got %s", validReferenceMediatype, manifest.MediaType) } } @@ -261,8 +263,6 @@ func TestORASGetReferenceManifest_NotCachedDesc(t *testing.T) { firstDigest := digest.FromString("testDigest") artifactDigest := digest.FromString("testArtifactDigest") artifactDigestNotCached := digest.FromString("testArtifactDigestNotCached") - expectedReferenceMediatype := "application/vnd.oci.image.manifest.right.v1+json" - wrongReferenceMediatype := "application/vnd.oci.image.manifest.wrong.v1+json" store, err := createBaseStore("1.0.0", conf) if err != nil { t.Fatalf("failed to create oras store: %v", err) @@ -277,7 +277,7 @@ func TestORASGetReferenceManifest_NotCachedDesc(t *testing.T) { t.Fatalf("failed to marshal cached manifest: %v", err) } manifestNotCached := oci.Manifest{ - MediaType: expectedReferenceMediatype, + MediaType: validReferenceMediatype, Config: oci.Descriptor{}, Layers: []oci.Descriptor{}, } @@ -311,8 +311,156 @@ func TestORASGetReferenceManifest_NotCachedDesc(t *testing.T) { if err != nil { t.Fatalf("failed to get reference manifest: %v", err) } - if manifest.MediaType != expectedReferenceMediatype { - t.Fatalf("expected media type %s, got %s", expectedReferenceMediatype, manifest.MediaType) + if manifest.MediaType != validReferenceMediatype { + t.Fatalf("expected media type %s, got %s", validReferenceMediatype, manifest.MediaType) + } +} + +func TestORASGetReferenceManifest_CacheFetchManifestFailure(t *testing.T) { + conf := config.StorePluginConfig{ + "name": "oras", + } + ctx := context.Background() + firstDigest := digest.FromString("testDigest") + artifactDigest := digest.FromString("testArtifactDigest") + store, err := createBaseStore("1.0.0", conf) + if err != nil { + t.Fatalf("failed to create oras store: %v", err) + } + manifestCached := oci.Manifest{ + MediaType: wrongReferenceMediatype, + Config: oci.Descriptor{}, + Layers: []oci.Descriptor{}, + } + manifestCachedBytes, err := json.Marshal(manifestCached) + if err != nil { + t.Fatalf("failed to marshal cached manifest: %v", err) + } + manifestNotCached := oci.Manifest{ + MediaType: validReferenceMediatype, + Config: oci.Descriptor{}, + Layers: []oci.Descriptor{}, + } + manifestNotCachedBytes, err := json.Marshal(manifestNotCached) + if err != nil { + t.Fatalf("failed to marshal not cached manifest: %v", err) + } + testRepo := mocks.TestRepository{ + FetchMap: map[digest.Digest]io.ReadCloser{ + artifactDigest: io.NopCloser(bytes.NewReader(manifestNotCachedBytes)), + }, + } + store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) { + return testRepo, nil + } + store.localCache = mocks.TestStorage{ + ExistsMap: map[digest.Digest]io.Reader{ + artifactDigest: bytes.NewReader(manifestCachedBytes), + }, + FetchErr: errors.New("cache fetch error"), + } + inputRef := common.Reference{ + Original: inputOriginalPath, + Digest: firstDigest, + } + manifest, err := store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{ + Descriptor: oci.Descriptor{ + MediaType: ocispecs.MediaTypeArtifactManifest, + Digest: artifactDigest, + }, + }) + if err != nil { + t.Fatalf("failed to get reference manifest: %v", err) + } + if manifest.MediaType != validReferenceMediatype { + t.Fatalf("expected media type %s, got %s", validReferenceMediatype, manifest.MediaType) + } +} + +func TestORASGetReferenceManifest_CacheExistsFailure(t *testing.T) { + conf := config.StorePluginConfig{ + "name": "oras", + } + ctx := context.Background() + firstDigest := digest.FromString("testDigest") + artifactDigest := digest.FromString("testArtifactDigest") + store, err := createBaseStore("1.0.0", conf) + if err != nil { + t.Fatalf("failed to create oras store: %v", err) + } + if err != nil { + t.Fatalf("failed to marshal cached manifest: %v", err) + } + + testRepo := mocks.TestRepository{ + FetchMap: map[digest.Digest]io.ReadCloser{}, + } + store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) { + return testRepo, nil + } + store.localCache = mocks.TestStorage{ + ExistsErr: errors.New("cache exists error"), + } + inputRef := common.Reference{ + Original: inputOriginalPath, + Digest: firstDigest, + } + _, err = store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{ + Descriptor: oci.Descriptor{ + MediaType: ocispecs.MediaTypeArtifactManifest, + Digest: artifactDigest, + }, + }) + if err == nil { + t.Fatalf("expected error fetching reference manifest") + } +} + +func TestORASGetReferenceManifest_NotCachedDescAndFetchFailed(t *testing.T) { + conf := config.StorePluginConfig{ + "name": "oras", + } + ctx := context.Background() + firstDigest := digest.FromString("testDigest") + artifactDigest := digest.FromString("testArtifactDigest") + artifactDigestNotCached := digest.FromString("testArtifactDigestNotCached") + store, err := createBaseStore("1.0.0", conf) + if err != nil { + t.Fatalf("failed to create oras store: %v", err) + } + manifestCached := oci.Manifest{ + MediaType: wrongReferenceMediatype, + Config: oci.Descriptor{}, + Layers: []oci.Descriptor{}, + } + manifestCachedBytes, err := json.Marshal(manifestCached) + if err != nil { + t.Fatalf("failed to marshal cached manifest: %v", err) + } + + testRepo := mocks.TestRepository{ + FetchMap: map[digest.Digest]io.ReadCloser{}, + } + store.createRepository = func(_ context.Context, _ *orasStore, _ common.Reference) (registry.Repository, error) { + return testRepo, nil + } + store.localCache = mocks.TestStorage{ + ExistsMap: map[digest.Digest]io.Reader{ + artifactDigestNotCached: bytes.NewReader(manifestCachedBytes), + }, + } + inputRef := common.Reference{ + Original: inputOriginalPath, + Digest: firstDigest, + } + _, err = store.GetReferenceManifest(ctx, inputRef, ocispecs.ReferenceDescriptor{ + Descriptor: oci.Descriptor{ + MediaType: ocispecs.MediaTypeArtifactManifest, + Digest: artifactDigest, + }, + }) + if err == nil { + t.Fatalf("expected error fetching reference manifest") } } diff --git a/pkg/verifier/notation/notation.go b/pkg/verifier/notation/notation.go index 1a4349347..aa6be7c0e 100644 --- a/pkg/verifier/notation/notation.go +++ b/pkg/verifier/notation/notation.go @@ -150,30 +150,29 @@ func (v *notationPluginVerifier) Verify(ctx context.Context, return verifier.VerifierResult{IsSuccess: false}, re.ErrorCodeGetReferenceManifestFailure.NewError(re.ReferrerStore, store.Name(), re.EmptyLink, err, fmt.Sprintf("failed to resolve reference manifest: %+v", referenceDescriptor), re.HideStackTrace) } - if len(referenceManifest.Blobs) == 0 { - return verifier.VerifierResult{IsSuccess: false}, re.ErrorCodeSignatureNotFound.NewError(re.Verifier, v.name, re.EmptyLink, nil, fmt.Sprintf("no signature content found for referrer: %s@%s", subjectReference.Path, referenceDescriptor.Digest.String()), re.HideStackTrace) + if len(referenceManifest.Blobs) != 1 { + return verifier.VerifierResult{IsSuccess: false}, re.ErrorCodeVerifyPluginFailure.WithDetail(fmt.Sprintf("Notation signature manifest requires exactly one signature envelope blob, got %d", len(referenceManifest.Blobs))).WithRemediation(fmt.Sprintf("Please inspect the artifact [%s@%s] is correctly signed by Notation signer", subjectReference.Path, referenceDescriptor.Digest.String())) } - for _, blobDesc := range referenceManifest.Blobs { - refBlob, err := store.GetBlobContent(ctx, subjectReference, blobDesc.Digest) - if err != nil { - return verifier.VerifierResult{IsSuccess: false}, re.ErrorCodeGetBlobContentFailure.NewError(re.ReferrerStore, store.Name(), re.EmptyLink, err, fmt.Sprintf("failed to get blob content of digest: %s", blobDesc.Digest), re.HideStackTrace) - } - - // TODO: notation verify API only accepts digested reference now. - // Pass in tagged reference instead once notation-go supports it. - subjectRef := fmt.Sprintf("%s@%s", subjectReference.Path, subjectReference.Digest.String()) - outcome, err := v.verifySignature(ctx, subjectRef, blobDesc.MediaType, subjectDesc.Descriptor, refBlob) - if err != nil { - return verifier.VerifierResult{IsSuccess: false, Extensions: extensions}, re.ErrorCodeVerifyPluginFailure.NewError(re.Verifier, v.name, re.NotationTsgLink, err, "failed to verify signature of digest", re.HideStackTrace) - } + blobDesc := referenceManifest.Blobs[0] + refBlob, err := store.GetBlobContent(ctx, subjectReference, blobDesc.Digest) + if err != nil { + return verifier.VerifierResult{IsSuccess: false}, re.ErrorCodeGetBlobContentFailure.NewError(re.ReferrerStore, store.Name(), re.EmptyLink, err, fmt.Sprintf("failed to get blob content of digest: %s", blobDesc.Digest), re.HideStackTrace) + } - // Note: notation verifier already validates certificate chain is not empty. - cert := outcome.EnvelopeContent.SignerInfo.CertificateChain[0] - extensions["Issuer"] = cert.Issuer.String() - extensions["SN"] = cert.Subject.String() + // TODO: notation verify API only accepts digested reference now. + // Pass in tagged reference instead once notation-go supports it. + subjectRef := fmt.Sprintf("%s@%s", subjectReference.Path, subjectReference.Digest.String()) + outcome, err := v.verifySignature(ctx, subjectRef, blobDesc.MediaType, subjectDesc.Descriptor, refBlob) + if err != nil { + return verifier.VerifierResult{IsSuccess: false, Extensions: extensions}, re.ErrorCodeVerifyPluginFailure.NewError(re.Verifier, v.name, re.NotationTsgLink, err, "failed to verify signature of digest", re.HideStackTrace) } + // Note: notation verifier already validates certificate chain is not empty. + cert := outcome.EnvelopeContent.SignerInfo.CertificateChain[0] + extensions["Issuer"] = cert.Issuer.String() + extensions["SN"] = cert.Subject.String() + return verifier.VerifierResult{ Name: v.name, Type: v.verifierType, diff --git a/pkg/verifier/notation/notation_test.go b/pkg/verifier/notation/notation_test.go index c320a8263..3bbebe042 100644 --- a/pkg/verifier/notation/notation_test.go +++ b/pkg/verifier/notation/notation_test.go @@ -367,6 +367,26 @@ func TestVerify(t *testing.T) { expect: failedResult, expectErr: true, }, + { + name: "multiple signature blobs", + ref: validRef2, + refBlob: testRefBlob2, + manifest: ocispecs.ReferenceManifest{ + Blobs: []ocispec.Descriptor{validBlobDesc, validBlobDesc2}, + }, + expect: failedResult, + expectErr: true, + }, + { + name: "get blob content failed", + ref: validRef, + refBlob: nil, + manifest: ocispecs.ReferenceManifest{ + Blobs: []ocispec.Descriptor{validBlobDesc}, + }, + expect: failedResult, + expectErr: true, + }, { name: "verified successfully", ref: validRef,