Merge pull request #730 from pilu/fix/bip39-entropy-strength

Fix BIP39 entropy strength validation

extkeys/hdkey_test.go

@@ -1,4 +1,4 @@
-package extkeys_test
+package extkeys
 
 import (
 	"bytes"
@@ -8,7 +8,6 @@ import (
 	"testing"
 
 	"github.com/btcsuite/btcd/chaincfg"
-	"github.com/status-im/status-go/extkeys"
 )
 
 func TestBIP32Vectors(t *testing.T) {
@@ -30,35 +29,35 @@ func TestBIP32Vectors(t *testing.T) {
 		{
 			"test vector 1 chain m/0H",
 			"000102030405060708090a0b0c0d0e0f",
-			[]uint32{extkeys.HardenedKeyStart},
+			[]uint32{HardenedKeyStart},
 			"xpub68Gmy5EdvgibQVfPdqkBBCHxA5htiqg55crXYuXoQRKfDBFA1WEjWgP6LHhwBZeNK1VTsfTFUHCdrfp1bgwQ9xv5ski8PX9rL2dZXvgGDnw",
 			"xprv9uHRZZhk6KAJC1avXpDAp4MDc3sQKNxDiPvvkX8Br5ngLNv1TxvUxt4cV1rGL5hj6KCesnDYUhd7oWgT11eZG7XnxHrnYeSvkzY7d2bhkJ7",
 		},
 		{
 			"test vector 1 chain m/0H/1",
 			"000102030405060708090a0b0c0d0e0f",
-			[]uint32{extkeys.HardenedKeyStart, 1},
+			[]uint32{HardenedKeyStart, 1},
 			"xpub6ASuArnXKPbfEwhqN6e3mwBcDTgzisQN1wXN9BJcM47sSikHjJf3UFHKkNAWbWMiGj7Wf5uMash7SyYq527Hqck2AxYysAA7xmALppuCkwQ",
 			"xprv9wTYmMFdV23N2TdNG573QoEsfRrWKQgWeibmLntzniatZvR9BmLnvSxqu53Kw1UmYPxLgboyZQaXwTCg8MSY3H2EU4pWcQDnRnrVA1xe8fs",
 		},
 		{
 			"test vector 1 chain m/0H/1/2H",
 			"000102030405060708090a0b0c0d0e0f",
-			[]uint32{extkeys.HardenedKeyStart, 1, extkeys.HardenedKeyStart + 2},
+			[]uint32{HardenedKeyStart, 1, HardenedKeyStart + 2},
 			"xpub6D4BDPcP2GT577Vvch3R8wDkScZWzQzMMUm3PWbmWvVJrZwQY4VUNgqFJPMM3No2dFDFGTsxxpG5uJh7n7epu4trkrX7x7DogT5Uv6fcLW5",
 			"xprv9z4pot5VBttmtdRTWfWQmoH1taj2axGVzFqSb8C9xaxKymcFzXBDptWmT7FwuEzG3ryjH4ktypQSAewRiNMjANTtpgP4mLTj34bhnZX7UiM",
 		},
 		{
 			"test vector 1 chain m/0H/1/2H/2",
 			"000102030405060708090a0b0c0d0e0f",
-			[]uint32{extkeys.HardenedKeyStart, 1, extkeys.HardenedKeyStart + 2, 2},
+			[]uint32{HardenedKeyStart, 1, HardenedKeyStart + 2, 2},
 			"xpub6FHa3pjLCk84BayeJxFW2SP4XRrFd1JYnxeLeU8EqN3vDfZmbqBqaGJAyiLjTAwm6ZLRQUMv1ZACTj37sR62cfN7fe5JnJ7dh8zL4fiyLHV",
 			"xprvA2JDeKCSNNZky6uBCviVfJSKyQ1mDYahRjijr5idH2WwLsEd4Hsb2Tyh8RfQMuPh7f7RtyzTtdrbdqqsunu5Mm3wDvUAKRHSC34sJ7in334",
 		},
 		{
 			"test vector 1 chain m/0H/1/2H/2/1000000000",
 			"000102030405060708090a0b0c0d0e0f",
-			[]uint32{extkeys.HardenedKeyStart, 1, extkeys.HardenedKeyStart + 2, 2, 1000000000},
+			[]uint32{HardenedKeyStart, 1, HardenedKeyStart + 2, 2, 1000000000},
 			"xpub6H1LXWLaKsWFhvm6RVpEL9P4KfRZSW7abD2ttkWP3SSQvnyA8FSVqNTEcYFgJS2UaFcxupHiYkro49S8yGasTvXEYBVPamhGW6cFJodrTHy",
 			"xprvA41z7zogVVwxVSgdKUHDy1SKmdb533PjDz7J6N6mV6uS3ze1ai8FHa8kmHScGpWmj4WggLyQjgPie1rFSruoUihUZREPSL39UNdE3BBDu76",
 		},
@@ -80,28 +79,28 @@ func TestBIP32Vectors(t *testing.T) {
 		{
 			"test vector 2 chain m/0/2147483647H",
 			"fffcf9f6f3f0edeae7e4e1dedbd8d5d2cfccc9c6c3c0bdbab7b4b1aeaba8a5a29f9c999693908d8a8784817e7b7875726f6c696663605d5a5754514e4b484542",
-			[]uint32{0, extkeys.HardenedKeyStart + 2147483647},
+			[]uint32{0, HardenedKeyStart + 2147483647},
 			"xpub6ASAVgeehLbnwdqV6UKMHVzgqAG8Gr6riv3Fxxpj8ksbH9ebxaEyBLZ85ySDhKiLDBrQSARLq1uNRts8RuJiHjaDMBU4Zn9h8LZNnBC5y4a",
 			"xprv9wSp6B7kry3Vj9m1zSnLvN3xH8RdsPP1Mh7fAaR7aRLcQMKTR2vidYEeEg2mUCTAwCd6vnxVrcjfy2kRgVsFawNzmjuHc2YmYRmagcEPdU9",
 		},
 		{
 			"test vector 2 chain m/0/2147483647H/1",
 			"fffcf9f6f3f0edeae7e4e1dedbd8d5d2cfccc9c6c3c0bdbab7b4b1aeaba8a5a29f9c999693908d8a8784817e7b7875726f6c696663605d5a5754514e4b484542",
-			[]uint32{0, extkeys.HardenedKeyStart + 2147483647, 1},
+			[]uint32{0, HardenedKeyStart + 2147483647, 1},
 			"xpub6DF8uhdarytz3FWdA8TvFSvvAh8dP3283MY7p2V4SeE2wyWmG5mg5EwVvmdMVCQcoNJxGoWaU9DCWh89LojfZ537wTfunKau47EL2dhHKon",
 			"xprv9zFnWC6h2cLgpmSA46vutJzBcfJ8yaJGg8cX1e5StJh45BBciYTRXSd25UEPVuesF9yog62tGAQtHjXajPPdbRCHuWS6T8XA2ECKADdw4Ef",
 		},
 		{
 			"test vector 2 chain m/0/2147483647H/1/2147483646H",
 			"fffcf9f6f3f0edeae7e4e1dedbd8d5d2cfccc9c6c3c0bdbab7b4b1aeaba8a5a29f9c999693908d8a8784817e7b7875726f6c696663605d5a5754514e4b484542",
-			[]uint32{0, extkeys.HardenedKeyStart + 2147483647, 1, extkeys.HardenedKeyStart + 2147483646},
+			[]uint32{0, HardenedKeyStart + 2147483647, 1, HardenedKeyStart + 2147483646},
 			"xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL",
 			"xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc",
 		},
 		{
 			"test vector 2 chain m/0/2147483647H/1/2147483646H/2",
 			"fffcf9f6f3f0edeae7e4e1dedbd8d5d2cfccc9c6c3c0bdbab7b4b1aeaba8a5a29f9c999693908d8a8784817e7b7875726f6c696663605d5a5754514e4b484542",
-			[]uint32{0, extkeys.HardenedKeyStart + 2147483647, 1, extkeys.HardenedKeyStart + 2147483646, 2},
+			[]uint32{0, HardenedKeyStart + 2147483647, 1, HardenedKeyStart + 2147483646, 2},
 			"xpub6FnCn6nSzZAw5Tw7cgR9bi15UV96gLZhjDstkXXxvCLsUXBGXPdSnLFbdpq8p9HmGsApME5hQTZ3emM2rnY5agb9rXpVGyy3bdW6EEgAtqt",
 			"xprvA2nrNbFZABcdryreWet9Ea4LvTJcGsqrMzxHx98MMrotbir7yrKCEXw7nadnHM8Dq38EGfSh6dqA9QWTyefMLEcBYJUuekgW4BYPJcr9E7j",
 		},
@@ -115,7 +114,7 @@ tests:
 			continue
 		}
 
-		extKey, err := extkeys.NewMaster(seed, []byte("Bitcoin seed"))
+		extKey, err := NewMaster(seed, []byte("Bitcoin seed"))
 		if err != nil {
 			t.Errorf("NewMasterKey #%d (%s): %v", i, test.name, err)
 			continue
@@ -354,7 +353,7 @@ func TestChildDerivation(t *testing.T) {
 
 	runTests := func(tests []testCase) {
 		for i, test := range tests {
-			extKey, err := extkeys.NewKeyFromString(test.master)
+			extKey, err := NewKeyFromString(test.master)
 			if err != nil {
 				t.Errorf("NewKeyFromString #%d (%s): unexpected error creating extended key: %v", i, test.name, err)
 				continue
@@ -381,29 +380,29 @@ func TestChildDerivation(t *testing.T) {
 
 func TestErrors(t *testing.T) {
 	// Should get an error when seed has too few bytes.
-	_, err := extkeys.NewMaster(bytes.Repeat([]byte{0x00}, 15), []byte{0x00})
+	_, err := NewMaster(bytes.Repeat([]byte{0x00}, 15), []byte{0x00})
-	if err != extkeys.ErrInvalidSeedLen {
+	if err != ErrInvalidSeedLen {
 		t.Errorf("NewMaster: mismatched error -- got: %v, want: %v",
-			err, extkeys.ErrInvalidSeedLen)
+			err, ErrInvalidSeedLen)
 	}
 
 	// Should get an error when seed has too many bytes.
-	_, err = extkeys.NewMaster(bytes.Repeat([]byte{0x00}, 65), []byte{0x00})
+	_, err = NewMaster(bytes.Repeat([]byte{0x00}, 65), []byte{0x00})
-	if err != extkeys.ErrInvalidSeedLen {
+	if err != ErrInvalidSeedLen {
 		t.Errorf("NewMaster: mismatched error -- got: %v, want: %v",
-			err, extkeys.ErrInvalidSeedLen)
+			err, ErrInvalidSeedLen)
 	}
 
 	// Generate a new key and neuter it to a public extended key.
-	mnemonic := extkeys.NewMnemonic(extkeys.Salt)
+	mnemonic := NewMnemonic(Salt)
 
-	phrase, err := mnemonic.MnemonicPhrase(128, extkeys.EnglishLanguage)
+	phrase, err := mnemonic.MnemonicPhrase(128, EnglishLanguage)
 	if err != nil {
 		t.Errorf("Test failed: could not create seed: %s", err)
 	}
 
 	password := "badpassword"
-	extKey, err := extkeys.NewMaster(mnemonic.MnemonicSeed(phrase, password), []byte(extkeys.Salt))
+	extKey, err := NewMaster(mnemonic.MnemonicSeed(phrase, password), []byte(Salt))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 		return
@@ -416,20 +415,20 @@ func TestErrors(t *testing.T) {
 	}
 
 	// Deriving a hardened child extended key should fail from a public key.
-	_, err = pubKey.Child(extkeys.HardenedKeyStart)
+	_, err = pubKey.Child(HardenedKeyStart)
-	if err != extkeys.ErrDerivingHardenedFromPublic {
+	if err != ErrDerivingHardenedFromPublic {
-		t.Errorf("Child: mismatched error -- got: %v, want: %v", err, extkeys.ErrDerivingHardenedFromPublic)
+		t.Errorf("Child: mismatched error -- got: %v, want: %v", err, ErrDerivingHardenedFromPublic)
 	}
 
-	_, err = pubKey.BIP44Child(extkeys.CoinTypeETH, 0)
+	_, err = pubKey.BIP44Child(CoinTypeETH, 0)
-	if err != extkeys.ErrInvalidMasterKey {
+	if err != ErrInvalidMasterKey {
-		t.Errorf("BIP44Child: mistmatched error -- got: %v, want: %v", err, extkeys.ErrInvalidMasterKey)
+		t.Errorf("BIP44Child: mistmatched error -- got: %v, want: %v", err, ErrInvalidMasterKey)
 	}
 
-	childKey, _ := extKey.Child(extkeys.HardenedKeyStart + 1)
+	childKey, _ := extKey.Child(HardenedKeyStart + 1)
-	_, err = childKey.BIP44Child(extkeys.CoinTypeETH, 0) // this should be called from master only
+	_, err = childKey.BIP44Child(CoinTypeETH, 0) // this should be called from master only
-	if err != extkeys.ErrInvalidMasterKey {
+	if err != ErrInvalidMasterKey {
-		t.Errorf("BIP44Child: mistmatched error -- got: %v, want: %v", err, extkeys.ErrInvalidMasterKey)
+		t.Errorf("BIP44Child: mistmatched error -- got: %v, want: %v", err, ErrInvalidMasterKey)
 	}
 
 	// NewKeyFromString failure tests.
@@ -439,16 +438,17 @@ func TestErrors(t *testing.T) {
 		err       error
 		neuter    bool
 		neuterErr error
+		extKey    *ExtendedKey
 	}{
 		{
 			name: "invalid key length",
 			key:  "xpub1234",
-			err:  extkeys.ErrInvalidKeyLen,
+			err:  ErrInvalidKeyLen,
 		},
 		{
 			name: "bad checksum",
 			key:  "xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EBygr15",
-			err:  extkeys.ErrBadChecksum,
+			err:  ErrBadChecksum,
 		},
 		{
 			name: "pubkey not on curve",
@@ -462,10 +462,26 @@ func TestErrors(t *testing.T) {
 			neuter:    true,
 			neuterErr: chaincfg.ErrUnknownHDKeyID,
 		},
+		{
+			name:      "zeroed extended key",
+			key:       EmptyExtendedKeyString,
+			err:       nil,
+			neuter:    false,
+			neuterErr: nil,
+			extKey:    &ExtendedKey{},
+		},
+		{
+			name:      "empty string",
+			key:       "",
+			err:       nil,
+			neuter:    false,
+			neuterErr: nil,
+			extKey:    &ExtendedKey{},
+		},
 	}
 
 	for i, test := range tests {
-		extKey, err := extkeys.NewKeyFromString(test.key)
+		extKey, err := NewKeyFromString(test.key)
 		if !reflect.DeepEqual(err, test.err) {
 			t.Errorf("NewKeyFromString #%d (%s): mismatched error -- got: %v, want: %v", i, test.name, err, test.err)
 			continue
@@ -478,6 +494,13 @@ func TestErrors(t *testing.T) {
 				continue
 			}
 		}
+
+		if test.extKey != nil {
+			if !reflect.DeepEqual(extKey, test.extKey) {
+				t.Errorf("ExtKey #%d (%s): mismatched extended key -- got: %+v, want: %+v", i, test.name, extKey, test.extKey)
+				continue
+			}
+		}
 	}
 }
 
@@ -486,12 +509,12 @@ func TestBIP44ChildDerivation(t *testing.T) {
 	derivedKey1String := "xprvA38t8tFW4vbuB7WJXEqMFmZqRrcZUKWqqMcGjjKjr2hbfvPhRtLLJGL4ayWG8shF1VkuUikVGodGshLiKRS7WrdsrGSVDQCY33qoPBxG2Kp"
 	derivedKey2String := "xprvA38t8tFW4vbuDgBNpekPnuMSfpWziDLdF7W9Zd3mPy6eDEkM5F17vk59RtVoFbNdBBq84EJf5CqdZhhEoBkAM4DXHQsDqvUxVnncfnDQEFg"
 
-	extKey, err := extkeys.NewKeyFromString(keyString)
+	extKey, err := NewKeyFromString(keyString)
 	if err != nil {
 		t.Error("NewKeyFromString: cannot create extended key")
 	}
 
-	accounKey1, err := extKey.BIP44Child(extkeys.CoinTypeETH, 0)
+	accounKey1, err := extKey.BIP44Child(CoinTypeETH, 0)
 	if err != nil {
 		t.Error("Error dering BIP44-compliant key")
 	}
@@ -500,7 +523,7 @@ func TestBIP44ChildDerivation(t *testing.T) {
 	}
 	t.Logf("Account 1 key: %s", accounKey1.String())
 
-	accounKey2, err := extKey.BIP44Child(extkeys.CoinTypeETH, 1)
+	accounKey2, err := extKey.BIP44Child(CoinTypeETH, 1)
 	if err != nil {
 		t.Error("Error dering BIP44-compliant key")
 	}

extkeys/mnemonic.go

@@ -41,6 +41,17 @@ const (
 	totalAvailableLanguages
 )
 
+type entropyStrength int
+
+// Valid entropy strengths
+const (
+	EntropyStrength128 entropyStrength = 128 + 32*iota
+	EntropyStrength160
+	EntropyStrength192
+	EntropyStrength224
+	EntropyStrength256
+)
+
 // Languages is a list of supported languages for which mnemonic keys can be generated
 var Languages = [...]string{
 	"English",
@@ -53,6 +64,11 @@ var Languages = [...]string{
 	"Russian",
 }
 
+// ErrInvalidEntropyStrength is the error returned by MnemonicPhrase
+// when the entropy strength is not valid.
+// Valid entropy strength values are multiple of 32 between 128 and 256.
+var ErrInvalidEntropyStrength = errors.New("The mnemonic must encode entropy in a multiple of 32 bits, The recommended size of ENT is 128-256 bits")
+
 var (
 	last11BitsMask          = big.NewInt(2047)
 	rightShift11BitsDivider = big.NewInt(2048)
@@ -118,7 +134,7 @@ func (m *Mnemonic) MnemonicSeed(mnemonic string, password string) []byte {
 }
 
 // MnemonicPhrase returns a human readable seed for BIP32 Hierarchical Deterministic Wallets
-func (m *Mnemonic) MnemonicPhrase(strength, language Language) (string, error) {
+func (m *Mnemonic) MnemonicPhrase(strength entropyStrength, language Language) (string, error) {
 	wordList, err := m.WordList(language)
 	if err != nil {
 		return "", err
@@ -128,8 +144,8 @@ func (m *Mnemonic) MnemonicPhrase(strength, language Language) (string, error) {
 	// With more entropy security is improved but the sentence length increases.
 	// We refer to the initial entropy length as ENT. The recommended size of ENT is 128-256 bits.
 
-	if strength%32 > 0 && strength < 128 && strength > 256 {
+	if strength%32 > 0 || strength < 128 || strength > 256 {
-		return "", errors.New("The mnemonic must encode entropy in a multiple of 32 bits, The recommended size of ENT is 128-256 bits")
+		return "", ErrInvalidEntropyStrength
 	}
 
 	// First, an initial entropy of ENT bits is generated
@@ -220,7 +236,7 @@ func (m *Mnemonic) ValidMnemonic(mnemonic string, language Language) bool {
 
 // WordList returns list of words for a given language
 func (m *Mnemonic) WordList(language Language) (*WordList, error) {
-	if m.wordLists[language] == nil {
+	if int(language) < 0 || int(language) > len(m.wordLists)-1 || m.wordLists[language] == nil {
 		return nil, fmt.Errorf("language word list is missing (language id: %d)", language)
 	}
 	return m.wordLists[language], nil

extkeys/mnemonic_test.go

@@ -1,12 +1,10 @@
-package extkeys_test
+package extkeys
 
 import (
 	"encoding/json"
 	"fmt"
 	"os"
 	"testing"
-
-	"github.com/status-im/status-go/extkeys"
 )
 
 type VectorsFile struct {
@@ -18,16 +16,54 @@ type Vector struct {
 	language, salt, password, input, mnemonic, seed, xprv string
 }
 
+func TestNewMnemonic(t *testing.T) {
+	m1 := NewMnemonic("")
+	if m1.salt != Salt {
+		t.Errorf("expected default salt, got: %q", m1.salt)
+	}
+
+	customSalt := "custom-salt"
+	m2 := NewMnemonic(customSalt)
+	if m2.salt != customSalt {
+		t.Errorf("expected %q, got: %q", customSalt, m2.salt)
+	}
+}
+
+func TestMnemonic_WordList(t *testing.T) {
+	m := NewMnemonic(Salt)
+	_, err := m.WordList(EnglishLanguage)
+	if err != nil {
+		t.Errorf("expected WordList to return WordList without errors, got: %s", err)
+	}
+
+	indexes := []Language{-1, Language(len(m.wordLists))}
+	for _, index := range indexes {
+		_, err := m.WordList(index)
+		if err == nil {
+			t.Errorf("expected WordList to return an error with index %d", index)
+		}
+	}
+}
+
 // TestMnemonicPhrase
 func TestMnemonicPhrase(t *testing.T) {
 
-	mnemonic := extkeys.NewMnemonic(extkeys.Salt)
+	mnemonic := NewMnemonic(Salt)
+
+	// test strength validation
+	strengths := []entropyStrength{127, 129, 257}
+	for _, s := range strengths {
+		_, err := mnemonic.MnemonicPhrase(s, EnglishLanguage)
+		if err != ErrInvalidEntropyStrength {
+			t.Errorf("Entropy strength '%d' should be invalid", s)
+		}
+	}
 
 	// test mnemonic generation
 	t.Log("Test mnemonic generation:")
 	for _, language := range mnemonic.AvailableLanguages() {
-		phrase, err := mnemonic.MnemonicPhrase(128, language)
+		phrase, err := mnemonic.MnemonicPhrase(EntropyStrength128, language)
-		t.Logf("Mnemonic (%s): %s", extkeys.Languages[language], phrase)
+		t.Logf("Mnemonic (%s): %s", Languages[language], phrase)
 
 		if err != nil {
 			t.Errorf("Test failed: could not create seed: %s", err)
@@ -47,8 +83,8 @@ func TestMnemonicPhrase(t *testing.T) {
 	t.Log("Test against pre-computed seed vectors:")
 	stats := map[string]int{}
 	for _, vector := range vectorsFile.vectors {
-		stats[vector.language] += 1
+		stats[vector.language]++
-		mnemonic := extkeys.NewMnemonic(vector.salt)
+		mnemonic := NewMnemonic(vector.salt)
 		seed := mnemonic.MnemonicSeed(vector.mnemonic, vector.password)
 		if fmt.Sprintf("%x", seed) != vector.seed {
 			t.Errorf("Test failed (%s): incorrect seed (%x) generated (expected: %s)", vector.language, seed, vector.seed)

geth/account/accounts.go

@@ -48,7 +48,7 @@ func NewManager(nodeManager common.NodeManager) *Manager {
 func (m *Manager) CreateAccount(password string) (address, pubKey, mnemonic string, err error) {
 	// generate mnemonic phrase
 	mn := extkeys.NewMnemonic(extkeys.Salt)
-	mnemonic, err = mn.MnemonicPhrase(128, extkeys.EnglishLanguage)
+	mnemonic, err = mn.MnemonicPhrase(extkeys.EntropyStrength128, extkeys.EnglishLanguage)
 	if err != nil {
 		return "", "", "", fmt.Errorf("can not create mnemonic seed: %v", err)
 	}