refactor:keep sign interface unchanged. pass keyspec as parameter to provider

Signed-off-by: zaihaoyin <[email protected]>

Author: zaihaoyin

signature/plugin.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/notaryproject/notation-core-go/signature"
+	"github.com/notaryproject/notation-core-go/signature/jws"
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/plugin"
 )
@@ -25,13 +26,16 @@ type pluginSigner struct {
 // 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 NewSignerPlugin(runner plugin.Runner, keyID string, pluginConfig map[string]string, envelopeMediaType string) (notation.Signer, error) {
+func NewSignerPlugin(runner plugin.Runner, keyID string, pluginConfig map[string]string) (notation.Signer, error) {
 	if runner == nil {
 		return nil, errors.New("nil plugin runner")
 	}
 	if keyID == "" {
 		return nil, errors.New("nil signing keyID")
 	}
+
+	// TODO: pass media type as a parameter.
+	envelopeMediaType := jws.MediaTypeEnvelope
 	if err := ValidateEnvelopeMediaType(envelopeMediaType); err != nil {
 		return nil, err
 	}
@@ -115,8 +119,14 @@ func (s *pluginSigner) generateSignature(ctx context.Context, desc notation.Desc
 		return nil, fmt.Errorf("envelope payload can't be marshaled: %w", err)
 	}
 
-	// Create plugin signature provider
-	s.sigProvider.SetConfig(config)
+	// for external plugin, pass keySpec and config before signing
+	if extProvider, ok := s.sigProvider.(*externalProvider); ok {
+		ks, err := plugin.ParseKeySpec(key.KeySpec)
+		if err != nil {
+			return nil, err
+		}
+		extProvider.prepareSigning(config, ks)
+	}
 	signReq := &signature.SignRequest{
 		Payload: signature.Payload{
 			ContentType: notation.MediaTypePayloadV1,
@@ -125,8 +135,8 @@ func (s *pluginSigner) generateSignature(ctx context.Context, desc notation.Desc
 		Signer:                   s.sigProvider,
 		SigningTime:              time.Now(),
 		ExtendedSignedAttributes: nil,
-		SigningAgent:             notation.SigningAgent,
 		SigningScheme:            signature.SigningSchemeX509,
+		SigningAgent:             notation.SigningAgent, // TODO: include external signing plugin's name and version. https://github.com/notaryproject/notation-go/issues/80
 	}
 
 	if !opts.Expiry.IsZero() {

signature/provider.go

@@ -24,7 +24,6 @@ var builtInPluginMetaData = plugin.Metadata{
 type provider interface {
 	plugin.Runner
 	signature.Signer
-	SetConfig(map[string]string)
 }
 
 // builtinProvider is a builtin provider implementation
@@ -53,9 +52,6 @@ func (*builtinProvider) metadata() *plugin.Metadata {
 	return &builtInPluginMetaData
 }
 
-// SetConfig sets config when signing.
-func (*builtinProvider) SetConfig(map[string]string) {}
-
 // Run implements the plugin workflow.
 //
 // builtinProvider only supports metadata and describe key.
@@ -96,27 +92,10 @@ func newExternalProvider(runner plugin.Runner, keyID string) provider {
 	}
 }
 
-// SetConfig sets up config used by signing.
-func (p *externalProvider) SetConfig(cfg map[string]string) {
+// prepareSigning sets up config and keySpec used to sign.
+func (p *externalProvider) prepareSigning(cfg map[string]string, keySpec signature.KeySpec) {
 	p.config = cfg
-}
-
-// describeKey invokes plugin's DescribeKey command.
-func (p *externalProvider) describeKey(ctx context.Context) (*plugin.DescribeKeyResponse, error) {
-	req := &plugin.DescribeKeyRequest{
-		ContractVersion: plugin.ContractVersion,
-		KeyID:           p.keyID,
-		PluginConfig:    p.config,
-	}
-	out, err := p.Run(ctx, req)
-	if err != nil {
-		return nil, fmt.Errorf("describe-key command failed: %w", err)
-	}
-	resp, ok := out.(*plugin.DescribeKeyResponse)
-	if !ok {
-		return nil, fmt.Errorf("plugin runner returned incorrect describe-key response type '%T'", out)
-	}
-	return resp, nil
+	p.keySpec = keySpec
 }
 
 // Sign signs the digest by calling the underlying plugin.
@@ -159,18 +138,5 @@ func (p *externalProvider) Sign(payload []byte) ([]byte, []*x509.Certificate, er
 
 // KeySpec returns the keySpec of a keyID by calling describeKey and do some keySpec validation.
 func (p *externalProvider) KeySpec() (signature.KeySpec, error) {
-	if p.keySpec != (signature.KeySpec{}) {
-		return p.keySpec, nil
-	}
-	keyResp, err := p.describeKey(context.Background())
-	if err != nil {
-		return signature.KeySpec{}, err
-	}
-
-	// Check keyID is honored.
-	if p.keyID != keyResp.KeyID {
-		return signature.KeySpec{}, fmt.Errorf("keyID in describeKey response %q does not match request %q", keyResp.KeyID, p.keyID)
-	}
-	p.keySpec, err = plugin.ParseKeySpec(keyResp.KeySpec)
-	return p.keySpec, err
+	return p.keySpec, nil
 }

signature/signer.go

@@ -7,12 +7,13 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/notaryproject/notation-core-go/signature/jws"
 	"github.com/notaryproject/notation-go"
 )
 
 // NewSignerFromFiles creates a signer from key, certificate files
 // TODO: Add tests for this method. https://github.com/notaryproject/notation-go/issues/80
-func NewSignerFromFiles(keyPath, certPath, envelopeMediaType string) (notation.Signer, error) {
+func NewSignerFromFiles(keyPath, certPath string) (notation.Signer, error) {
 	if keyPath == "" {
 		return nil, errors.New("key path not specified")
 	}
@@ -39,14 +40,17 @@ func NewSignerFromFiles(keyPath, certPath, envelopeMediaType string) (notation.S
 	}
 
 	// create signer
-	return NewSigner(cert.PrivateKey, certs, envelopeMediaType)
+	return NewSigner(cert.PrivateKey, certs)
 }
 
 // NewSigner creates a signer with the recommended signing method and a signing key bundled
 // with a certificate chain.
 // The relation of the provided signing key and its certificate chain is not verified,
 // and should be verified by the caller.
-func NewSigner(key crypto.PrivateKey, certChain []*x509.Certificate, envelopeMediaType string) (notation.Signer, error) {
+func NewSigner(key crypto.PrivateKey, certChain []*x509.Certificate) (notation.Signer, error) {
+	// TODO: pass media type as a parameter
+	envelopeMediaType := jws.MediaTypeEnvelope
+
 	builtinProvider, err := newBuiltinProvider(key, certChain)
 	if err != nil {
 		return nil, err

signature/signer_test.go

@@ -118,7 +118,7 @@ func testSignerFromFile(t *testing.T, keyCert *keyCertPair, envelopeType, dir st
 	if err != nil {
 		t.Fatalf("prepareTestKeyCertFile() failed: %v", err)
 	}
-	s, err := NewSignerFromFiles(keyPath, certPath, envelopeType)
+	s, err := NewSignerFromFiles(keyPath, certPath)
 	if err != nil {
 		t.Fatalf("NewSignerFromFiles() failed: %v", err)
 	}
@@ -161,7 +161,7 @@ func TestSignWithTimestamp(t *testing.T) {
 	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 := NewSigner(keyCert.key, keyCert.certs, envelopeType)
+				s, err := NewSigner(keyCert.key, keyCert.certs)
 				if err != nil {
 					t.Fatalf("NewSigner() error = %v", err)
 				}
@@ -192,7 +192,7 @@ func TestSignWithoutExpiry(t *testing.T) {
 	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 := NewSigner(keyCert.key, keyCert.certs, envelopeType)
+				s, err := NewSigner(keyCert.key, keyCert.certs)
 				if err != nil {
 					t.Fatalf("NewSigner() error = %v", err)
 				}
@@ -246,7 +246,7 @@ func TestExternalSigner_Sign(t *testing.T) {
 	for _, envelopeType := range signature.RegisteredEnvelopeTypes() {
 		for _, keyCert := range keyCertPairCollections {
 			externalRunner := newMockProvider(keyCert.key, keyCert.certs, testKeyID)
-			s, err := NewSignerPlugin(externalRunner, testKeyID, nil, envelopeType)
+			s, err := NewSignerPlugin(externalRunner, testKeyID, nil)
 			if err != nil {
 				t.Fatalf("NewSigner() error = %v", err)
 			}
@@ -266,7 +266,7 @@ func TestExternalSigner_SignEnvelope(t *testing.T) {
 			t.Run(fmt.Sprintf("envelopeType=%v_keySpec=%v", envelopeType, keyCert.keySpecName), func(t *testing.T) {
 				externalRunner := newMockEnvelopeProvider(keyCert.key, keyCert.certs, testKeyID)
 				p := newExternalProvider(externalRunner, testKeyID)
-				s, err := NewSignerPlugin(p, testKeyID, nil, envelopeType)
+				s, err := NewSignerPlugin(p, testKeyID, nil)
 				if err != nil {
 					t.Fatalf("NewSigner() error = %v", err)
 				}
@@ -338,7 +338,7 @@ func basicVerification(t *testing.T, sig []byte, envelopeType string, trust *x50
 }
 
 func validateSignWithCerts(t *testing.T, envelopeType string, key crypto.PrivateKey, certs []*x509.Certificate) {
-	s, err := NewSigner(key, certs, envelopeType)
+	s, err := NewSigner(key, certs)
 	if err != nil {
 		t.Fatalf("NewSigner() error = %v", err)
 	}

signature/verifier.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 
 	"github.com/notaryproject/notation-core-go/signature"
+	"github.com/notaryproject/notation-core-go/signature/jws"
 	x509n "github.com/notaryproject/notation-core-go/x509"
 	"github.com/notaryproject/notation-go"
 )
@@ -51,7 +52,8 @@ func ValidatePayloadContentType(payload *signature.Payload) error {
 // Verify verifies the signature and returns the verified descriptor and
 // metadata of the signed artifact.
 func (v *Verifier) Verify(_ context.Context, sig []byte, opts notation.VerifyOptions) (notation.Descriptor, error) {
-	sigEnv, err := signature.ParseEnvelope(opts.SignatureMediaType, sig)
+	// TODO: pass media type as a parameter
+	sigEnv, err := signature.ParseEnvelope(jws.MediaTypeEnvelope, sig)
 	if err != nil {
 		return notation.Descriptor{}, err
 	}

signature/verifier_test.go

@@ -22,7 +22,7 @@ func testVerifierFromFile(t *testing.T, keyCert *keyCertPair, envelopeType, dir
 	if err != nil {
 		t.Fatalf("prepare key cert file failed: %v", err)
 	}
-	s, err := NewSignerFromFiles(keyPath, certPath, envelopeType)
+	s, err := NewSignerFromFiles(keyPath, certPath)
 	if err != nil {
 		t.Fatalf("NewSignerFromFiles() failed: %v", err)
 	}
@@ -67,7 +67,7 @@ func TestVerifyWithCertChain(t *testing.T) {
 	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 := NewSigner(keyCert.key, keyCert.certs, envelopeType)
+				s, err := NewSigner(keyCert.key, keyCert.certs)
 				if err != nil {
 					t.Fatalf("NewSigner() error = %v", err)
 				}