hdkeychain: Add a strict BIP32 child derivation method.

This creates a new ExtendedKey child derivation method, ChildBIP32Std, which creates an ExtendedKey with leading zeroes retained for strict compliance with BIP32. This is not intended for Decred wallet use.
decred · Dec 10, 2021 · ec3ba65 · ec3ba65
1 parent 3bcb7d9
commit ec3ba65
Show file tree

Hide file tree

Showing 4 changed files with 222 additions and 32 deletions.
diff --git a/hdkeychain/README.md b/hdkeychain/README.md
@@ -33,6 +33,13 @@ A comprehensive suite of tests is provided to ensure proper functionality.
 - Comprehensive test coverage including the BIP0032 test vectors
 - Benchmarks
 
+## BIP0032 Conformity
+
+Two different child key derivation functions are provided: the Child function
+derives extended keys using a modified scheme based on BIP0032, whereas
+ChildBIP32Std produces keys that strictly conform to the standard.  The Child
+function should be used for Decred wallet key derivation for legacy reasons.
+
 ## Installation and Updating
 
 This package is part of the `github.com/decred/dcrd/hdkeychain/v3` module.  Use

diff --git a/hdkeychain/doc.go b/hdkeychain/doc.go
@@ -38,13 +38,24 @@ create a random seed for use with the NewMaster function.
 Deriving Children
 
 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
-function.  The Child function supports deriving both normal (non-hardened) and
+discussed later), the child extended keys can be derived by using either the
+Child or ChildBIP32Std function.  The difference is described in the following
+section.  These functions support deriving both normal (non-hardened) and
 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
 Child function.  This provides the ability to cascade the keys into a tree and
 hence generate the hierarchical deterministic key chains.
 
+BIP0032 Conformity
+
+The Child function derives extended keys with a modified scheme based on
+BIP0032, whereas ChildBIP32Std produces keys that strictly conform to the
+standard.  Specifically, the Decred variation strips leading zeros of a private
+key, causing subsequent child keys to differ from the keys expected by standard
+BIP0032.  The ChildBIP32Std method retains leading zeros, ensuring the child
+keys expected by BIP0032 are derived.  The Child function must be used for
+Decred wallet key derivation for legacy reasons.
+
 Normal vs Hardened Child Extended Keys
 
 A private extended key can be used to derive both hardened and non-hardened

diff --git a/hdkeychain/extendedkey.go b/hdkeychain/extendedkey.go
@@ -1,5 +1,5 @@
 // Copyright (c) 2014-2016 The btcsuite developers
-// Copyright (c) 2015-2020 The Decred developers
+// Copyright (c) 2015-2021 The Decred developers
 // Use of this source code is governed by an ISC
 // license that can be found in the LICENSE file.
 
@@ -210,28 +210,12 @@ func doubleBlake256Cksum(v []byte) []byte {
 	return second[:4]
 }
 
