-
Notifications
You must be signed in to change notification settings - Fork 47
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
Changes from 4 commits
b74eecc
ef5fb9f
88f43b2
93f3406
4b19844
fd82cc9
d370993
0094cae
d94e1c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0 | ||
bech32 iov1u29wnfhtjn7g3de7kl9adwrmlyltn0hsudlacf | ||
hex E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,27 @@ | ||
package main | ||
|
||
import ( | ||
"bytes" | ||
"crypto/sha256" | ||
"errors" | ||
"flag" | ||
"fmt" | ||
"io" | ||
"io/ioutil" | ||
"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" | ||
) | ||
|
||
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 +31,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.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default is always printed as part of the help message.
|
||
) | ||
fl.Parse(args) | ||
|
||
|
@@ -35,9 +42,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) | ||
|
@@ -55,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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we test this validation for
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code processing the mnemonic is not ours and usually I would argue that we should not test it. But here it is 4b19844
I do not understand this case, so I did not include it in the tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We don't need to test every single code branch of that external library but we should at least cover our integration of it as well as the bahaviour we expect from the API.
I'm glad we talk about it: A BIP39 mnemonic is not just a list of 12, 15, 18, 21 or 24 words from a 2048 element wordlist. It is encoded data plus a checksum. In a 12 word mnemonic for example, 11.64 words are arbitrary data and 0.36 words are a checksum. e.g.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems as the implementation is not complete.
I think the best is to create a pull request with a fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree. Good that we checked that ;) Regarding 1.: the behaviour is fair indeed. We should just be explicit about it since not every implementations wirks that way. In this case we should have a test that verifies that mnemonic and menonic+whitespace leads to the same keypair. |
||
} | ||
|
||
// 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() { | ||
|
@@ -66,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.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. req: please add the 2 formats we have for There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
||
|
@@ -83,6 +118,50 @@ 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pp: |
||
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! | ||
`) | ||
} | ||
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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
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") | ||
} | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. req: Please move const and vars before the methods using them. |
There was a problem hiding this comment.
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.