Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/bnscli: use mnemonic to generate private key #923

Merged
merged 9 commits into from Aug 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions cmd/bnscli/clitests/keyaddr.test
Expand Up @@ -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
3 changes: 2 additions & 1 deletion cmd/bnscli/clitests/keyaddr.test.gold
@@ -1 +1,2 @@
E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0
bech32 iov1u29wnfhtjn7g3de7kl9adwrmlyltn0hsudlacf
hex E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0
6 changes: 5 additions & 1 deletion cmd/bnscli/clitests/keygen.test
Expand Up @@ -2,12 +2,16 @@

set -e

mnemonic=`mktemp`
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`
keypath=$tempdir/key.priv
export BNSCLI_PRIV_KEY=$keypath

bnscli keygen
bnscli keygen < $mnemonic
rm $mnemonic
Copy link
Contributor

Choose a reason for hiding this comment

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

req: please update the comments below as key derivation is deterministic.


# Key generation in non deterministic (cryptography 101) so we cannot compare
# its value. We can only ensure that enough bytes was generated.
Expand Down
114 changes: 110 additions & 4 deletions cmd/bnscli/cmd_key.go
@@ -1,21 +1,28 @@
package main

import (
"bytes"
"crypto/sha256"
"errors"
"flag"
"fmt"
"io"
"io/ioutil"
"os"
"strings"

"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"
)

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.
Expand All @@ -25,6 +32,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.")
Copy link
Contributor

Choose a reason for hiding this comment

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

pp: please add default path to the description so that it can be copied and modified knowing our exact format and numbers if required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default is always printed as part of the help message.

  -path string
        Derivation path as described in BIP-44. (default "m/44'/234'/0'")

)
fl.Parse(args)

Expand All @@ -35,9 +43,14 @@ 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)
}

priv, err := keygen(string(mnemonic), *pathFl)
if err != nil {
return fmt.Errorf("cannot generate key: %s", err)
}

fd, err := os.OpenFile(*keyPathFl, os.O_CREATE|os.O_WRONLY, 0400)
Expand All @@ -55,6 +68,53 @@ 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 err := validateMnemonic(string(mnemonic)); err != nil {
return nil, fmt.Errorf("invalid mnemonic: %s", err)
}

// 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
}

// isMnemonicValid returns true if given mnemonic string is valid. Whitespaces
// are relevant.
//
// Use this instead of bip39.IsMnemonicValid because this function ensures the
// 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), " ")
Copy link
Contributor

Choose a reason for hiding this comment

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

Works for whitespace. Is upper/lower case still considered a problem here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think validation is case-sensitive as it has a fixed set of words that it is using to map each word to a numeric value. Using non lowercase letter would not return a match in the index.

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 {
fl := flag.NewFlagSet("", flag.ExitOnError)
fl.Usage = func() {
Expand All @@ -66,6 +126,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.")
Copy link
Contributor

Choose a reason for hiding this comment

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

req: please add the 2 formats we have for tiov testnet and iov as default to the description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? You can have only one default value.

)
fl.Parse(args)

Expand All @@ -83,6 +144,51 @@ Print out a hex-address associated with your private key.
Ed25519: raw,
},
}
_, err = fmt.Fprintln(output, key.PublicKey().Address())

bech, err := toBech32(*bechPrefixFl, 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
}

// 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)
Copy link
Contributor

Choose a reason for hiding this comment

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

pp: hash[:20] is the same as weave.NewCondition("sigs", "ed25519", pubkey).Address((). Please use existing logic instead for this sensible part.

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. 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.")
)
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
}
105 changes: 105 additions & 0 deletions cmd/bnscli/cmd_key_test.go
@@ -0,0 +1,105 @@
package main

import (
"testing"

"golang.org/x/crypto/ed25519"
)

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
// 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")
}
})
}
}

func TestMnemonic(t *testing.T) {
cases := map[string]struct {
mnemonic string
wantErr bool
}{

"valid mnemonic 12 words": {
mnemonic: "super bulk plunge better rookie donor reward obscure rescue type trade pelican",
wantErr: false,
},
"valid mnemonic 15 words": {
mnemonic: "say debris entry orange grief deer train until flock scrub volume artist skill obscure immense",
wantErr: false,
},
"valid mnemonic 18 words": {
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: "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": {
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,
Copy link
Contributor

Choose a reason for hiding this comment

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

The last mnemonic is valid but the cut ones (15–21 words) are not due to invalid checksum. How can those tests pass?

},
"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,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the additional whitespace test passing? When you add additional whitespace to a mnemonic, you get a different kypair/address

"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,
},
"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 {
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)
}
})
}
}
20 changes: 20 additions & 0 deletions cmd/bnscli/common.go
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"strconv"
"strings"
Expand Down Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

req: Please move const and vars before the methods using them.

1 change: 1 addition & 0 deletions cmd/bnscli/main.go
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Expand Up @@ -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.1-0.20181017060643-dbb3b84ba2ef
golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f
google.golang.org/grpc v1.21.0 // indirect
)
6 changes: 4 additions & 2 deletions go.sum
Expand Up @@ -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=
Expand All @@ -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.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=
Expand Down