Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ 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
* chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue
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
Loading