Skip to content

Commit

Permalink
fix: Enforce validation on notation signature blob number (ratify-pro…
Browse files Browse the repository at this point in the history 
…ject#1726)

Signed-off-by: akashsinghal <[email protected]>
  • Loading branch information
@binbin-li @akashsinghal
binbin-li authored and akashsinghal committed Sep 13, 2024
1 parent 93f55ca commit f84c2cb
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 37 deletions.
8 changes: 8 additions & 0 deletions pkg/referrerstore/oras/mocks/oras_storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
17 changes: 10 additions & 7 deletions pkg/referrerstore/oras/oras.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}

Expand Down
170 changes: 159 additions & 11 deletions pkg/referrerstore/oras/oras_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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{},
}
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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)
Expand All @@ -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{},
}
Expand Down Expand Up @@ -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")
}
}

Expand Down
37 changes: 18 additions & 19 deletions pkg/verifier/notation/notation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewVerifierResult("", v.name, v.verifierType, "Signature verification success", true, nil, extensions), nil
}

Expand Down
20 changes: 20 additions & 0 deletions pkg/verifier/notation/notation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit f84c2cb

Please sign in to comment.