hdkeychain: correct BIP-32 derivation issue

fixes issue #172
This commit is contained in:
Devrandom 2020-09-01 13:15:53 +02:00
parent 24e673ae72
commit dde9e31e50
5 changed files with 157 additions and 28 deletions

View file

@ -26,7 +26,7 @@ func BenchmarkDeriveHardened(b *testing.B) {
b.StartTimer() b.StartTimer()
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
masterKey.Child(hdkeychain.HardenedKeyStart) masterKey.Derive(hdkeychain.HardenedKeyStart)
} }
} }
@ -41,7 +41,7 @@ func BenchmarkDeriveNormal(b *testing.B) {
b.StartTimer() b.StartTimer()
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
masterKey.Child(0) masterKey.Derive(0)
} }
} }

View file

@ -39,14 +39,14 @@ create a random seed for use with the NewMaster function.
Deriving Children Deriving Children
Once you have created a tree root (or have deserialized an extended key as Once you have created a tree root (or have deserialized an extended key as
discussed later), the child extended keys can be derived by using the Child discussed later), the child extended keys can be derived by using the Derive
function. The Child function supports deriving both normal (non-hardened) and function. The Derive function supports deriving both normal (non-hardened) and
hardened child extended keys. In order to derive a hardened extended key, use hardened child extended keys. In order to derive a hardened extended key, use
the HardenedKeyStart constant + the hardened key number as the index to the the HardenedKeyStart constant + the hardened key number as the index to the
Child function. This provides the ability to cascade the keys into a tree and Derive function. This provides the ability to cascade the keys into a tree and
hence generate the hierarchical deterministic key chains. hence generate the hierarchical deterministic key chains.
Normal vs Hardened Child Extended Keys Normal vs Hardened Derived Extended Keys
A private extended key can be used to derive both hardened and non-hardened A private extended key can be used to derive both hardened and non-hardened
(normal) child private and public extended keys. A public extended key can only (normal) child private and public extended keys. A public extended key can only

View file

