Skip to content

Commit

Permalink
chore(deps, crypto): use secp directly instead of wrapper (Finschia#1163
Browse files Browse the repository at this point in the history
)

* chore(deps, crypto): use secp directly instead of wrapper

* replace it in rosetta
  • Loading branch information
tkxkd0159 authored and jaeseung-bae committed May 7, 2024
1 parent 64e0e2f commit 81d1c61
Show file tree
Hide file tree
Showing 13 changed files with 236 additions and 99 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (docs) [\#1120](https://github.com/Finschia/finschia-sdk/pull/1120) Update links in x/foundation README.md
* (feat) [\#1121](https://github.com/Finschia/finschia-sdk/pull/1121) Add update-censorship cmd to x/foundation cli
* (server) [#1153](https://github.com/Finschia/finschia-sdk/pull/1153) remove grpc replace directive
* (crypto) [\#1163](https://github.com/Finschia/finschia-sdk/pull/1163) Update some secp256k1 logics with latest `dcrec`

### Bug Fixes
* (x/auth) [#1281](https://github.com/Finschia/finschia-sdk/pull/1281) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. (backport #1274)
* (x/foundation) [\#1283](https://github.com/Finschia/finschia-sdk/pull/1283) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic (backport #1277)
Expand Down
6 changes: 3 additions & 3 deletions crypto/hd/hdpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"strconv"
"strings"

"github.com/btcsuite/btcd/btcec"
secp "github.com/decred/dcrd/dcrec/secp256k1/v4"
)

// BIP44Params wraps BIP 44 params (5 level BIP 32 path).
Expand Down Expand Up @@ -237,7 +237,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 := secp.PrivKeyFromBytes(privKeyBytes[:]).PubKey()
pubkeyBytes := ecPub.SerializeCompressed()
data = pubkeyBytes

Expand All @@ -260,7 +260,7 @@ func addScalars(a []byte, b []byte) [32]byte {
aInt := new(big.Int).SetBytes(a)
bInt := new(big.Int).SetBytes(b)
sInt := new(big.Int).Add(aInt, bInt)
x := sInt.Mod(sInt, btcec.S256().N).Bytes()
x := sInt.Mod(sInt, secp.S256().N).Bytes()
x2 := [32]byte{}
copy(x2[32-len(x):], x)

Expand Down
4 changes: 2 additions & 2 deletions crypto/keys/secp256k1/secp256k1.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"io"
"math/big"

secp256k1 "github.com/btcsuite/btcd/btcec"
"github.com/decred/dcrd/dcrec/secp256k1/v4"
"golang.org/x/crypto/ripemd160" //nolint: staticcheck // necessary for Bitcoin address format

"github.com/Finschia/ostracon/crypto"
Expand Down Expand Up @@ -38,7 +38,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).PubKey()
pk := pubkeyObject.SerializeCompressed()
return &PubKey{Key: pk}
}
Expand Down
6 changes: 3 additions & 3 deletions crypto/keys/secp256k1/secp256k1_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"math/big"
"testing"

btcSecp256k1 "github.com/btcsuite/btcd/btcec"
secp "github.com/decred/dcrd/dcrec/secp256k1/v4"
"github.com/stretchr/testify/require"
)

Expand All @@ -23,7 +23,7 @@ func Test_genPrivKey(t *testing.T) {
shouldPanic bool
}{
{"empty bytes (panics because 1st 32 bytes are zero and 0 is not a valid field element)", empty, true},
{"curve order: N", btcSecp256k1.S256().N.Bytes(), true},
{"curve order: N", secp.S256().N.Bytes(), true},
{"valid because 0 < 1 < N", validOne, false},
}
for _, tt := range tests {
Expand All @@ -37,7 +37,7 @@ func Test_genPrivKey(t *testing.T) {
}
got := genPrivKey(bytes.NewReader(tt.notSoRand))
fe := new(big.Int).SetBytes(got[:])
require.True(t, fe.Cmp(btcSecp256k1.S256().N) < 0)
require.True(t, fe.Cmp(secp.S256().N) < 0)
require.True(t, fe.Sign() > 0)
})
}
Expand Down
57 changes: 21 additions & 36 deletions crypto/keys/secp256k1/secp256k1_nocgo.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,22 @@
package secp256k1

import (
"math/big"
"errors"

secp256k1 "github.com/btcsuite/btcd/btcec"
"github.com/decred/dcrd/dcrec/secp256k1/v4"
"github.com/decred/dcrd/dcrec/secp256k1/v4/ecdsa"

"github.com/Finschia/ostracon/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))
if err != nil {
return nil, err
}
sigBytes := serializeSig(sig)
return sigBytes, nil
priv := secp256k1.PrivKeyFromBytes(privKey.Key)
sig := ecdsa.SignCompact(priv, crypto.Sha256(msg), false)

// remove the first byte which is compactSigRecoveryCode
return sig[1:], nil
}

// VerifyBytes verifies a signature of the form R || S.
Expand All @@ -35,37 +28,29 @@ 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
}
// parse the signature:
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 {
// parse the signature, will return error if it is not in lower-S form
signature, err := signatureFromBytes(sigStr)
if err != nil {
return false
}
return signature.Verify(crypto.Sha256(msg), pub)
}

// 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]),
// Rejects malleable signatures (if S value if it is over half order).
func signatureFromBytes(sigStr []byte) (*ecdsa.Signature, error) {
var r secp256k1.ModNScalar
r.SetByteSlice(sigStr[:32])
var s secp256k1.ModNScalar
s.SetByteSlice(sigStr[32:64])
if s.IsOverHalfOrder() {
return nil, errors.New("signature is not in lower-S form")
}
}

// 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
return ecdsa.NewSignature(&r, &s), nil
}
24 changes: 17 additions & 7 deletions crypto/keys/secp256k1/secp256k1_nocgo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package secp256k1
import (
"testing"

secp256k1 "github.com/btcsuite/btcd/btcec"
"github.com/decred/dcrd/dcrec/secp256k1/v4"
"github.com/stretchr/testify/require"
)

Expand All @@ -19,20 +19,30 @@ 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,
)
}
Expand Down
Loading

0 comments on commit 81d1c61

Please sign in to comment.