Skip to content

Commit

Permalink
fix(crypto): error if incorrect ledger public key (backport cosmos#19691
Browse files Browse the repository at this point in the history
) (cosmos#19746)

Co-authored-by: Rootul P <rootulp@gmail.com>
Co-authored-by: sontrinh16 <trinhleson2000@gmail.com>
(cherry picked from commit b9b1324)
  • Loading branch information
mergify[bot] authored and karzak committed Dec 18, 2024
1 parent 2d6a76d commit c14d3da
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Bug Fixes

* (x/gov) [#19725](https://github.com/cosmos/cosmos-sdk/pull/19725) Fetch a failed proposal tally from proposal.FinalTallyResult in the gprc query.
* (crypto) [#19691](https://github.com/cosmos/cosmos-sdk/pull/19746) Fix tx sign doesn't throw an error when incorrect Ledger is used.

## [v0.47.10](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.10) - 2024-02-27

Expand Down
8 changes: 8 additions & 0 deletions crypto/keyring/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,14 @@ func SignWithLedger(k *Record, msg []byte) (sig []byte, pub types.PubKey, err er
if err != nil {
return nil, nil, err
}
ledgerPubKey := priv.PubKey()
pubKey, err := k.GetPubKey()
if err != nil {
return nil, nil, err
}
if !pubKey.Equals(ledgerPubKey) {
return nil, nil, fmt.Errorf("the public key that the user attempted to sign with does not match the public key on the ledger device. %v does not match %v", pubKey.String(), ledgerPubKey.String())
}

if !priv.PubKey().VerifySignature(msg, sig) {
return nil, nil, errors.New("Ledger generated an invalid signature. Perhaps you have multiple ledgers and need to try another one")
Expand Down
66 changes: 66 additions & 0 deletions crypto/keyring/keyring_ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/crypto/hd"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/crypto/ledger"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/types"
)

Expand Down Expand Up @@ -133,3 +137,65 @@ func TestAltKeyring_SaveLedgerKey(t *testing.T) {
path := ledgerInfo.GetPath()
require.Equal(t, "m/44'/118'/3'/0/1", path.String())
}

func TestSignWithLedger(t *testing.T) {
// Create two distinct Ledger records: recordA and recordB.
// RecordA is added to the Ledger but recordB is not added.
pathA := hd.NewFundraiserParams(0, types.CoinType, 0)
privA, _, err := ledger.NewPrivKeySecp256k1(*pathA, "cosmos")
require.NoError(t, err)
recordA, err := NewLedgerRecord("ledgerA", privA.PubKey(), pathA)
require.NoError(t, err)
pubA, err := recordA.GetPubKey()
require.NoError(t, err)

pathB := hd.NewFundraiserParams(0, types.CoinType, 1)
// privB won't be added to the Ledger because it doesn't use ledger.NewPrivKeySecp256k1
privB := secp256k1.GenPrivKey()
recordB, err := NewLedgerRecord("ledgerB", privB.PubKey(), pathB)
require.NoError(t, err)
pubB, err := recordB.GetPubKey()
require.NoError(t, err)

require.NotEqual(t, pubA, pubB)
type testCase struct {
name string
record *Record
msg []byte
wantSig []byte
wantPub cryptotypes.PubKey
wantErr bool
wantErrContains string
}
testCases := []testCase{
{
name: "ordinary ledger tx",
record: recordA,
msg: []byte("msg"),
wantSig: []byte{0xfb, 0x93, 0x1b, 0xb9, 0x75, 0x25, 0xe7, 0x99, 0x64, 0xc2, 0x78, 0xf7, 0x94, 0x9a, 0x63, 0x83, 0xe2, 0x59, 0x76, 0x48, 0x1d, 0x2, 0xbc, 0xc2, 0x83, 0x21, 0x24, 0x4b, 0x95, 0x99, 0x25, 0x8b, 0x30, 0x38, 0x6, 0x61, 0x79, 0x9a, 0x9e, 0x8, 0x98, 0xfd, 0x34, 0xc6, 0x7e, 0x47, 0x4d, 0x5f, 0xe, 0xf3, 0xc3, 0xe7, 0xdd, 0xe3, 0x89, 0x80, 0xda, 0x8b, 0x48, 0x15, 0x34, 0xce, 0xdf, 0x1c},
wantPub: pubA,
wantErr: false,
},
{
name: "want error when the public key the user attempted to sign with doesn't match the public key on the ledger",
record: recordB,
msg: []byte("msg"),
wantSig: []byte(nil),
wantPub: nil,
wantErr: true,
wantErrContains: "the public key that the user attempted to sign with does not match the public key on the ledger device",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
sig, pub, err := SignWithLedger(tc.record, tc.msg)
assert.Equal(t, tc.wantSig, sig)
assert.Equal(t, tc.wantPub, pub)
if tc.wantErr {
assert.Error(t, err)
assert.Contains(t, err.Error(), tc.wantErrContains)
}
})
}
}

0 comments on commit c14d3da

Please sign in to comment.