From 1d7fcd845893c84768ac786c1cee8948f3868cdd Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 29 Jan 2019 10:54:51 +0100 Subject: [PATCH 01/17] Upgrade cosmos-ledger-go to v0.9.4 --- Gopkg.lock | 28 ++++++++++++++-------------- Gopkg.toml | 5 ++++- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 97fcf673472d..f4be56f50d09 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -2,20 +2,12 @@ [[projects]] - digest = "1:7736fc6da04620727f8f3aa2ced8d77be8e074a302820937aa5993848c769b27" - name = "github.com/ZondaX/hid-go" - packages = ["."] - pruneopts = "UT" - revision = "48b08affede2cea076a3cf13b2e3f72ed262b743" - version = "v0.4.0" - -[[projects]] - digest = "1:1ba351898f7efc68c7c9ff3145b920e478f716b077fdaaf06b967c5d883fa988" + digest = "1:0e3c4434de19ab6378518212ae289f32a93b58fe3cbadef8d5e18c4430ade1a9" name = "github.com/ZondaX/ledger-go" packages = ["."] pruneopts = "UT" - revision = "c3225ab10c2f53397d4aa419a588466493572b22" - version = "v0.4.0" + revision = "5271d68c0c1fa7718e5781fea56169bba82942f6" + version = "v0.5.0" [[projects]] branch = "master" @@ -207,6 +199,14 @@ pruneopts = "UT" revision = "c42d9e0ca023e2198120196f842701bb4c55d7b9" +[[projects]] + branch = "master" + digest = "1:447562773a19dc1719359c2cd70d275c62c0b89f79d763f41d5deedb0e69873f" + name = "github.com/karalabe/hid" + packages = ["."] + pruneopts = "T" + revision = "d815e0c1a2e2082a287a2806bc90bc8fc7b276a9" + [[projects]] branch = "master" digest = "1:a64e323dc06b73892e5bb5d040ced475c4645d456038333883f58934abbf6f72" @@ -522,12 +522,12 @@ revision = "v0.29.0" [[projects]] - digest = "1:29b886f00694ae7c18c4559a2901f2a057d5a62308ed5eb6cd52b9a31016fb14" + digest = "1:4f00ff9ea968ebd4e591884513604087e01aadd0bef4ab4ab7644a304723ae13" name = "github.com/zondax/ledger-cosmos-go" packages = ["."] pruneopts = "UT" - revision = "71aa0ab6e03d2d320c82bbe13678a36584a5b813" - version = "v0.9.3" + revision = "e236ef273df616b984def5f9188e2d9bebb39e8b" + version = "v0.9.4" [[projects]] digest = "1:6f6dc6060c4e9ba73cf28aa88f12a69a030d3d19d518ef8e931879eaa099628d" diff --git a/Gopkg.toml b/Gopkg.toml index 1bf115d61992..b6eed92612a2 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -44,7 +44,7 @@ [[constraint]] name = "github.com/zondax/ledger-cosmos-go" - version = "=v0.9.3" + version = "=v0.9.4" ## deps without releases: @@ -84,3 +84,6 @@ [prune] go-tests = true unused-packages = true + [[prune.project]] + name = "github.com/karalabe/hid" + unused-packages = false From a0b517ac399a5f55f85ee76f677dd3988016b42d Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 29 Jan 2019 18:55:52 +0100 Subject: [PATCH 02/17] upgrading to cosmos-ledger-go v0.9.5 --- Gopkg.lock | 38 +++++++++++++++++++------------------- Gopkg.toml | 4 ++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index f4be56f50d09..500655257f70 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1,14 +1,6 @@ # This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. -[[projects]] - digest = "1:0e3c4434de19ab6378518212ae289f32a93b58fe3cbadef8d5e18c4430ade1a9" - name = "github.com/ZondaX/ledger-go" - packages = ["."] - pruneopts = "UT" - revision = "5271d68c0c1fa7718e5781fea56169bba82942f6" - version = "v0.5.0" - [[projects]] branch = "master" digest = "1:09a7f74eb6bb3c0f14d8926610c87f569c5cff68e978d30e9a3540aeb626fdf0" @@ -199,14 +191,6 @@ pruneopts = "UT" revision = "c42d9e0ca023e2198120196f842701bb4c55d7b9" -[[projects]] - branch = "master" - digest = "1:447562773a19dc1719359c2cd70d275c62c0b89f79d763f41d5deedb0e69873f" - name = "github.com/karalabe/hid" - packages = ["."] - pruneopts = "T" - revision = "d815e0c1a2e2082a287a2806bc90bc8fc7b276a9" - [[projects]] branch = "master" digest = "1:a64e323dc06b73892e5bb5d040ced475c4645d456038333883f58934abbf6f72" @@ -522,12 +506,28 @@ revision = "v0.29.0" [[projects]] - digest = "1:4f00ff9ea968ebd4e591884513604087e01aadd0bef4ab4ab7644a304723ae13" + digest = "1:b73f5e117bc7c6e8fc47128f20db48a873324ad5cfeeebfc505e85c58682b5e4" + name = "github.com/zondax/hid" + packages = ["."] + pruneopts = "T" + revision = "302fd402163c34626286195dfa9adac758334acc" + version = "v0.9.0" + +[[projects]] + digest = "1:1327096745ba36bd6c7b4c9ea09c25b3c262d36f09ce21b8c5e34ea41fb71798" name = "github.com/zondax/ledger-cosmos-go" packages = ["."] pruneopts = "UT" - revision = "e236ef273df616b984def5f9188e2d9bebb39e8b" - version = "v0.9.4" + revision = "a93249f81d8e5a294fb2d2ead4c17abc88d0fd3a" + version = "v0.9.5" + +[[projects]] + digest = "1:312416a870d6ef9941e1ea7893480ff1e6627cc66517ce2760bf97af427d8f8c" + name = "github.com/zondax/ledger-go" + packages = ["."] + pruneopts = "UT" + revision = "6c752dfd14666393766f120c0c3ae940b5aaa8d5" + version = "v0.6.0" [[projects]] digest = "1:6f6dc6060c4e9ba73cf28aa88f12a69a030d3d19d518ef8e931879eaa099628d" diff --git a/Gopkg.toml b/Gopkg.toml index b6eed92612a2..7f6f1eed22f2 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -44,7 +44,7 @@ [[constraint]] name = "github.com/zondax/ledger-cosmos-go" - version = "=v0.9.4" + version = "=v0.9.5" ## deps without releases: @@ -85,5 +85,5 @@ go-tests = true unused-packages = true [[prune.project]] - name = "github.com/karalabe/hid" + name = "github.com/zondax/hid" unused-packages = false From ac1ceb68e5e507aa6e01c9253a22ed0b96d6c374 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Tue, 29 Jan 2019 21:14:58 +0100 Subject: [PATCH 03/17] upgrading to v0.9.6 removing case mismatches in dependencies --- Gopkg.lock | 12 ++++++------ Gopkg.toml | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 500655257f70..0c9d8fdf921f 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -514,20 +514,20 @@ version = "v0.9.0" [[projects]] - digest = "1:1327096745ba36bd6c7b4c9ea09c25b3c262d36f09ce21b8c5e34ea41fb71798" + digest = "1:20e33bf30e3b9b95eeb5ab28cd7fdb1b6f171433053666a18b245bf9e1cc6c73" name = "github.com/zondax/ledger-cosmos-go" packages = ["."] pruneopts = "UT" - revision = "a93249f81d8e5a294fb2d2ead4c17abc88d0fd3a" - version = "v0.9.5" + revision = "a67896da91d935e859755d080d7b2e4c2a5a163d" + version = "v0.9.6" [[projects]] - digest = "1:312416a870d6ef9941e1ea7893480ff1e6627cc66517ce2760bf97af427d8f8c" + digest = "1:65384c591a38a7fe5b2593cb195e4da7870970687b38f6da64a94a4bd47a4275" name = "github.com/zondax/ledger-go" packages = ["."] pruneopts = "UT" - revision = "6c752dfd14666393766f120c0c3ae940b5aaa8d5" - version = "v0.6.0" + revision = "918a8a722678b284fc313bd9f2c28df6458a313f" + version = "v0.7.0" [[projects]] digest = "1:6f6dc6060c4e9ba73cf28aa88f12a69a030d3d19d518ef8e931879eaa099628d" diff --git a/Gopkg.toml b/Gopkg.toml index 7f6f1eed22f2..a1e08c37d4a6 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -44,7 +44,7 @@ [[constraint]] name = "github.com/zondax/ledger-cosmos-go" - version = "=v0.9.5" + version = "=v0.9.6" ## deps without releases: From bd7d6741c4edad3f51a2980d8e8914778af044d2 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Fri, 1 Feb 2019 19:31:55 +0100 Subject: [PATCH 04/17] wip --- crypto/ledger.go | 2 +- crypto/ledger_integration_test.go | 128 ++++++++++++++++++++++++++++++ crypto/ledger_secp256k1.go | 78 ++++++++---------- crypto/ledger_test.go | 54 +------------ 4 files changed, 166 insertions(+), 96 deletions(-) create mode 100644 crypto/ledger_integration_test.go diff --git a/crypto/ledger.go b/crypto/ledger.go index 36b5646b86f7..f3d0e3859b91 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -1,4 +1,4 @@ -// +build cgo,ledger +// +build cgo ledger package crypto diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go new file mode 100644 index 000000000000..59a0b378769b --- /dev/null +++ b/crypto/ledger_integration_test.go @@ -0,0 +1,128 @@ +// +build cgo ledger real_ledger + +package crypto + +import ( + "testing" + + ledger "github.com/zondax/ledger-cosmos-go" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/encoding/amino" +) + +const ( + // These tests expect a ledger device initialized to the following mnemonic + testMnemonic = "equip will roof matter pink blind book anxiety banner elbow sun young" +) + +func TestDiscoverDevice(t *testing.T) { + device, err := discoverLedger() + require.NoError(t, err) + require.NotNil(t, device) +} + +func TestDiscoverDeviceTwice(t *testing.T) { + // We expect the second call not to find a device + device, err := discoverLedger() + require.NoError(t, err) + require.NotNil(t, device) + + device2, err := discoverLedger() + require.Error(t, err) + require.Equal(t, "no ledger connected", err.Error()) + require.Nil(t, device2) +} + +func TestDiscoverDeviceTwiceClosing(t *testing.T) { + { + device, err := ledger.FindLedgerCosmosUserApp() + require.NoError(t, err) + require.NotNil(t, device) + require.NoError(t, device.Close()) + } + + device2, err := discoverLedger() + require.NoError(t, err) + require.NotNil(t, device2) +} + +func TestPublicKey(t *testing.T) { + path := DerivationPath{44, 118, 0, 0, 0} + priv, err := NewPrivKeyLedgerSecp256k1(path) + require.Nil(t, err, "%s", err) + require.NotNil(t, priv) + + pubKeyAddr, err := sdk.Bech32ifyAccPub(priv.PubKey()) + require.NoError(t, err) + require.Equal(t, "cosmospub1addwnpepqd87l8xhcnrrtzxnkql7k55ph8fr9jarf4hn6udwukfprlalu8lgw0urza0", pubKeyAddr) +} + +func TestPublicKeyHDPath(t *testing.T) { + expectedAnswers := []string{ + "cosmospub1addwnpepqd87l8xhcnrrtzxnkql7k55ph8fr9jarf4hn6udwukfprlalu8lgw0urza0", + "cosmospub1addwnpepqfsdqjr68h7wjg5wacksmqaypasnra232fkgu5sxdlnlu8j22ztxvlqvd65", + "cosmospub1addwnpepqw3xwqun6q43vtgw6p4qspq7srvxhcmvq4jrx5j5ma6xy3r7k6dtxmrkh3d", + "cosmospub1addwnpepqvez9lrp09g8w7gkv42y4yr5p6826cu28ydrhrujv862yf4njmqyyjr4pjs", + "cosmospub1addwnpepq06hw3enfrtmq8n67teytcmtnrgcr0yntmyt25kdukfjkerdc7lqg32rcz7", + "cosmospub1addwnpepqg3trf2gd0s2940nckrxherwqhgmm6xd5h4pcnrh4x7y35h6yafmcpk5qns", + "cosmospub1addwnpepqdm6rjpx6wsref8wjn7ym6ntejet430j4szpngfgc20caz83lu545vuv8hp", + "cosmospub1addwnpepqvdhtjzy2wf44dm03jxsketxc07vzqwvt3vawqqtljgsr9s7jvydjmt66ew", + "cosmospub1addwnpepqwystfpyxwcava7v3t7ndps5xzu6s553wxcxzmmnxevlzvwrlqpzz695nw9", + "cosmospub1addwnpepqw970u6gjqkccg9u3rfj99857wupj2z9fqfzy2w7e5dd7xn7kzzgkgqch0r", + } + + // TODO: Check data with locally generated pubkey + // TODO: ONHOLD, it needs another PR to get merged first + //seed, _ := bip39.NewSeedWithErrorChecking(testMnemonic, "") + //masterPriv, ch := hd.ComputeMastersFromSeed(seed) + //derivedPriv, err := hd.DerivePrivateKeyForPath(masterPriv, ch, fullHdPath) + + // Check with device + for i := uint32(0); i < 10; i++ { + path := DerivationPath{44, 118, 0, 0, i} + priv, err := NewPrivKeyLedgerSecp256k1(path) + require.Nil(t, err, "%s", err) + require.NotNil(t, priv) + + pubKeyAddr, err := sdk.Bech32ifyAccPub(priv.PubKey()) + require.NoError(t, err) + require.Equal(t, expectedAnswers[i], pubKeyAddr) + } +} + +func TestRealLedgerSecp256k1(t *testing.T) { + msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") + path := DerivationPath{44, 118, 0, 0, 0} + + priv, err := NewPrivKeyLedgerSecp256k1(path) + require.Nil(t, err, "%s", err) + + pub := priv.PubKey() + sig, err := priv.Sign(msg) + require.Nil(t, err) + + valid := pub.VerifyBytes(msg, sig) + require.True(t, valid) + + // now, let's serialize the public key and make sure it still works + bs := priv.PubKey().Bytes() + pub2, err := cryptoAmino.PubKeyFromBytes(bs) + require.Nil(t, err, "%+v", err) + + // make sure we get the same pubkey when we load from disk + require.Equal(t, pub, pub2) + + // signing with the loaded key should match the original pubkey + sig, err = priv.Sign(msg) + require.Nil(t, err) + valid = pub.VerifyBytes(msg, sig) + require.True(t, valid) + + // make sure pubkeys serialize properly as well + bs = pub.Bytes() + bpub, err := cryptoAmino.PubKeyFromBytes(bs) + require.NoError(t, err) + require.Equal(t, pub, bpub) +} diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index ff05d31bae66..63396d58ffe9 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -22,12 +22,14 @@ type ( // dependencies when Ledger support is potentially not enabled. discoverLedgerFn func() (LedgerSECP256K1, error) + // TODO: Improve this // DerivationPath represents a Ledger derivation path. DerivationPath []uint32 // LedgerSECP256K1 reflects an interface a Ledger API must implement for // the SECP256K1 scheme. LedgerSECP256K1 interface { + Close() error GetPublicKeySECP256K1([]uint32) ([]byte, error) SignSECP256K1([]uint32, []byte) ([]byte, error) } @@ -40,7 +42,6 @@ type ( // ledger attached. CachedPubKey tmcrypto.PubKey Path DerivationPath - ledger LedgerSECP256K1 } ) @@ -58,16 +59,14 @@ func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { if err != nil { return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") } + defer device.Close() - pkl := &PrivKeyLedgerSecp256k1{Path: path, ledger: device} - - pubKey, err := pkl.getPubKey() + pubKey, err := getPubKey(device, path) if err != nil { return nil, err } - pkl.CachedPubKey = pubKey - return pkl, err + return PrivKeyLedgerSecp256k1{pubKey, path}, nil } // PubKey returns the cached public key. @@ -78,16 +77,16 @@ func (pkl PrivKeyLedgerSecp256k1) PubKey() tmcrypto.PubKey { // ValidateKey allows us to verify the sanity of a public key after loading it // from disk. func (pkl PrivKeyLedgerSecp256k1) ValidateKey() error { - // getPubKey will return an error if the ledger is not - pub, err := pkl.getPubKey() - if err != nil { - return err - } - - // verify this matches cached address - if !pub.Equals(pkl.CachedPubKey) { - return fmt.Errorf("cached key does not match retrieved key") - } + //// getPubKey will return an error if the ledger is not + //pub, err := pkl.getPubKey() + //if err != nil { + // return err + //} + // + //// verify this matches cached address + //if !pub.Equals(pkl.CachedPubKey) { + // return fmt.Errorf("cached key does not match retrieved key") + //} return nil } @@ -117,44 +116,35 @@ func (pkl PrivKeyLedgerSecp256k1) Equals(other tmcrypto.PrivKey) bool { // an error, so this should only trigger if the private key is held in memory // for a while before use. func (pkl PrivKeyLedgerSecp256k1) Sign(msg []byte) ([]byte, error) { - sig, err := pkl.signLedgerSecp256k1(msg) - if err != nil { - return nil, err - } - - return sig, nil + //sig, err := pkl.signLedgerSecp256k1(msg) + //if err != nil { + // return nil, err + //} + //return sig, nil + return nil, nil } +//func (pkl PrivKeyLedgerSecp256k1) signLedgerSecp256k1(msg []byte) ([]byte, error) { +// return pkl.ledger.SignSECP256K1(pkl.Path, msg) +//} + // getPubKey reads the pubkey the ledger itself // 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 (pkl PrivKeyLedgerSecp256k1) getPubKey() (key tmcrypto.PubKey, err error) { - key, err = pkl.pubkeyLedgerSecp256k1() +func getPubKey(device LedgerSECP256K1, path DerivationPath) (tmcrypto.PubKey, error) { + publicKey, err := device.GetPublicKeySECP256K1(path) if err != nil { - return key, fmt.Errorf("please open Cosmos app on the Ledger device - error: %v", err) + return nil, fmt.Errorf("please open Cosmos app on the Ledger device - error: %v", err) } - return key, err -} - -func (pkl PrivKeyLedgerSecp256k1) signLedgerSecp256k1(msg []byte) ([]byte, error) { - return pkl.ledger.SignSECP256K1(pkl.Path, msg) -} - -func (pkl PrivKeyLedgerSecp256k1) pubkeyLedgerSecp256k1() (pub tmcrypto.PubKey, err error) { - key, err := pkl.ledger.GetPublicKeySECP256K1(pkl.Path) - if err != nil { - return nil, fmt.Errorf("error fetching public key: %v", err) - } - - var pk tmsecp256k1.PubKeySecp256k1 - // re-serialize in the 33-byte compressed format - cmp, err := secp256k1.ParsePubKey(key[:], secp256k1.S256()) + cmp, err := secp256k1.ParsePubKey(publicKey[:], secp256k1.S256()) if err != nil { - return nil, fmt.Errorf("error parsing public key: %v", err) + return nil, fmt.Errorf("error parsing public publicKey: %v", err) } - copy(pk[:], cmp.SerializeCompressed()) - return pk, nil + var compressedPublicKey tmsecp256k1.PubKeySecp256k1 + copy(compressedPublicKey[:], cmp.SerializeCompressed()) + + return compressedPublicKey, nil } diff --git a/crypto/ledger_test.go b/crypto/ledger_test.go index 1aae158eff95..106fbdcdbcec 100644 --- a/crypto/ledger_test.go +++ b/crypto/ledger_test.go @@ -1,64 +1,16 @@ package crypto import ( - "fmt" - "os" "testing" "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto/encoding/amino" ) -var ledgerEnabledEnv = "TEST_WITH_LEDGER" - -func TestRealLedgerSecp256k1(t *testing.T) { - if os.Getenv(ledgerEnabledEnv) == "" { - t.Skip(fmt.Sprintf("Set '%s' to run code on a real ledger", ledgerEnabledEnv)) - } - msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") - path := DerivationPath{44, 60, 0, 0, 0} - - priv, err := NewPrivKeyLedgerSecp256k1(path) - require.Nil(t, err, "%s", err) - - pub := priv.PubKey() - sig, err := priv.Sign(msg) - require.Nil(t, err) - - valid := pub.VerifyBytes(msg, sig) - require.True(t, valid) - - // now, let's serialize the public key and make sure it still works - bs := priv.PubKey().Bytes() - pub2, err := cryptoAmino.PubKeyFromBytes(bs) - require.Nil(t, err, "%+v", err) - - // make sure we get the same pubkey when we load from disk - require.Equal(t, pub, pub2) - - // signing with the loaded key should match the original pubkey - sig, err = priv.Sign(msg) - require.Nil(t, err) - valid = pub.VerifyBytes(msg, sig) - require.True(t, valid) - - // make sure pubkeys serialize properly as well - bs = pub.Bytes() - bpub, err := cryptoAmino.PubKeyFromBytes(bs) - require.NoError(t, err) - require.Equal(t, pub, bpub) -} - -// TestRealLedgerErrorHandling calls. These tests assume -// the ledger is not plugged in.... -func TestRealLedgerErrorHandling(t *testing.T) { - if os.Getenv(ledgerEnabledEnv) != "" { - t.Skip(fmt.Sprintf("Unset '%s' to run code as if without a real Ledger", ledgerEnabledEnv)) - } - +// This tests assume a ledger is not plugged in +func TestLedgerErrorHandling(t *testing.T) { // first, try to generate a key, must return an error // (no panic) - path := DerivationPath{44, 60, 0, 0, 0} + path := DerivationPath{44, 555, 0, 0, 0} _, err := NewPrivKeyLedgerSecp256k1(path) require.Error(t, err) } From ca1cac1026d68f3963ba2bb51a4de37ea8abb75c Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Fri, 1 Feb 2019 21:11:57 +0100 Subject: [PATCH 05/17] improving tests --- crypto/ledger_integration_test.go | 3 +++ crypto/ledger_secp256k1.go | 37 ++++++++++++++++++++++--------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go index 59a0b378769b..6f4f26c004ee 100644 --- a/crypto/ledger_integration_test.go +++ b/crypto/ledger_integration_test.go @@ -21,6 +21,7 @@ func TestDiscoverDevice(t *testing.T) { device, err := discoverLedger() require.NoError(t, err) require.NotNil(t, device) + defer device.Close() } func TestDiscoverDeviceTwice(t *testing.T) { @@ -28,6 +29,7 @@ func TestDiscoverDeviceTwice(t *testing.T) { device, err := discoverLedger() require.NoError(t, err) require.NotNil(t, device) + defer device.Close() device2, err := discoverLedger() require.Error(t, err) @@ -46,6 +48,7 @@ func TestDiscoverDeviceTwiceClosing(t *testing.T) { device2, err := discoverLedger() require.NoError(t, err) require.NotNil(t, device2) + require.NoError(t, device2.Close()) } func TestPublicKey(t *testing.T) { diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 63396d58ffe9..011a33d32de8 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -74,6 +74,20 @@ func (pkl PrivKeyLedgerSecp256k1) PubKey() tmcrypto.PubKey { return pkl.CachedPubKey } +func (pkl PrivKeyLedgerSecp256k1) Sign(message []byte) ([]byte, error) { + if discoverLedger == nil { + return nil, errors.New("no Ledger discovery function defined") + } + + device, err := discoverLedger() + if err != nil { + return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") + } + defer device.Close() + + return sign(device, pkl.Path, message) +} + // ValidateKey allows us to verify the sanity of a public key after loading it // from disk. func (pkl PrivKeyLedgerSecp256k1) ValidateKey() error { @@ -115,18 +129,19 @@ func (pkl PrivKeyLedgerSecp256k1) Equals(other tmcrypto.PrivKey) bool { // Communication is checked on NewPrivKeyLedger and PrivKeyFromBytes, returning // an error, so this should only trigger if the private key is held in memory // for a while before use. -func (pkl PrivKeyLedgerSecp256k1) Sign(msg []byte) ([]byte, error) { - //sig, err := pkl.signLedgerSecp256k1(msg) - //if err != nil { - // return nil, err - //} - //return sig, nil - return nil, nil -} +func sign(device LedgerSECP256K1, path DerivationPath, msg []byte) ([]byte, error) { + sig, err := device.SignSECP256K1(path, msg) + if err != nil { + return nil, err + } -//func (pkl PrivKeyLedgerSecp256k1) signLedgerSecp256k1(msg []byte) ([]byte, error) { -// return pkl.ledger.SignSECP256K1(pkl.Path, msg) -//} + sig_, err := secp256k1.ParseDERSignature(sig[:], secp256k1.S256()) + if err != nil { + return nil, err + } + + return sig_.Serialize(), nil +} // getPubKey reads the pubkey the ledger itself // since this involves IO, it may return an error, which is not exposed From f48f17747c695a8a88fcbdda709d692e38714d97 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Fri, 1 Feb 2019 23:42:45 +0100 Subject: [PATCH 06/17] Upgrading to v0.9.7 --- Gopkg.lock | 13 +++++++------ Gopkg.toml | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 0c9d8fdf921f..9711a8447413 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -514,20 +514,20 @@ version = "v0.9.0" [[projects]] - digest = "1:20e33bf30e3b9b95eeb5ab28cd7fdb1b6f171433053666a18b245bf9e1cc6c73" + digest = "1:fca24169988a61ea725d1326de30910d8049fe68bcbc194d28803f9a76dda380" name = "github.com/zondax/ledger-cosmos-go" packages = ["."] pruneopts = "UT" - revision = "a67896da91d935e859755d080d7b2e4c2a5a163d" - version = "v0.9.6" + revision = "69fdb8ce5e5b9d9c3b22b9248e117b231d4f06dd" + version = "v0.9.7" [[projects]] - digest = "1:65384c591a38a7fe5b2593cb195e4da7870970687b38f6da64a94a4bd47a4275" + digest = "1:f8e4c0b959174a1fa5946b12f1f2ac7ea5651bef20a9e4a8dac55dbffcaa6cd6" name = "github.com/zondax/ledger-go" packages = ["."] pruneopts = "UT" - revision = "918a8a722678b284fc313bd9f2c28df6458a313f" - version = "v0.7.0" + revision = "69c15f1333a9b6866e5f66096561c7d138894bc5" + version = "v0.8.0" [[projects]] digest = "1:6f6dc6060c4e9ba73cf28aa88f12a69a030d3d19d518ef8e931879eaa099628d" @@ -681,6 +681,7 @@ "github.com/stretchr/testify/assert", "github.com/stretchr/testify/require", "github.com/syndtr/goleveldb/leveldb/opt", + "github.com/tendermint/btcd/btcec", "github.com/tendermint/go-amino", "github.com/tendermint/iavl", "github.com/tendermint/tendermint/abci/server", diff --git a/Gopkg.toml b/Gopkg.toml index a1e08c37d4a6..bb3b016c0432 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -44,7 +44,7 @@ [[constraint]] name = "github.com/zondax/ledger-cosmos-go" - version = "=v0.9.6" + version = "=v0.9.7" ## deps without releases: From 612c8d3379a922ab8fdfc72e1523963d6c8cd904 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 11:27:24 +0100 Subject: [PATCH 07/17] Convert signatures DER->BER + tests --- crypto/ledger_integration_test.go | 23 +++++++++++++++++++++-- crypto/ledger_secp256k1.go | 22 +++++++++++++--------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go index 6f4f26c004ee..046508fc0b88 100644 --- a/crypto/ledger_integration_test.go +++ b/crypto/ledger_integration_test.go @@ -3,13 +3,14 @@ package crypto import ( + "fmt" "testing" - ledger "github.com/zondax/ledger-cosmos-go" + "github.com/tendermint/tendermint/crypto/encoding/amino" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto/encoding/amino" + ledger "github.com/zondax/ledger-cosmos-go" ) const ( @@ -60,6 +61,9 @@ func TestPublicKey(t *testing.T) { pubKeyAddr, err := sdk.Bech32ifyAccPub(priv.PubKey()) require.NoError(t, err) require.Equal(t, "cosmospub1addwnpepqd87l8xhcnrrtzxnkql7k55ph8fr9jarf4hn6udwukfprlalu8lgw0urza0", pubKeyAddr) + require.Equal(t, "5075624b6579536563703235366b317b303334464546394344374334433633353838443342303"+ + "3464542353238314239443233324342413334443646334437314145453539323131464642464531464538377d", + fmt.Sprintf("%x", priv.PubKey())) } func TestPublicKeyHDPath(t *testing.T) { @@ -95,6 +99,21 @@ func TestPublicKeyHDPath(t *testing.T) { } } +func TestSignature(t *testing.T) { + msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") + path := DerivationPath{44, 118, 0, 0, 0} + + priv, err := NewPrivKeyLedgerSecp256k1(path) + require.Nil(t, err, "%s", err) + + pub := priv.PubKey() + sig, err := priv.Sign(msg) + require.Nil(t, err) + + valid := pub.VerifyBytes(msg, sig) + require.True(t, valid) +} + func TestRealLedgerSecp256k1(t *testing.T) { msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") path := DerivationPath{44, 118, 0, 0, 0} diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 011a33d32de8..a73e40de40e0 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -5,7 +5,8 @@ import ( "github.com/pkg/errors" - secp256k1 "github.com/btcsuite/btcd/btcec" + "github.com/btcsuite/btcd/btcec" + tmbtcec "github.com/tendermint/btcd/btcec" tmcrypto "github.com/tendermint/tendermint/crypto" tmsecp256k1 "github.com/tendermint/tendermint/crypto/secp256k1" ) @@ -124,6 +125,15 @@ func (pkl PrivKeyLedgerSecp256k1) Equals(other tmcrypto.PrivKey) bool { return false } +func convertDERtoBER(signatureDER []byte) ([]byte, error) { + sigDER, err := btcec.ParseDERSignature(signatureDER[:], btcec.S256()) + if err != nil { + return nil, err + } + sigBER := tmbtcec.Signature{R: sigDER.R, S: sigDER.S} + return sigBER.Serialize(), nil +} + // Sign calls the ledger and stores the PubKey for future use. // // Communication is checked on NewPrivKeyLedger and PrivKeyFromBytes, returning @@ -134,13 +144,7 @@ func sign(device LedgerSECP256K1, path DerivationPath, msg []byte) ([]byte, erro if err != nil { return nil, err } - - sig_, err := secp256k1.ParseDERSignature(sig[:], secp256k1.S256()) - if err != nil { - return nil, err - } - - return sig_.Serialize(), nil + return convertDERtoBER(sig) } // getPubKey reads the pubkey the ledger itself @@ -153,7 +157,7 @@ func getPubKey(device LedgerSECP256K1, path DerivationPath) (tmcrypto.PubKey, er } // re-serialize in the 33-byte compressed format - cmp, err := secp256k1.ParsePubKey(publicKey[:], secp256k1.S256()) + cmp, err := btcec.ParsePubKey(publicKey[:], btcec.S256()) if err != nil { return nil, fmt.Errorf("error parsing public publicKey: %v", err) } From 651169abb5b6a1e33a7fb507a9bcdf124aea7966 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 11:54:05 +0100 Subject: [PATCH 08/17] refactoring device handling --- crypto/ledger_secp256k1.go | 74 ++++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index a73e40de40e0..4a69dc2a9fb7 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -52,13 +52,9 @@ type ( // CONTRACT: The ledger device, ledgerDevice, must be loaded and set prior to // any creation of a PrivKeyLedgerSecp256k1. func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { - if discoverLedger == nil { - return nil, errors.New("no Ledger discovery function defined") - } - - device, err := discoverLedger() + device, err := getLedgerDevice() if err != nil { - return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") + return nil, err } defer device.Close() @@ -75,35 +71,27 @@ func (pkl PrivKeyLedgerSecp256k1) PubKey() tmcrypto.PubKey { return pkl.CachedPubKey } +// Sign returns a secp256k1 signature for the corresponding message func (pkl PrivKeyLedgerSecp256k1) Sign(message []byte) ([]byte, error) { - if discoverLedger == nil { - return nil, errors.New("no Ledger discovery function defined") - } - - device, err := discoverLedger() + device, err := getLedgerDevice() if err != nil { - return nil, errors.Wrap(err, "failed to create PrivKeyLedgerSecp256k1") + return nil, err } defer device.Close() - return sign(device, pkl.Path, message) + return sign(device, pkl, message) } // ValidateKey allows us to verify the sanity of a public key after loading it // from disk. func (pkl PrivKeyLedgerSecp256k1) ValidateKey() error { - //// getPubKey will return an error if the ledger is not - //pub, err := pkl.getPubKey() - //if err != nil { - // return err - //} - // - //// verify this matches cached address - //if !pub.Equals(pkl.CachedPubKey) { - // return fmt.Errorf("cached key does not match retrieved key") - //} + device, err := getLedgerDevice() + if err != nil { + return err + } + defer device.Close() - return nil + return validateKey(device, pkl) } // AssertIsPrivKeyInner implements the PrivKey interface. It performs a no-op. @@ -121,7 +109,6 @@ func (pkl PrivKeyLedgerSecp256k1) Equals(other tmcrypto.PrivKey) bool { if ledger, ok := other.(*PrivKeyLedgerSecp256k1); ok { return pkl.CachedPubKey.Equals(ledger.CachedPubKey) } - return false } @@ -134,16 +121,49 @@ func convertDERtoBER(signatureDER []byte) ([]byte, error) { return sigBER.Serialize(), nil } +func getLedgerDevice() (LedgerSECP256K1, error) { + if discoverLedger == nil { + return nil, errors.New("no Ledger discovery function defined") + } + + device, err := discoverLedger() + if err != nil { + return nil, errors.Wrap(err, "ledger nano S") + } + + return device, nil +} + +func validateKey(device LedgerSECP256K1, pkl PrivKeyLedgerSecp256k1) error { + pub, err := getPubKey(device, pkl.Path) + if err != nil { + return err + } + + // verify this matches cached address + if !pub.Equals(pkl.CachedPubKey) { + return fmt.Errorf("cached key does not match retrieved key") + } + + return nil +} + // Sign calls the ledger and stores the PubKey for future use. // // Communication is checked on NewPrivKeyLedger and PrivKeyFromBytes, returning // an error, so this should only trigger if the private key is held in memory // for a while before use. -func sign(device LedgerSECP256K1, path DerivationPath, msg []byte) ([]byte, error) { - sig, err := device.SignSECP256K1(path, msg) +func sign(device LedgerSECP256K1, pkl PrivKeyLedgerSecp256k1, msg []byte) ([]byte, error) { + err := validateKey(device, pkl) if err != nil { return nil, err } + + sig, err := device.SignSECP256K1(pkl.Path, msg) + if err != nil { + return nil, err + } + return convertDERtoBER(sig) } From 6c4840ce1def5a070fc88bdad4beebbda301f718 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 12:02:32 +0100 Subject: [PATCH 09/17] moving to existing types for consistency --- crypto/ledger_integration_test.go | 14 ++++++-------- crypto/ledger_secp256k1.go | 18 +++++++----------- crypto/ledger_test.go | 3 ++- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go index 046508fc0b88..9840942ac9dc 100644 --- a/crypto/ledger_integration_test.go +++ b/crypto/ledger_integration_test.go @@ -6,10 +6,10 @@ import ( "fmt" "testing" - "github.com/tendermint/tendermint/crypto/encoding/amino" - + "github.com/cosmos/cosmos-sdk/crypto/keys/hd" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/encoding/amino" ledger "github.com/zondax/ledger-cosmos-go" ) @@ -53,7 +53,7 @@ func TestDiscoverDeviceTwiceClosing(t *testing.T) { } func TestPublicKey(t *testing.T) { - path := DerivationPath{44, 118, 0, 0, 0} + path := *hd.NewFundraiserParams(0, 0) priv, err := NewPrivKeyLedgerSecp256k1(path) require.Nil(t, err, "%s", err) require.NotNil(t, priv) @@ -88,7 +88,7 @@ func TestPublicKeyHDPath(t *testing.T) { // Check with device for i := uint32(0); i < 10; i++ { - path := DerivationPath{44, 118, 0, 0, i} + path := *hd.NewFundraiserParams(0, i) priv, err := NewPrivKeyLedgerSecp256k1(path) require.Nil(t, err, "%s", err) require.NotNil(t, priv) @@ -101,8 +101,7 @@ func TestPublicKeyHDPath(t *testing.T) { func TestSignature(t *testing.T) { msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") - path := DerivationPath{44, 118, 0, 0, 0} - + path := *hd.NewFundraiserParams(0, 0) priv, err := NewPrivKeyLedgerSecp256k1(path) require.Nil(t, err, "%s", err) @@ -116,8 +115,7 @@ func TestSignature(t *testing.T) { func TestRealLedgerSecp256k1(t *testing.T) { msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") - path := DerivationPath{44, 118, 0, 0, 0} - + path := *hd.NewFundraiserParams(0, 0) priv, err := NewPrivKeyLedgerSecp256k1(path) require.Nil(t, err, "%s", err) diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 4a69dc2a9fb7..e9c2d8f559c1 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -3,9 +3,9 @@ package crypto import ( "fmt" - "github.com/pkg/errors" - "github.com/btcsuite/btcd/btcec" + "github.com/cosmos/cosmos-sdk/crypto/keys/hd" + "github.com/pkg/errors" tmbtcec "github.com/tendermint/btcd/btcec" tmcrypto "github.com/tendermint/tendermint/crypto" tmsecp256k1 "github.com/tendermint/tendermint/crypto/secp256k1" @@ -23,10 +23,6 @@ type ( // dependencies when Ledger support is potentially not enabled. discoverLedgerFn func() (LedgerSECP256K1, error) - // TODO: Improve this - // DerivationPath represents a Ledger derivation path. - DerivationPath []uint32 - // LedgerSECP256K1 reflects an interface a Ledger API must implement for // the SECP256K1 scheme. LedgerSECP256K1 interface { @@ -42,7 +38,7 @@ type ( // go-amino so we can view the address later, even without having the // ledger attached. CachedPubKey tmcrypto.PubKey - Path DerivationPath + Path hd.BIP44Params } ) @@ -51,7 +47,7 @@ type ( // // CONTRACT: The ledger device, ledgerDevice, must be loaded and set prior to // any creation of a PrivKeyLedgerSecp256k1. -func NewPrivKeyLedgerSecp256k1(path DerivationPath) (tmcrypto.PrivKey, error) { +func NewPrivKeyLedgerSecp256k1(path hd.BIP44Params) (tmcrypto.PrivKey, error) { device, err := getLedgerDevice() if err != nil { return nil, err @@ -159,7 +155,7 @@ func sign(device LedgerSECP256K1, pkl PrivKeyLedgerSecp256k1, msg []byte) ([]byt return nil, err } - sig, err := device.SignSECP256K1(pkl.Path, msg) + sig, err := device.SignSECP256K1(pkl.Path.DerivationPath(), msg) if err != nil { return nil, err } @@ -170,8 +166,8 @@ func sign(device LedgerSECP256K1, pkl PrivKeyLedgerSecp256k1, msg []byte) ([]byt // getPubKey reads the pubkey the ledger itself // 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 DerivationPath) (tmcrypto.PubKey, error) { - publicKey, err := device.GetPublicKeySECP256K1(path) +func getPubKey(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) } diff --git a/crypto/ledger_test.go b/crypto/ledger_test.go index 106fbdcdbcec..cb2c7b325153 100644 --- a/crypto/ledger_test.go +++ b/crypto/ledger_test.go @@ -3,6 +3,7 @@ package crypto import ( "testing" + "github.com/cosmos/cosmos-sdk/crypto/keys/hd" "github.com/stretchr/testify/require" ) @@ -10,7 +11,7 @@ import ( func TestLedgerErrorHandling(t *testing.T) { // first, try to generate a key, must return an error // (no panic) - path := DerivationPath{44, 555, 0, 0, 0} + path := *hd.NewParams(44, 555, 0, false, 0) _, err := NewPrivKeyLedgerSecp256k1(path) require.Error(t, err) } From 5100162542b41025f5f23eb7a9d9b13b143d67ee Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 12:21:20 +0100 Subject: [PATCH 10/17] make target + more tests --- Makefile | 3 +++ crypto/ledger_integration_test.go | 43 +++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index d583e0c763fd..46fb34f6bcf0 100644 --- a/Makefile +++ b/Makefile @@ -144,6 +144,9 @@ test: test_unit test_cli: @go test -p 4 `go list github.com/cosmos/cosmos-sdk/cmd/gaia/cli_test` -tags=cli_test +test_ledger: + @go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags=ledger_device + test_unit: @VERSION=$(VERSION) go test $(PACKAGES_NOSIMULATION) diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go index 9840942ac9dc..667a6c40cc38 100644 --- a/crypto/ledger_integration_test.go +++ b/crypto/ledger_integration_test.go @@ -60,7 +60,7 @@ func TestPublicKey(t *testing.T) { pubKeyAddr, err := sdk.Bech32ifyAccPub(priv.PubKey()) require.NoError(t, err) - require.Equal(t, "cosmospub1addwnpepqd87l8xhcnrrtzxnkql7k55ph8fr9jarf4hn6udwukfprlalu8lgw0urza0", pubKeyAddr) + require.Equal(t, "cosmospub1addwnpepqd87l8xhcnrrtzxnkql7k55ph8fr9jarf4hn6udwukfprlalu8lgw0urza0", pubKeyAddr, "Is your device using test mnemonic: %s ?", testMnemonic) require.Equal(t, "5075624b6579536563703235366b317b303334464546394344374334433633353838443342303"+ "3464542353238314239443233324342413334443646334437314145453539323131464642464531464538377d", fmt.Sprintf("%x", priv.PubKey())) @@ -81,7 +81,7 @@ func TestPublicKeyHDPath(t *testing.T) { } // TODO: Check data with locally generated pubkey - // TODO: ONHOLD, it needs another PR to get merged first + // TODO: ONHOLD, it needs PR #3461 to get merged first //seed, _ := bip39.NewSeedWithErrorChecking(testMnemonic, "") //masterPriv, ch := hd.ComputeMastersFromSeed(seed) //derivedPriv, err := hd.DerivePrivateKeyForPath(masterPriv, ch, fullHdPath) @@ -89,32 +89,47 @@ func TestPublicKeyHDPath(t *testing.T) { // Check with device for i := uint32(0); i < 10; i++ { path := *hd.NewFundraiserParams(0, i) + fmt.Printf("Checking keys at %v\n", path) + priv, err := NewPrivKeyLedgerSecp256k1(path) require.Nil(t, err, "%s", err) require.NotNil(t, priv) pubKeyAddr, err := sdk.Bech32ifyAccPub(priv.PubKey()) require.NoError(t, err) - require.Equal(t, expectedAnswers[i], pubKeyAddr) + require.Equal(t, expectedAnswers[i], pubKeyAddr, "Is your device using test mnemonic: %s ?", testMnemonic) } } -func TestSignature(t *testing.T) { - msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") - path := *hd.NewFundraiserParams(0, 0) - priv, err := NewPrivKeyLedgerSecp256k1(path) - require.Nil(t, err, "%s", err) +func getFakeTx(accountNumber uint32) []byte { + tmp := fmt.Sprintf( + `{"account_number":"%d","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"5000"},"memo":"memo","msgs":[[""]],"sequence":"6"}`, + accountNumber) - pub := priv.PubKey() - sig, err := priv.Sign(msg) - require.Nil(t, err) + return []byte(tmp) +} - valid := pub.VerifyBytes(msg, sig) - require.True(t, valid) +func TestSignaturesHD(t *testing.T) { + for account := uint32(0); account < 100; account += 30 { + msg := getFakeTx(account) + + path := *hd.NewFundraiserParams(account, account/5) + fmt.Printf("Checking signature at %v\n", path) + + priv, err := NewPrivKeyLedgerSecp256k1(path) + require.Nil(t, err, "%s", err) + + pub := priv.PubKey() + sig, err := priv.Sign(msg) + require.Nil(t, err) + + valid := pub.VerifyBytes(msg, sig) + require.True(t, valid, "Is your device using test mnemonic: %s ?", testMnemonic) + } } func TestRealLedgerSecp256k1(t *testing.T) { - msg := []byte("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"5000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}") + msg := getFakeTx(50) path := *hd.NewFundraiserParams(0, 0) priv, err := NewPrivKeyLedgerSecp256k1(path) require.Nil(t, err, "%s", err) From eff4dcdf2e4f5afbc4ba87cc926adf73777a9dea Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 12:31:17 +0100 Subject: [PATCH 11/17] Removing old redundant types --- client/keys/add.go | 3 +-- crypto/keys/keybase.go | 4 ++-- crypto/keys/types.go | 12 +++++------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index b3af588dc8b4..ee3d3bb0f953 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -20,7 +20,6 @@ import ( "github.com/tendermint/tendermint/libs/cli" "github.com/cosmos/cosmos-sdk/client" - ccrypto "github.com/cosmos/cosmos-sdk/crypto" "github.com/cosmos/cosmos-sdk/crypto/keys" "github.com/cosmos/cosmos-sdk/crypto/keys/hd" sdk "github.com/cosmos/cosmos-sdk/types" @@ -179,7 +178,7 @@ func runAddCmd(cmd *cobra.Command, args []string) error { if viper.GetBool(client.FlagUseLedger) { account := uint32(viper.GetInt(flagAccount)) index := uint32(viper.GetInt(flagIndex)) - path := ccrypto.DerivationPath{44, 118, account, 0, index} + path := *hd.NewFundraiserParams(account, index) info, err := kb.CreateLedger(name, path, keys.Secp256k1) if err != nil { return err diff --git a/crypto/keys/keybase.go b/crypto/keys/keybase.go index e202cd6d816f..60c98f76d755 100644 --- a/crypto/keys/keybase.go +++ b/crypto/keys/keybase.go @@ -158,7 +158,7 @@ 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, path crypto.DerivationPath, algo SigningAlgo) (Info, error) { +func (kb dbKeybase) CreateLedger(name string, path hd.BIP44Params, algo SigningAlgo) (Info, error) { if algo != Secp256k1 { return nil, ErrUnsupportedSigningAlgo } @@ -432,7 +432,7 @@ func (kb dbKeybase) writeLocalKey(priv tmcrypto.PrivKey, name, passphrase string return info } -func (kb dbKeybase) writeLedgerKey(pub tmcrypto.PubKey, path crypto.DerivationPath, name string) Info { +func (kb dbKeybase) writeLedgerKey(pub tmcrypto.PubKey, path hd.BIP44Params, name string) Info { info := newLedgerInfo(name, pub, path) kb.writeInfo(info, name) return info diff --git a/crypto/keys/types.go b/crypto/keys/types.go index 14d050961135..713b083ab093 100644 --- a/crypto/keys/types.go +++ b/crypto/keys/types.go @@ -3,8 +3,6 @@ package keys import ( "github.com/tendermint/tendermint/crypto" - ccrypto "github.com/cosmos/cosmos-sdk/crypto" - "github.com/cosmos/cosmos-sdk/crypto/keys/hd" "github.com/cosmos/cosmos-sdk/types" ) @@ -34,7 +32,7 @@ type Keybase interface { Derive(name, mnemonic, bip39Passwd, encryptPasswd string, params hd.BIP44Params) (Info, error) // Create, store, and return a new Ledger key reference - CreateLedger(name string, path ccrypto.DerivationPath, algo SigningAlgo) (info Info, err error) + CreateLedger(name string, path hd.BIP44Params, algo SigningAlgo) (info Info, err error) // Create, store, and return a new offline key reference CreateOffline(name string, pubkey crypto.PubKey) (info Info, err error) @@ -123,12 +121,12 @@ func (i localInfo) GetAddress() types.AccAddress { // ledgerInfo is the public information about a Ledger key type ledgerInfo struct { - Name string `json:"name"` - PubKey crypto.PubKey `json:"pubkey"` - Path ccrypto.DerivationPath `json:"path"` + Name string `json:"name"` + PubKey crypto.PubKey `json:"pubkey"` + Path hd.BIP44Params `json:"path"` } -func newLedgerInfo(name string, pub crypto.PubKey, path ccrypto.DerivationPath) Info { +func newLedgerInfo(name string, pub crypto.PubKey, path hd.BIP44Params) Info { return &ledgerInfo{ Name: name, PubKey: pub, From 8bc473644da8b91256105cf2ba7a47ad6b3810f8 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 12:57:38 +0100 Subject: [PATCH 12/17] fixing linter issue in defer calls --- crypto/ledger_secp256k1.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index e9c2d8f559c1..5edaf98b7b0c 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -2,6 +2,7 @@ package crypto import ( "fmt" + "os" "github.com/btcsuite/btcd/btcec" "github.com/cosmos/cosmos-sdk/crypto/keys/hd" @@ -52,7 +53,7 @@ func NewPrivKeyLedgerSecp256k1(path hd.BIP44Params) (tmcrypto.PrivKey, error) { if err != nil { return nil, err } - defer device.Close() + defer warnIfErrors(device.Close) pubKey, err := getPubKey(device, path) if err != nil { @@ -73,7 +74,7 @@ func (pkl PrivKeyLedgerSecp256k1) Sign(message []byte) ([]byte, error) { if err != nil { return nil, err } - defer device.Close() + defer warnIfErrors(device.Close) return sign(device, pkl, message) } @@ -85,7 +86,7 @@ func (pkl PrivKeyLedgerSecp256k1) ValidateKey() error { if err != nil { return err } - defer device.Close() + defer warnIfErrors(device.Close) return validateKey(device, pkl) } @@ -108,6 +109,14 @@ func (pkl PrivKeyLedgerSecp256k1) Equals(other tmcrypto.PrivKey) bool { return false } +// warnIfErrors wraps a function and writes a warning to stderr. This is required +// to avoid ignoring errors when defer is used. Using defer may result in linter warnings. +func warnIfErrors(f func() error) { + if err := f(); err != nil { + _, _ = fmt.Fprint(os.Stderr, "received error when closing ledger connection", err) + } +} + func convertDERtoBER(signatureDER []byte) ([]byte, error) { sigDER, err := btcec.ParseDERSignature(signatureDER[:], btcec.S256()) if err != nil { From fbf586935f4dfd9d5989072e0edf7f8bb345c1e4 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 13:03:01 +0100 Subject: [PATCH 13/17] updating pending.md --- PENDING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/PENDING.md b/PENDING.md index bb5e7a4c6b21..f07c619b831e 100644 --- a/PENDING.md +++ b/PENDING.md @@ -78,6 +78,7 @@ BUG FIXES - [\#3419](https://github.com/cosmos/cosmos-sdk/pull/3419) Fix `q distr slashes` panic - [\#3453](https://github.com/cosmos/cosmos-sdk/pull/3453) The `rest-server` command didn't respect persistent flags such as `--chain-id` and `--trust-node` if they were passed on the command line. + - [\#3441](https://github.com/cosmos/cosmos-sdk/pull/3431) Improved resource management and connection handling (ledger devices). Fixes issue with DER vs BER signatures. * Gaia From e3b2a21f290e54185f2330b552be2104f22a2b23 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sat, 2 Feb 2019 14:11:11 +0100 Subject: [PATCH 14/17] fixing tagging issue --- Makefile | 2 +- crypto/ledger.go | 2 +- crypto/ledger_integration_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 46fb34f6bcf0..5568c49e2da3 100644 --- a/Makefile +++ b/Makefile @@ -145,7 +145,7 @@ test_cli: @go test -p 4 `go list github.com/cosmos/cosmos-sdk/cmd/gaia/cli_test` -tags=cli_test test_ledger: - @go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags=ledger_device + @go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger real_ledger' test_unit: @VERSION=$(VERSION) go test $(PACKAGES_NOSIMULATION) diff --git a/crypto/ledger.go b/crypto/ledger.go index f3d0e3859b91..36b5646b86f7 100644 --- a/crypto/ledger.go +++ b/crypto/ledger.go @@ -1,4 +1,4 @@ -// +build cgo ledger +// +build cgo,ledger package crypto diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go index 667a6c40cc38..0ad9748101f7 100644 --- a/crypto/ledger_integration_test.go +++ b/crypto/ledger_integration_test.go @@ -1,4 +1,4 @@ -// +build cgo ledger real_ledger +// +build cgo,ledger,real_ledger package crypto From 1bfcf51469d93f48dbd0462b0a2bd0fe40b2fbfb Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Sun, 3 Feb 2019 19:52:12 +0100 Subject: [PATCH 15/17] Update crypto/ledger_secp256k1.go Co-Authored-By: jleni --- crypto/ledger_secp256k1.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/ledger_secp256k1.go b/crypto/ledger_secp256k1.go index 5edaf98b7b0c..5be8405f5e60 100644 --- a/crypto/ledger_secp256k1.go +++ b/crypto/ledger_secp256k1.go @@ -184,7 +184,7 @@ func getPubKey(device LedgerSECP256K1, path hd.BIP44Params) (tmcrypto.PubKey, er // 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 publicKey: %v", err) + return nil, fmt.Errorf("error parsing public key: %v", err) } var compressedPublicKey tmsecp256k1.PubKeySecp256k1 From 58f30605298409daf1024852bf2328ac9cb71215 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Sun, 3 Feb 2019 20:23:48 +0100 Subject: [PATCH 16/17] Update ledger_integration_test.go --- crypto/ledger_integration_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go index 0ad9748101f7..d8458b4021fe 100644 --- a/crypto/ledger_integration_test.go +++ b/crypto/ledger_integration_test.go @@ -80,12 +80,6 @@ func TestPublicKeyHDPath(t *testing.T) { "cosmospub1addwnpepqw970u6gjqkccg9u3rfj99857wupj2z9fqfzy2w7e5dd7xn7kzzgkgqch0r", } - // TODO: Check data with locally generated pubkey - // TODO: ONHOLD, it needs PR #3461 to get merged first - //seed, _ := bip39.NewSeedWithErrorChecking(testMnemonic, "") - //masterPriv, ch := hd.ComputeMastersFromSeed(seed) - //derivedPriv, err := hd.DerivePrivateKeyForPath(masterPriv, ch, fullHdPath) - // Check with device for i := uint32(0); i < 10; i++ { path := *hd.NewFundraiserParams(0, i) From 16e036204037ea2bba0ec56f9dd13f16f975f407 Mon Sep 17 00:00:00 2001 From: Juan Leni Date: Mon, 4 Feb 2019 07:53:05 +0100 Subject: [PATCH 17/17] Making explicit this tag is test related only --- Makefile | 2 +- crypto/ledger_integration_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 5568c49e2da3..0b2c2c76be62 100644 --- a/Makefile +++ b/Makefile @@ -145,7 +145,7 @@ test_cli: @go test -p 4 `go list github.com/cosmos/cosmos-sdk/cmd/gaia/cli_test` -tags=cli_test test_ledger: - @go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger real_ledger' + @go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger test_real_ledger' test_unit: @VERSION=$(VERSION) go test $(PACKAGES_NOSIMULATION) diff --git a/crypto/ledger_integration_test.go b/crypto/ledger_integration_test.go index d8458b4021fe..30c66c765286 100644 --- a/crypto/ledger_integration_test.go +++ b/crypto/ledger_integration_test.go @@ -1,4 +1,4 @@ -// +build cgo,ledger,real_ledger +// +build cgo,ledger,test_real_ledger package crypto