From c6efab6932558d703064bae562c245783fdacde3 Mon Sep 17 00:00:00 2001 From: Elias Naur <103319121+elias-orijtech@users.noreply.github.com> Date: Mon, 17 Oct 2022 08:33:43 -0600 Subject: [PATCH] chore(crypto): update btcec to v2 (#13513) Similar to Tendermint's PR, https://github.com/tendermint/tendermint/pull/9250 Note that crypto/ledger is not updated in this PR, because if its dependency on the R and S values being exposed by ParseDERSignature. Updates #13423 Signed-off-by: Elias Naur Signed-off-by: Elias Naur Co-authored-by: Marko --- crypto/hd/hdpath.go | 2 +- crypto/keys/secp256k1/secp256k1.go | 2 +- crypto/keys/secp256k1/secp256k1_nocgo.go | 50 +++++++------------ crypto/keys/secp256k1/secp256k1_nocgo_test.go | 21 +++++--- crypto/keys/secp256k1/secp256k1_test.go | 11 ++-- server/rosetta/converter.go | 2 +- 6 files changed, 44 insertions(+), 44 deletions(-) diff --git a/crypto/hd/hdpath.go b/crypto/hd/hdpath.go index 15dd33012246..3216bf40e2c6 100644 --- a/crypto/hd/hdpath.go +++ b/crypto/hd/hdpath.go @@ -225,7 +225,7 @@ func derivePrivateKey(privKeyBytes [32]byte, chainCode [32]byte, index uint32, h data = append([]byte{byte(0)}, privKeyBytes[:]...) } else { // this can't return an error: - _, ecPub := btcec.PrivKeyFromBytes(btcec.S256(), privKeyBytes[:]) + _, ecPub := btcec.PrivKeyFromBytes(privKeyBytes[:]) pubkeyBytes := ecPub.SerializeCompressed() data = pubkeyBytes diff --git a/crypto/keys/secp256k1/secp256k1.go b/crypto/keys/secp256k1/secp256k1.go index 14333e9b731b..0d5d849d438b 100644 --- a/crypto/keys/secp256k1/secp256k1.go +++ b/crypto/keys/secp256k1/secp256k1.go @@ -37,7 +37,7 @@ func (privKey *PrivKey) Bytes() []byte { // PubKey performs the point-scalar multiplication from the privKey on the // generator point to get the pubkey. func (privKey *PrivKey) PubKey() cryptotypes.PubKey { - _, pubkeyObject := secp256k1.PrivKeyFromBytes(secp256k1.S256(), privKey.Key) + _, pubkeyObject := secp256k1.PrivKeyFromBytes(privKey.Key) pk := pubkeyObject.SerializeCompressed() return &PubKey{Key: pk} } diff --git a/crypto/keys/secp256k1/secp256k1_nocgo.go b/crypto/keys/secp256k1/secp256k1_nocgo.go index 7e41a9f51341..ac7f521488c0 100644 --- a/crypto/keys/secp256k1/secp256k1_nocgo.go +++ b/crypto/keys/secp256k1/secp256k1_nocgo.go @@ -4,29 +4,22 @@ package secp256k1 import ( - "math/big" - secp256k1 "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcec/v2/ecdsa" "github.com/tendermint/tendermint/crypto" ) -// used to reject malleable signatures -// see: -// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93 -// - https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/crypto.go#L39 -var secp256k1halfN = new(big.Int).Rsh(secp256k1.S256().N, 1) - // Sign creates an ECDSA signature on curve Secp256k1, using SHA256 on the msg. // The returned signature will be of the form R || S (in lower-S form). func (privKey *PrivKey) Sign(msg []byte) ([]byte, error) { - priv, _ := secp256k1.PrivKeyFromBytes(secp256k1.S256(), privKey.Key) - sig, err := priv.Sign(crypto.Sha256(msg)) + priv, _ := secp256k1.PrivKeyFromBytes(privKey.Key) + sig, err := ecdsa.SignCompact(priv, crypto.Sha256(msg), false) if err != nil { return nil, err } - sigBytes := serializeSig(sig) - return sigBytes, nil + // remove the first byte which is compactSigRecoveryCode + return sig[1:], nil } // VerifyBytes verifies a signature of the form R || S. @@ -35,7 +28,7 @@ func (pubKey *PubKey) VerifySignature(msg []byte, sigStr []byte) bool { if len(sigStr) != 64 { return false } - pub, err := secp256k1.ParsePubKey(pubKey.Key, secp256k1.S256()) + pub, err := secp256k1.ParsePubKey(pubKey.Key) if err != nil { return false } @@ -43,7 +36,13 @@ func (pubKey *PubKey) VerifySignature(msg []byte, sigStr []byte) bool { signature := signatureFromBytes(sigStr) // Reject malleable signatures. libsecp256k1 does this check but btcec doesn't. // see: https://github.com/ethereum/go-ethereum/blob/f9401ae011ddf7f8d2d95020b7446c17f8d98dc1/crypto/signature_nocgo.go#L90-L93 - if signature.S.Cmp(secp256k1halfN) > 0 { + // Serialize() would negate S value if it is over half order. + // Hence, if the signature is different after Serialize() if should be rejected. + modifiedSignature, parseErr := ecdsa.ParseDERSignature(signature.Serialize()) + if parseErr != nil { + return false + } + if !signature.IsEqual(modifiedSignature) { return false } return signature.Verify(crypto.Sha256(msg), pub) @@ -51,21 +50,10 @@ func (pubKey *PubKey) VerifySignature(msg []byte, sigStr []byte) bool { // Read Signature struct from R || S. Caller needs to ensure // that len(sigStr) == 64. -func signatureFromBytes(sigStr []byte) *secp256k1.Signature { - return &secp256k1.Signature{ - R: new(big.Int).SetBytes(sigStr[:32]), - S: new(big.Int).SetBytes(sigStr[32:64]), - } -} - -// Serialize signature to R || S. -// R, S are padded to 32 bytes respectively. -func serializeSig(sig *secp256k1.Signature) []byte { - rBytes := sig.R.Bytes() - sBytes := sig.S.Bytes() - sigBytes := make([]byte, 64) - // 0 pad the byte arrays from the left if they aren't big enough. - copy(sigBytes[32-len(rBytes):32], rBytes) - copy(sigBytes[64-len(sBytes):64], sBytes) - return sigBytes +func signatureFromBytes(sigStr []byte) *ecdsa.Signature { + var r secp256k1.ModNScalar + r.SetByteSlice(sigStr[:32]) + var s secp256k1.ModNScalar + s.SetByteSlice(sigStr[32:64]) + return ecdsa.NewSignature(&r, &s) } diff --git a/crypto/keys/secp256k1/secp256k1_nocgo_test.go b/crypto/keys/secp256k1/secp256k1_nocgo_test.go index d46182e54151..f38a5bf4fd45 100644 --- a/crypto/keys/secp256k1/secp256k1_nocgo_test.go +++ b/crypto/keys/secp256k1/secp256k1_nocgo_test.go @@ -19,20 +19,29 @@ func TestSignatureVerificationAndRejectUpperS(t *testing.T) { priv := GenPrivKey() sigStr, err := priv.Sign(msg) require.NoError(t, err) - sig := signatureFromBytes(sigStr) - require.False(t, sig.S.Cmp(secp256k1halfN) > 0) + var r secp256k1.ModNScalar + r.SetByteSlice(sigStr[:32]) + var s secp256k1.ModNScalar + s.SetByteSlice(sigStr[32:64]) + require.False(t, s.IsOverHalfOrder()) pub := priv.PubKey() require.True(t, pub.VerifySignature(msg, sigStr)) // malleate: - sig.S.Sub(secp256k1.S256().CurveParams.N, sig.S) - require.True(t, sig.S.Cmp(secp256k1halfN) > 0) - malSigStr := serializeSig(sig) + var S256 secp256k1.ModNScalar + S256.SetByteSlice(secp256k1.S256().N.Bytes()) + s.Negate().Add(&S256) + require.True(t, s.IsOverHalfOrder()) + rBytes := r.Bytes() + sBytes := s.Bytes() + malSigStr := make([]byte, 64) + copy(malSigStr[32-len(rBytes):32], rBytes[:]) + copy(malSigStr[64-len(sBytes):64], sBytes[:]) require.False(t, pub.VerifySignature(msg, malSigStr), "VerifyBytes incorrect with malleated & invalid S. sig=%v, key=%v", - sig, + malSigStr, priv, ) } diff --git a/crypto/keys/secp256k1/secp256k1_test.go b/crypto/keys/secp256k1/secp256k1_test.go index 89b02042e1b0..651665ad4e8e 100644 --- a/crypto/keys/secp256k1/secp256k1_test.go +++ b/crypto/keys/secp256k1/secp256k1_test.go @@ -8,6 +8,7 @@ import ( "testing" btcSecp256k1 "github.com/btcsuite/btcd/btcec/v2" + btcecdsa "github.com/btcsuite/btcd/btcec/v2/ecdsa" "github.com/cosmos/btcutil/base58" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -64,7 +65,7 @@ func TestSignAndValidateSecp256k1(t *testing.T) { // ---- // Test cross packages verification msgHash := crypto.Sha256(msg) - btcPrivKey, btcPubKey := btcSecp256k1.PrivKeyFromBytes(btcSecp256k1.S256(), privKey.Key) + btcPrivKey, btcPubKey := btcSecp256k1.PrivKeyFromBytes(privKey.Key) // This fails: malformed signature: no header magic // btcSig, err := secp256k1.ParseSignature(sig, secp256k1.S256()) // require.NoError(t, err) @@ -77,9 +78,11 @@ func TestSignAndValidateSecp256k1(t *testing.T) { ok := ecdsa.Verify(btcPubKey.ToECDSA(), msgHash, r, s) require.True(t, ok) - sig2, err := btcPrivKey.Sign(msgHash) + sig2, err := btcecdsa.SignCompact(btcPrivKey, msgHash, false) + // Chop off compactSigRecoveryCode. + sig2 = sig2[1:] require.NoError(t, err) - pubKey.VerifySignature(msg, sig2.Serialize()) + pubKey.VerifySignature(msg, sig2) // ---- // Mutate the signature, just one bit. @@ -98,7 +101,7 @@ func TestSecp256k1LoadPrivkeyAndSerializeIsIdentity(t *testing.T) { // This function creates a private and public key in the underlying libraries format. // The private key is basically calling new(big.Int).SetBytes(pk), which removes leading zero bytes - priv, _ := btcSecp256k1.PrivKeyFromBytes(btcSecp256k1.S256(), privKeyBytes[:]) + priv, _ := btcSecp256k1.PrivKeyFromBytes(privKeyBytes[:]) // this takes the bytes returned by `(big int).Bytes()`, and if the length is less than 32 bytes, // pads the bytes from the left with zero bytes. Therefore these two functions composed // result in the identity function on privKeyBytes, hence the following equality check diff --git a/server/rosetta/converter.go b/server/rosetta/converter.go index 082a52dc503d..e7cce56bd134 100644 --- a/server/rosetta/converter.go +++ b/server/rosetta/converter.go @@ -649,7 +649,7 @@ func (c converter) PubKey(pubKey *rosettatypes.PublicKey) (cryptotypes.PubKey, e return nil, crgerrs.WrapError(crgerrs.ErrUnsupportedCurve, "only secp256k1 supported") } - cmp, err := btcec.ParsePubKey(pubKey.Bytes, btcec.S256()) + cmp, err := btcec.ParsePubKey(pubKey.Bytes) if err != nil { return nil, crgerrs.WrapError(crgerrs.ErrBadArgument, err.Error()) }