cmd/bnscli: use mnemonic to generate private key #923
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| bech32 iov1u29wnfhtjn7g3de7kl9adwrmlyltn0hsudlacf | ||
| hex E28AE9A6EB94FC88B73EB7CBD6B87BF93EB9BEF0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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(), ` | ||
| 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. 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. 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) | ||
| } | ||
|
|
||
| mnemonic, err := readInput(input) | ||
| if err != nil { | ||
| 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,37 @@ 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 !isMnemonicValid(string(mnemonic)) { | ||
| return nil, errors.New("invalid mnemonic") | ||
|
There was a problem hiding this comment. Can we test this validation for
There was a problem hiding this comment. 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.
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. 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.
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 | ||
| } | ||
|
|
||
| // 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() { | ||
|
|
@@ -66,6 +109,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. req: please add the 2 formats we have for There was a problem hiding this comment. What do you mean? You can have only one default value. |
||
| ) | ||
| fl.Parse(args) | ||
|
|
||
|
|
@@ -83,6 +127,51 @@ Print out a hex-address associated with your private key. | |
| Ed25519: raw, | ||
| }, | ||
| } | ||
|
|
||
| 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. 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! | ||
| `) | ||
| 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 | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| 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, | ||
|
There was a problem hiding this comment. 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 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, | ||
| }, | ||
|
There was a problem hiding this comment. 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) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.