From b74eecc9741f29f0cb1506065d3169b261758132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Mon, 29 Jul 2019 15:29:24 +0200 Subject: [PATCH 1/9] cmd/bnscli: use mnemonic to generate private key Instead of generating a random private key use algorithm described in https://github.com/iov-one/iov-core/blob/master/docs/address-derivation-v1.md to create a mnemonic and later use that mnemonic to create a private key. resolve #855 Tests are missing. --- cmd/bnscli/cmd_key.go | 58 ++++++++++++++++++++++++++++++++++++++++--- cmd/bnscli/common.go | 20 +++++++++++++++ cmd/bnscli/main.go | 1 + go.mod | 3 ++- go.sum | 6 +++-- 5 files changed, 81 insertions(+), 7 deletions(-) diff --git a/cmd/bnscli/cmd_key.go b/cmd/bnscli/cmd_key.go index e3db89e7..782667b9 100644 --- a/cmd/bnscli/cmd_key.go +++ b/cmd/bnscli/cmd_key.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "flag" "fmt" "io" @@ -8,6 +9,9 @@ import ( "os" "github.com/iov-one/weave/crypto" + "github.com/iov-one/weave/crypto/bech32" + "github.com/stellar/go/exp/crypto/derivation" + "github.com/tyler-smith/go-bip39" "golang.org/x/crypto/ed25519" ) @@ -15,7 +19,7 @@ func cmdKeygen(input io.Reader, output io.Writer, args []string) error { fl := flag.NewFlagSet("", flag.ExitOnError) fl.Usage = func() { fmt.Fprint(flag.CommandLine.Output(), ` -Generate a new private key. +Read mnemonic and generate a new private key. When successful a new file with binary content containing private key is created. This command fails if the private key file already exists. @@ -25,6 +29,7 @@ created. This command fails if the private key file already exists. var ( keyPathFl = fl.String("key", env("BNSCLI_PRIV_KEY", os.Getenv("HOME")+"/.bnsd.priv.key"), "Path to the private key file that transaction should be signed with. You can use BNSCLI_PRIV_KEY environment variable to set it.") + pathFl = fl.String("path", "m/44'/234'/0'", "Derivation path as described in BIP-44.") ) fl.Parse(args) @@ -35,9 +40,17 @@ created. This command fails if the private key file already exists. return fmt.Errorf("private key file %q already exists, delete this file and try again", *keyPathFl) } - _, priv, err := ed25519.GenerateKey(nil) + mnemonic, err := readInput(input) if err != nil { - return fmt.Errorf("cannot generate ed25519 key: %s", err) + return fmt.Errorf("cannot read mnemonic: %s", err) + } + + // We do not allow for passphrase. + seed := bip39.NewSeed(string(mnemonic), "") + + key, err := derivation.DeriveForPath(*pathFl, seed) + if err != nil { + return fmt.Errorf("cannot deriviate master key from seed: %s", err) } fd, err := os.OpenFile(*keyPathFl, os.O_CREATE|os.O_WRONLY, 0400) @@ -46,6 +59,11 @@ created. This command fails if the private key file already exists. } defer fd.Close() + _, priv, err := ed25519.GenerateKey(bytes.NewReader(key.Key)) + if err != nil { + return fmt.Errorf("cannot generate ed25519 private key: %s", err) + } + if _, err := fd.Write(priv); err != nil { return fmt.Errorf("cannot write private key: %s", err) } @@ -83,6 +101,38 @@ Print out a hex-address associated with your private key. Ed25519: raw, }, } - _, err = fmt.Fprintln(output, key.PublicKey().Address()) + + bech, err := bech32.Encode("iov", key.PublicKey().GetEd25519()) + if err != nil { + return fmt.Errorf("cannot generate bech32 address format: %s", err) + } + + fmt.Fprintf(output, "bech32\t%s\n", bech) + fmt.Fprintf(output, "hex\t%s\n", key.PublicKey().Address()) + return nil +} + +func cmdMnemonic(input io.Reader, output io.Writer, args []string) error { + fl := flag.NewFlagSet("", flag.ExitOnError) + fl.Usage = func() { + fmt.Fprint(flag.CommandLine.Output(), ` +Generate and print out a mnemonic. +`) + } + var ( + bitSizeFl = fl.Uint("size", 256, "Bit size of the entropy. Must be between 128 and 256.") + ) + fl.Parse(args) + + entropy, err := bip39.NewEntropy(int(*bitSizeFl)) + if err != nil { + return fmt.Errorf("cannot create entropy instance: %s", err) + } + mnemonic, err := bip39.NewMnemonic(entropy) + if err != nil { + return fmt.Errorf("cannot create mnemonic instance: %s", err) + } + + _, err = fmt.Fprintln(output, mnemonic) return err } diff --git a/cmd/bnscli/common.go b/cmd/bnscli/common.go index da6cd141..7d791d4c 100644 --- a/cmd/bnscli/common.go +++ b/cmd/bnscli/common.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "os" "strconv" "strings" @@ -182,3 +183,22 @@ func tendermintStore(nodeURL string) weave.ReadOnlyKVStore { tm := rpcclient.NewHTTP(nodeURL, "/websocket") return app.NewABCIStore(rpcQueryWrapper{tm}) } + +// readInput returns all bytes waiting on given input. This function immediatly +// returns errNoPipe error if the input is not piped to avoid forever waiting. +func readInput(input io.Reader) ([]byte, error) { + // If the given reader is providing a stat information (ie os.Stdin) + // then check if the data is being piped. That should prevent us from + // waiting for a data on a reader that no one ever writes to. + if s, ok := input.(stater); ok { + if info, err := s.Stat(); err == nil { + isPipe := (info.Mode() & os.ModeCharDevice) == 0 + if !isPipe { + return nil, errNoPipe + } + } + } + return ioutil.ReadAll(input) +} + +var errNoPipe = errors.New("no data piped") diff --git a/cmd/bnscli/main.go b/cmd/bnscli/main.go index 6679e472..bc83b6aa 100644 --- a/cmd/bnscli/main.go +++ b/cmd/bnscli/main.go @@ -42,6 +42,7 @@ var commands = map[string]func(input io.Reader, output io.Writer, args []string) "from-sequence": cmdFromSequence, "keyaddr": cmdKeyaddr, "keygen": cmdKeygen, + "mnemonic": cmdMnemonic, "multisig": cmdMultisig, "query": cmdQuery, "register-username": cmdRegisterUsername, diff --git a/go.mod b/go.mod index 2d13bd93..bc94e572 100644 --- a/go.mod +++ b/go.mod @@ -17,12 +17,13 @@ require ( github.com/prometheus/client_golang v0.9.3 // indirect github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a // indirect github.com/rs/cors v1.6.0 // indirect - github.com/stellar/go v0.0.0-20190524153138-e5e03dc34e2d + github.com/stellar/go v0.0.0-20190723221356-14eed5a46caf github.com/stellar/go-xdr v0.0.0-20180917104419-0bc96f33a18e // indirect github.com/syndtr/goleveldb v1.0.0 // indirect github.com/tendermint/go-amino v0.15.0 github.com/tendermint/iavl v0.12.2 github.com/tendermint/tendermint v0.31.5 + github.com/tyler-smith/go-bip39 v1.0.0 golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f google.golang.org/grpc v1.21.0 // indirect ) diff --git a/go.sum b/go.sum index dbe19459..11266ab3 100644 --- a/go.sum +++ b/go.sum @@ -108,8 +108,8 @@ github.com/rs/cors v1.6.0 h1:G9tHG9lebljV9mfp9SNPDL36nCDxmo3zTlAf1YgvzmI= github.com/rs/cors v1.6.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= -github.com/stellar/go v0.0.0-20190524153138-e5e03dc34e2d h1:qlw1pPjIH7Uo9t1BeP+/pHvqAxB0Yq22zLpWzY1Uu4I= -github.com/stellar/go v0.0.0-20190524153138-e5e03dc34e2d/go.mod h1:Kkro8X6IWn/5XtSicGd6N2LZKMKUCWS5wS5Ctjh6+Vw= +github.com/stellar/go v0.0.0-20190723221356-14eed5a46caf h1:gLIFkwCtIquj9iFCPy595EFSmgJbQIZMLAG6gFHcNrI= +github.com/stellar/go v0.0.0-20190723221356-14eed5a46caf/go.mod h1:Kkro8X6IWn/5XtSicGd6N2LZKMKUCWS5wS5Ctjh6+Vw= github.com/stellar/go-xdr v0.0.0-20180917104419-0bc96f33a18e h1:n/hfey8pO+RYMoGXyvyzuw5pdO8IFDoyAL/g5OiCesY= github.com/stellar/go-xdr v0.0.0-20180917104419-0bc96f33a18e/go.mod h1:gpOLVzy6TVYTQ3LvHSN9RJC700FkhFCpSE82u37aNRM= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -125,6 +125,8 @@ github.com/tendermint/iavl v0.12.2 h1:Ls5p5VINCM1HRT9g5Vvs2zmDOCU/CCIvIHzd/pZ8P0 github.com/tendermint/iavl v0.12.2/go.mod h1:EoKMMv++tDOL5qKKVnoIqtVPshRrEPeJ0WsgDOLAauM= github.com/tendermint/tendermint v0.31.5 h1:vTet8tCq3B9/J9Yo11dNZ8pOB7NtSy++bVSfkP4KzR4= github.com/tendermint/tendermint v0.31.5/go.mod h1:ymcPyWblXCplCPQjbOYbrF1fWnpslATMVqiGgWbZrlc= +github.com/tyler-smith/go-bip39 v1.0.0 h1:FOHg9gaQLeBBRbHE/QrTLfEiBHy5pQ/yXzf9JG5pYFM= +github.com/tyler-smith/go-bip39 v1.0.0/go.mod h1:sJ5fKU0s6JVwZjjcUEX2zFOnvq0ASQ2K9Zr6cf67kNs= golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= From ef5fb9f53c074f9bd2c6687008e6054dc5fff3ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Mon, 29 Jul 2019 16:12:08 +0200 Subject: [PATCH 2/9] bnscli: validate mnemonic before using --- cmd/bnscli/cmd_key.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/bnscli/cmd_key.go b/cmd/bnscli/cmd_key.go index 782667b9..70c34eba 100644 --- a/cmd/bnscli/cmd_key.go +++ b/cmd/bnscli/cmd_key.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "errors" "flag" "fmt" "io" @@ -45,6 +46,10 @@ created. This command fails if the private key file already exists. return fmt.Errorf("cannot read mnemonic: %s", err) } + if !bip39.IsMnemonicValid(string(mnemonic)) { + return errors.New("invalid mnemonic") + } + // We do not allow for passphrase. seed := bip39.NewSeed(string(mnemonic), "") From 88f43b217a679c33b308095f45231783abc400f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Tue, 30 Jul 2019 11:28:07 +0200 Subject: [PATCH 3/9] update bnscli key management --- cmd/bnscli/clitests/keyaddr.test | 2 + cmd/bnscli/clitests/keyaddr.test.gold | 3 +- cmd/bnscli/clitests/keygen.test | 6 ++- cmd/bnscli/cmd_key.go | 56 +++++++++++++++++++-------- cmd/bnscli/cmd_key_test.go | 39 +++++++++++++++++++ 5 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 cmd/bnscli/cmd_key_test.go diff --git a/cmd/bnscli/clitests/keyaddr.test b/cmd/bnscli/clitests/keyaddr.test index 1618515f..87ec43e7 100644 --- a/cmd/bnscli/clitests/keyaddr.test +++ b/cmd/bnscli/clitests/keyaddr.test @@ -8,6 +8,8 @@ set -e keyfile=`mktemp` echo 00wZcK6QrPNAXy2Z3KyhbQx9s3n0vq/P32Z7nWnONQ0n9ftEBQnfp57Ig6BRC8mpYUw9RBiIgfDF5AKJi0vzyQ== | base64 --decode > $keyfile +# The output of this command can be verified using iov-core +# https://iov-one.github.io/token-finder/#E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0 bnscli keyaddr -key $keyfile rm $keyfile diff --git a/cmd/bnscli/clitests/keyaddr.test.gold b/cmd/bnscli/clitests/keyaddr.test.gold index f304761c..ba4b0d0a 100644 --- a/cmd/bnscli/clitests/keyaddr.test.gold +++ b/cmd/bnscli/clitests/keyaddr.test.gold @@ -1 +1,2 @@ -E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0 +bech32 iov1u29wnfhtjn7g3de7kl9adwrmlyltn0hsudlacf +hex E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0 diff --git a/cmd/bnscli/clitests/keygen.test b/cmd/bnscli/clitests/keygen.test index 6ae97efa..ee9a4d1e 100644 --- a/cmd/bnscli/clitests/keygen.test +++ b/cmd/bnscli/clitests/keygen.test @@ -2,12 +2,16 @@ set -e +mnemonic=`mktemp` +echo 'neutral abandon month park disease forum engage dutch coconut base morning icon wide stock coast fork girl fish despair kiss dilemma pass slide major' > $mnemonic + # Use a custom key path just in case the host is using original one. tempdir=`mktemp -d` keypath=$tempdir/key.priv export BNSCLI_PRIV_KEY=$keypath -bnscli keygen +bnscli keygen < $mnemonic +rm $mnemonic # Key generation in non deterministic (cryptography 101) so we cannot compare # its value. We can only ensure that enough bytes was generated. diff --git a/cmd/bnscli/cmd_key.go b/cmd/bnscli/cmd_key.go index 70c34eba..30e6d6e1 100644 --- a/cmd/bnscli/cmd_key.go +++ b/cmd/bnscli/cmd_key.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "crypto/sha256" "errors" "flag" "fmt" @@ -46,16 +47,9 @@ created. This command fails if the private key file already exists. return fmt.Errorf("cannot read mnemonic: %s", err) } - if !bip39.IsMnemonicValid(string(mnemonic)) { - return errors.New("invalid mnemonic") - } - - // We do not allow for passphrase. - seed := bip39.NewSeed(string(mnemonic), "") - - key, err := derivation.DeriveForPath(*pathFl, seed) + priv, err := keygen(string(mnemonic), *pathFl) if err != nil { - return fmt.Errorf("cannot deriviate master key from seed: %s", err) + return fmt.Errorf("cannot generate key: %s", err) } fd, err := os.OpenFile(*keyPathFl, os.O_CREATE|os.O_WRONLY, 0400) @@ -64,11 +58,6 @@ created. This command fails if the private key file already exists. } defer fd.Close() - _, priv, err := ed25519.GenerateKey(bytes.NewReader(key.Key)) - if err != nil { - return fmt.Errorf("cannot generate ed25519 private key: %s", err) - } - if _, err := fd.Write(priv); err != nil { return fmt.Errorf("cannot write private key: %s", err) } @@ -78,6 +67,28 @@ created. This command fails if the private key file already exists. return nil } +// keygen returns a private key generated using given mnemonic and derivation +// path. +func keygen(mnemonic, derivationPath string) (ed25519.PrivateKey, error) { + if !bip39.IsMnemonicValid(string(mnemonic)) { + return nil, errors.New("invalid mnemonic") + } + + // We do not allow for passphrase. + seed := bip39.NewSeed(string(mnemonic), "") + + key, err := derivation.DeriveForPath(derivationPath, seed) + if err != nil { + return nil, fmt.Errorf("cannot deriviate master key from seed: %s", err) + } + + _, priv, err := ed25519.GenerateKey(bytes.NewReader(key.Key)) + if err != nil { + return nil, fmt.Errorf("cannot generate ed25519 private key: %s", err) + } + return priv, nil +} + func cmdKeyaddr(input io.Reader, output io.Writer, args []string) error { fl := flag.NewFlagSet("", flag.ExitOnError) fl.Usage = func() { @@ -89,6 +100,7 @@ Print out a hex-address associated with your private key. var ( keyPathFl = fl.String("key", env("BNSCLI_PRIV_KEY", os.Getenv("HOME")+"/.bnsd.priv.key"), "Path to the private key file that transaction should be signed with. You can use BNSCLI_PRIV_KEY environment variable to set it.") + bechPrefixFl = fl.String("bp", "iov", "Bech32 prefix.") ) fl.Parse(args) @@ -107,7 +119,7 @@ Print out a hex-address associated with your private key. }, } - bech, err := bech32.Encode("iov", key.PublicKey().GetEd25519()) + bech, err := toBech32(*bechPrefixFl, key.PublicKey().GetEd25519()) if err != nil { return fmt.Errorf("cannot generate bech32 address format: %s", err) } @@ -117,11 +129,23 @@ Print out a hex-address associated with your private key. return nil } +// toBech32 computes the bech32 address representation as described in +// https://github.com/iov-one/iov-core/blob/8846fed17443766a9ad9c908c3d7fc9d205e02ef/docs/address-derivation-v1.md#deriving-addresses-from-keypairs +func toBech32(prefix string, pubkey []byte) ([]byte, error) { + data := append([]byte("sigs/ed25519/"), pubkey...) + hash := sha256.Sum256(data) + bech, err := bech32.Encode(prefix, hash[:20]) + if err != nil { + return nil, fmt.Errorf("cannot compute bech32: %s", err) + } + return bech, nil +} + func cmdMnemonic(input io.Reader, output io.Writer, args []string) error { fl := flag.NewFlagSet("", flag.ExitOnError) fl.Usage = func() { fmt.Fprint(flag.CommandLine.Output(), ` -Generate and print out a mnemonic. +Generate and print out a mnemonic. Keep the result in safe place! `) } var ( diff --git a/cmd/bnscli/cmd_key_test.go b/cmd/bnscli/cmd_key_test.go new file mode 100644 index 00000000..93b4f616 --- /dev/null +++ b/cmd/bnscli/cmd_key_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "testing" + + "golang.org/x/crypto/ed25519" +) + +func TestKeygent(t *testing.T) { + const mnemonic = `shy else mystery outer define there front bracket dawn honey excuse virus lazy book kiss cannon oven law coconut hedgehog veteran narrow great cage` + + // Result of this test can be verified using iov-core implementation + // available at https://iov-one.github.io/token-finder/ + cases := map[string]string{ + "m/44'/234'/0'": "tiov1c3n70dph9m2jepszfmmh84pu75zuga3zrsd7jw", + "m/44'/234'/1'": "tiov10lzv8v2lds7jvmkdt6t6khmhydr920r2yux8p9", + "m/44'/234'/2'": "tiov18gwds8rx8cajav3m4lr5j98vlly9n8ms930z2l", + "m/44'/234'/3'": "tiov1casuhjhjcqlxhlcfpqak5uccpqyajzp0nj3639", + "m/44'/234'/4'": "tiov16rjld9tw88yrcc954cvvtnern576daunnn8jmn", + } + + for path, bech := range cases { + t.Run(path, func(t *testing.T) { + priv, err := keygen(mnemonic, path) + if err != nil { + t.Fatalf("cannot generate key: %s", err) + } + b, err := toBech32("tiov", priv.Public().(ed25519.PublicKey)) + if err != nil { + t.Fatalf("cannot serialize to bech32: %s", err) + } + if got := string(b); got != bech { + t.Logf("want: %s", bech) + t.Logf(" got: %s", got) + t.Fatal("unexpected bech address") + } + }) + } +} From 93f3406bf9d5247bcf0a98e83eb3e25efadb2706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Tue, 30 Jul 2019 11:56:41 +0200 Subject: [PATCH 4/9] CHANGELOG --- CHANGELOG.md | 8 ++++++++ cmd/bnscli/cmd_key_test.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a5373bc..69dd8a81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## HEAD +- `cmd/bnscli`: a new command `mnemonic` was added for generating a random + mnemonic as described in BIP-39. + +Breaking changes + +- `cmd/bnscli`: `keygen` command was updated and requires a mnemonic to + generate a key. + ## 0.20.0 diff --git a/cmd/bnscli/cmd_key_test.go b/cmd/bnscli/cmd_key_test.go index 93b4f616..24755f0e 100644 --- a/cmd/bnscli/cmd_key_test.go +++ b/cmd/bnscli/cmd_key_test.go @@ -6,7 +6,7 @@ import ( "golang.org/x/crypto/ed25519" ) -func TestKeygent(t *testing.T) { +func TestKeygen(t *testing.T) { const mnemonic = `shy else mystery outer define there front bracket dawn honey excuse virus lazy book kiss cannon oven law coconut hedgehog veteran narrow great cage` // Result of this test can be verified using iov-core implementation From 4b19844a839fe8f091a0cb49952c968dd57a2342 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Wed, 31 Jul 2019 09:36:28 +0200 Subject: [PATCH 5/9] bnscli: mnemonic tests --- cmd/bnscli/cmd_key_test.go | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/cmd/bnscli/cmd_key_test.go b/cmd/bnscli/cmd_key_test.go index 24755f0e..76bddec0 100644 --- a/cmd/bnscli/cmd_key_test.go +++ b/cmd/bnscli/cmd_key_test.go @@ -37,3 +37,59 @@ func TestKeygen(t *testing.T) { }) } } + +func TestMnemonic(t *testing.T) { + cases := map[string]struct { + mnemonic string + wantErr bool + }{ + + "valid mnemonic 12 words": { + mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology", + wantErr: false, + }, + "valid mnemonic 15 words": { + mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key", + wantErr: false, + }, + "valid mnemonic 18 words": { + mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key gain belt same", + wantErr: false, + }, + "valid mnemonic 21 words": { + mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key gain belt same matrix chase mom", + wantErr: false, + }, + "valid mnemonic 24 words": { + mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key gain belt same matrix chase mom beyond model toy", + wantErr: false, + }, + "additional whitespace around mnemonnic is ignored": { + mnemonic: ` + usage + mountain + noodle + inspire distance lyrics caution wait mansion + never announce biology + `, + wantErr: false, + }, + "mnenomic that is valid in a language other than English (Italian)": { + mnemonic: "acrobata acuto adagio addebito addome adeguato aderire adipe adottare adulare affabile affetto affisso affranto aforisma", + wantErr: true, + }, + "mnenomic that is valid in a language other than English (Japanese)": { + mnemonic: " あつかう あっしゅく あつまり あつめる あてな あてはまる あひる あぶら あぶる あふれる あまい あまど ", + wantErr: true, + }, + } + + for testName, tc := range cases { + t.Run(testName, func(t *testing.T) { + _, err := keygen(tc.mnemonic, "m/44'/234'/0'") + if hasErr := err != nil; hasErr != tc.wantErr { + t.Fatalf("returned erorr value: %+v", err) + } + }) + } +} From fd82cc981f08f41ecbcc16f9ed8ad48665c17fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Wed, 31 Jul 2019 14:19:21 +0200 Subject: [PATCH 6/9] bnscli: improve mnemonic validation --- cmd/bnscli/cmd_key.go | 12 +++++++++++- cmd/bnscli/cmd_key_test.go | 24 +++++++++++++++--------- go.mod | 2 +- go.sum | 4 ++-- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/cmd/bnscli/cmd_key.go b/cmd/bnscli/cmd_key.go index 30e6d6e1..653d95f0 100644 --- a/cmd/bnscli/cmd_key.go +++ b/cmd/bnscli/cmd_key.go @@ -70,7 +70,7 @@ created. This command fails if the private key file already exists. // keygen returns a private key generated using given mnemonic and derivation // path. func keygen(mnemonic, derivationPath string) (ed25519.PrivateKey, error) { - if !bip39.IsMnemonicValid(string(mnemonic)) { + if !isMnemonicValid(string(mnemonic)) { return nil, errors.New("invalid mnemonic") } @@ -89,6 +89,15 @@ func keygen(mnemonic, derivationPath string) (ed25519.PrivateKey, error) { return priv, nil } +// isMnemonicValid returns true if given mnemonic string is valid. Whitespaces +// are ignored. +// Use this instead of bip39.IsMnemonicValid because this function ensures the +// checksum consistency. bip39.IsMnemonicValid does not test the checksum. +func isMnemonicValid(mnemonic string) bool { + _, err := bip39.EntropyFromMnemonic(mnemonic) + return err == nil +} + func cmdKeyaddr(input io.Reader, output io.Writer, args []string) error { fl := flag.NewFlagSet("", flag.ExitOnError) fl.Usage = func() { @@ -147,6 +156,7 @@ func cmdMnemonic(input io.Reader, output io.Writer, args []string) error { fmt.Fprint(flag.CommandLine.Output(), ` Generate and print out a mnemonic. Keep the result in safe place! `) + fl.PrintDefaults() } var ( bitSizeFl = fl.Uint("size", 256, "Bit size of the entropy. Must be between 128 and 256.") diff --git a/cmd/bnscli/cmd_key_test.go b/cmd/bnscli/cmd_key_test.go index 76bddec0..bd3bc980 100644 --- a/cmd/bnscli/cmd_key_test.go +++ b/cmd/bnscli/cmd_key_test.go @@ -45,19 +45,19 @@ func TestMnemonic(t *testing.T) { }{ "valid mnemonic 12 words": { - mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology", + mnemonic: "super bulk plunge better rookie donor reward obscure rescue type trade pelican", wantErr: false, }, "valid mnemonic 15 words": { - mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key", + mnemonic: "say debris entry orange grief deer train until flock scrub volume artist skill obscure immense", wantErr: false, }, "valid mnemonic 18 words": { - mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key gain belt same", + mnemonic: "fetch height snow poverty space follow seven festival wasp pet asset tattoo cement twist exile trend bench eternal", wantErr: false, }, "valid mnemonic 21 words": { - mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key gain belt same matrix chase mom", + mnemonic: "increase shine pumpkin curtain trash cabbage juice canal ugly naive name insane indoor assault snap taxi casual unhappy buddy defense artefact", wantErr: false, }, "valid mnemonic 24 words": { @@ -66,11 +66,13 @@ func TestMnemonic(t *testing.T) { }, "additional whitespace around mnemonnic is ignored": { mnemonic: ` - usage - mountain - noodle - inspire distance lyrics caution wait mansion - never announce biology + forget + rely tiny + ostrich drop edit + assault mechanic pony extend + together twelve + observe bullet dream + short glide crack orchard exotic zero fly spice final `, wantErr: false, }, @@ -82,6 +84,10 @@ func TestMnemonic(t *testing.T) { mnemonic: " あつかう あっしゅく あつまり あつめる あてな あてはまる あひる あぶら あぶる あふれる あまい あまど ", wantErr: true, }, + "initially valid mnemonic that the last word was changed": { + mnemonic: "super bulk plunge better rookie donor reward obscure rescue type trade trade", + wantErr: true, + }, } for testName, tc := range cases { diff --git a/go.mod b/go.mod index bc94e572..96c610b8 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/tendermint/go-amino v0.15.0 github.com/tendermint/iavl v0.12.2 github.com/tendermint/tendermint v0.31.5 - github.com/tyler-smith/go-bip39 v1.0.0 + github.com/tyler-smith/go-bip39 v1.0.1-0.20181017060643-dbb3b84ba2ef golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f google.golang.org/grpc v1.21.0 // indirect ) diff --git a/go.sum b/go.sum index 11266ab3..d2979bac 100644 --- a/go.sum +++ b/go.sum @@ -125,8 +125,8 @@ github.com/tendermint/iavl v0.12.2 h1:Ls5p5VINCM1HRT9g5Vvs2zmDOCU/CCIvIHzd/pZ8P0 github.com/tendermint/iavl v0.12.2/go.mod h1:EoKMMv++tDOL5qKKVnoIqtVPshRrEPeJ0WsgDOLAauM= github.com/tendermint/tendermint v0.31.5 h1:vTet8tCq3B9/J9Yo11dNZ8pOB7NtSy++bVSfkP4KzR4= github.com/tendermint/tendermint v0.31.5/go.mod h1:ymcPyWblXCplCPQjbOYbrF1fWnpslATMVqiGgWbZrlc= -github.com/tyler-smith/go-bip39 v1.0.0 h1:FOHg9gaQLeBBRbHE/QrTLfEiBHy5pQ/yXzf9JG5pYFM= -github.com/tyler-smith/go-bip39 v1.0.0/go.mod h1:sJ5fKU0s6JVwZjjcUEX2zFOnvq0ASQ2K9Zr6cf67kNs= +github.com/tyler-smith/go-bip39 v1.0.1-0.20181017060643-dbb3b84ba2ef h1:wHSqTBrZW24CsNJDfeh9Ex6Pm0Rcpc7qrgKBiL44vF4= +github.com/tyler-smith/go-bip39 v1.0.1-0.20181017060643-dbb3b84ba2ef/go.mod h1:sJ5fKU0s6JVwZjjcUEX2zFOnvq0ASQ2K9Zr6cf67kNs= golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= From d370993f1dcdc4a5b7583b357bca29bb3559178c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Wed, 31 Jul 2019 14:49:33 +0200 Subject: [PATCH 7/9] ensure mnemonic whitespace does not influence output --- cmd/bnscli/cmd_key_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/cmd/bnscli/cmd_key_test.go b/cmd/bnscli/cmd_key_test.go index bd3bc980..50b70f0e 100644 --- a/cmd/bnscli/cmd_key_test.go +++ b/cmd/bnscli/cmd_key_test.go @@ -1,8 +1,11 @@ package main import ( + "bytes" + "strings" "testing" + "github.com/tyler-smith/go-bip39" "golang.org/x/crypto/ed25519" ) @@ -99,3 +102,34 @@ func TestMnemonic(t *testing.T) { }) } } + +func TestMnemonicWhitespaceIsIgnored(t *testing.T) { + // The same mnemonic words formatted differently (different whitespace) + // produce the same key. + + words := []string{ + "super", "bulk", "plunge", "better", "rookie", "donor", + "reward", "obscure", "rescue", "type", "trade", "pelican", + } + + // Standard mnemonic is created by separating words with a single space. + stdMnemonic := strings.Join(words, " ") + stdSeed := bip39.NewSeed(stdMnemonic, "") + + // All other mnemonics are non standard but somehow supported by this + // bip39 implementation. + mnemonics := []string{ + strings.Join(words, "\t"), + strings.Join(words, " \n"), + strings.Join(words, " \t \n "), + } + + for i, m := range mnemonics { + s := bip39.NewSeed(m, "") + if !bytes.Equal(stdSeed, s) { + t.Logf("reference: %x", stdSeed) + t.Logf(" got: %x", s) + t.Fatalf("seed %d is different than standard seed", i) + } + } +} From 0094caee91b76c8fe5d541ed258e2f1527926435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Wed, 31 Jul 2019 15:49:03 +0200 Subject: [PATCH 8/9] more bip-39 package issues solved --- cmd/bnscli/cmd_key.go | 31 +++++++++++++++----- cmd/bnscli/cmd_key_test.go | 60 ++++++++++---------------------------- 2 files changed, 39 insertions(+), 52 deletions(-) diff --git a/cmd/bnscli/cmd_key.go b/cmd/bnscli/cmd_key.go index 653d95f0..108cf252 100644 --- a/cmd/bnscli/cmd_key.go +++ b/cmd/bnscli/cmd_key.go @@ -9,6 +9,7 @@ import ( "io" "io/ioutil" "os" + "strings" "github.com/iov-one/weave/crypto" "github.com/iov-one/weave/crypto/bech32" @@ -70,8 +71,8 @@ created. This command fails if the private key file already exists. // keygen returns a private key generated using given mnemonic and derivation // path. func keygen(mnemonic, derivationPath string) (ed25519.PrivateKey, error) { - if !isMnemonicValid(string(mnemonic)) { - return nil, errors.New("invalid mnemonic") + if err := validateMnemonic(string(mnemonic)); err != nil { + return nil, fmt.Errorf("invalid mnemonic: %s", err) } // We do not allow for passphrase. @@ -90,12 +91,28 @@ func keygen(mnemonic, derivationPath string) (ed25519.PrivateKey, error) { } // isMnemonicValid returns true if given mnemonic string is valid. Whitespaces -// are ignored. +// are relevant. +// // Use this instead of bip39.IsMnemonicValid because this function ensures the -// checksum consistency. bip39.IsMnemonicValid does not test the checksum. -func isMnemonicValid(mnemonic string) bool { - _, err := bip39.EntropyFromMnemonic(mnemonic) - return err == nil +// checksum consistency. bip39.IsMnemonicValid does not test the checksum. It +// also ignores whitespaces. +// +// This function ensures that the mnemonic is a single space separated list of +// words as this is important during seed creation. +func validateMnemonic(mnemonic string) error { + // A lazy way to check that words are exactly single space separated. + expected := strings.Join(strings.Fields(mnemonic), " ") + if mnemonic != expected { + return errors.New("whitespace violation") + } + + // Entropy generation does base validation of checking if words are + // valid and in the right amount. It also tests the checksum. + if _, err := bip39.EntropyFromMnemonic(mnemonic); err != nil { + return fmt.Errorf("entropy: %s", err) + } + + return nil } func cmdKeyaddr(input io.Reader, output io.Writer, args []string) error { diff --git a/cmd/bnscli/cmd_key_test.go b/cmd/bnscli/cmd_key_test.go index 50b70f0e..3b7d3610 100644 --- a/cmd/bnscli/cmd_key_test.go +++ b/cmd/bnscli/cmd_key_test.go @@ -1,11 +1,8 @@ package main import ( - "bytes" - "strings" "testing" - "github.com/tyler-smith/go-bip39" "golang.org/x/crypto/ed25519" ) @@ -67,17 +64,21 @@ func TestMnemonic(t *testing.T) { mnemonic: "usage mountain noodle inspire distance lyrics caution wait mansion never announce biology squirrel guess key gain belt same matrix chase mom beyond model toy", wantErr: false, }, - "additional whitespace around mnemonnic is ignored": { - mnemonic: ` - forget - rely tiny - ostrich drop edit - assault mechanic pony extend - together twelve - observe bullet dream - short glide crack orchard exotic zero fly spice final - `, - wantErr: false, + "additional whitespace around mnemonnic is not allowed (beginning)": { + mnemonic: " super bulk plunge better rookie donor reward obscure rescue type trade pelican", + wantErr: true, + }, + "additional whitespace around mnemonnic is not allowed (end)": { + mnemonic: "super bulk plunge better rookie donor reward obscure rescue type trade pelican ", + wantErr: true, + }, + "additional whitespace around mnemonnic is not allowed (middle)": { + mnemonic: "super bulk plunge better rookie donor reward obscure rescue type trade pelican", + wantErr: true, + }, + "mnemonnic cannot be tab separated": { + mnemonic: "super\tbulk plunge better rookie donor reward obscure rescue type trade pelican", + wantErr: true, }, "mnenomic that is valid in a language other than English (Italian)": { mnemonic: "acrobata acuto adagio addebito addome adeguato aderire adipe adottare adulare affabile affetto affisso affranto aforisma", @@ -102,34 +103,3 @@ func TestMnemonic(t *testing.T) { }) } } - -func TestMnemonicWhitespaceIsIgnored(t *testing.T) { - // The same mnemonic words formatted differently (different whitespace) - // produce the same key. - - words := []string{ - "super", "bulk", "plunge", "better", "rookie", "donor", - "reward", "obscure", "rescue", "type", "trade", "pelican", - } - - // Standard mnemonic is created by separating words with a single space. - stdMnemonic := strings.Join(words, " ") - stdSeed := bip39.NewSeed(stdMnemonic, "") - - // All other mnemonics are non standard but somehow supported by this - // bip39 implementation. - mnemonics := []string{ - strings.Join(words, "\t"), - strings.Join(words, " \n"), - strings.Join(words, " \t \n "), - } - - for i, m := range mnemonics { - s := bip39.NewSeed(m, "") - if !bytes.Equal(stdSeed, s) { - t.Logf("reference: %x", stdSeed) - t.Logf(" got: %x", s) - t.Fatalf("seed %d is different than standard seed", i) - } - } -} From d94e1c2712e8b897c8d17847800c89f0e2eccd2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Husiaty=C5=84ski?= Date: Thu, 1 Aug 2019 09:09:17 +0200 Subject: [PATCH 9/9] fix bnscli tests --- cmd/bnscli/clitests/keygen.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/bnscli/clitests/keygen.test b/cmd/bnscli/clitests/keygen.test index ee9a4d1e..3998b57d 100644 --- a/cmd/bnscli/clitests/keygen.test +++ b/cmd/bnscli/clitests/keygen.test @@ -3,7 +3,7 @@ set -e mnemonic=`mktemp` -echo 'neutral abandon month park disease forum engage dutch coconut base morning icon wide stock coast fork girl fish despair kiss dilemma pass slide major' > $mnemonic +echo -n 'neutral abandon month park disease forum engage dutch coconut base morning icon wide stock coast fork girl fish despair kiss dilemma pass slide major' > $mnemonic # Use a custom key path just in case the host is using original one. tempdir=`mktemp -d`