Updated cose branch

go.mod

@@ -1,8 +1,8 @@
 module github.com/notaryproject/notation-core-go
 
-go 1.17
+go 1.18
 
-require github.com/golang-jwt/jwt/v4 v4.4.1
+require github.com/golang-jwt/jwt/v4 v4.4.2
 
 require (
 	github.com/fxamacker/cbor/v2 v2.4.0 // indirect

go.sum

@@ -1,7 +1,7 @@
 github.com/fxamacker/cbor/v2 v2.4.0 h1:ri0ArlOR+5XunOP8CRUowT0pSJOwhW098ZCUyskZD88=
 github.com/fxamacker/cbor/v2 v2.4.0/go.mod h1:TA1xS00nchWmaBnEIxPSE5oHLuJBAVvqrtAnWBwBCVo=
-github.com/golang-jwt/jwt/v4 v4.4.1 h1:pC5DB52sCeK48Wlb9oPcdhnjkz1TKt1D/P7WKJ0kUcQ=
-github.com/golang-jwt/jwt/v4 v4.4.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
+github.com/golang-jwt/jwt/v4 v4.4.2 h1:rcc4lwaZgFMCZ5jxF9ABolDcIHdBytAFgqFPbSJQAYs=
+github.com/golang-jwt/jwt/v4 v4.4.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
 github.com/veraison/go-cose v1.0.0-rc.1.0.20220824135457-9d2fab636b83 h1:g8vDfnNOPcGzg6mnlBGc0J5t5lAJkaepXqbc9qFRnFs=
 github.com/veraison/go-cose v1.0.0-rc.1.0.20220824135457-9d2fab636b83/go.mod h1:7ziE85vSq4ScFTg6wyoMXjucIGOf4JkFEZi/an96Ct4=
 github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=

signature/algorithm.go

@@ -31,18 +31,6 @@ const (
 	KeyTypeEC                     // KeyType EC
 )
 
-// String is the stringer function for KeyType
-func (keyType KeyType) String() string {
-	switch keyType {
-	case KeyTypeRSA:
-		return "RSA"
-	case KeyTypeEC:
-		return "ECDSA"
-	default:
-		return fmt.Sprintf("unknown key type: %d", keyType)
-	}
-}
-
 // KeySpec defines a key type and size.
 type KeySpec struct {
 	Type KeyType
@@ -74,7 +62,7 @@ func ExtractKeySpec(signingCert *x509.Certificate) (KeySpec, error) {
 			}, nil
 		default:
 			return KeySpec{}, &UnsupportedSigningKeyError{
-				fmt.Sprintf("rsa key size %d is not supported", bitSize),
+				Msg: fmt.Sprintf("rsa key size %d is not supported", bitSize),
 			}
 		}
 	case *ecdsa.PublicKey:
@@ -86,11 +74,13 @@ func ExtractKeySpec(signingCert *x509.Certificate) (KeySpec, error) {
 			}, nil
 		default:
 			return KeySpec{}, &UnsupportedSigningKeyError{
-				fmt.Sprintf("ecdsa key size %d is not supported", bitSize),
+				Msg: fmt.Sprintf("ecdsa key size %d is not supported", bitSize),
 			}
 		}
 	}
-	return KeySpec{}, fmt.Errorf("invalid public key type")
+	return KeySpec{}, &UnsupportedSigningKeyError{
+		Msg: "invalid public key type",
+	}
 }
 
 // SignatureAlgorithm returns the signing algorithm associated with the KeySpec.

signature/algorithm_test.go

@@ -229,28 +229,3 @@ func TestSignatureAlgorithm(t *testing.T) {
 		})
 	}
 }
-
-func TestKeyTypeStringer(t *testing.T) {
-	testCase := []struct {
-		keyType KeyType
-		str     string
-	}{
-		{
-			keyType: KeyTypeEC,
-			str:     "ECDSA",
-		},
-		{
-			keyType: KeyTypeRSA,
-			str:     "RSA",
-		},
-		{
-			keyType: 10,
-			str:     "unknown key type: 10",
-		},
-	}
-	for _, tt := range testCase {
-		if tt.keyType.String() != tt.str {
-			t.Fatalf("keyType: %s stringer test failed", tt.keyType)
-		}
-	}
-}

signature/cose/envelope_test.go

@@ -336,19 +336,19 @@ func TestVerifyErrors(t *testing.T) {
 		}
 	})
 
