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.

Sign up for GitHub

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

refactor: update crypto/ledger to btcec/v2 #14123

Merged
merged 8 commits into from
Dec 5, 2022
Merged

refactor: update crypto/ledger to btcec/v2 #14123

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cd713e6
chore(crypto): update crypto/ledger to v2
julienrbrt Dec 1, 2022
2a072c0
go mod tidy
julienrbrt Dec 1, 2022
2d83fd3
Merge branch 'main' into julien/btcec-ledger
julienrbrt Dec 1, 2022
ecab39c
updates
julienrbrt Dec 1, 2022
d2a00a9
add comment
julienrbrt Dec 1, 2022
90a71b5
Merge branch 'main' into julien/btcec-ledger
julienrbrt Dec 3, 2022
8be7ee5
Merge branch 'main' into julien/btcec-ledger
julienrbrt Dec 5, 2022
c9d8f8b
Merge branch 'main' into julien/btcec-ledger
julienrbrt Dec 5, 2022
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
14 changes: 8 additions & 6 deletions crypto/ledger/ledger_mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
package ledger

import (
"errors"
"fmt"

"github.com/btcsuite/btcd/btcec"
"github.com/pkg/errors"

btcec "github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcec/v2/ecdsa"
"github.com/cosmos/go-bip39"
secp256k1 "github.com/tendermint/btcd/btcec"
"github.com/tendermint/tendermint/crypto"
Expand Down Expand Up @@ -73,7 +73,7 @@ func (mock LedgerSECP256K1Mock) GetAddressPubKeySECP256K1(derivationPath []uint3
}

// re-serialize in the 33-byte compressed format
cmp, err := btcec.ParsePubKey(pk[:], btcec.S256())
cmp, err := btcec.ParsePubKey(pk[:])
if err != nil {
return nil, "", fmt.Errorf("error parsing public key: %v", err)
}
Expand Down Expand Up @@ -108,8 +108,10 @@ func (mock LedgerSECP256K1Mock) SignSECP256K1(derivationPath []uint32, message [
}

// Need to return DER as the ledger does
sig2 := btcec.Signature{R: sig.R, S: sig.S}
return sig2.Serialize(), nil
var r, s btcec.ModNScalar
r.SetByteSlice(sig.R.Bytes())
s.SetByteSlice(sig.S.Bytes())
return ecdsa.NewSignature(&r, &s).Serialize(), nil
}

// ShowAddressSECP256K1 shows the address for the corresponding bip32 derivation path
Expand Down
2 changes: 1 addition & 1 deletion crypto/ledger/ledger_notavail.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package ledger

import (
"github.com/pkg/errors"
"errors"
)

// If ledger support (build tag) has been enabled, which implies a CGO dependency,
Expand Down
24 changes: 17 additions & 7 deletions crypto/ledger/ledger_secp256k1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package ledger

import (
"errors"
"fmt"
"math/big"
"os"

"github.com/btcsuite/btcd/btcec"
"github.com/pkg/errors"
btcec "github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcec/v2/ecdsa"

tmbtcec "github.com/tendermint/btcd/btcec"

Expand Down Expand Up @@ -210,11 +212,19 @@ func warnIfErrors(f func() error) {
}

func convertDERtoBER(signatureDER []byte) ([]byte, error) {
sigDER, err := btcec.ParseDERSignature(signatureDER, btcec.S256())
sigDER, err := ecdsa.ParseDERSignature(signatureDER)
if err != nil {
return nil, err
}
sigBER := tmbtcec.Signature{R: sigDER.R, S: sigDER.S}

sigStr := sigDER.Serialize()
var r, s big.Int
// The format of a DER encoded signature is as follows:
// 0x30 <total length> 0x02 <length of R> <R> 0x02 <length of S> <S>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might seem paranoid, but I suggest that we add a length minimum check here

// Validating that we have a somewhat proper DER signature.
if n := len(sigDER); n < 4 {
    return nil, fmt.Errorf("invalid DER length: want >=4 got %d", n)
}
if g, w := sigDER[0], 0x30; g != w {
    return nil, fmt.Errorf("invalid DER: byte[0]=%x, want=%x", g, w)
}
if wantLen, gotLen := int(sigDER[1]), len(sigDER[3:]); gotLen != wantLen {
    return nil, fmt.Errorf("invalid DER len: got=%d, want=%d",
       gotLen, wantLen)
}

// Eliminate compiler Bounds checking.
_ = sigDER[9:]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a bit paranoid indeed, given that Serialize always returns this format.
https://go.dev/play/p/WuZKekW5s9x

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool no biggie.

r.SetBytes(sigStr[4 : 4+sigStr[3]])
s.SetBytes(sigStr[4+sigStr[3]+2:])
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
sigBER := tmbtcec.Signature{R: &r, S: &s}

return sigBER.Serialize(), nil
}

Expand All @@ -225,7 +235,7 @@ func getDevice() (SECP256K1, error) {

device, err := options.discoverLedger()
if err != nil {
return nil, errors.Wrap(err, "ledger nano S")
return nil, fmt.Errorf("ledger nano S: %w", err)
}

return device, nil
Expand Down Expand Up @@ -283,7 +293,7 @@ func getPubKeyUnsafe(device SECP256K1, path hd.BIP44Params) (types.PubKey, error
}

// re-serialize in the 33-byte compressed format
cmp, err := btcec.ParsePubKey(publicKey, btcec.S256())
cmp, err := btcec.ParsePubKey(publicKey)
if err != nil {
return nil, fmt.Errorf("error parsing public key: %v", err)
}
Expand All @@ -307,7 +317,7 @@ func getPubKeyAddrSafe(device SECP256K1, path hd.BIP44Params, hrp string) (types
}

// re-serialize in the 33-byte compressed format
cmp, err := btcec.ParsePubKey(publicKey, btcec.S256())
cmp, err := btcec.ParsePubKey(publicKey)
if err != nil {
return nil, "", fmt.Errorf("error parsing public key: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ require (
github.com/99designs/keyring v1.2.1
github.com/armon/go-metrics v0.4.1
github.com/bgentry/speakeasy v0.1.0
github.com/btcsuite/btcd v0.22.3
github.com/btcsuite/btcd/btcec/v2 v2.3.2
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e
github.com/cockroachdb/apd/v2 v2.0.2
Expand Down Expand Up @@ -74,6 +73,7 @@ require (
github.com/aws/aws-sdk-go v1.40.45 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
github.com/btcsuite/btcd v0.22.3 // indirect
github.com/cenkalti/backoff/v4 v4.1.3 // indirect
github.com/cespare/xxhash v1.1.0 // indirect
github.com/cespare/xxhash/v2 v2.1.2 // indirect
Expand Down
1 change: 0 additions & 1 deletion simapp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
github.com/bgentry/speakeasy v0.1.0 // indirect
github.com/btcsuite/btcd v0.22.3 // indirect
github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect
github.com/cenkalti/backoff/v4 v4.1.3 // indirect
github.com/cespare/xxhash v1.1.0 // indirect
Expand Down
1 change: 0 additions & 1 deletion simapp/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ github.com/bgentry/speakeasy v0.1.0 h1:ByYyxL9InA1OWqxJqqp2A5pYHUrCiAL6K3J+LKSsQ
github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ=
github.com/btcsuite/btcd v0.22.3 h1:kYNaWFvOw6xvqP0vR20RP1Zq1DVMBxEO8QN5d1/EfNg=
github.com/btcsuite/btcd v0.22.3/go.mod h1:wqgTSL29+50LRkmOVknEdmt8ZojIzhuWvgu/iptuN7Y=
github.com/btcsuite/btcd/btcec/v2 v2.3.2 h1:5n0X6hX0Zk+6omWcihdYvdAlGf2DfasC0GMf7DClJ3U=
github.com/btcsuite/btcd/btcec/v2 v2.3.2/go.mod h1:zYzJ8etWJQIv1Ogk7OzpWjowwOdXY1W/17j2MW85J04=
github.com/btcsuite/btcd/btcutil v1.1.2 h1:XLMbX8JQEiwMcYft2EGi8zPUkoa0abKIU6/BJSRsjzQ=
Expand Down
1 change: 0 additions & 1 deletion tests/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
github.com/bgentry/speakeasy v0.1.0 // indirect
github.com/btcsuite/btcd v0.22.3 // indirect
github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect
github.com/cenkalti/backoff/v4 v4.1.3 // indirect
github.com/cespare/xxhash v1.1.0 // indirect
Expand Down
1 change: 0 additions & 1 deletion tests/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ github.com/bgentry/speakeasy v0.1.0 h1:ByYyxL9InA1OWqxJqqp2A5pYHUrCiAL6K3J+LKSsQ
github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ=
github.com/btcsuite/btcd v0.22.3 h1:kYNaWFvOw6xvqP0vR20RP1Zq1DVMBxEO8QN5d1/EfNg=
github.com/btcsuite/btcd v0.22.3/go.mod h1:wqgTSL29+50LRkmOVknEdmt8ZojIzhuWvgu/iptuN7Y=
github.com/btcsuite/btcd/btcec/v2 v2.3.2 h1:5n0X6hX0Zk+6omWcihdYvdAlGf2DfasC0GMf7DClJ3U=
github.com/btcsuite/btcd/btcec/v2 v2.3.2/go.mod h1:zYzJ8etWJQIv1Ogk7OzpWjowwOdXY1W/17j2MW85J04=
github.com/btcsuite/btcd/btcutil v1.1.2 h1:XLMbX8JQEiwMcYft2EGi8zPUkoa0abKIU6/BJSRsjzQ=
Expand Down