refactor(crypto): handle errors and more in bls12381 (cometbft#2832)

- pin cometbft-db-testing image version - extract const in a separate file - return err in GenPrivKey and NewPrivateKeyFromBytes Follow-up to cometbft#2765 --- #### PR checklist - [x] Tests written/updated - [ ] ~~Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~~ - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Sergio Mena <[email protected]>
InjectiveLabs · Apr 18, 2024 · d27732a · d27732a
1 parent b846639
commit d27732a
Show file tree

Hide file tree

Showing 7 changed files with 173 additions and 72 deletions.
diff --git a/.changelog/unreleased/features/2765-bls12-381-curve.md b/.changelog/unreleased/features/2765-bls12-381-curve.md
@@ -1,2 +1,3 @@
-- `[crypto]` Add support for BLS12-381 keys. Use `bls12381` build flag to enable
-  it ([\#2765](https://github.com/cometbft/cometbft/pull/2765))
+- `[crypto]` Add support for BLS12-381 keys. Since the implementation needs
+  `cgo` and brings in new dependencies, we use the `bls12381` build flag to
+  enable it ([\#2765](https://github.com/cometbft/cometbft/pull/2765))
diff --git a/crypto/bls12381/const.go b/crypto/bls12381/const.go
@@ -0,0 +1,18 @@
+package bls12381
+
+const (
+	// PrivKeySize defines the length of the PrivKey byte array.
+	PrivKeySize = 32
+	// PubKeySize defines the length of the PubKey byte array.
+	PubKeySize = 48
+	// SignatureLength defines the byte length of a BLS signature.
+	SignatureLength = 96
+	// KeyType is the string constant for the BLS12-381 algorithm.
+	KeyType = "bls12_381"
+	// MaxMsgLen defines the maximum length of the message bytes as passed to Sign.
+	MaxMsgLen = 32
+	// BLS12-381 private key name.
+	PrivKeyName = "cometbft/PrivKeyBls12_381"
+	// BLS12-381 public key name.
+	PubKeyName = "cometbft/PubKeyBls12_381"
+)
diff --git a/crypto/bls12381/key.go b/crypto/bls12381/key.go
@@ -1,72 +1,66 @@
-//go:build !bls12381 || !((linux && amd64) || (linux && arm64) || (darwin && amd64) || (darwin && arm64) || (windows && amd64))
+//go:build !(((linux && amd64) || (linux && arm64) || (darwin && amd64) || (darwin && arm64) || (windows && amd64)) && bls12381)
 
 package bls12381
 
 import (
+	"errors"
+
 	"github.com/cometbft/cometbft/crypto"
 )
 
 const (
-	// PrivKeySize defines the length of the PrivKey byte array.
-	PrivKeySize = 32
-	// PubKeySize defines the length of the PubKey byte array.
-	PubKeySize = 48
-	// SignatureLength defines the byte length of a BLS signature.
-	SignatureLength = 96
-	// KeyType is the string constant for the bls12_381 algorithm.
-	KeyType = "bls12_381"
 	// Enabled indicates if this curve is enabled.
 	Enabled = false
 )
 
-// -------------------------------------.
-const (
-	PubKeyName = "cometbft/PubKeyBLS12_381"
-)
+// ErrDisabled is returned if the caller didn't use the `bls12381` build tag or has an incompatible OS.
+var ErrDisabled = errors.New("bls12_381 is disabled")
 
 // ===============================================================================================
 // Private Key
 // ===============================================================================================
 
-// PrivKey is a wrapper around the Ethereum bls12_381 private key type. This
+// PrivKey is a wrapper around the Ethereum BLS12-381 private key type. This
 // wrapper conforms to crypto.Pubkey to allow for the use of the Ethereum
-// bls12_381 private key type.
+// BLS12-381 private key type.
 
 // Compile-time type assertion.
 var _ crypto.PrivKey = &PrivKey{}
 
 // PrivKey represents a BLS private key noop when blst is not set as a build flag and cgo is disabled.
 type PrivKey []byte
 
+// NewPrivateKeyFromBytes returns ErrDisabled.
 func NewPrivateKeyFromBytes([]byte) (PrivKey, error) {
-	panic("bls12_381 is disabled")
+	return nil, ErrDisabled
 }
 
+// GenPrivKey returns ErrDisabled.
 func GenPrivKey() (PrivKey, error) {
-	panic("bls12_381 is disabled")
+	return nil, ErrDisabled
 }
 
-// Bytes returns the byte representation of the ECDSA Private Key.
+// Bytes returns the byte representation of the Key.
 func (privKey PrivKey) Bytes() []byte {
 	return privKey
 }
 
-// PubKey returns the ECDSA private key's public key. If the privkey is not valid
-// it returns a nil value.
+// PubKey always panics.
 func (PrivKey) PubKey() crypto.PubKey {
 	panic("bls12_381 is disabled")
 }
 
-// Equals returns true if two ECDSA private keys are equal and false otherwise.
+// Equals always panics.
 func (PrivKey) Equals(crypto.PrivKey) bool {
 	panic("bls12_381 is disabled")
 }
 
-// Type returns eth_bls12_381.
+// Type returns the key's type.
 func (PrivKey) Type() string {
 	return KeyType
 }
 
+// Sign always panics.
 func (PrivKey) Sign([]byte) ([]byte, error) {
 	panic("bls12_381 is disabled")
 }
@@ -75,37 +69,37 @@ func (PrivKey) Sign([]byte) ([]byte, error) {
 // Public Key
 // ===============================================================================================
 
-// Pubkey is a wrapper around the Ethereum bls12_381 public key type. This
+// Pubkey is a wrapper around the Ethereum BLS12-381 public key type. This
 // wrapper conforms to crypto.Pubkey to allow for the use of the Ethereum
-// bls12_381 public key type.
+// BLS12-381 public key type.
 
 // Compile-time type assertion.
 var _ crypto.PubKey = &PubKey{}
 
 // PubKey represents a BLS private key noop when blst is not set as a build flag and cgo is disabled.
 type PubKey []byte
 
-// Address returns the address of the ECDSA public key.
-// The function will return an empty address if the public key is invalid.
+// Address always panics.
 func (PubKey) Address() crypto.Address {
 	panic("bls12_381 is disabled")
 }
 
+// VerifySignature always panics.
 func (PubKey) VerifySignature([]byte, []byte) bool {
 	panic("bls12_381 is disabled")
 }
 
-// Bytes returns the pubkey byte format.
+// Bytes always panics.
 func (PubKey) Bytes() []byte {
 	panic("bls12_381 is disabled")
 }
 
-// Type returns eth_bls12_381.
+// Type returns the key's type.
 func (PubKey) Type() string {
 	return KeyType
 }
 
-// Equals returns true if the pubkey type is the same and their bytes are deeply equal.
+// Equals always panics.
 func (PubKey) Equals(crypto.PubKey) bool {
 	panic("bls12_381 is disabled")
 }
diff --git a/crypto/bls12381/key_bls12381.go b/crypto/bls12381/key_bls12381.go
@@ -15,23 +15,11 @@ import (
 )
 
 const (
-	// PrivKeySize defines the length of the PrivKey byte array.
-	PrivKeySize = 32
-	// PubKeySize defines the length of the PubKey byte array.
-	PubKeySize = 48
-	// SignatureLength defines the byte length of a BLS signature.
-	SignatureLength = 96
-	// KeyType is the string constant for the bls12_381 algorithm.
-	KeyType = "bls12_381"
 	// Enabled indicates if this curve is enabled.
 	Enabled = true
 )
 
 // -------------------------------------.
-const (
-	PrivKeyName = "cometbft/PrivKeyBLS12_381"
-	PubKeyName  = "cometbft/PubKeyBLS12_381"
-)
 
 func init() {
 	cmtjson.RegisterType(PubKey{}, PubKeyName)
@@ -42,14 +30,15 @@ func init() {
 // Private Key
 // ===============================================================================================
 
-// PrivKey is a wrapper around the Ethereum bls12_381 private key type. This
+// PrivKey is a wrapper around the Ethereum BLS12-381 private key type. This
 // wrapper conforms to crypto.Pubkey to allow for the use of the Ethereum
-// bls12_381 private key type.
+// BLS12-381 private key type.
 
 var _ crypto.PrivKey = &PrivKey{}
 
 type PrivKey []byte
 
+// NewPrivateKeyFromBytes build a new key from the given bytes.
 func NewPrivateKeyFromBytes(bz []byte) (PrivKey, error) {
 	secretKey, err := bls12381.SecretKeyFromBytes(bz)
 	if err != nil {
@@ -58,102 +47,113 @@ func NewPrivateKeyFromBytes(bz []byte) (PrivKey, error) {
 	return secretKey.Marshal(), nil
 }
 
+// GenPrivKey generates a new key.
 func GenPrivKey() (PrivKey, error) {
 	secretKey, err := bls12381.RandKey()
 	return PrivKey(secretKey.Marshal()), err
 }
 
-// Bytes returns the byte representation of the Private Key.
+// Bytes returns the byte representation of the Key.
 func (privKey PrivKey) Bytes() []byte {
 	return privKey
 }
 
 // PubKey returns the private key's public key. If the privkey is not valid
 // it returns a nil value.
 func (privKey PrivKey) PubKey() crypto.PubKey {
-	secretKey, _ := bls12381.SecretKeyFromBytes(privKey)
+	secretKey, err := bls12381.SecretKeyFromBytes(privKey)
+	if err != nil {
+		return nil
+	}
 
 	return PubKey(secretKey.PublicKey().Marshal())
 }
 
-// Equals returns true if two private keys are equal and false otherwise.
+// Equals returns true if two keys are equal and false otherwise.
 func (privKey PrivKey) Equals(other crypto.PrivKey) bool {
 	return privKey.Type() == other.Type() && bytes.Equal(privKey.Bytes(), other.Bytes())
 }
 
-// Type returns eth_bls12_381.
+// Type returns the type.
 func (PrivKey) Type() string {
 	return KeyType
 }
 
-func (privKey PrivKey) Sign(digestBz []byte) ([]byte, error) {
+// Sign signs the given byte array. If msg is larger than
+// MaxMsgLen, SHA256 sum will be signed instead of the raw bytes.
+func (privKey PrivKey) Sign(msg []byte) ([]byte, error) {
 	secretKey, err := bls12381.SecretKeyFromBytes(privKey)
 	if err != nil {
 		return nil, err
 	}
 
-	bz := digestBz
-	if len(bz) > 32 {
-		hash := sha256.Sum256(bz)
-		bz = hash[:]
+	if len(msg) > MaxMsgLen {
+		hash := sha256.Sum256(msg)
+		sig := secretKey.Sign(hash[:])
+		return sig.Marshal(), nil
 	}
-
-	sig := secretKey.Sign(bz)
+	sig := secretKey.Sign(msg)
 	return sig.Marshal(), nil
 }
 
 // ===============================================================================================
 // Public Key
 // ===============================================================================================
 
-// Pubkey is a wrapper around the Ethereum bls12_381 public key type. This
+// Pubkey is a wrapper around the Ethereum BLS12-381 public key type. This
 // wrapper conforms to crypto.Pubkey to allow for the use of the Ethereum
-// bls12_381 public key type.
+// BLS12-381 public key type.
 
 var _ crypto.PubKey = &PubKey{}
 
 type PubKey []byte
 
-// Address returns the address of the public key.
+// Address returns the address of the key.
+//
 // The function will panic if the public key is invalid.
 func (pubKey PubKey) Address() crypto.Address {
 	pk, _ := bls12381.PublicKeyFromBytes(pubKey)
 	if len(pk.Marshal()) != PubKeySize {
 		panic("pubkey is incorrect size")
 	}
-	// TODO: do we want to keep this address format?
 	return crypto.Address(tmhash.SumTruncated(pubKey))
 }
 
+// VerifySignature verifies the given signature.
 func (pubKey PubKey) VerifySignature(msg, sig []byte) bool {
 	if len(sig) != SignatureLength {
 		return false
 	}
-	bz := msg
-	if len(msg) > 32 {
+
+	pubK, err := bls12381.PublicKeyFromBytes(pubKey)
+	if err != nil { // invalid pubkey
+		return false
+	}
+
+	if len(msg) > MaxMsgLen {
 		hash := sha256.Sum256(msg)
-		bz = hash[:]
+		msg = hash[:]
 	}
 
-	pubK, _ := bls12381.PublicKeyFromBytes(pubKey)
-	ok, err := bls12381.VerifySignature(sig, [32]byte(bz[:32]), pubK)
-	if err != nil {
+	ok, err := bls12381.VerifySignature(sig, [MaxMsgLen]byte(msg[:MaxMsgLen]), pubK)
+	if err != nil { // bad signature
 		return false
 	}
+
 	return ok
 }
 
-// Bytes returns the pubkey byte format.
+// Bytes returns the byte format.
 func (pubKey PubKey) Bytes() []byte {
 	return pubKey
 }
 
-// Type returns eth_bls12_381.
+// Type returns the key's type.
 func (PubKey) Type() string {
 	return KeyType
 }
 
-// Equals returns true if the pubkey type is the same and their bytes are deeply equal.
+// Equals returns true if the other's type is the same and their bytes are deeply equal.
 func (pubKey PubKey) Equals(other crypto.PubKey) bool {
 	return pubKey.Type() == other.Type() && bytes.Equal(pubKey.Bytes(), other.Bytes())
 }