Merge pull request #270 from onflow/revert-266-tarak/6216-crypto-kms

Revert "[Crypto] general updates"

account.go

@@ -95,32 +95,15 @@ func (a AccountKey) Encode() []byte {
 	return mustRLPEncode(&temp)
 }
 
-// accountCompatibleAlgorithms returns true if the signature and hash algorithms are a valid pair
-// for a key on a Flow account.
-func accountCompatibleAlgorithms(sigAlgo crypto.SignatureAlgorithm, hashAlgo crypto.HashAlgorithm) bool {
-	switch sigAlgo {
-	case crypto.ECDSA_P256:
-		fallthrough
-	case crypto.ECDSA_secp256k1:
-		switch hashAlgo {
-		case crypto.SHA2_256:
-			fallthrough
-		case crypto.SHA3_256:
-			return true
-		}
-	}
-	return false
-}
-
 // Validate returns an error if this account key is invalid.
 //
 // An account key can be invalid for the following reasons:
-// - It specifies an incompatible signature/hash algorithm pair with regards to Flow accounts
+// - It specifies an incompatible signature/hash algorithm pairing
 // - It specifies a valid key weight
 func (a AccountKey) Validate() error {
-	if !accountCompatibleAlgorithms(a.SigAlgo, a.HashAlgo) {
+	if !crypto.CompatibleAlgorithms(a.SigAlgo, a.HashAlgo) {
 		return errors.Errorf(
-			"signing algorithm (%s) and hashing algorithm (%s) are not a valid pair for a Flow account key",
+			"signing algorithm (%s) is incompatible with hashing algorithm (%s)",
 			a.SigAlgo,
 			a.HashAlgo,
 		)

account_test.go

@@ -20,7 +20,6 @@ package flow
 
 import (
 	"crypto/rand"
-	"fmt"
 	"testing"
 
 	"github.com/onflow/flow-go-sdk/crypto"
@@ -78,43 +77,18 @@ func TestAccountKey(t *testing.T) {
 		assert.EqualError(t, key.Validate(), "invalid key weight: -1")
 	})
 
-	t.Run("Key Algorithm", func(t *testing.T) {
-		hashAlgos := []crypto.HashAlgorithm{
-			crypto.UnknownHashAlgorithm,
-			crypto.SHA2_256,
-			crypto.SHA2_384,
-			crypto.SHA3_256,
-			crypto.SHA3_384,
-			crypto.Keccak256,
-		}
-		signAlgos := []crypto.SignatureAlgorithm{
-			crypto.UnknownSignatureAlgorithm,
-			crypto.ECDSA_P256,
-			crypto.ECDSA_secp256k1,
+	t.Run("Invalid Key Algorithm", func(t *testing.T) {
+		privateKey := generateKey()
+		key := AccountKey{
+			PublicKey: privateKey.PublicKey(),
 		}
 
-		validPairs := map[crypto.SignatureAlgorithm]map[crypto.HashAlgorithm]bool{
-			crypto.ECDSA_P256: map[crypto.HashAlgorithm]bool{
-				crypto.SHA2_256: true,
-				crypto.SHA3_256: true,
-			},
-			crypto.ECDSA_secp256k1: map[crypto.HashAlgorithm]bool{
-				crypto.SHA2_256: true,
-				crypto.SHA3_256: true,
-			},
-		}
+		key.SetSigAlgo(privateKey.Algorithm())
+		assert.EqualError(t, key.Validate(), "signing algorithm (ECDSA_P256) is incompatible with hashing algorithm (UNKNOWN)")
 
-		key := AccountKey{}
-		for _, s := range signAlgos {
-			for _, h := range hashAlgos {
-				key.SetSigAlgo(s)
-				key.SetHashAlgo(h)
-				if validPairs[s][h] {
-					assert.NoError(t, key.Validate())
-				} else {
-					assert.EqualError(t, key.Validate(), fmt.Sprintf("signing algorithm (%s) and hashing algorithm (%s) are not a valid pair for a Flow account key", s, h))
-				}
-			}
-		}
+		key.SetHashAlgo(crypto.SHA3_256)
+		key.SetSigAlgo(crypto.UnknownSignatureAlgorithm)
+		assert.EqualError(t, key.Validate(), "signing algorithm (UNKNOWN) is incompatible with hashing algorithm (SHA3_256)")
 	})
+
 }

crypto/cloudkms/cloudkms.go

@@ -64,7 +64,6 @@ func (k Key) ResourceID() string {
 	)
 }
 
-// KeyFromResourceID returns a `Key` from a resource ID.
 func KeyFromResourceID(resourceID string) (Key, error) {
 	key := Key{}
 
@@ -104,8 +103,7 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
 
 // GetPublicKey fetches the public key portion of a KMS asymmetric signing key version.
 //
-// KMS keys of the type `CryptoKeyVersion_EC_SIGN_P256_SHA256` and `CryptoKeyVersion_EC_SIGN_SECP256K1_SHA256`
-// are the only keys supported by the SDK.
+// ECDSA_P256 is currently the only Flow signature algorithm supported by Google Cloud KMS.
 //
 // Ref: https://cloud.google.com/kms/docs/retrieve-public-key
 func (c *Client) GetPublicKey(ctx context.Context, key Key) (crypto.PublicKey, crypto.HashAlgorithm, error) {
@@ -156,25 +154,22 @@ func (c *Client) KMSClient() *kms.KeyManagementClient {
 	return c.client
 }
 
-// ParseSignatureAlgorithm returns the `SignatureAlgorithm` corresponding to the input KMS key type.
 func ParseSignatureAlgorithm(algo kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm) crypto.SignatureAlgorithm {
 	if algo == kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256 {
 		return crypto.ECDSA_P256
 	}
 
-	if algo == kmspb.CryptoKeyVersion_EC_SIGN_SECP256K1_SHA256 {
-		return crypto.ECDSA_secp256k1
-	}
-
-	return crypto.UnknownSignatureAlgorithm
+	// TODO: update this once Google KMS API supports ECDSA_secp256k1
+	// https://github.com/onflow/flow-go-sdk/issues/193
+	return crypto.ECDSA_secp256k1
 }
 
-// ParseHashAlgorithm returns the `HashAlgorithm` corresponding to the input KMS key type.
 func ParseHashAlgorithm(algo kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm) crypto.HashAlgorithm {
-	if algo == kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256 || algo == kmspb.CryptoKeyVersion_EC_SIGN_SECP256K1_SHA256 {
+	if algo == kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256 {
 		return crypto.SHA2_256
 	}
 
-	// the function can be extended to return SHA3-256 if it becomes supported by KMS.
-	return crypto.UnknownHashAlgorithm
+	// TODO: update this once Google KMS API supports ECDSA_secp256k1
+	// https://github.com/onflow/flow-go-sdk/issues/193
+	return crypto.SHA2_256
 }

crypto/cloudkms/signer.go

@@ -22,38 +22,31 @@ import (
 	"context"
 	"encoding/asn1"
 	"fmt"
-	"hash/crc32"
 	"math/big"
 
 	kms "cloud.google.com/go/kms/apiv1"
-	"github.com/onflow/flow-go-sdk/crypto"
 	kmspb "google.golang.org/genproto/googleapis/cloud/kms/v1"
-	"google.golang.org/protobuf/types/known/wrapperspb"
+
+	"github.com/onflow/flow-go-sdk/crypto"
 )
 
 var _ crypto.Signer = (*Signer)(nil)
 
 // Signer is a Google Cloud KMS implementation of crypto.Signer.
 type Signer struct {
-	ctx    context.Context
-	client *kms.KeyManagementClient
-	key    Key
-	// ECDSA is the only algorithm supported by this package. The signature algorithm
-	// therefore represents the elliptic curve used. The curve is needed to parse the kms signature.
-	curve crypto.SignatureAlgorithm
-	// public key for easier access
+	ctx       context.Context
+	client    *kms.KeyManagementClient
+	key       Key
+	hashAlgo  crypto.HashAlgorithm
 	publicKey crypto.PublicKey
 }
 
-// SignerForKey returns a new Google Cloud KMS signer for an asymmetric signing key version.
-//
-// Only ECDSA keys on P-256 and secp256k1 curves and SHA2-256 are supported.
+// SignerForKey returns a new Google Cloud KMS signer for an asymmetric key version.
 func (c *Client) SignerForKey(
 	ctx context.Context,
 	key Key,
 ) (*Signer, error) {
-
-	pk, _, err := c.GetPublicKey(ctx, key)
+	pk, hashAlgo, err := c.GetPublicKey(ctx, key)
 	if err != nil {
 		return nil, err
 	}
@@ -62,7 +55,7 @@ func (c *Client) SignerForKey(
 		ctx:       ctx,
 		client:    c.client,
 		key:       key,
-		curve:     pk.Algorithm(),
+		hashAlgo:  hashAlgo,
 		publicKey: pk,
 	}, nil
 }
@@ -71,64 +64,77 @@ func (c *Client) SignerForKey(
 //
 // Reference: https://cloud.google.com/kms/docs/create-validate-signatures
 func (s *Signer) Sign(message []byte) ([]byte, error) {
+	hasher, err := crypto.NewHasher(s.hashAlgo)
+	if err != nil {
+		return nil, fmt.Errorf("cloudkms: failed to instantiate hasher: %w", err)
+	}
+	digest := hasher.ComputeHash(message)
+
+	digestMessage, err := makeDigest(s.hashAlgo, digest)
+	if err != nil {
+		return nil, fmt.Errorf("cloudkms: failed to construct digest: %w", err)
+	}
 
 	request := &kmspb.AsymmetricSignRequest{
-		Name:       s.key.ResourceID(),
-		Data:       message,
-		DataCrc32C: checksum(message),
+		Name:   s.key.ResourceID(),
+		Digest: digestMessage,
 	}
 
 	result, err := s.client.AsymmetricSign(s.ctx, request)
 	if err != nil {
 		return nil, fmt.Errorf("cloudkms: failed to sign: %w", err)
 	}
 
-	sig, err := parseSignature(result.Signature, s.curve)
+	sig, err := parseSignature(result.Signature)
 	if err != nil {
 		return nil, fmt.Errorf("cloudkms: failed to parse signature: %w", err)
 	}
 
 	return sig, nil
 }
 
-func checksum(data []byte) *wrapperspb.Int64Value {
-	// compute the checksum
-	checksum := crc32.ChecksumIEEE(data)
-	val := wrapperspb.Int64(int64(checksum))
-	return val
+func makeDigest(hashAlgo crypto.HashAlgorithm, digest []byte) (*kmspb.Digest, error) {
+	switch hashAlgo {
+	case crypto.SHA2_256:
+		return &kmspb.Digest{Digest: &kmspb.Digest_Sha256{Sha256: digest}}, nil
+	case crypto.SHA2_384:
+		return &kmspb.Digest{Digest: &kmspb.Digest_Sha384{Sha384: digest}}, nil
+	}
+
+	return nil, fmt.Errorf("unsupported hash algorithm %s", hashAlgo)
 }
 
-// parseSignature parses an asn1 stucture (R,S) into a slice of bytes as required by the `Siger.Sign` method.
-func parseSignature(kmsSignature []byte, curve crypto.SignatureAlgorithm) ([]byte, error) {
+// ecCoupleComponentSize is size of a component in either (r,s) couple for an elliptical curve signature
+// or (x,y) identifying a public key. Component size is needed for encoding couples comprised of variable length
+// numbers to []byte encoding. They are not always the same length, so occasionally padding is required.
+// Here's how one calculates the required length of each component:
+// 		ECDSA_CurveBits = 256
+// 		ecCoupleComponentSize := ECDSA_CurveBits / 8
+// 		if ECDSA_CurveBits % 8 > 0 {
+//			ecCoupleComponentSize++
+// 		}
+const ecCoupleComponentSize = 32
+
+func parseSignature(signature []byte) ([]byte, error) {
 	var parsedSig struct{ R, S *big.Int }
-	if _, err := asn1.Unmarshal(kmsSignature, &parsedSig); err != nil {
+	if _, err := asn1.Unmarshal(signature, &parsedSig); err != nil {
 		return nil, fmt.Errorf("asn1.Unmarshal: %w", err)
 	}
 
-	curveOrderLen := curveOrder(curve)
-	signature := make([]byte, 2*curveOrderLen)
-
-	// left pad R and S with zeroes
 	rBytes := parsedSig.R.Bytes()
+	rBytesPadded := rightPad(rBytes, ecCoupleComponentSize)
+
 	sBytes := parsedSig.S.Bytes()
-	copy(signature[curveOrderLen-len(rBytes):], rBytes)
-	copy(signature[len(signature)-len(sBytes):], sBytes)
+	sBytesPadded := rightPad(sBytes, ecCoupleComponentSize)
 
-	return signature, nil
+	return append(rBytesPadded, sBytesPadded...), nil
 }
 
-// returns the curve order size in bytes (used to padd R and S of the ECDSA signature)
-// Only P-256 and secp256k1 are supported. The calling function should make sure
-// the function is only called with one of the 2 curves.
-func curveOrder(curve crypto.SignatureAlgorithm) int {
-	switch curve {
-	case crypto.ECDSA_P256:
-		return 32
-	case crypto.ECDSA_secp256k1:
-		return 32
-	default:
-		return 0 // or panic? this only happens if there is an implementation bug
-	}
+// rightPad pads a byte slice with empty bytes (0x00) to the given length.
+func rightPad(b []byte, length int) []byte {
+	padded := make([]byte, length)
+	copy(padded[length-len(b):], b)
+	return padded
 }
 
 func (s *Signer) PublicKey() crypto.PublicKey {

crypto/crypto.go

@@ -53,11 +53,31 @@ func StringToSignatureAlgorithm(s string) SignatureAlgorithm {
 	}
 }
 
-// CompatibleAlgorithms returns true if the signature and hash algorithms is a valid pair for a signing key
-// supported by the package.
-//
-// The package currently supports ECDSA with the 2 curves P-256 and secp256k1. Both curves can be paired with
-// a supported hash function of 256-bits output (SHA2-256, SHA3-256, Keccak256)
+// HashAlgorithm is an identifier for a hash algorithm.
+type HashAlgorithm = hash.HashingAlgorithm
+
+const (
+	UnknownHashAlgorithm HashAlgorithm = hash.UnknownHashingAlgorithm
+	SHA2_256                           = hash.SHA2_256
+	SHA2_384                           = hash.SHA2_384
+	SHA3_256                           = hash.SHA3_256
+	SHA3_384                           = hash.SHA3_384
+)
+
+// StringToHashAlgorithm converts a string to a HashAlgorithm.
+func StringToHashAlgorithm(s string) HashAlgorithm {
+	switch s {
+	case SHA2_256.String():
+		return SHA2_256
+	case SHA3_256.String():
+		return SHA3_256
+
+	default:
+		return UnknownHashAlgorithm
+	}
+}
+
+// CompatibleAlgorithms returns true if the signature and hash algorithms are compatible.
 func CompatibleAlgorithms(sigAlgo SignatureAlgorithm, hashAlgo HashAlgorithm) bool {
 	switch sigAlgo {
 	case ECDSA_P256:
@@ -67,8 +87,6 @@ func CompatibleAlgorithms(sigAlgo SignatureAlgorithm, hashAlgo HashAlgorithm) bo
 		case SHA2_256:
 			fallthrough
 		case SHA3_256:
-			fallthrough
-		case Keccak256:
 			return true
 		}
 	}
@@ -101,15 +119,8 @@ type InMemorySigner struct {
 var _ Signer = (*InMemorySigner)(nil)
 
 // NewInMemorySigner initializes and returns a new in-memory signer with the provided private key
-// and hashing algorithm.
+// and hasher.
 func NewInMemorySigner(privateKey PrivateKey, hashAlgo HashAlgorithm) InMemorySigner {
-	// check compatibility to form a signing key
-	if !CompatibleAlgorithms(privateKey.Algorithm(), hashAlgo) {
-		// TODO: panic?
-		return InMemorySigner{}
-	}
-
-	// The error is ignored because the hash algorithm is valid at this point
 	hasher, _ := NewHasher(hashAlgo)
 
 	return InMemorySigner{