-// Child returns a derived child extended key at the given index.  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
-// extended key will be also be a public extended key.
-//
-// When the index is greater to or equal than the HardenedKeyStart constant, the
-// derived extended key will be a hardened extended key.  It is only possible to
-// derive a hardened extended key from a private extended key.  Consequently,
-// this function will return ErrDeriveHardFromPublic if a hardened child
-// extended key is requested from a public extended key.
-//
-// A hardened extended key is useful since, as previously mentioned, it requires
-// a parent private extended key to derive.  In other words, normal child
-// extended public keys can be derived from a parent public extended key (no
-// knowledge of the parent private key) whereas hardened extended keys may not
-// be.
-//
-// NOTE: There is an extremely small chance (< 1 in 2^127) the specific child
-// 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
-// invalid child and simply increment to the next index.
-func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) {
+// child derives a child extended key at the given index. The derived key will
+// retain any leading zeros of a private key if the strict BIP32 flag is true,
+// otherwise they will be stripped.  Strict BIP32 derivation is not intended for
+// Decred wallets.  The derived extended key will be either public or private as
+// determined by the IsPrivate function.
+func (k *ExtendedKey) child(i uint32, strictBIP32 bool) (*ExtendedKey, error) {
 	// There are four scenarios that could happen here:
 	// 1) Private extended key -> Hardened child private extended key
 	// 2) Private extended key -> Non-hardened child private extended key
@@ -317,12 +301,12 @@ func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) {
 		childKeyBytes := ilModN.Bytes()
 		childKey = childKeyBytes[:]
 
-		// Strip leading zeroes to maintain legacy behavior.  Note that per
-		// [BIP32] this should be the fully zero-padded 32-bytes, however, the
-		// Decred variation strips leading zeros for legacy reasons and changing
-		// it now would break derivation for a lot of Decred wallets that rely
-		// on this behavior.
-		for len(childKey) > 0 && childKey[0] == 0x00 {
+		// Optionally strip leading zeroes to maintain legacy behavior.  Note
+		// that per [BIP32] this should be the fully zero-padded 32-bytes,
+		// however, the Decred variation strips leading zeros for legacy reasons
+		// and changing it now would break derivation for a lot of Decred
+		// wallets that rely on this behavior.
+		for !strictBIP32 && len(childKey) > 0 && childKey[0] == 0x00 {
 			childKey = childKey[1:]
 		}
 		isPrivate = true
@@ -364,6 +348,43 @@ func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) {
 		parentFP, k.depth+1, i, isPrivate), nil
 }
 
+// Child returns a derived child extended key at the given index.  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
+// extended key will be also be a public extended key.
+//
+// When the index is greater to or equal than the HardenedKeyStart constant, the
+// derived extended key will be a hardened extended key.  It is only possible to
+// derive a hardened extended key from a private extended key.  Consequently,
+// this function will return ErrDeriveHardFromPublic if a hardened child
+// extended key is requested from a public extended key.
+//
+// A hardened extended key is useful since, as previously mentioned, it requires
+// a parent private extended key to derive.  In other words, normal child
+// extended public keys can be derived from a parent public extended key (no
+// knowledge of the parent private key) whereas hardened extended keys may not
+// be.
+//
+// NOTE: There is an extremely small chance (< 1 in 2^127) the specific child
+// 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
+// invalid child and simply increment to the next index.
+//
+// NOTE 2: Child keys derived from the returned extended key will follow the
+// modified Decred variation of the BIP32 derivation scheme such that any
+// leading zero bytes of private keys are stripped, resulting in different
+// subsequent child keys. This should be used for legacy compatibility purposes.
+func (k *ExtendedKey) Child(i uint32) (*ExtendedKey, error) {
+	return k.child(i, false)
+}
+
+// ChildBIP32Std is like Child, except that derived keys will follow BIP32
+// strictly by retaining leading zeros in the keys, always generating 32-byte
+// keys, and thus different subsequently derived child keys.
+func (k *ExtendedKey) ChildBIP32Std(i uint32) (*ExtendedKey, error) {
+	return k.child(i, true)
+}
+
 // Neuter returns a new extended public key from this extended private key.  The
 // same extended key will be returned unaltered if it is already an extended
 // public key.

diff --git a/hdkeychain/extendedkey_test.go b/hdkeychain/extendedkey_test.go
@@ -1,5 +1,5 @@
 // Copyright (c) 2014 The btcsuite developers
-// Copyright (c) 2015-2020 The Decred developers
+// Copyright (c) 2015-2021 The Decred developers
 // Use of this source code is governed by an ISC
 // license that can be found in the LICENSE file.
 
@@ -893,3 +893,154 @@ func TestZero(t *testing.T) {
 		}
 	}
 }