-	t.Run("when getSignatureAlgorithm fails due to invalid public key type", func(t *testing.T) {
+	t.Run("when getSignatureAlgorithm fails due to unsupported public key", func(t *testing.T) {
 		env, err := getVerifyCOSE("notary.x509", signature.KeyTypeRSA, 3072)
 		if err != nil {
 			t.Fatalf("getVerifyCOSE() failed. Error = %s", err)
 		}
-		certs := []*x509.Certificate{testhelper.GetED25519LeafCertificate().Cert, testhelper.GetED25519RootCertificate().Cert}
+		certs := []*x509.Certificate{testhelper.GetUnsupportedRSACert().Cert}
 		certChain := make([]interface{}, len(certs))
 		for i, c := range certs {
 			certChain[i] = c.Raw
 		}
 		env.base.Headers.Unprotected[cose.HeaderLabelX5Chain] = certChain
 		_, _, err = env.Verify()
-		expected := errors.New("invalid public key type")
+		expected := errors.New("rsa key size 1024 is not supported")
 		if !isErrEqual(expected, err) {
 			t.Fatalf("Verify() expects error: %v, but got: %v.", expected, err)
 		}

signature/envelope.go

@@ -1,3 +1,13 @@
+// Package signature provides operations for types that implement
+// signature.Envelope or signature.Signer.
+//
+// An Envelope is a structure that creates and verifies a signature using the
+// specified signing algorithm with required validation. To register a new
+// envelope, call RegisterEnvelopeType first during the initialization.
+//
+// A Signer is a structure used to sign payload generated after signature
+// envelope created. The underlying signing logic is provided by the underlying
+// local crypto library or the external signing plugin.
 package signature
 
 import "fmt"
@@ -33,9 +43,9 @@ type envelopeFunc struct {
 	parseFunc ParseEnvelopeFunc
 }
 
-// envelopeFuncs maps envelope media type to corresponding constructors and 
+// envelopeFuncs maps envelope media type to corresponding constructors and
 // parsers.
-var envelopeFuncs = make(map[string]envelopeFunc)
+var envelopeFuncs map[string]envelopeFunc
 
 // RegisterEnvelopeType registers newFunc and parseFunc for the given mediaType.
 // Those functions are intended to be called when creating a new envelope.
@@ -44,6 +54,9 @@ func RegisterEnvelopeType(mediaType string, newFunc NewEnvelopeFunc, parseFunc P
 	if newFunc == nil || parseFunc == nil {
 		return fmt.Errorf("required functions not provided")
 	}
+	if envelopeFuncs == nil {
+		envelopeFuncs = make(map[string]envelopeFunc)
+	}
 
 	envelopeFuncs[mediaType] = envelopeFunc{
 		newFunc:   newFunc,
@@ -54,7 +67,7 @@ func RegisterEnvelopeType(mediaType string, newFunc NewEnvelopeFunc, parseFunc P
 
 // RegisteredEnvelopeTypes lists registered envelope media types.
 func RegisteredEnvelopeTypes() []string {
-	types := []string{}
+	var types []string
 
 	for envelopeType := range envelopeFuncs {
 		types = append(types, envelopeType)
@@ -72,7 +85,7 @@ func NewEnvelope(mediaType string) (Envelope, error) {
 	return envelopeFunc.newFunc(), nil
 }
 
-// NewEnvelope generates an envelope by given envelope bytes with specified
+// ParseEnvelope generates an envelope by given envelope bytes with specified
 // media type.
 func ParseEnvelope(mediaType string, envelopeBytes []byte) (Envelope, error) {
 	envelopeFunc, ok := envelopeFuncs[mediaType]

signature/envelope_test.go

@@ -89,7 +89,7 @@ func TestRegisteredEnvelopeTypes(t *testing.T) {
 		{
 			name:          "empty map",
 			envelopeFuncs: make(map[string]envelopeFunc),
-			expect:        []string{},
+			expect:        nil,
 		},
 		{
 			name: "nonempty map",
@@ -106,7 +106,7 @@ func TestRegisteredEnvelopeTypes(t *testing.T) {
 			types := RegisteredEnvelopeTypes()
 
 			if !reflect.DeepEqual(types, tt.expect) {
-				t.Errorf("got types: %v, expect types; %v", types, tt.expect)
+				t.Errorf("got types: %v, expect types: %v", types, tt.expect)
 			}
 		})
 	}

signature/internal/base/envelope.go

@@ -11,16 +11,17 @@ import (
 
 // Envelope represents a general envelope wrapping a raw signature and envelope
 // in specific format.
+// Envelope manipulates the common validation shared by internal envelopes.
 type Envelope struct {
-	signature.Envelope        // internal envelope in a specific format(COSE/JWS)
+	signature.Envelope        // internal envelope in a specific format(e.g. Cose, JWS)
 	Raw                []byte // raw signature
 }
 
 // Sign generates signature in terms of given SignRequest.
 //
 // Reference: https://github.com/notaryproject/notaryproject/blob/main/signing-and-verification-workflow.md#signing-steps
 func (e *Envelope) Sign(req *signature.SignRequest) ([]byte, error) {
-	// Sanitize request
+	// Canonicalize request.
 	req.SigningTime = req.SigningTime.Truncate(time.Second)
 	req.Expiry = req.Expiry.Truncate(time.Second)
 	err := validateSignRequest(req)
@@ -63,10 +64,6 @@ func (e *Envelope) Verify() (*signature.Payload, *signature.SignerInfo, error) {
 		return nil, nil, &signature.MalformedSignatureError{}
 	}
 
-	if err := e.validatePayload(); err != nil {
-		return nil, nil, err
-	}
-
 	// core verify process.
 	payload, signerInfo, err := e.Envelope.Verify()
 	if err != nil {

signature/internal/base/envelope_test.go

@@ -251,26 +251,6 @@ func TestVerify(t *testing.T) {
 			expectSignerInfo: nil,
 			expectErr:        true,
 		},
-		{
-			name: "invalid payload",
-			env: &Envelope{
-				Raw:      validBytes,
-				Envelope: &mockEnvelope{},
-			},
-			expectPayload:    nil,
-			expectSignerInfo: nil,
-			expectErr:        true,
-		},
-		{
-			name: "invalid payload",
-			env: &Envelope{
-				Raw:      validBytes,
-				Envelope: &mockEnvelope{},
-			},
-			expectPayload:    nil,
-			expectSignerInfo: nil,
-			expectErr:        true,
-		},
 		{
 			name: "err returned by internal envelope",
 			env: &Envelope{
@@ -736,17 +716,7 @@ func TestValidateCertificateChain(t *testing.T) {
 			expectErr: true,
 		},
 		{
-			name: "unsupported algorithm",
-			certs: []*x509.Certificate{
-				testhelper.GetED25519LeafCertificate().Cert,
-				testhelper.GetED25519RootCertificate().Cert,
-			},
-			signTime:  testhelper.GetED25519LeafCertificate().Cert.NotBefore,
-			alg:       signature.AlgorithmES256,
-			expectErr: true,
-		},
-		{
-			name: "unmacthed signing algorithm",
+			name: "unmatched signing algorithm",
 			certs: []*x509.Certificate{
 				testhelper.GetRSALeafCertificate().Cert,
 				testhelper.GetRSARootCertificate().Cert,
@@ -772,9 +742,7 @@ func TestValidateCertificateChain(t *testing.T) {
 			err := validateCertificateChain(tt.certs, tt.signTime, tt.alg)
 
 			if (err != nil) != tt.expectErr {
-				if (err != nil) != tt.expectErr {
-					t.Errorf("error = %v, expectErr = %v", err, tt.expectErr)
-				}
+				t.Errorf("error = %v, expectErr = %v", err, tt.expectErr)
 			}
 		})
 	}
@@ -789,7 +757,7 @@ func TestGetSignatureAlgorithm(t *testing.T) {
 	}{
 		{
 			name:      "unsupported cert",
-			cert:      testhelper.GetUnsupportedCertificate().Cert,
+			cert:      testhelper.GetUnsupportedRSACert().Cert,
 			expect:    0,
 			expectErr: true,
 		},

signature/signer.go

@@ -29,8 +29,12 @@ type LocalSigner interface {
 	PrivateKey() crypto.PrivateKey
 }
 
-// signer is a LocalSigner implementation.
-type signer struct {
+// localSigner implements LocalSigner interface.
+//
+// Note that localSigner only holds the signing key, keySpec and certificate
+// chain. The underlying signing implementation is provided by the underlying
+// crypto library for the specific signature format like go-jwt or go-cose.
+type localSigner struct {
 	keySpec KeySpec
 	key     crypto.PrivateKey
 	certs   []*x509.Certificate
@@ -57,7 +61,7 @@ func NewLocalSigner(certs []*x509.Certificate, key crypto.PrivateKey) (LocalSign
 		}
 	}
 
-	return &signer{
+	return &localSigner{
 		keySpec: keySpec,
 		key:     key,
 		certs:   certs,
@@ -84,24 +88,24 @@ func isKeyPair(priv crypto.PrivateKey, pub crypto.PublicKey, keySpec KeySpec) bo
 	}
 }
 
-// Sign signs the digest and returns the raw signature and certificates.
+// Sign signs the content and returns the raw signature and certificates.
 // This implementation should never be used by built-in signers.
-func (s *signer) Sign(digest []byte) ([]byte, []*x509.Certificate, error) {
-	return nil, nil, fmt.Errorf("local signer doesn't support sign with digest")
+func (s *localSigner) Sign(content []byte) ([]byte, []*x509.Certificate, error) {
+	return nil, nil, fmt.Errorf("local signer doesn't support sign")
 }
 
 // KeySpec returns the key specification.
-func (s *signer) KeySpec() (KeySpec, error) {
+func (s *localSigner) KeySpec() (KeySpec, error) {
 	return s.keySpec, nil
 }
 
 // CertificateChain returns the certificate chain.
-func (s *signer) CertificateChain() ([]*x509.Certificate, error) {
+func (s *localSigner) CertificateChain() ([]*x509.Certificate, error) {
 	return s.certs, nil
 }
 
 // PrivateKey returns the private key.
-func (s *signer) PrivateKey() crypto.PrivateKey {
+func (s *localSigner) PrivateKey() crypto.PrivateKey {
 	return s.key
 }
 
@@ -120,8 +124,8 @@ func VerifyAuthenticity(signerInfo *SignerInfo, trustedCerts []*x509.Certificate
 	}
 
 	for _, trust := range trustedCerts {
-		for _, sig := range signerInfo.CertificateChain {
-			if trust.Equal(sig) {
+		for _, cert := range signerInfo.CertificateChain {
+			if trust.Equal(cert) {
 				return trust, nil
 			}
 		}