fix child keys derivation adding bytes padding

extkeys/hdkey.go

@@ -194,7 +194,22 @@ func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) {
  keyBigInt.Add(keyBigInt, parentKeyBigInt)
  keyBigInt.Mod(keyBigInt, btcec.S256().N)
 
- child.KeyData = keyBigInt.Bytes()
+ // Make sure that child.KeyData is 32 bytes of data even if the value is represented with less bytes.
+ // When we derive a child of this key, we call splitHMAC that does a sha512 of a seed that is:
+ // - 1 byte with 0x00
+ // - 32 bytes for the key data
+ // - 4 bytes for the child key index
+ // If we don't padd the KeyData, it will be shifted to left in that 32 bytes space
+ // generating a different seed and different child key.
+ // This part fixes a bug we had previously and described at:
+ // https://medium.com/@alexberegszaszi/why-do-my-bip32-wallets-disagree-6f3254cc5846#.86inuifuq
+ keyData := keyBigInt.Bytes()
+ if len(keyData) < 32 {
+ extra := make([]byte, 32-len(keyData))
+ keyData = append(extra, keyData...)
+ }
+
+ child.KeyData = keyData
  child.Version = PrivateKeyVersion
  } else {
  // Case #3: childKey = serP(point(parse256(IL)) + parentKey)

extkeys/hdkey_test.go

@@ -18,6 +18,8 @@ const (
 )
 
 func TestBIP32Vectors(t *testing.T) {
+ // Test vectors 1, 2, and 3 are taken from the BIP32 specs:
+ // https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#test-vectors
  tests := []struct {
  name string
  seed string
@@ -111,6 +113,21 @@ func TestBIP32Vectors(t *testing.T) {
  "xpub6FnCn6nSzZAw5Tw7cgR9bi15UV96gLZhjDstkXXxvCLsUXBGXPdSnLFbdpq8p9HmGsApME5hQTZ3emM2rnY5agb9rXpVGyy3bdW6EEgAtqt",
  "xprvA2nrNbFZABcdryreWet9Ea4LvTJcGsqrMzxHx98MMrotbir7yrKCEXw7nadnHM8Dq38EGfSh6dqA9QWTyefMLEcBYJUuekgW4BYPJcr9E7j",
  },
+ // Test vector 3
+ {
+ "test vector 3 chain m",
+ "4b381541583be4423346c643850da4b320e46a87ae3d2a4e6da11eba819cd4acba45d239319ac14f863b8d5ab5a0d0c64d2e8a1e7d1457df2e5a3c51c73235be",
+ []uint32{},
+ "xpub661MyMwAqRbcEZVB4dScxMAdx6d4nFc9nvyvH3v4gJL378CSRZiYmhRoP7mBy6gSPSCYk6SzXPTf3ND1cZAceL7SfJ1Z3GC8vBgp2epUt13",
+ "xprv9s21ZrQH143K25QhxbucbDDuQ4naNntJRi4KUfWT7xo4EKsHt2QJDu7KXp1A3u7Bi1j8ph3EGsZ9Xvz9dGuVrtHHs7pXeTzjuxBrCmmhgC6",
+ },
+ {
+ "test vector 3 chain m/0H",
+ "4b381541583be4423346c643850da4b320e46a87ae3d2a4e6da11eba819cd4acba45d239319ac14f863b8d5ab5a0d0c64d2e8a1e7d1457df2e5a3c51c73235be",
+ []uint32{HardenedKeyStart},
+ "xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y",
+ "xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L",
+ },
  }
 
 tests:
@@ -622,6 +639,50 @@ func TestHDWalletCompatibility(t *testing.T) {
  }
 }
 
+// TestPrivateKeyDataWithLeadingZeros is a regression test that checks
+// we don't re-introduce a bug we had in the past.
+// For a specific mnemonic phrase, we were deriving a wrong key/address
+// at path m/44'/60'/0'/0/0 compared to other wallets.
+// In this specific case, the second child key is represented in 31 bytes.
+// The problem raises when deriving its child key.
+// One of the step to derive the child key is calling our splitHMAC
+// that returns a secretKey and a chainCode.
+// Inside this function we make a sha512 of a seed that is a 37 bytes with:
+// 1 byte with 0x00
+// 32 bytes for the key data
+// 4 bytes for the child key index
+// In our case, if the key was less then 32 bytes, it was shifted to the left of that 32 bytes space,
+// resulting in a different seed, and a different data returned from the sha512 call.
+// https://medium.com/@alexberegszaszi/why-do-my-bip32-wallets-disagree-6f3254cc5846#.86inuifuq
+// https://github.com/iancoleman/bip39/issues/58
+func TestPrivateKeyDataWithLeadingZeros(t *testing.T) {
+ mn := NewMnemonic()
+ words := "radar blur cabbage chef fix engine embark joy scheme fiction master release"
+ key, _ := NewMaster(mn.MnemonicSeed(words, ""))
+
+ path := []uint32{
+ HardenedKeyStart + 44, // purpose
+ HardenedKeyStart + 60, // cointype
+ HardenedKeyStart + 0, // account
+ 0, // change
+ 0, // index
+ }
+
+ for _, part := range path {
+ key, _ = key.Child(part)
+ if length := len(key.KeyData); length != 32 {
+ t.Errorf("expected key length to be 32, got: %d", length)
+ }
+ }
+
+ expectedAddress := "0xaC39b311DCEb2A4b2f5d8461c1cdaF756F4F7Ae9"
+ address := crypto.PubkeyToAddress(key.ToECDSA().PublicKey).Hex()
+
+ if address != expectedAddress {
+ t.Errorf("expected address %s, got: %s", expectedAddress, address)
+ }
+}
+
 //func TestNewKey(t *testing.T) {
 // mnemonic := NewMnemonic()
 //