+
+// TestBIP0032Vector4 tests the BIP32 test vector 4 against keys derived from
+// the ChildBIP32Std method, which derives keys that retain the leading zeros.
+func TestBIP0032Vector4(t *testing.T) {
+	testVec4MasterHex := "3ddd5602285899a946114506157c7997e5444528f3003f6134712147db19b678"
+	hkStart := uint32(0x80000000)
+
+	mainNetParams := mockMainNetParams()
+	tests := []struct {
+		name         string
+		master       string
+		path         []uint32
+		wantPub      string
+		wantPriv     string
+		wantPrivSer  string
+		leadingZeros int
+		net          NetworkParams
+	}{
+		{
+			name:        "test vector 4 chain m",
+			master:      testVec4MasterHex,
+			path:        []uint32{},
+			wantPub:     "dpubZ9169KDAEUnyovAoD3NuxWgVNvAWqC3eUFUrip8ZnT4emsEvAPCyhVgoymUF6p7gug7K6ewzEaUvy77tT7LStEjj56CaZxLRMSYjypMK1yC",
+			wantPriv:    "dprv3hCznBesA6jBuWH2xXKZSHtEkucFQFTaGhrXyuCCnNU24KDHHRsNcvAnq4596P5sYKTSgqbjUe2UNVxid8J9oDyFsuSWb3L93Jrka3vPtYV",
+			wantPrivSer: "12c0d59c7aa3a10973dbd3f478b65f2516627e3fe61e00c345be9a477ad2e215",
+			net:         mainNetParams,
+		},
+		{ // This test fails when using Child instead of ChildBIP32Std.
+			name:         "test vector 4 chain m/0H -- leading zeros in private key serialization",
+			master:       testVec4MasterHex,
+			path:         []uint32{hkStart},
+			wantPub:      "dpubZBYwK8RvCX3KjyCPhRQ9BYdX1ZYTvcFf67sRtoZGPnEYMCVgVyF5uWToDwePapvd1GtLYHAWGpQWo9ffod84MPvwpHmvKeZnZZf47EJcUmT",
+			wantPriv:     "dprv3jkqwzsd88yXqZJdSuLnfKqGPYzCVffataF79tcuPhdudeU3d1uUpvwn5Btmbai3BoVYhzbsEQ7tcH1XtEJ1BhzNSAtZfqv8BtSpdq5FEpj",
+			wantPrivSer:  "00d948e9261e41362a688b916f297121ba6bfb2274a3575ac0e456551dfd7f7e",
+			leadingZeros: 1, // 1 zero byte
+			net:          mainNetParams,
+		},
+		{
+			name:        "test vector 4 chain m/0H/1H -- completely different key",
+			master:      testVec4MasterHex,
+			path:        []uint32{hkStart, hkStart + 1},
+			wantPub:     "dpubZDVQT3iY5Bz6NtyCE8zfnpgwkN7AEUv5mr6H6vPVPRMupBbjTJojs7qRrvddW9adSJt6obdBgW6DsUAaFD8xRVQfVB4XfWCEYdTYRZ7updr",
+			wantPriv:    "dprv3mhK5vAEzovJUV5RycwKGbth8MYtoYL1aJTxN1T8PLmH6da6aMU8nYKQiC5Zj7mC7pH4G8xPxGZVR51FG9Ssptzx1c39A1JxADS6zDrsLPr",
+			wantPrivSer: "3a2086edd7d9df86c3487a5905a1712a9aa664bce8cc268141e07549eaa8661d",
+			net:         mainNetParams,
+		},
+	}
+
+tests:
+	for i, test := range tests {
+		masterSeed, err := hex.DecodeString(test.master)
+		if err != nil {
+			t.Errorf("DecodeString #%d (%s): unexpected error: %v",
+				i, test.name, err)
+			continue
+		}
+
+		extKey, err := NewMaster(masterSeed, test.net)
+		if err != nil {
+			t.Errorf("NewMaster #%d (%s): unexpected error when "+
+				"creating new master key: %v", i, test.name,
+				err)
+			continue
+		}
+
+		for _, childNum := range test.path {
+			var err error
+			extKey, err = extKey.ChildBIP32Std(childNum)
+			if err != nil {
+				t.Errorf("err: %v", err)
+				continue tests
+			}
+		}
+
+		priv, err := extKey.SerializedPrivKey()
+		if err != nil {
+			t.Errorf("SerializedPrivKey #%d (%s): unexpected error: %v",
+				i, test.name, err)
+			continue
+		}
+		if len(priv) != 32 {
+			t.Errorf("serialized private key length %d, want 32", len(priv))
+		}
+
+		privStr := hex.EncodeToString(priv)
+		if privStr != test.wantPrivSer {
+			t.Errorf("Serialize #%d (%s): mismatched serialized "+
+				"private extended key -- got: %s, want: %s", i,
+				test.name, privStr, test.wantPrivSer)
+			continue
+		}
+
+		extKeyStr := extKey.String()
+		if extKeyStr != test.wantPriv {
+			t.Errorf("Serialize #%d (%s): mismatched serialized "+
+				"private extended key -- got: %s, want: %s", i,
+				test.name, extKeyStr, test.wantPriv)
+			continue
+		}
+
+		pubKey := extKey.Neuter()
+
+		// Neutering a second time should have no effect.
+		// Test for referencial equality.
+		if pubKey != pubKey.Neuter() {
+			t.Errorf("Neuter of extended public key returned " +
+				"different object address")
+			return
+		}
+
+		pubStr := pubKey.String()
+		if pubStr != test.wantPub {
+			t.Errorf("Neuter #%d (%s): mismatched serialized "+
+				"public extended key -- got: %s, want: %s", i,
+				test.name, pubStr, test.wantPub)
+			continue
+		}
+
+		// If this path generates a private key with leading zeros, ensure they
+		// are stripped with the legacy Decred child derivation.
+		if test.leadingZeros > 0 {
+			wantLen := 32 - test.leadingZeros
+			extKey, err := NewMaster(masterSeed, test.net)
+			if err != nil {
+				t.Errorf("NewMaster #%d (%s): unexpected error when "+
+					"creating new master key: %v", i, test.name,
+					err)
+				continue
+			}
+
+			for _, childNum := range test.path {
+				var err error
+				extKey, err = extKey.Child(childNum) // modified/legacy BIP32
+				if err != nil {
+					t.Errorf("err: %v", err)
+					continue tests
+				}
+			}
+
+			priv, err := extKey.SerializedPrivKey()
+			if err != nil {
+				t.Errorf("SerializedPrivKey #%d (%s): unexpected error: %v",
+					i, test.name, err)
+				continue
+			}
+			if len(priv) != wantLen {
+				t.Errorf("serialized private key length %d, want %d", len(priv), wantLen)
+			}
+		}
+	}
+}