From ab113ebd2a6cbffd64c880621492b2a9ce6b2d0d Mon Sep 17 00:00:00 2001 From: Patrick Zheng Date: Fri, 25 Nov 2022 10:20:16 +0800 Subject: [PATCH] update: check SignatureMediaType in notation.Verify (#208) Signed-off-by: Patrick Zheng --- notation.go | 86 ++++++++++++++++++++++++++++++------------------ notation_test.go | 60 ++++++++++++++++++++++++++++----- 2 files changed, 106 insertions(+), 40 deletions(-) diff --git a/notation.go b/notation.go index 5d71efdb..bb813554 100644 --- a/notation.go +++ b/notation.go @@ -72,28 +72,6 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, opts Sig return targetDesc, nil } -// VerifyOptions contains parameters for Verifier.Verify. -type VerifyOptions struct { - // ArtifactReference is the reference of the artifact that is been - // verified against to. - ArtifactReference string - - // SignatureMediaType is the envelope type of the signature. - // Currently both `application/jose+json` and `application/cose` are - // supported. - SignatureMediaType string - - // PluginConfig is a map of plugin configs. - PluginConfig map[string]string - - // MaxSignatureAttempts is the maximum number of signature envelopes that - // can be associated with the target artifact. If set to less than or equals - // to zero, an error will be returned. - // Note: this option is scoped to notation.Verify(). verifier.Verify() is - // for signle signature verification, and therefore, does not use it. - MaxSignatureAttempts int -} - // ValidationResult encapsulates the verification result (passed or failed) // for a verification type, including the desired verification action as // specified in the trust policy @@ -132,6 +110,21 @@ type VerificationOutcome struct { Error error } +// VerifyOptions contains parameters for Verifier.Verify. +type VerifyOptions struct { + // ArtifactReference is the reference of the artifact that is been + // verified against to. + ArtifactReference string + + // SignatureMediaType is the envelope type of the signature. + // Currently both `application/jose+json` and `application/cose` are + // supported. + SignatureMediaType string + + // PluginConfig is a map of plugin configs. + PluginConfig map[string]string +} + // Verifier is a generic interface for verifying an artifact. type Verifier interface { // Verify verifies the signature blob and returns the outcome upon @@ -141,12 +134,33 @@ type Verifier interface { Verify(ctx context.Context, desc ocispec.Descriptor, signature []byte, opts VerifyOptions) (*VerificationOutcome, error) } +// RemoteVerifyOptions contains parameters for notation.Verify. +type RemoteVerifyOptions struct { + // ArtifactReference is the reference of the artifact that is been + // verified against to. + ArtifactReference string + + // PluginConfig is a map of plugin configs. + PluginConfig map[string]string + + // MaxSignatureAttempts is the maximum number of signature envelopes that + // will be processed for verification. If set to less than or equals + // to zero, an error will be returned. + MaxSignatureAttempts int +} + // Verify performs signature verification on each of the notation supported // verification types (like integrity, authenticity, etc.) and return the // successful signature verification outcomes. // 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, opts VerifyOptions) (ocispec.Descriptor, []*VerificationOutcome, error) { +func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, remoteOpts RemoteVerifyOptions) (ocispec.Descriptor, []*VerificationOutcome, error) { + // opts to be passed in verifier.Verify() + opts := VerifyOptions{ + ArtifactReference: remoteOpts.ArtifactReference, + PluginConfig: remoteOpts.PluginConfig, + } + // passing nil signature to check 'skip' outcome, err := verifier.Verify(ctx, ocispec.Descriptor{}, nil, opts) if err != nil { @@ -156,31 +170,38 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, op } else if reflect.DeepEqual(outcome.VerificationLevel, trustpolicy.LevelSkip) { return ocispec.Descriptor{}, []*VerificationOutcome{outcome}, nil } + + // check MaxSignatureAttempts + if remoteOpts.MaxSignatureAttempts <= 0 { + return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("verifyOptions.MaxSignatureAttempts expects a positive number, got %d", remoteOpts.MaxSignatureAttempts)} + } + // get signature manifests - artifactRef := opts.ArtifactReference + artifactRef := remoteOpts.ArtifactReference artifactDescriptor, err := repo.Resolve(ctx, artifactRef) if err != nil { return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: err.Error()} } var verificationOutcomes []*VerificationOutcome - if opts.MaxSignatureAttempts <= 0 { - return ocispec.Descriptor{}, verificationOutcomes, ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("verifyOptions.MaxSignatureAttempts expects a positive number, got %d", opts.MaxSignatureAttempts)} - } - errExceededMaxVerificationLimit := ErrorVerificationFailed{Msg: fmt.Sprintf("total number of signatures associated with an artifact should be less than: %d", opts.MaxSignatureAttempts)} + errExceededMaxVerificationLimit := ErrorVerificationFailed{Msg: fmt.Sprintf("total number of signatures associated with an artifact should be less than: %d", remoteOpts.MaxSignatureAttempts)} numOfSignatureProcessed := 0 err = repo.ListSignatures(ctx, artifactDescriptor, func(signatureManifests []ocispec.Descriptor) error { // process signatures for _, sigManifestDesc := range signatureManifests { - if numOfSignatureProcessed >= opts.MaxSignatureAttempts { + if numOfSignatureProcessed >= remoteOpts.MaxSignatureAttempts { break } numOfSignatureProcessed++ // get signature envelope - sigBlob, _, err := repo.FetchSignatureBlob(ctx, sigManifestDesc) + sigBlob, sigDesc, err := repo.FetchSignatureBlob(ctx, sigManifestDesc) if err != nil { return ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("unable to retrieve digital signature with digest %q associated with %q from the registry, error : %v", sigManifestDesc.Digest, artifactRef, err.Error())} } + // using signature media type fetched from registry + opts.SignatureMediaType = sigDesc.MediaType + + // verify each signature outcome, err := verifier.Verify(ctx, artifactDescriptor, sigBlob, opts) if err != nil { if outcome == nil { @@ -190,14 +211,15 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, op continue } - // At this point, the signature is verified successfully. Add + // at this point, the signature is verified successfully. Add // it to the verificationOutcomes. verificationOutcomes = append(verificationOutcomes, outcome) + // early break on success return errDoneVerification } - if numOfSignatureProcessed >= opts.MaxSignatureAttempts { + if numOfSignatureProcessed >= remoteOpts.MaxSignatureAttempts { return errExceededMaxVerificationLimit } diff --git a/notation_test.go b/notation_test.go index c732ce33..f3f31611 100644 --- a/notation_test.go +++ b/notation_test.go @@ -22,7 +22,7 @@ func TestRegistryResolveError(t *testing.T) { // mock the repository repo.ResolveError = errors.New(errorMessage) - opts := VerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} + opts := RemoteVerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} _, _, err := Verify(context.Background(), &verifier, repo, opts) if err == nil || !errors.Is(err, expectedErr) { @@ -35,7 +35,7 @@ func TestSkippedSignatureVerification(t *testing.T) { repo := mock.NewRepository() verifier := dummyVerifier{&policyDocument, mock.PluginManager{}, false, *trustpolicy.LevelSkip} - opts := VerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} + opts := RemoteVerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} _, outcomes, err := Verify(context.Background(), &verifier, repo, opts) if err != nil || outcomes[0].VerificationLevel.Name != trustpolicy.LevelSkip.Name { @@ -52,7 +52,7 @@ func TestRegistryNoSignatureManifests(t *testing.T) { // mock the repository repo.ListSignaturesResponse = []ocispec.Descriptor{} - opts := VerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} + opts := RemoteVerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} _, _, err := Verify(context.Background(), &verifier, repo, opts) if err == nil || !errors.Is(err, expectedErr) { @@ -69,11 +69,55 @@ func TestRegistryFetchSignatureBlobError(t *testing.T) { // mock the repository repo.FetchSignatureBlobError = errors.New("network error") - opts := VerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} + opts := RemoteVerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} _, _, err := Verify(context.Background(), &verifier, repo, opts) if err == nil || !errors.Is(err, expectedErr) { - t.Fatalf("RegistryGetBlob expected: %v got: %v", expectedErr, err) + t.Fatalf("RegistryFetchSignatureBlob expected: %v got: %v", expectedErr, err) + } +} + +func TestVerifyValid(t *testing.T) { + policyDocument := dummyPolicyDocument() + repo := mock.NewRepository() + verifier := dummyVerifier{&policyDocument, mock.PluginManager{}, false, *trustpolicy.LevelStrict} + + // mock the repository + opts := RemoteVerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} + _, _, err := Verify(context.Background(), &verifier, repo, opts) + + if err != nil { + t.Fatalf("SignaureMediaTypeMismatch expected: %v got: %v", nil, err) + } +} + +func TestMaxSignatureAttemptsMissing(t *testing.T) { + policyDocument := dummyPolicyDocument() + repo := mock.NewRepository() + verifier := dummyVerifier{&policyDocument, mock.PluginManager{}, false, *trustpolicy.LevelStrict} + expectedErr := ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("verifyOptions.MaxSignatureAttempts expects a positive number, got %d", 0)} + + // mock the repository + opts := RemoteVerifyOptions{ArtifactReference: mock.SampleArtifactUri} + _, _, err := Verify(context.Background(), &verifier, repo, opts) + + if err == nil || !errors.Is(err, expectedErr) { + t.Fatalf("VerificationFailed expected: %v got: %v", expectedErr, err) + } +} + +func TestVerifyFailed(t *testing.T) { + policyDocument := dummyPolicyDocument() + repo := mock.NewRepository() + verifier := dummyVerifier{&policyDocument, mock.PluginManager{}, true, *trustpolicy.LevelStrict} + expectedErr := ErrorVerificationFailed{} + + // mock the repository + opts := RemoteVerifyOptions{ArtifactReference: mock.SampleArtifactUri, MaxSignatureAttempts: 50} + _, _, err := Verify(context.Background(), &verifier, repo, opts) + + if err == nil || !errors.Is(err, expectedErr) { + t.Fatalf("VerificationFailed expected: %v got: %v", expectedErr, err) } } @@ -104,13 +148,13 @@ type dummyVerifier struct { } func (v *dummyVerifier) Verify(ctx context.Context, desc ocispec.Descriptor, signature []byte, opts VerifyOptions) (*VerificationOutcome, error) { - if v.FailVerify { - return nil, errors.New("failed verify") - } outcome := &VerificationOutcome{ VerificationResults: []*ValidationResult{}, VerificationLevel: &v.VerificationLevel, } + if v.FailVerify { + return outcome, errors.New("failed verify") + } return outcome, nil }