Adding support for Ledger Cosmos App v1.5 (cosmos#4227)

This PR adds support for the latest version of the Cosmos App (v.1.5).
The app is not been released yet by Ledger but the PR is backwards compatible.
We can later remove backwards compatibility and enforce v1.5 only.

When creating a new account, `gaiacli` now shows the account/index and address in the device and requires user confirmation.

Related PRs:
cosmos/ledger-cosmos-go#3
cosmos/ledger-cosmos-go#4
cosmos/ledger-cosmos-go#5
cosmos/ledger-cosmos-go#6

Changes in the app can be found here:
LedgerHQ/ledger-app-cosmos#5

.pending/improvements/gaiacli/4227-Support-for-Ledger-App-1.5

@@ -0,0 +1 @@
+#4227 Support for Ledger App v1.5

client/keys/add.go

@@ -171,9 +171,10 @@ func runAddCmd(_ *cobra.Command, args []string) error {
 	account := uint32(viper.GetInt(flagAccount))
 	index := uint32(viper.GetInt(flagIndex))
 
-	// If we're using ledger, only thing we need is the path. So generate key and we're done.
+	// If we're using ledger, only thing we need is the path and the bech32 prefix.
 	if viper.GetBool(client.FlagUseLedger) {
-		info, err := kb.CreateLedger(name, keys.Secp256k1, account, index)
+		bech32PrefixAccAddr := sdk.GetConfig().GetBech32AccountAddrPrefix()
+		info, err := kb.CreateLedger(name, keys.Secp256k1, bech32PrefixAccAddr, account, index)
 		if err != nil {
 			return err
 		}

client/keys/show.go

@@ -25,7 +25,7 @@ const (
 	FlagPublicKey = "pubkey"
 	// FlagBechPrefix defines a desired Bech32 prefix encoding for a key.
 	FlagBechPrefix = "bech"
-	// FlagBechPrefix defines a desired Bech32 prefix encoding for a key.
+	// FlagDevice indicates that the information should be shown in the device
 	FlagDevice = "device"
 
 	flagMultiSigThreshold = "multisig-threshold"
@@ -48,7 +48,7 @@ consisting of all the keys provided by name and multisig threshold.`,
 	cmd.Flags().String(FlagBechPrefix, sdk.PrefixAccount, "The Bech32 prefix encoding for a key (acc|val|cons)")
 	cmd.Flags().BoolP(FlagAddress, "a", false, "Output the address only (overrides --output)")
 	cmd.Flags().BoolP(FlagPublicKey, "p", false, "Output the public key only (overrides --output)")
-	cmd.Flags().BoolP(FlagDevice, "d", false, "Output the address in the device")
+	cmd.Flags().BoolP(FlagDevice, "d", false, "Output the address in a ledger device")
 	cmd.Flags().Uint(flagMultiSigThreshold, 1, "K out of N required signatures")
 	cmd.Flags().BoolP(flagShowMultiSig, "m", false, "Output multisig pubkey constituents, threshold, and weights")
 	cmd.Flags().Bool(client.FlagIndentResponse, false, "Add indent to JSON response")

crypto/keys/keybase.go

@@ -137,18 +137,19 @@ func (kb dbKeybase) Derive(name, mnemonic, bip39Passphrase, encryptPasswd string
 
 // CreateLedger creates a new locally-stored reference to a Ledger keypair
 // It returns the created key info and an error if the Ledger could not be queried
-func (kb dbKeybase) CreateLedger(name string, algo SigningAlgo, account uint32, index uint32) (Info, error) {
+func (kb dbKeybase) CreateLedger(name string, algo SigningAlgo, hrp string, account, index uint32) (Info, error) {
 	if algo != Secp256k1 {
 		return nil, ErrUnsupportedSigningAlgo
 	}
 
 	hdPath := hd.NewFundraiserParams(account, index)
-	priv, err := crypto.NewPrivKeyLedgerSecp256k1(*hdPath)
+	priv, _, err := crypto.NewPrivKeyLedgerSecp256k1(*hdPath, hrp)
 	if err != nil {
 		return nil, err
 	}
 	pub := priv.PubKey()
 
+	// Note: Once Cosmos App v1.3.1 is compulsory, it could be possible to check that pubkey and addr match
 	return kb.writeLedgerKey(name, pub, *hdPath), nil
 }
 
@@ -246,7 +247,7 @@ func (kb dbKeybase) Sign(name, passphrase string, msg []byte) (sig []byte, pub t
 
 	case ledgerInfo:
 		linfo := info.(ledgerInfo)
-		priv, err = crypto.NewPrivKeyLedgerSecp256k1(linfo.Path)
+		priv, err = crypto.NewPrivKeyLedgerSecp256k1Unsafe(linfo.Path)
 		if err != nil {
 			return
 		}

crypto/keys/keybase_test.go

@@ -38,7 +38,7 @@ func TestCreateAccountInvalidMnemonic(t *testing.T) {
 
 func TestCreateLedgerUnsupportedAlgo(t *testing.T) {
 	kb := NewInMemory()
-	_, err := kb.CreateLedger("some_account", Ed25519, 0, 1)
+	_, err := kb.CreateLedger("some_account", Ed25519, "cosmos", 0, 1)
 	assert.Error(t, err)
 	assert.Equal(t, "unsupported signing algo: only secp256k1 is supported", err.Error())
 }
@@ -50,7 +50,7 @@ func TestCreateLedger(t *testing.T) {
 	// test_cover does not compile some dependencies so ledger is disabled
 	// test_unit may add a ledger mock
 	// both cases are acceptable
-	ledger, err := kb.CreateLedger("some_account", Secp256k1, 3, 1)
+	ledger, err := kb.CreateLedger("some_account", Secp256k1, "cosmos", 3, 1)
 
 	if err != nil {
 		assert.Error(t, err)

crypto/keys/lazy_keybase.go

@@ -106,14 +106,14 @@ func (lkb lazyKeybase) Derive(name, mnemonic, bip39Passwd, encryptPasswd string,
 	return newDbKeybase(db).Derive(name, mnemonic, bip39Passwd, encryptPasswd, params)
 }
 
-func (lkb lazyKeybase) CreateLedger(name string, algo SigningAlgo, account uint32, index uint32) (info Info, err error) {
+func (lkb lazyKeybase) CreateLedger(name string, algo SigningAlgo, hrp string, account, index uint32) (info Info, err error) {
 	db, err := sdk.NewLevelDB(lkb.name, lkb.dir)
 	if err != nil {
 		return nil, err
 	}
 	defer db.Close()
 
-	return newDbKeybase(db).CreateLedger(name, algo, account, index)
+	return newDbKeybase(db).CreateLedger(name, algo, hrp, account, index)
 }
 
 func (lkb lazyKeybase) CreateOffline(name string, pubkey crypto.PubKey) (info Info, err error) {

crypto/keys/types.go

@@ -35,7 +35,7 @@ type Keybase interface {
 	Derive(name, mnemonic, bip39Passwd, encryptPasswd string, params hd.BIP44Params) (Info, error)
 
 	// CreateLedger creates, stores, and returns a new Ledger key reference
-	CreateLedger(name string, algo SigningAlgo, account uint32, index uint32) (info Info, err error)
+	CreateLedger(name string, algo SigningAlgo, hrp string, account, index uint32) (info Info, err error)
 
 	// CreateOffline creates, stores, and returns a new offline key reference
 	CreateOffline(name string, pubkey crypto.PubKey) (info Info, err error)

crypto/ledger_mock.go

@@ -4,15 +4,17 @@ package crypto
 
 import (
 	"fmt"
-
 	"github.com/btcsuite/btcd/btcec"
-	bip39 "github.com/cosmos/go-bip39"
 	"github.com/pkg/errors"
-	secp256k1 "github.com/tendermint/btcd/btcec"
-	"github.com/tendermint/tendermint/crypto"
 
 	"github.com/cosmos/cosmos-sdk/crypto/keys/hd"
 	"github.com/cosmos/cosmos-sdk/tests"
+	"github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/go-bip39"
+
+	secp256k1 "github.com/tendermint/btcd/btcec"
+	"github.com/tendermint/tendermint/crypto"
+	tmsecp256k1 "github.com/tendermint/tendermint/crypto/secp256k1"
 )
 
 // If ledger support (build tag) has been enabled, which implies a CGO dependency,
@@ -31,6 +33,8 @@ func (mock LedgerSECP256K1Mock) Close() error {
 	return nil
 }
 
+// GetPublicKeySECP256K1 mocks a ledger device
+// as per the original API, it returns an uncompressed key
 func (mock LedgerSECP256K1Mock) GetPublicKeySECP256K1(derivationPath []uint32) ([]byte, error) {
 	if derivationPath[0] != 44 {
 		return nil, errors.New("Invalid derivation path")
@@ -56,6 +60,28 @@ func (mock LedgerSECP256K1Mock) GetPublicKeySECP256K1(derivationPath []uint32) (
 	return pubkeyObject.SerializeUncompressed(), nil
 }
 
+// GetAddressPubKeySECP256K1 mocks a ledger device
+// as per the original API, it returns a compressed key and a bech32 address
+func (mock LedgerSECP256K1Mock) GetAddressPubKeySECP256K1(derivationPath []uint32, hrp string) ([]byte, string, error) {
+	pk, err := mock.GetPublicKeySECP256K1(derivationPath)
+	if err != nil {
+		return nil, "", err
+	}
+
+	// re-serialize in the 33-byte compressed format
+	cmp, err := btcec.ParsePubKey(pk[:], btcec.S256())
+	if err != nil {
+		return nil, "", fmt.Errorf("error parsing public key: %v", err)
+	}
+
+	var compressedPublicKey tmsecp256k1.PubKeySecp256k1
+	copy(compressedPublicKey[:], cmp.SerializeCompressed())
+
+	// Generate the bech32 addr using existing tmcrypto/etc.
+	addr := types.AccAddress(compressedPublicKey.Address()).String()
+	return pk, addr, err
+}
+
 func (mock LedgerSECP256K1Mock) SignSECP256K1(derivationPath []uint32, message []byte) ([]byte, error) {
 	path := hd.NewParams(derivationPath[0], derivationPath[1], derivationPath[2], derivationPath[3] != 0, derivationPath[4])
 	seed, err := bip39.NewSeedWithErrorChecking(tests.TestMnemonic, "")

crypto/ledger_secp256k1.go

@@ -5,12 +5,11 @@ import (
 	"os"
 
 	"github.com/btcsuite/btcd/btcec"
+	"github.com/pkg/errors"
 
 	"github.com/cosmos/cosmos-sdk/crypto/keys/hd"
 	"github.com/cosmos/cosmos-sdk/types"
 
-	"github.com/pkg/errors"
-
 	tmbtcec "github.com/tendermint/btcd/btcec"
 	tmcrypto "github.com/tendermint/tendermint/crypto"
 	tmsecp256k1 "github.com/tendermint/tendermint/crypto/secp256k1"
@@ -28,13 +27,15 @@ type (
 	// dependencies when Ledger support is potentially not enabled.
 	discoverLedgerFn func() (LedgerSECP256K1, error)
 
-	// LedgerSECP256K1 reflects an interface a Ledger API must implement for
-	// the SECP256K1 scheme.
+	// LedgerSECP256K1 reflects an interface a Ledger API must implement for SECP256K1
 	LedgerSECP256K1 interface {
 		Close() error
+		// Returns an uncompressed pubkey
 		GetPublicKeySECP256K1([]uint32) ([]byte, error)
+		// Returns a compressed pubkey and bech32 address (requires user confirmation)
+		GetAddressPubKeySECP256K1([]uint32, string) ([]byte, string, error)
+		// Signs a message (requires user confirmation)
 		SignSECP256K1([]uint32, []byte) ([]byte, error)
-		ShowAddressSECP256K1([]uint32, string) error
 	}
 
 	// PrivKeyLedgerSecp256k1 implements PrivKey, calling the ledger nano we
@@ -48,41 +49,41 @@ type (
 	}
 )
 
-// NewPrivKeyLedgerSecp256k1 will generate a new key and store the public key
-// for later use.
-func NewPrivKeyLedgerSecp256k1(path hd.BIP44Params) (tmcrypto.PrivKey, error) {
+// NewPrivKeyLedgerSecp256k1Unsafe will generate a new key and store the public key for later use.
+//
+// This function is marked as unsafe as it will retrieve a pubkey without user verification.
+// It can only be used to verify a pubkey but never to create new accounts/keys. In that case,
+// please refer to NewPrivKeyLedgerSecp256k1
+func NewPrivKeyLedgerSecp256k1Unsafe(path hd.BIP44Params) (tmcrypto.PrivKey, error) {
 	device, err := getLedgerDevice()
 	if err != nil {
 		return nil, err
 	}
 	defer warnIfErrors(device.Close)
 
-	pubKey, err := getPubKey(device, path)
+	pubKey, err := getPubKeyUnsafe(device, path)
 	if err != nil {
 		return nil, err
 	}
 
 	return PrivKeyLedgerSecp256k1{pubKey, path}, nil
 }
 
-// LedgerShowAddress triggers a ledger device to show the corresponding address.
-func LedgerShowAddress(path hd.BIP44Params, expectedPubKey tmcrypto.PubKey) error {
+// NewPrivKeyLedgerSecp256k1 will generate a new key and store the public key for later use.
+// The request will require user confirmation and will show account and index in the device
+func NewPrivKeyLedgerSecp256k1(path hd.BIP44Params, hrp string) (tmcrypto.PrivKey, string, error) {
 	device, err := getLedgerDevice()
 	if err != nil {
-		return err
+		return nil, "", err
 	}
 	defer warnIfErrors(device.Close)
 
-	pubKey, err := getPubKey(device, path)
+	pubKey, addr, err := getPubKeyAddrSafe(device, path, hrp)
 	if err != nil {
-		return err
-	}
-
-	if pubKey != expectedPubKey {
-		return fmt.Errorf("pubkey does not match, Check this is the same device")
+		return nil, "", err
 	}
 
-	return device.ShowAddressSECP256K1(path.DerivationPath(), types.Bech32PrefixAccAddr)
+	return PrivKeyLedgerSecp256k1{pubKey, path}, addr, nil
 }
 
 // PubKey returns the cached public key.
@@ -101,6 +102,35 @@ func (pkl PrivKeyLedgerSecp256k1) Sign(message []byte) ([]byte, error) {
 	return sign(device, pkl, message)
 }
 
+// LedgerShowAddress triggers a ledger device to show the corresponding address.
+func LedgerShowAddress(path hd.BIP44Params, expectedPubKey tmcrypto.PubKey) error {
+	device, err := getLedgerDevice()
+	if err != nil {
+		return err
+	}
+	defer warnIfErrors(device.Close)
+
+	pubKey, err := getPubKeyUnsafe(device, path)
+	if err != nil {
+		return err
+	}
+
+	if pubKey != expectedPubKey {
+		return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
+	}
+
+	pubKey2, _, err := getPubKeyAddrSafe(device, path, types.Bech32PrefixAccAddr)
+	if err != nil {
+		return err
+	}
+
+	if pubKey2 != expectedPubKey {
+		return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
+	}
+
+	return nil
+}
+
 // ValidateKey allows us to verify the sanity of a public key after loading it
 // from disk.
 func (pkl PrivKeyLedgerSecp256k1) ValidateKey() error {
@@ -162,7 +192,7 @@ func getLedgerDevice() (LedgerSECP256K1, error) {
 }
 
 func validateKey(device LedgerSECP256K1, pkl PrivKeyLedgerSecp256k1) error {
-	pub, err := getPubKey(device, pkl.Path)
+	pub, err := getPubKeyUnsafe(device, pkl.Path)
 	if err != nil {
 		return err
 	}
@@ -194,10 +224,15 @@ func sign(device LedgerSECP256K1, pkl PrivKeyLedgerSecp256k1, msg []byte) ([]byt
 	return convertDERtoBER(sig)
 }
 
-// getPubKey reads the pubkey the ledger itself
+// getPubKeyUnsafe reads the pubkey from a ledger device
+//
+// This function is marked as unsafe as it will retrieve a pubkey without user verification
+// It can only be used to verify a pubkey but never to create new accounts/keys. In that case,
+// please refer to getPubKeyAddrSafe
+//
 // since this involves IO, it may return an error, which is not exposed
 // in the PubKey interface, so this function allows better error handling
-func getPubKey(device LedgerSECP256K1, path hd.BIP44Params) (tmcrypto.PubKey, error) {
+func getPubKeyUnsafe(device LedgerSECP256K1, path hd.BIP44Params) (tmcrypto.PubKey, error) {
 	publicKey, err := device.GetPublicKeySECP256K1(path.DerivationPath())
 	if err != nil {
 		return nil, fmt.Errorf("please open Cosmos app on the Ledger device - error: %v", err)
@@ -214,3 +249,27 @@ func getPubKey(device LedgerSECP256K1, path hd.BIP44Params) (tmcrypto.PubKey, er
 
 	return compressedPublicKey, nil
 }
+
+// getPubKeyAddr reads the pubkey and the address from a ledger device.
+// This function is marked as Safe as it will require user confirmation and
+// account and index will be shown in the device.
+//
+// Since this involves IO, it may return an error, which is not exposed
+// in the PubKey interface, so this function allows better error handling.
+func getPubKeyAddrSafe(device LedgerSECP256K1, path hd.BIP44Params, hrp string) (tmcrypto.PubKey, string, error) {
+	publicKey, addr, err := device.GetAddressPubKeySECP256K1(path.DerivationPath(), hrp)
+	if err != nil {
+		return nil, "", fmt.Errorf("address %s rejected", addr)
+	}
+
+	// re-serialize in the 33-byte compressed format
+	cmp, err := btcec.ParsePubKey(publicKey[:], btcec.S256())
+	if err != nil {
+		return nil, "", fmt.Errorf("error parsing public key: %v", err)
+	}
+
+	var compressedPublicKey tmsecp256k1.PubKeySecp256k1
+	copy(compressedPublicKey[:], cmp.SerializeCompressed())
+
+	return compressedPublicKey, addr, nil
+}