From ed268db7038b73e4b3a734448a14a82f9aa8ea50 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Fri, 31 May 2024 09:30:34 +0800 Subject: [PATCH] Revert "feat: add support for signing blob (#379)" This reverts commit ec42378613e1e189da0e94f1c225dba0b06a0fed. --- example_localSign_test.go | 4 +- example_remoteSign_test.go | 7 +-- notation.go | 110 +++++-------------------------------- notation_test.go | 108 +----------------------------------- signer/plugin.go | 103 +++++++++------------------------- signer/plugin_test.go | 102 +++++++++------------------------- signer/signer.go | 57 +++---------------- signer/signer_test.go | 60 -------------------- 8 files changed, 82 insertions(+), 469 deletions(-) diff --git a/example_localSign_test.go b/example_localSign_test.go index 9672c02f..996784ce 100644 --- a/example_localSign_test.go +++ b/example_localSign_test.go @@ -47,8 +47,8 @@ func Example_localSign() { // Users should replace `exampleCertTuple.PrivateKey` with their own private // key and replace `exampleCerts` with the corresponding full certificate // chain, following the Notary certificate requirements: - // https://github.com/notaryproject/notaryproject/blob/v1.0.0/specs/signature-specification.md#certificate-requirements - exampleSigner, err := signer.NewGenericSigner(exampleCertTuple.PrivateKey, exampleCerts) + // https://github.com/notaryproject/notaryproject/blob/v1.0.0-rc.1/specs/signature-specification.md#certificate-requirements + exampleSigner, err := signer.New(exampleCertTuple.PrivateKey, exampleCerts) if err != nil { panic(err) // Handle error } diff --git a/example_remoteSign_test.go b/example_remoteSign_test.go index 03565e98..9cf51058 100644 --- a/example_remoteSign_test.go +++ b/example_remoteSign_test.go @@ -18,13 +18,12 @@ import ( "crypto/x509" "fmt" - "oras.land/oras-go/v2/registry/remote" - "github.com/notaryproject/notation-core-go/signature/cose" "github.com/notaryproject/notation-core-go/testhelper" "github.com/notaryproject/notation-go" "github.com/notaryproject/notation-go/registry" "github.com/notaryproject/notation-go/signer" + "oras.land/oras-go/v2/registry/remote" ) // Both COSE ("application/cose") and JWS ("application/jose+json") @@ -46,8 +45,8 @@ func Example_remoteSign() { // Users should replace `exampleCertTuple.PrivateKey` with their own private // key and replace `exampleCerts` with the corresponding full certificate // chain, following the Notary certificate requirements: - // https://github.com/notaryproject/notaryproject/blob/v1.0.0/specs/signature-specification.md#certificate-requirements - exampleSigner, err := signer.NewGenericSigner(exampleCertTuple.PrivateKey, exampleCerts) + // https://github.com/notaryproject/notaryproject/blob/v1.0.0-rc.1/specs/signature-specification.md#certificate-requirements + exampleSigner, err := signer.New(exampleCertTuple.PrivateKey, exampleCerts) if err != nil { panic(err) // Handle error } diff --git a/notation.go b/notation.go index e5738062..2e1cf603 100644 --- a/notation.go +++ b/notation.go @@ -22,8 +22,6 @@ import ( "encoding/json" "errors" "fmt" - "io" - "mime" "strings" "time" @@ -31,8 +29,6 @@ import ( "oras.land/oras-go/v2/registry/remote" "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/internal/envelope" "github.com/notaryproject/notation-go/log" "github.com/notaryproject/notation-go/registry" @@ -48,7 +44,7 @@ var reservedAnnotationPrefixes = [...]string{"io.cncf.notary"} // SignerSignOptions contains parameters for Signer.Sign. type SignerSignOptions struct { // SignatureMediaType is the envelope type of the signature. - // Currently, both `application/jose+json` and `application/cose` are + // Currently both `application/jose+json` and `application/cose` are // supported. SignatureMediaType string @@ -63,37 +59,15 @@ type SignerSignOptions struct { SigningAgent string } -// Signer is a generic interface for signing an OCI artifact. +// Signer is a generic interface for signing an artifact. // The interface allows signing with local or remote keys, // and packing in various signature formats. type Signer interface { - // Sign signs the OCI artifact described by its descriptor, + // Sign signs the artifact described by its descriptor, // and returns the signature and SignerInfo. Sign(ctx context.Context, desc ocispec.Descriptor, opts SignerSignOptions) ([]byte, *signature.SignerInfo, error) } -// SignBlobOptions contains parameters for notation.SignBlob. -type SignBlobOptions struct { - SignerSignOptions - // ContentMediaType is the media-type of the blob being signed. - ContentMediaType string - // UserMetadata contains key-value pairs that are added to the signature - // payload - UserMetadata map[string]string -} - -// BlobDescriptorGenerator creates descriptor using the digest Algorithm. -type BlobDescriptorGenerator func(digest.Algorithm) (ocispec.Descriptor, error) - -// BlobSigner is a generic interface for signing arbitrary data. -// The interface allows signing with local or remote keys, -// and packing in various signature formats. -type BlobSigner interface { - // SignBlob signs the descriptor returned by genDesc , - // and returns the signature and SignerInfo - SignBlob(ctx context.Context, genDesc BlobDescriptorGenerator, opts SignerSignOptions) ([]byte, *signature.SignerInfo, error) -} - // signerAnnotation facilitates return of manifest annotations by signers type signerAnnotation interface { // PluginAnnotations returns signature manifest annotations returned from @@ -114,16 +88,22 @@ type SignOptions struct { UserMetadata map[string]string } -// Sign signs the OCI artifact and push the signature to the Repository. -// The descriptor of the sign content is returned upon successful signing. +// Sign signs the artifact and push the signature to the Repository. +// The descriptor of the sign content is returned upon sucessful signing. func Sign(ctx context.Context, signer Signer, repo registry.Repository, signOpts SignOptions) (ocispec.Descriptor, error) { // sanity check - if err := validateSignArguments(signer, signOpts.SignerSignOptions); err != nil { - return ocispec.Descriptor{}, err + if signer == nil { + return ocispec.Descriptor{}, errors.New("signer cannot be nil") } if repo == nil { return ocispec.Descriptor{}, errors.New("repo cannot be nil") } + if signOpts.ExpiryDuration < 0 { + return ocispec.Descriptor{}, fmt.Errorf("expiry duration cannot be a negative value") + } + if signOpts.ExpiryDuration%time.Second != 0 { + return ocispec.Descriptor{}, fmt.Errorf("expiry duration supports minimum granularity of seconds") + } logger := log.GetLogger(ctx) artifactRef := signOpts.ArtifactReference @@ -179,50 +159,6 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, signOpts return targetDesc, nil } -// SignBlob signs the arbitrary data and returns the signature -func SignBlob(ctx context.Context, signer BlobSigner, blobReader io.Reader, signBlobOpts SignBlobOptions) ([]byte, *signature.SignerInfo, error) { - // sanity checks - if err := validateSignArguments(signer, signBlobOpts.SignerSignOptions); err != nil { - return nil, nil, err - } - - if blobReader == nil { - return nil, nil, errors.New("blobReader cannot be nil") - } - - if signBlobOpts.ContentMediaType == "" { - return nil, nil, errors.New("content media-type cannot be empty") - } - - if _, _, err := mime.ParseMediaType(signBlobOpts.ContentMediaType); err != nil { - return nil, nil, fmt.Errorf("invalid content media-type '%s': %v", signBlobOpts.ContentMediaType, err) - } - - getDescFunc := getDescriptorFunc(ctx, blobReader, signBlobOpts.ContentMediaType, signBlobOpts.UserMetadata) - return signer.SignBlob(ctx, getDescFunc, signBlobOpts.SignerSignOptions) -} - -func validateSignArguments(signer any, signOpts SignerSignOptions) error { - if signer == nil { - return errors.New("signer cannot be nil") - } - if signOpts.ExpiryDuration < 0 { - return errors.New("expiry duration cannot be a negative value") - } - if signOpts.ExpiryDuration%time.Second != 0 { - return errors.New("expiry duration supports minimum granularity of seconds") - } - if signOpts.SignatureMediaType == "" { - return errors.New("signature media-type cannot be empty") - } - - if !(signOpts.SignatureMediaType == jws.MediaTypeEnvelope || signOpts.SignatureMediaType == cose.MediaTypeEnvelope) { - return fmt.Errorf("invalid signature media-type '%s'", signOpts.SignatureMediaType) - } - - return nil -} - func addUserMetadataToDescriptor(ctx context.Context, desc ocispec.Descriptor, userMetadata map[string]string) (ocispec.Descriptor, error) { logger := log.GetLogger(ctx) @@ -307,7 +243,7 @@ func (outcome *VerificationOutcome) UserMetadata() (map[string]string, error) { // VerifierVerifyOptions contains parameters for Verifier.Verify. type VerifierVerifyOptions struct { - // ArtifactReference is the reference of the artifact that is being + // ArtifactReference is the reference of the artifact that is been // verified against to. It must be a full reference. ArtifactReference string @@ -341,7 +277,7 @@ type verifySkipper interface { // VerifyOptions contains parameters for notation.Verify. type VerifyOptions struct { - // ArtifactReference is the reference of the artifact that is being + // ArtifactReference is the reference of the artifact that is been // verified against to. ArtifactReference string @@ -520,19 +456,3 @@ func generateAnnotations(signerInfo *signature.SignerInfo, annotations map[strin annotations[ocispec.AnnotationCreated] = signingTime.Format(time.RFC3339) return annotations, nil } - -func getDescriptorFunc(ctx context.Context, reader io.Reader, contentMediaType string, userMetadata map[string]string) BlobDescriptorGenerator { - return func(hashAlgo digest.Algorithm) (ocispec.Descriptor, error) { - digester := hashAlgo.Digester() - bytes, err := io.Copy(digester.Hash(), reader) - if err != nil { - return ocispec.Descriptor{}, err - } - targetDesc := ocispec.Descriptor{ - MediaType: contentMediaType, - Digest: digester.Digest(), - Size: bytes, - } - return addUserMetadataToDescriptor(ctx, targetDesc, userMetadata) - } -} diff --git a/notation_test.go b/notation_test.go index 03ef24a5..32805d22 100644 --- a/notation_test.go +++ b/notation_test.go @@ -18,11 +18,9 @@ import ( "encoding/json" "errors" "fmt" - "io" "math" "os" "path/filepath" - "strings" "testing" "time" @@ -35,7 +33,6 @@ import ( "github.com/notaryproject/notation-go/plugin" "github.com/notaryproject/notation-go/registry" "github.com/notaryproject/notation-go/verifier/trustpolicy" - "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/registry/remote" ) @@ -55,7 +52,6 @@ func TestSignSuccess(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(b *testing.T) { opts := SignOptions{} - opts.SignatureMediaType = jws.MediaTypeEnvelope opts.ExpiryDuration = tc.dur opts.ArtifactReference = mock.SampleArtifactUri @@ -67,91 +63,11 @@ func TestSignSuccess(t *testing.T) { } } -func TestSignBlobSuccess(t *testing.T) { - reader := strings.NewReader("some content") - testCases := []struct { - name string - dur time.Duration - mtype string - agent string - pConfig map[string]string - metadata map[string]string - }{ - {"expiryInHours", 24 * time.Hour, "video/mp4", "", nil, nil}, - {"oneSecondExpiry", 1 * time.Second, "video/mp4", "", nil, nil}, - {"zeroExpiry", 0, "video/mp4", "", nil, nil}, - {"validContentType", 1 * time.Second, "video/mp4", "", nil, nil}, - {"emptyContentType", 1 * time.Second, "video/mp4", "someDummyAgent", map[string]string{"hi": "hello"}, map[string]string{"bye": "tata"}}, - } - for _, tc := range testCases { - t.Run(tc.name, func(b *testing.T) { - opts := SignBlobOptions{ - SignerSignOptions: SignerSignOptions{ - SignatureMediaType: jws.MediaTypeEnvelope, - ExpiryDuration: tc.dur, - PluginConfig: tc.pConfig, - SigningAgent: tc.agent, - }, - UserMetadata: expectedMetadata, - ContentMediaType: tc.mtype, - } - - _, _, err := SignBlob(context.Background(), &dummySigner{}, reader, opts) - if err != nil { - b.Fatalf("Sign failed with error: %v", err) - } - }) - } -} - -func TestSignBlobError(t *testing.T) { - reader := strings.NewReader("some content") - testCases := []struct { - name string - signer BlobSigner - dur time.Duration - rdr io.Reader - sigMType string - ctMType string - errMsg string - }{ - {"negativeExpiry", &dummySigner{}, -1 * time.Second, nil, "video/mp4", jws.MediaTypeEnvelope, "expiry duration cannot be a negative value"}, - {"milliSecExpiry", &dummySigner{}, 1 * time.Millisecond, nil, "video/mp4", jws.MediaTypeEnvelope, "expiry duration supports minimum granularity of seconds"}, - {"invalidContentMediaType", &dummySigner{}, 1 * time.Second, reader, "video/mp4/zoping", jws.MediaTypeEnvelope, "invalid content media-type 'video/mp4/zoping': mime: unexpected content after media subtype"}, - {"emptyContentMediaType", &dummySigner{}, 1 * time.Second, reader, "", jws.MediaTypeEnvelope, "content media-type cannot be empty"}, - {"invalidSignatureMediaType", &dummySigner{}, 1 * time.Second, reader, "", "", "content media-type cannot be empty"}, - {"nilReader", &dummySigner{}, 1 * time.Second, nil, "video/mp4", jws.MediaTypeEnvelope, "blobReader cannot be nil"}, - {"nilSigner", nil, 1 * time.Second, reader, "video/mp4", jws.MediaTypeEnvelope, "signer cannot be nil"}, - {"signerError", &dummySigner{fail: true}, 1 * time.Second, reader, "video/mp4", jws.MediaTypeEnvelope, "expected SignBlob failure"}, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - opts := SignBlobOptions{ - SignerSignOptions: SignerSignOptions{ - SignatureMediaType: jws.MediaTypeEnvelope, - ExpiryDuration: tc.dur, - PluginConfig: nil, - }, - ContentMediaType: tc.sigMType, - } - - _, _, err := SignBlob(context.Background(), tc.signer, tc.rdr, opts) - if err == nil { - t.Fatalf("expected error but didnt found") - } - if err.Error() != tc.errMsg { - t.Fatalf("expected err message to be '%s' but found '%s'", tc.errMsg, err.Error()) - } - }) - } -} - func TestSignSuccessWithUserMetadata(t *testing.T) { repo := mock.NewRepository() opts := SignOptions{} opts.ArtifactReference = mock.SampleArtifactUri opts.UserMetadata = expectedMetadata - opts.SignatureMediaType = jws.MediaTypeEnvelope _, err := Sign(context.Background(), &verifyMetadataSigner{}, repo, opts) if err != nil { @@ -374,9 +290,6 @@ func TestSignDigestNotMatchResolve(t *testing.T) { repo := mock.NewRepository() repo.MissMatchDigest = true signOpts := SignOptions{ - SignerSignOptions: SignerSignOptions{ - SignatureMediaType: jws.MediaTypeEnvelope, - }, ArtifactReference: mock.SampleArtifactUri, } @@ -544,9 +457,7 @@ func dummyPolicyStatement() (policyStatement trustpolicy.TrustPolicy) { return } -type dummySigner struct { - fail bool -} +type dummySigner struct{} func (s *dummySigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts SignerSignOptions) ([]byte, *signature.SignerInfo, error) { return []byte("ABC"), &signature.SignerInfo{ @@ -556,23 +467,6 @@ func (s *dummySigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts Si }, nil } -func (s *dummySigner) SignBlob(_ context.Context, descGenFunc BlobDescriptorGenerator, _ SignerSignOptions) ([]byte, *signature.SignerInfo, error) { - if s.fail { - return nil, nil, errors.New("expected SignBlob failure") - } - - _, err := descGenFunc(digest.SHA384) - if err != nil { - return nil, nil, err - } - - return []byte("ABC"), &signature.SignerInfo{ - SignedAttributes: signature.SignedAttributes{ - SigningTime: time.Now(), - }, - }, nil -} - type verifyMetadataSigner struct{} func (s *verifyMetadataSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts SignerSignOptions) ([]byte, *signature.SignerInfo, error) { diff --git a/signer/plugin.go b/signer/plugin.go index 37bb352d..5187bf65 100644 --- a/signer/plugin.go +++ b/signer/plugin.go @@ -15,7 +15,6 @@ package signer import ( "context" - "crypto" "crypto/x509" "encoding/json" "errors" @@ -30,38 +29,22 @@ import ( "github.com/notaryproject/notation-go/log" "github.com/notaryproject/notation-go/plugin/proto" "github.com/notaryproject/notation-plugin-framework-go/plugin" - "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) -// PluginSigner signs artifacts and generates signatures. +// pluginSigner signs artifacts and generates signatures. // It implements notation.Signer -type PluginSigner struct { +type pluginSigner struct { plugin plugin.SignPlugin keyID string pluginConfig map[string]string manifestAnnotations map[string]string } -var algorithms = map[crypto.Hash]digest.Algorithm{ - crypto.SHA256: digest.SHA256, - crypto.SHA384: digest.SHA384, - crypto.SHA512: digest.SHA512, -} - // NewFromPlugin creates a notation.Signer that signs artifacts and generates // signatures by delegating the one or more operations to the named plugin, // as defined in https://github.com/notaryproject/notaryproject/blob/main/specs/plugin-extensibility.md#signing-interfaces. -// Deprecated: NewFromPlugin function exists for historical compatibility and should not be used. -// To create PluginSigner, use NewPluginSigner() function. func NewFromPlugin(plugin plugin.SignPlugin, keyID string, pluginConfig map[string]string) (notation.Signer, error) { - return NewPluginSigner(plugin, keyID, pluginConfig) -} - -// NewPluginSigner creates a notation.Signer that signs artifacts and generates -// signatures by delegating the one or more operations to the named plugin, -// as defined in https://github.com/notaryproject/notaryproject/blob/main/specs/plugin-extensibility.md#signing-interfaces. -func NewPluginSigner(plugin plugin.SignPlugin, keyID string, pluginConfig map[string]string) (*PluginSigner, error) { if plugin == nil { return nil, errors.New("nil plugin") } @@ -69,7 +52,7 @@ func NewPluginSigner(plugin plugin.SignPlugin, keyID string, pluginConfig map[st return nil, errors.New("keyID not specified") } - return &PluginSigner{ + return &pluginSigner{ plugin: plugin, keyID: keyID, pluginConfig: pluginConfig, @@ -77,91 +60,57 @@ func NewPluginSigner(plugin plugin.SignPlugin, keyID string, pluginConfig map[st } // PluginAnnotations returns signature manifest annotations returned from plugin -func (s *PluginSigner) PluginAnnotations() map[string]string { +func (s *pluginSigner) PluginAnnotations() map[string]string { return s.manifestAnnotations } // Sign signs the artifact described by its descriptor and returns the // marshalled envelope. -func (s *PluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { +func (s *pluginSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { logger := log.GetLogger(ctx) - mergedConfig := s.mergeConfig(opts.PluginConfig) - logger.Debug("Invoking plugin's get-plugin-metadata command") - metadata, err := s.plugin.GetMetadata(ctx, &plugin.GetMetadataRequest{PluginConfig: mergedConfig}) + req := &plugin.GetMetadataRequest{ + PluginConfig: s.mergeConfig(opts.PluginConfig), + } + metadata, err := s.plugin.GetMetadata(ctx, req) if err != nil { return nil, nil, err } - logger.Debugf("Using plugin %v with capabilities %v to sign oci artifact %v in signature media type %v", metadata.Name, metadata.Capabilities, desc.Digest, opts.SignatureMediaType) + logger.Debugf("Using plugin %v with capabilities %v to sign artifact %v in signature media type %v", metadata.Name, metadata.Capabilities, desc.Digest, opts.SignatureMediaType) if metadata.HasCapability(plugin.CapabilitySignatureGenerator) { - ks, err := s.getKeySpec(ctx, mergedConfig) - if err != nil { - return nil, nil, err - } - return s.generateSignature(ctx, desc, opts, ks, metadata, mergedConfig) + return s.generateSignature(ctx, desc, opts, metadata) } else if metadata.HasCapability(plugin.CapabilityEnvelopeGenerator) { return s.generateSignatureEnvelope(ctx, desc, opts) } return nil, nil, fmt.Errorf("plugin does not have signing capabilities") } -// SignBlob signs the arbitrary data and returns the marshalled envelope. -func (s *PluginSigner) SignBlob(ctx context.Context, descGenFunc notation.BlobDescriptorGenerator, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { +func (s *pluginSigner) generateSignature(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions, metadata *plugin.GetMetadataResponse) ([]byte, *signature.SignerInfo, error) { logger := log.GetLogger(ctx) - mergedConfig := s.mergeConfig(opts.PluginConfig) - - logger.Debug("Invoking plugin's get-plugin-metadata command") - metadata, err := s.plugin.GetMetadata(ctx, &plugin.GetMetadataRequest{PluginConfig: mergedConfig}) + logger.Debug("Generating signature by plugin") + config := s.mergeConfig(opts.PluginConfig) + // Get key info. + key, err := s.describeKey(ctx, config) if err != nil { return nil, nil, err } - logger.Debug("Invoking plugin's describe-key command") - ks, err := s.getKeySpec(ctx, mergedConfig) - if err != nil { - return nil, nil, err + // Check keyID is honored. + if s.keyID != key.KeyID { + return nil, nil, fmt.Errorf("keyID in describeKey response %q does not match request %q", key.KeyID, s.keyID) } - - // get descriptor to sign - desc, err := getDescriptor(ks, descGenFunc) + ks, err := proto.DecodeKeySpec(key.KeySpec) if err != nil { return nil, nil, err } - logger.Debugf("Using plugin %v with capabilities %v to sign blob using descriptor %+v", metadata.Name, metadata.Capabilities, desc) - if metadata.HasCapability(plugin.CapabilitySignatureGenerator) { - return s.generateSignature(ctx, desc, opts, ks, metadata, mergedConfig) - } else if metadata.HasCapability(plugin.CapabilityEnvelopeGenerator) { - return s.generateSignatureEnvelope(ctx, desc, opts) - } - return nil, nil, fmt.Errorf("plugin does not have signing capabilities") -} - -func (s *PluginSigner) getKeySpec(ctx context.Context, config map[string]string) (signature.KeySpec, error) { - logger := log.GetLogger(ctx) - logger.Debug("Invoking plugin's describe-key command") - descKeyResp, err := s.describeKey(ctx, config) - if err != nil { - return signature.KeySpec{}, err - } - - if s.keyID != descKeyResp.KeyID { - return signature.KeySpec{}, fmt.Errorf("keyID in describeKey response %q does not match request %q", descKeyResp.KeyID, s.keyID) - } - - return proto.DecodeKeySpec(descKeyResp.KeySpec) -} - -func (s *PluginSigner) generateSignature(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions, ks signature.KeySpec, metadata *plugin.GetMetadataResponse, pluginConfig map[string]string) ([]byte, *signature.SignerInfo, error) { - logger := log.GetLogger(ctx) - logger.Debug("Generating signature by plugin") - genericSigner := GenericSigner{ - signer: &pluginPrimitiveSigner{ + genericSigner := genericSigner{ + Signer: &pluginPrimitiveSigner{ ctx: ctx, plugin: s.plugin, keyID: s.keyID, - pluginConfig: pluginConfig, + pluginConfig: config, keySpec: ks, }, } @@ -170,7 +119,7 @@ func (s *PluginSigner) generateSignature(ctx context.Context, desc ocispec.Descr return genericSigner.Sign(ctx, desc, opts) } -func (s *PluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { +func (s *pluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { logger := log.GetLogger(ctx) logger.Debug("Generating signature envelope by plugin") payload := envelope.Payload{TargetArtifact: envelope.SanitizeTargetArtifact(desc)} @@ -233,7 +182,7 @@ func (s *PluginSigner) generateSignatureEnvelope(ctx context.Context, desc ocisp return resp.SignatureEnvelope, &envContent.SignerInfo, nil } -func (s *PluginSigner) mergeConfig(config map[string]string) map[string]string { +func (s *pluginSigner) mergeConfig(config map[string]string) map[string]string { c := make(map[string]string, len(s.pluginConfig)+len(config)) // First clone s.PluginConfig. for k, v := range s.pluginConfig { @@ -246,7 +195,7 @@ func (s *PluginSigner) mergeConfig(config map[string]string) map[string]string { return c } -func (s *PluginSigner) describeKey(ctx context.Context, config map[string]string) (*plugin.DescribeKeyResponse, error) { +func (s *pluginSigner) describeKey(ctx context.Context, config map[string]string) (*plugin.DescribeKeyResponse, error) { req := &plugin.DescribeKeyRequest{ ContractVersion: plugin.ContractVersion, KeyID: s.keyID, diff --git a/signer/plugin_test.go b/signer/plugin_test.go index 634df66f..c3dd2b30 100644 --- a/signer/plugin_test.go +++ b/signer/plugin_test.go @@ -32,7 +32,6 @@ import ( "github.com/notaryproject/notation-go/internal/envelope" "github.com/notaryproject/notation-go/plugin" "github.com/notaryproject/notation-go/plugin/proto" - "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -71,16 +70,6 @@ type mockPlugin struct { keySpec signature.KeySpec } -func getDescriptorFunc(throwError bool) func(hashAlgo digest.Algorithm) (ocispec.Descriptor, error) { - return func(hashAlgo digest.Algorithm) (ocispec.Descriptor, error) { - if throwError { - return ocispec.Descriptor{}, errors.New("") - } - return validSignDescriptor, nil - } - -} - func newMockPlugin(key crypto.PrivateKey, certs []*x509.Certificate, keySpec signature.KeySpec) *mockPlugin { return &mockPlugin{ key: key, @@ -147,7 +136,7 @@ func (p *mockPlugin) GenerateSignature(ctx context.Context, req *proto.GenerateS // GenerateEnvelope generates the Envelope with signature based on the request. func (p *mockPlugin) GenerateEnvelope(ctx context.Context, req *proto.GenerateEnvelopeRequest) (*proto.GenerateEnvelopeResponse, error) { - internalPluginSigner := PluginSigner{ + internalPluginSigner := pluginSigner{ plugin: newMockPlugin(p.key, p.certs, p.keySpec), } @@ -217,34 +206,15 @@ func (p *mockPlugin) GenerateEnvelope(ctx context.Context, req *proto.GenerateEn } func TestNewFromPluginFailed(t *testing.T) { - tests := map[string]struct { - pl plugin.SignPlugin - keyID string - errMsg string - }{ - "Invalid KeyID": { - pl: &plugin.CLIPlugin{}, - keyID: "", - errMsg: "keyID not specified", - }, - "nilPlugin": { - pl: nil, - keyID: "someKeyId", - errMsg: "nil plugin", - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - _, err := NewFromPlugin(tc.pl, tc.keyID, make(map[string]string)) - if err == nil || err.Error() != tc.errMsg { - t.Fatalf("TestNewFromPluginFailed expects error %q, got %q", tc.errMsg, err.Error()) - } - }) + wantErr := "keyID not specified" + _, err := NewFromPlugin(&plugin.CLIPlugin{}, "", make(map[string]string)) + if err == nil || err.Error() != wantErr { + t.Fatalf("TestNewFromPluginFailed expects error %q, got %q", wantErr, err.Error()) } } func TestSigner_Sign_EnvelopeNotSupported(t *testing.T) { - signer := PluginSigner{ + signer := pluginSigner{ plugin: newMockPlugin(nil, nil, signature.KeySpec{Type: signature.KeyTypeRSA, Size: 2048}), } opts := notation.SignerSignOptions{SignatureMediaType: "unsupported"} @@ -255,7 +225,7 @@ func TestSigner_Sign_DescribeKeyIDMismatch(t *testing.T) { respKeyId := "" for _, envelopeType := range signature.RegisteredEnvelopeTypes() { t.Run(fmt.Sprintf("envelopeType=%v", envelopeType), func(t *testing.T) { - signer := PluginSigner{ + signer := pluginSigner{ plugin: newMockPlugin(nil, nil, signature.KeySpec{}), keyID: "1", } @@ -268,7 +238,7 @@ func TestSigner_Sign_ExpiryInValid(t *testing.T) { for _, envelopeType := range signature.RegisteredEnvelopeTypes() { t.Run(fmt.Sprintf("envelopeType=%v", envelopeType), func(t *testing.T) { ks, _ := signature.ExtractKeySpec(keyCertPairCollections[0].certs[0]) - signer := PluginSigner{ + signer := pluginSigner{ plugin: newMockPlugin(keyCertPairCollections[0].key, keyCertPairCollections[0].certs, ks), } _, _, err := signer.Sign(context.Background(), ocispec.Descriptor{}, notation.SignerSignOptions{ExpiryDuration: -24 * time.Hour, SignatureMediaType: envelopeType}) @@ -285,7 +255,7 @@ func TestSigner_Sign_InvalidCertChain(t *testing.T) { t.Run(fmt.Sprintf("envelopeType=%v", envelopeType), func(t *testing.T) { mockPlugin := newMockPlugin(defaultKeyCert.key, defaultKeyCert.certs, defaultKeySpec) mockPlugin.invalidCertChain = true - signer := PluginSigner{ + signer := pluginSigner{ plugin: mockPlugin, } testSignerError(t, signer, "x509: malformed certificate", notation.SignerSignOptions{SignatureMediaType: envelopeType}) @@ -299,7 +269,7 @@ func TestSigner_Sign_InvalidDescriptor(t *testing.T) { mockPlugin := newMockPlugin(defaultKeyCert.key, defaultKeyCert.certs, defaultKeySpec) mockPlugin.wantEnvelope = true mockPlugin.invalidDescriptor = true - signer := PluginSigner{ + signer := pluginSigner{ plugin: mockPlugin, } testSignerError(t, signer, "during signing, following unknown attributes were added to subject descriptor: [\"additional_field\"]", notation.SignerSignOptions{SignatureMediaType: envelopeType}) @@ -312,7 +282,7 @@ func TestPluginSigner_Sign_SignatureVerifyError(t *testing.T) { t.Run(fmt.Sprintf("envelopeType=%v", envelopeType), func(t *testing.T) { mockPlugin := newMockPlugin(defaultKeyCert.key, defaultKeyCert.certs, defaultKeySpec) mockPlugin.invalidSig = true - signer := PluginSigner{ + signer := pluginSigner{ plugin: mockPlugin, } testSignerError(t, signer, "signature is invalid", notation.SignerSignOptions{SignatureMediaType: envelopeType}) @@ -325,28 +295,10 @@ func TestPluginSigner_Sign_Valid(t *testing.T) { for _, keyCert := range keyCertPairCollections { t.Run(fmt.Sprintf("external plugin,envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) { keySpec, _ := proto.DecodeKeySpec(proto.KeySpec(keyCert.keySpecName)) - pluginSigner := PluginSigner{ - plugin: newMockPlugin(keyCert.key, keyCert.certs, keySpec), - } - validSignOpts.SignatureMediaType = envelopeType - data, signerInfo, err := pluginSigner.Sign(context.Background(), validSignDescriptor, validSignOpts) - basicSignTest(t, &pluginSigner, envelopeType, data, signerInfo, err) - }) - } - } -} - -func TestPluginSigner_SignBlob_Valid(t *testing.T) { - for _, envelopeType := range signature.RegisteredEnvelopeTypes() { - for _, keyCert := range keyCertPairCollections { - t.Run(fmt.Sprintf("external plugin,envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) { - keySpec, _ := proto.DecodeKeySpec(proto.KeySpec(keyCert.keySpecName)) - pluginSigner := PluginSigner{ + pluginSigner := pluginSigner{ plugin: newMockPlugin(keyCert.key, keyCert.certs, keySpec), } - validSignOpts.SignatureMediaType = envelopeType - data, signerInfo, err := pluginSigner.SignBlob(context.Background(), getDescriptorFunc(false), validSignOpts) - basicSignTest(t, &pluginSigner, envelopeType, data, signerInfo, err) + basicSignTest(t, &pluginSigner, envelopeType, &validMetadata) }) } } @@ -359,7 +311,7 @@ func TestPluginSigner_SignEnvelope_RunFailed(t *testing.T) { wantEnvelope: true, failEnvelope: true, } - signer := PluginSigner{ + signer := pluginSigner{ plugin: p, } testSignerError(t, signer, "failed GenerateEnvelope", notation.SignerSignOptions{SignatureMediaType: envelopeType}) @@ -374,12 +326,10 @@ func TestPluginSigner_SignEnvelope_Valid(t *testing.T) { keySpec, _ := proto.DecodeKeySpec(proto.KeySpec(keyCert.keySpecName)) mockPlugin := newMockPlugin(keyCert.key, keyCert.certs, keySpec) mockPlugin.wantEnvelope = true - pluginSigner := PluginSigner{ + pluginSigner := pluginSigner{ plugin: mockPlugin, } - validSignOpts.SignatureMediaType = envelopeType - data, signerInfo, err := pluginSigner.Sign(context.Background(), validSignDescriptor, validSignOpts) - basicSignTest(t, &pluginSigner, envelopeType, data, signerInfo, err) + basicSignTest(t, &pluginSigner, envelopeType, &validMetadata) }) } } @@ -391,7 +341,7 @@ func TestPluginSigner_SignWithAnnotations_Valid(t *testing.T) { t.Run(fmt.Sprintf("external plugin,envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) { keySpec, _ := proto.DecodeKeySpec(proto.KeySpec(keyCert.keySpecName)) annts := map[string]string{"key": "value"} - pluginSigner := PluginSigner{ + pluginSigner := pluginSigner{ plugin: &mockPlugin{ key: keyCert.key, certs: keyCert.certs, @@ -400,9 +350,7 @@ func TestPluginSigner_SignWithAnnotations_Valid(t *testing.T) { wantEnvelope: true, }, } - validSignOpts.SignatureMediaType = envelopeType - data, signerInfo, err := pluginSigner.Sign(context.Background(), validSignDescriptor, validSignOpts) - basicSignTest(t, &pluginSigner, envelopeType, data, signerInfo, err) + basicSignTest(t, &pluginSigner, envelopeType, &validMetadata) if !reflect.DeepEqual(pluginSigner.PluginAnnotations(), annts) { fmt.Println(pluginSigner.PluginAnnotations()) t.Errorf("mismatch in annotations returned from PluginAnnotations()") @@ -412,7 +360,7 @@ func TestPluginSigner_SignWithAnnotations_Valid(t *testing.T) { } } -func testSignerError(t *testing.T, signer PluginSigner, wantEr string, opts notation.SignerSignOptions) { +func testSignerError(t *testing.T, signer pluginSigner, wantEr string, opts notation.SignerSignOptions) { t.Helper() _, _, err := signer.Sign(context.Background(), ocispec.Descriptor{}, opts) if err == nil || !strings.Contains(err.Error(), wantEr) { @@ -420,7 +368,9 @@ func testSignerError(t *testing.T, signer PluginSigner, wantEr string, opts nota } } -func basicSignTest(t *testing.T, ps *PluginSigner, envelopeType string, data []byte, signerInfo *signature.SignerInfo, err error) { +func basicSignTest(t *testing.T, pluginSigner *pluginSigner, envelopeType string, metadata *proto.GetMetadataResponse) { + validSignOpts.SignatureMediaType = envelopeType + data, signerInfo, err := pluginSigner.Sign(context.Background(), validSignDescriptor, validSignOpts) if err != nil { t.Fatalf("Signer.Sign() error = %v, wantErr nil", err) } @@ -449,12 +399,12 @@ func basicSignTest(t *testing.T, ps *PluginSigner, envelopeType string, data []b TargetArtifact: validSignDescriptor, } if !reflect.DeepEqual(expectedPayload, gotPayload) { - t.Fatalf("Signer.Sign() descriptor subject changed, expect: %+v, got: %+v", expectedPayload, payload) + t.Fatalf("Signer.Sign() descriptor subject changed, expect: %v, got: %v", expectedPayload, payload) } if signerInfo.SignedAttributes.SigningScheme != signature.SigningSchemeX509 { - t.Fatalf("Signer.Sign() signing scheme changed, expect: %+v, got: %+v", signerInfo.SignedAttributes.SigningScheme, signature.SigningSchemeX509) + t.Fatalf("Signer.Sign() signing scheme changed, expect: %v, got: %v", signerInfo.SignedAttributes.SigningScheme, signature.SigningSchemeX509) } - mockPlugin := ps.plugin.(*mockPlugin) + mockPlugin := pluginSigner.plugin.(*mockPlugin) if mockPlugin.keySpec.SignatureAlgorithm() != signerInfo.SignatureAlgorithm { t.Fatalf("Signer.Sign() signing algorithm changed") } @@ -464,5 +414,5 @@ func basicSignTest(t *testing.T, ps *PluginSigner, envelopeType string, data []b if !reflect.DeepEqual(mockPlugin.certs, signerInfo.CertificateChain) { t.Fatalf(" Signer.Sign() cert chain changed") } - basicVerification(t, data, envelopeType, mockPlugin.certs[len(mockPlugin.certs)-1], &validMetadata) + basicVerification(t, data, envelopeType, mockPlugin.certs[len(mockPlugin.certs)-1], metadata) } diff --git a/signer/signer.go b/signer/signer.go index 05d0ea8c..cefc7788 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -13,7 +13,7 @@ // Package signer provides notation signing functionality. It implements the // notation.Signer interface by providing builtinSigner for local signing and -// PluginSigner for remote signing. +// pluginSigner for remote signing. package signer import ( @@ -36,36 +36,24 @@ import ( // signingAgent is the unprotected header field used by signature. const signingAgent = "Notation/1.0.0" -// GenericSigner implements notation.Signer and embeds signature.Signer -type GenericSigner struct { - signer signature.Signer +// genericSigner implements notation.Signer and embeds signature.Signer +type genericSigner struct { + signature.Signer } // New returns a builtinSigner given key and cert chain -// Deprecated: New function exists for historical compatibility and should not be used. -// To create GenericSigner, use NewGenericSigner() function. func New(key crypto.PrivateKey, certChain []*x509.Certificate) (notation.Signer, error) { - return NewGenericSigner(key, certChain) -} - -// NewGenericSigner returns a builtinSigner given key and cert chain -func NewGenericSigner(key crypto.PrivateKey, certChain []*x509.Certificate) (*GenericSigner, error) { localSigner, err := signature.NewLocalSigner(certChain, key) if err != nil { return nil, err } - return &GenericSigner{ - signer: localSigner, + return &genericSigner{ + Signer: localSigner, }, nil } // NewFromFiles returns a builtinSigner given key and certChain paths. func NewFromFiles(keyPath, certChainPath string) (notation.Signer, error) { - return NewGenericSignerFromFiles(keyPath, certChainPath) -} - -// NewGenericSignerFromFiles returns a builtinSigner given key and certChain paths. -func NewGenericSignerFromFiles(keyPath, certChainPath string) (*GenericSigner, error) { if keyPath == "" { return nil, errors.New("key path not specified") } @@ -92,12 +80,12 @@ func NewGenericSignerFromFiles(keyPath, certChainPath string) (*GenericSigner, e } // create signer - return NewGenericSigner(cert.PrivateKey, certs) + return New(cert.PrivateKey, certs) } // Sign signs the artifact described by its descriptor and returns the // marshalled envelope. -func (s *GenericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { +func (s *genericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { logger := log.GetLogger(ctx) logger.Debugf("Generic signing for %v in signature media type %v", desc.Digest, opts.SignatureMediaType) // Generate payload to be signed. @@ -118,7 +106,7 @@ func (s *GenericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts ContentType: envelope.MediaTypePayloadV1, Content: payloadBytes, }, - Signer: s.signer, + Signer: s.Signer, SigningTime: time.Now(), SigningScheme: signature.SigningSchemeX509, SigningAgent: signingAgentId, @@ -158,30 +146,3 @@ func (s *GenericSigner) Sign(ctx context.Context, desc ocispec.Descriptor, opts // TODO: re-enable timestamping https://github.com/notaryproject/notation-go/issues/78 return sig, &envContent.SignerInfo, nil } - -// SignBlob signs the descriptor returned by blobGen and returns the marshalled envelope -func (s *GenericSigner) SignBlob(ctx context.Context, descGenFunc notation.BlobDescriptorGenerator, opts notation.SignerSignOptions) ([]byte, *signature.SignerInfo, error) { - logger := log.GetLogger(ctx) - logger.Debugf("Generic blob signing for signature media type %v", opts.SignatureMediaType) - - ks, err := s.signer.KeySpec() - if err != nil { - return nil, nil, err - } - - desc, err := getDescriptor(ks, descGenFunc) - if err != nil { - return nil, nil, err - } - - return s.Sign(ctx, desc, opts) -} - -func getDescriptor(ks signature.KeySpec, descGenFunc notation.BlobDescriptorGenerator) (ocispec.Descriptor, error) { - digestAlg, ok := algorithms[ks.SignatureAlgorithm().Hash()] - if !ok { - return ocispec.Descriptor{}, fmt.Errorf("unknown hashing algo %v", ks.SignatureAlgorithm().Hash()) - } - - return descGenFunc(digestAlg) -} diff --git a/signer/signer_test.go b/signer/signer_test.go index 31e16ca5..c8d0ae38 100644 --- a/signer/signer_test.go +++ b/signer/signer_test.go @@ -168,41 +168,6 @@ func TestNewFromFiles(t *testing.T) { } } -func TestNewFromFilesError(t *testing.T) { - tests := map[string]struct { - keyPath string - certPath string - errMsg string - }{ - "empty key path": { - keyPath: "", - certPath: "someCert", - errMsg: "key path not specified", - }, - "empty cert path": { - keyPath: "someKeyId", - certPath: "", - errMsg: "certificate path not specified", - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - _, err := NewFromFiles(tc.keyPath, tc.certPath) - if err == nil || err.Error() != tc.errMsg { - t.Fatalf("TestNewFromPluginFailed expects error %q, got %q", tc.errMsg, err.Error()) - } - }) - } -} - -func TestNewError(t *testing.T) { - wantErr := "\"certs\" param is invalid. Error: empty certs" - _, err := New(nil, nil) - if err == nil || err.Error() != wantErr { - t.Fatalf("TestNewFromPluginFailed expects error %q, got %q", wantErr, err.Error()) - } -} - func TestSignWithCertChain(t *testing.T) { // sign with key for _, envelopeType := range signature.RegisteredEnvelopeTypes() { @@ -214,31 +179,6 @@ func TestSignWithCertChain(t *testing.T) { } } -func TestSignBlobWithCertChain(t *testing.T) { - // sign with key - for _, envelopeType := range signature.RegisteredEnvelopeTypes() { - for _, keyCert := range keyCertPairCollections { - t.Run(fmt.Sprintf("envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) { - s, err := NewGenericSigner(keyCert.key, keyCert.certs) - if err != nil { - t.Fatalf("NewSigner() error = %v", err) - } - - sOpts := notation.SignerSignOptions{ - SignatureMediaType: envelopeType, - } - sig, _, err := s.SignBlob(context.Background(), getDescriptorFunc(false), sOpts) - if err != nil { - t.Fatalf("Sign() error = %v", err) - } - - // basic verification - basicVerification(t, sig, envelopeType, keyCert.certs[len(keyCert.certs)-1], nil) - }) - } - } -} - func TestSignWithoutExpiry(t *testing.T) { // sign with key for _, envelopeType := range signature.RegisteredEnvelopeTypes() {