@ -71,7 +71,7 @@ func Example_defaultWalletLayout() {
// Derive the extended key for account 0. This gives the path: // Derive the extended key for account 0. This gives the path:
// m/0H // m/0H
acct0, err := masterKey.Child(hdkeychain.HardenedKeyStart + 0) acct0, err := masterKey.Derive(hdkeychain.HardenedKeyStart + 0)
if err != nil { if err != nil {
fmt.Println(err) fmt.Println(err)
return return
@ -80,7 +80,7 @@ func Example_defaultWalletLayout() {
// Derive the extended key for the account 0 external chain. This // Derive the extended key for the account 0 external chain. This
// gives the path: // gives the path:
// m/0H/0 // m/0H/0
acct0Ext, err := acct0.Child(0) acct0Ext, err := acct0.Derive(0)
if err != nil { if err != nil {
fmt.Println(err) fmt.Println(err)
return return
@ -89,7 +89,7 @@ func Example_defaultWalletLayout() {
// Derive the extended key for the account 0 internal chain. This gives // Derive the extended key for the account 0 internal chain. This gives
// the path: // the path:
// m/0H/1 // m/0H/1
acct0Int, err := acct0.Child(1) acct0Int, err := acct0.Derive(1)
if err != nil { if err != nil {
fmt.Println(err) fmt.Println(err)
return return
@ -101,7 +101,7 @@ func Example_defaultWalletLayout() {
// Derive the 10th extended key for the account 0 external chain. This // Derive the 10th extended key for the account 0 external chain. This
// gives the path: // gives the path:
// m/0H/0/10 // m/0H/0/10
acct0Ext10, err := acct0Ext.Child(10) acct0Ext10, err := acct0Ext.Derive(10)
if err != nil { if err != nil {
fmt.Println(err) fmt.Println(err)
return return
@ -110,7 +110,7 @@ func Example_defaultWalletLayout() {
// Derive the 1st extended key for the account 0 internal chain. This // Derive the 1st extended key for the account 0 internal chain. This
// gives the path: // gives the path:
// m/0H/1/0 // m/0H/1/0
acct0Int0, err := acct0Int.Child(0) acct0Int0, err := acct0Int.Derive(0)
if err != nil { if err != nil {
fmt.Println(err) fmt.Println(err)
return return

View file

@ -120,7 +120,7 @@ type ExtendedKey struct {
// fields. No error checking is performed here as it's only intended to be a // fields. No error checking is performed here as it's only intended to be a
// convenience method used to create a populated struct. This function should // convenience method used to create a populated struct. This function should
// only be used by applications that need to create custom ExtendedKeys. All // only be used by applications that need to create custom ExtendedKeys. All
// other applications should just use NewMaster, Child, or Neuter. // other applications should just use NewMaster, Derive, or Neuter.
func NewExtendedKey(version, key, chainCode, parentFP []byte, depth uint8, func NewExtendedKey(version, key, chainCode, parentFP []byte, depth uint8,
childNum uint32, isPrivate bool) *ExtendedKey { childNum uint32, isPrivate bool) *ExtendedKey {
@ -198,8 +198,15 @@ func (k *ExtendedKey) ChainCode() []byte {
return append([]byte{}, k.chainCode...) return append([]byte{}, k.chainCode...)
} }
// Child returns a derived child extended key at the given index. When this // Derive returns a derived child extended key at the given index.
// extended key is a private extended key (as determined by the IsPrivate //
// IMPORTANT: if you were previously using the Child method, this method is incompatible.
// The Child method had a BIP-32 standard compatibility issue. You have to check whether
// any hardened derivations in your derivation path are affected by this issue, via the
// IsAffectedByIssue172 method and migrate the wallet if so. This method does conform
// to the standard. If you need the old behavior, use DeriveNonStandard.
//
// When this extended key is a private extended key (as determined by the IsPrivate
// function), a private extended key will be derived. Otherwise, the derived // function), a private extended key will be derived. Otherwise, the derived
// extended key will be also be a public extended key. // extended key will be also be a public extended key.
// //
@ -219,7 +226,7 @@ func (k *ExtendedKey) ChainCode() []byte {
// index does not derive to a usable child. The ErrInvalidChild error will be // index does not derive to a usable child. The ErrInvalidChild error will be
// returned if this should occur, and the caller is expected to ignore the // returned if this should occur, and the caller is expected to ignore the
// invalid child and simply increment to the next index. // invalid child and simply increment to the next index.
func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) { func (k *ExtendedKey) Derive(i uint32) (*ExtendedKey, error) {
// Prevent derivation of children beyond the max allowed depth. // Prevent derivation of children beyond the max allowed depth.
if k.depth == maxUint8 { if k.depth == maxUint8 {
return nil, ErrDeriveBeyondMaxDepth return nil, ErrDeriveBeyondMaxDepth
@ -254,7 +261,9 @@ func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) {
// When the child is a hardened child, the key is known to be a // When the child is a hardened child, the key is known to be a
// private key due to the above early return. Pad it with a // private key due to the above early return. Pad it with a
// leading zero as required by [BIP32] for deriving the child. // leading zero as required by [BIP32] for deriving the child.
copy(data[1:], k.key) // Additionally, right align it if it's shorter than 32 bytes.
offset := 33 - len(k.key)
copy(data[offset:], k.key)
} else { } else {
// Case #2 or #3. // Case #2 or #3.
// This is either a public or private extended key, but in // This is either a public or private extended key, but in
@ -342,6 +351,75 @@ func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) {
k.depth+1, i, isPrivate), nil k.depth+1, i, isPrivate), nil
} }
// Returns true if this key was affected by the BIP-32 issue in the Child
// method (since renamed to DeriveNonStandard).
func (k *ExtendedKey) IsAffectedByIssue172() bool {
return len(k.key) < 32
}
// Deprecated: This is a non-standard derivation that is affected by issue #172.
// 1-of-256 hardened derivations will be wrong. See note in the Derive method
// and IsAffectedByIssue172.
func (k *ExtendedKey) DeriveNonStandard(i uint32) (*ExtendedKey, error) {
if k.depth == maxUint8 {
return nil, ErrDeriveBeyondMaxDepth
}
isChildHardened := i >= HardenedKeyStart
if !k.isPrivate && isChildHardened {
return nil, ErrDeriveHardFromPublic
}
keyLen := 33
data := make([]byte, keyLen+4)
if isChildHardened {
copy(data[1:], k.key)
} else {
copy(data, k.pubKeyBytes())
}
binary.BigEndian.PutUint32(data[keyLen:], i)
hmac512 := hmac.New(sha512.New, k.chainCode)
hmac512.Write(data)
ilr := hmac512.Sum(nil)
il := ilr[:len(ilr)/2]
childChainCode := ilr[len(ilr)/2:]
ilNum := new(big.Int).SetBytes(il)
if ilNum.Cmp(btcec.S256().N) >= 0 || ilNum.Sign() == 0 {
return nil, ErrInvalidChild
}
var isPrivate bool
var childKey []byte
if k.isPrivate {
keyNum := new(big.Int).SetBytes(k.key)
ilNum.Add(ilNum, keyNum)
ilNum.Mod(ilNum, btcec.S256().N)
childKey = ilNum.Bytes()
isPrivate = true
} else {
ilx, ily := btcec.S256().ScalarBaseMult(il)
if ilx.Sign() == 0 || ily.Sign() == 0 {
return nil, ErrInvalidChild
}
pubKey, err := btcec.ParsePubKey(k.key, btcec.S256())
if err != nil {
return nil, err
}
childX, childY := btcec.S256().Add(ilx, ily, pubKey.X, pubKey.Y)
pk := btcec.PublicKey{Curve: btcec.S256(), X: childX, Y: childY}
childKey = pk.SerializeCompressed()
}
parentFP := btcutil.Hash160(k.pubKeyBytes())[:4]
return NewExtendedKey(k.version, childKey, childChainCode, parentFP,
k.depth+1, i, isPrivate), nil
}
// ChildNum returns the index at which the child extended key was derived. // ChildNum returns the index at which the child extended key was derived.
// //
// Extended keys with ChildNum value between 0 and 2^31-1 are normal child // Extended keys with ChildNum value between 0 and 2^31-1 are normal child

View file

@ -10,6 +10,7 @@ package hdkeychain
import ( import (
"bytes" "bytes"
"encoding/binary"
"encoding/hex" "encoding/hex"
"errors" "errors"
"math" "math"
@ -224,7 +225,7 @@ tests:
for _, childNum := range test.path { for _, childNum := range test.path {
var err error var err error
extKey, err = extKey.Child(childNum) extKey, err = extKey.Derive(childNum)
if err != nil { if err != nil {
t.Errorf("err: %v", err) t.Errorf("err: %v", err)
continue tests continue tests
@ -381,7 +382,7 @@ tests:
for _, childNum := range test.path { for _, childNum := range test.path {
var err error var err error
extKey, err = extKey.Child(childNum) extKey, err = extKey.Derive(childNum)
if err != nil { if err != nil {
t.Errorf("err: %v", err) t.Errorf("err: %v", err)
continue tests continue tests
@ -390,7 +391,7 @@ tests:
privStr := extKey.String() privStr := extKey.String()
if privStr != test.wantPriv { if privStr != test.wantPriv {
t.Errorf("Child #%d (%s): mismatched serialized "+ t.Errorf("Derive #%d (%s): mismatched serialized "+
"private extended key -- got: %s, want: %s", i, "private extended key -- got: %s, want: %s", i,
test.name, privStr, test.wantPriv) test.name, privStr, test.wantPriv)
continue continue
@ -500,7 +501,7 @@ tests:
for _, childNum := range test.path { for _, childNum := range test.path {
var err error var err error
extKey, err = extKey.Child(childNum) extKey, err = extKey.Derive(childNum)
if err != nil { if err != nil {
t.Errorf("err: %v", err) t.Errorf("err: %v", err)
continue tests continue tests
@ -509,7 +510,7 @@ tests:
pubStr := extKey.String() pubStr := extKey.String()
if pubStr != test.wantPub { if pubStr != test.wantPub {
t.Errorf("Child #%d (%s): mismatched serialized "+ t.Errorf("Derive #%d (%s): mismatched serialized "+
"public extended key -- got: %s, want: %s", i, "public extended key -- got: %s, want: %s", i,
test.name, pubStr, test.wantPub) test.name, pubStr, test.wantPub)
continue continue
@ -850,9 +851,9 @@ func TestErrors(t *testing.T) {
} }
// Deriving a hardened child extended key should fail from a public key. // Deriving a hardened child extended key should fail from a public key.
_, err = pubKey.Child(HardenedKeyStart) _, err = pubKey.Derive(HardenedKeyStart)
if err != ErrDeriveHardFromPublic { if err != ErrDeriveHardFromPublic {
t.Fatalf("Child: mismatched error -- got: %v, want: %v", t.Fatalf("Derive: mismatched error -- got: %v, want: %v",
err, ErrDeriveHardFromPublic) err, ErrDeriveHardFromPublic)
} }
@ -1072,20 +1073,20 @@ func TestMaximumDepth(t *testing.T) {
t.Fatalf("extendedkey depth %d should match expected value %d", t.Fatalf("extendedkey depth %d should match expected value %d",
extKey.Depth(), i) extKey.Depth(), i)
} }
newKey, err := extKey.Child(1) newKey, err := extKey.Derive(1)
if err != nil { if err != nil {
t.Fatalf("Child: unexpected error: %v", err) t.Fatalf("Derive: unexpected error: %v", err)
} }
extKey = newKey extKey = newKey
} }
noKey, err := extKey.Child(1) noKey, err := extKey.Derive(1)
if err != ErrDeriveBeyondMaxDepth { if err != ErrDeriveBeyondMaxDepth {
t.Fatalf("Child: mismatched error: want %v, got %v", t.Fatalf("Derive: mismatched error: want %v, got %v",
ErrDeriveBeyondMaxDepth, err) ErrDeriveBeyondMaxDepth, err)
} }
if noKey != nil { if noKey != nil {
t.Fatal("Child: deriving 256th key should not succeed") t.Fatal("Derive: deriving 256th key should not succeed")
} }
} }
@ -1156,3 +1157,53 @@ func TestCloneWithVersion(t *testing.T) {
} }
} }
} }
// TestLeadingZero ensures that deriving children from keys with a leading zero byte is done according
// to the BIP-32 standard and that the legacy method generates a backwards-compatible result.
func TestLeadingZero(t *testing.T) {
// The 400th seed results in a m/0' public key with a leading zero, allowing us to test
// the desired behavior.
ii := 399
seed := make([]byte, 32)
binary.BigEndian.PutUint32(seed[28:], uint32(ii))
masterKey, err := NewMaster(seed, &chaincfg.MainNetParams)
if err != nil {
t.Fatalf("hdkeychain.NewMaster failed: %v", err)
}
child0, err := masterKey.Derive(0 + HardenedKeyStart)
if err != nil {
t.Fatalf("masterKey.Derive failed: %v", err)
}
if !child0.IsAffectedByIssue172() {
t.Fatal("expected child0 to be affected by issue 172")
}
child1, err := child0.Derive(0 + HardenedKeyStart)
if err != nil {
t.Fatalf("child0.Derive failed: %v", err)
}
if child1.IsAffectedByIssue172() {
t.Fatal("did not expect child1 to be affected by issue 172")
}
child1nonstandard, err := child0.DeriveNonStandard(0 + HardenedKeyStart)
if err != nil {
t.Fatalf("child0.DeriveNonStandard failed: %v", err)
}
// This is the correct result based on BIP32
if hex.EncodeToString(child1.key) != "a9b6b30a5b90b56ed48728c73af1d8a7ef1e9cc372ec21afcc1d9bdf269b0988" {
t.Error("incorrect standard BIP32 derivation")
}
if hex.EncodeToString(child1nonstandard.key) != "ea46d8f58eb863a2d371a938396af8b0babe85c01920f59a8044412e70e837ee" {
t.Error("incorrect btcutil backwards compatible BIP32-like derivation")
}
if !child0.IsAffectedByIssue172() {
t.Error("child 0 should be affected by issue 172")
}
if child1.IsAffectedByIssue172() {
t.Error("child 1 should not be affected by issue 172")
}
}