Fix implementation of hd derivation to be bip32 compliant

https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#child-key-derivation-ckd-functions

lib/hdkeycache.js

@@ -8,17 +8,17 @@ module.exports = {
   _usedIndex: {},
   _CACHE_SIZE: 5000,
 
-  get: function(xkey, number, hardened) {
+  get: function(xkey, number, hardened, nonCompliant) {
     hardened = !!hardened;
-    var key = xkey + '/' + number + '/' + hardened;
+    var key = (nonCompliant ? '1' : '0') + xkey + '/' + number + '/' + hardened;
     if (this._cache[key]) {
       this._cacheHit(key);
       return this._cache[key];
     }
   },
-  set: function(xkey, number, hardened, derived) {
+  set: function(xkey, number, hardened, derived, nonCompliant) {
     hardened = !!hardened;
-    var key = xkey + '/' + number + '/' + hardened;
+    var key =  (nonCompliant ? '1' : '0') + xkey + '/' + number + '/' + hardened;
     this._cache[key] = derived;
     this._cacheHit(key);
   },

lib/hdprivatekey.js

@@ -139,6 +139,9 @@ HDPrivateKey._getDerivationIndexes = function(path) {
  * derived. If the second argument is truthy, the hardened version will be
  * derived. See the example usage for clarification.
  *
+ * WARNING: The `nonCompliant` option should NOT be used, except for older implementation
+ * that used a derivation strategy that used a non-zero padded private key.
+ *
  * @example
  * ```javascript
  * var parent = new HDPrivateKey('xprv...');
@@ -149,18 +152,38 @@ HDPrivateKey._getDerivationIndexes = function(path) {
  *
  * @param {string|number} arg
  * @param {boolean?} hardened
+ * @param {boolean?} nonCompliant - This should not be used and only for backwards compatibility
+ *
  */
-HDPrivateKey.prototype.derive = function(arg, hardened) {
+HDPrivateKey.prototype.derive = function(arg, hardened, nonCompliant) {
   if (_.isNumber(arg)) {
-    return this._deriveWithNumber(arg, hardened);
+    return this._deriveWithNumber(arg, hardened, nonCompliant);
   } else if (_.isString(arg)) {
-    return this._deriveFromString(arg);
+    return this._deriveFromString(arg, nonCompliant);
   } else {
     throw new hdErrors.InvalidDerivationArgument(arg);
   }
 };
 
-HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) {
+/**
+ * WARNING: If this is a new implementation you should NOT use this method, you should be using
+ * `derive` instead.
+ *
+ * This method is explicitly for use and compatibility with an implementation that
+ * was not compliant with BIP32 regarding the derivation algorithm. The private key
+ * must be 32 bytes hashing, and this implementation will use the non-zero padded
+ * serialization of a private key, such that it's still possible to derive the privateKey
+ * to recover those funds.
+ *
+ * @param {string|number} arg
+ * @param {boolean?} hardened
+ */
+HDPrivateKey.prototype.deriveNonCompliant = function(arg, hardened) {
+  var derived = this.derive(arg, hardened, true);
+  return derived;
+};
+
+HDPrivateKey.prototype._deriveWithNumber = function(index, hardened, nonCompliant) {
   /* jshint maxstatements: 20 */
   /* jshint maxcomplexity: 10 */
   if (!HDPrivateKey.isValidPath(index, hardened)) {
@@ -172,15 +195,23 @@ HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) {
     index += HDPrivateKey.Hardened;
   }
 
-  var cached = HDKeyCache.get(this.xprivkey, index, hardened);
+  var cached = HDKeyCache.get(this.xprivkey, index, hardened, nonCompliant);
   if (cached) {
     return cached;
   }
 
   var indexBuffer = BufferUtil.integerAsBuffer(index);
   var data;
-  if (hardened) {
-    data = BufferUtil.concat([new buffer.Buffer([0]), this.privateKey.toBuffer(), indexBuffer]);
+  if (hardened && nonCompliant) {
+    // The private key serialization in this case will not be exactly 32 bytes and can be
+    // any value less, and the value is not zero-padded.
+    var nonZeroPadded = this.privateKey.bn.toBuffer();
+    data = BufferUtil.concat([new buffer.Buffer([0]), nonZeroPadded, indexBuffer]);
+  } else if (hardened) {
+    // This will use a 32 byte zero padded serialization of the private key
+    var privateKeyBuffer = this.privateKey.bn.toBuffer({size: 32});
+    assert(privateKeyBuffer.length === 32, 'length of private key buffer is expected to be 32 bytes');
+    data = BufferUtil.concat([new buffer.Buffer([0]), privateKeyBuffer, indexBuffer]);
   } else {
     data = BufferUtil.concat([this.publicKey.toBuffer(), indexBuffer]);
   }
@@ -202,18 +233,18 @@ HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) {
     chainCode: chainCode,
     privateKey: privateKey
   });
-  HDKeyCache.set(this.xprivkey, index, hardened, derived);
+  HDKeyCache.set(this.xprivkey, index, hardened, derived, nonCompliant);
   return derived;
 };
 
-HDPrivateKey.prototype._deriveFromString = function(path) {
+HDPrivateKey.prototype._deriveFromString = function(path, nonCompliant) {
   if (!HDPrivateKey.isValidPath(path)) {
     throw new hdErrors.InvalidPath(path);
   }
 
   var indexes = HDPrivateKey._getDerivationIndexes(path);
   var derived = indexes.reduce(function(prev, index) {
-    return prev._deriveWithNumber(index);
+    return prev._deriveWithNumber(index, null, nonCompliant);
   }, this);
 
   return derived;

test/hdkeycache.js

@@ -25,22 +25,22 @@ describe('HDKey cache', function() {
 
   it('saves a derived key', function() {
     var child = master.derive(0);
-    expect(cache._cache[master.xprivkey + '/0/false'].xprivkey).to.equal(child.xprivkey);
+    expect(cache._cache['0' + master.xprivkey + '/0/false'].xprivkey).to.equal(child.xprivkey);
   });
   it('starts erasing unused keys', function() {
     var child1 = master.derive(0);
     var child2 = child1.derive(0);
     var child3 = child2.derive(0);
-    expect(cache._cache[master.xprivkey + '/0/false'].xprivkey).to.equal(child1.xprivkey);
+    expect(cache._cache['0' + master.xprivkey + '/0/false'].xprivkey).to.equal(child1.xprivkey);
     var child4 = child3.derive(0);
-    expect(cache._cache[master.xprivkey + '/0/false']).to.equal(undefined);
+    expect(cache._cache['0' + master.xprivkey + '/0/false']).to.equal(undefined);
   });
   it('avoids erasing keys that get cache hits ("hot keys")', function() {
     var child1 = master.derive(0);
     var child2 = master.derive(0).derive(0);
-    expect(cache._cache[master.xprivkey + '/0/false'].xprivkey).to.equal(child1.xprivkey);
+    expect(cache._cache['0' + master.xprivkey + '/0/false'].xprivkey).to.equal(child1.xprivkey);
     var child1_copy = master.derive(0);
-    expect(cache._cache[master.xprivkey + '/0/false'].xprivkey).to.equal(child1.xprivkey);
+    expect(cache._cache['0' + master.xprivkey + '/0/false'].xprivkey).to.equal(child1.xprivkey);
   });
   it('keeps the size of the cache small', function() {
     var child1 = master.derive(0);

test/hdkeys.js

@@ -221,6 +221,38 @@ describe('BIP32 compliance', function() {
       .xpubkey.should.equal(vector2_m02147483647h12147483646h2_public);
   });
 
+  it('should use full 32 bytes for private key data that is hashed (as per bip32)', function() {
+    // https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki
+    var privateKeyBuffer = new Buffer('00000055378cf5fafb56c711c674143f9b0ee82ab0ba2924f19b64f5ae7cdbfd', 'hex');
+    var chainCodeBuffer = new Buffer('9c8a5c863e5941f3d99453e6ba66b328bb17cf0b8dec89ed4fc5ace397a1c089', 'hex');
+    var key = HDPrivateKey.fromObject({
+      network: 'testnet',
+      depth: 0,
+      parentFingerPrint: 0,
+      childIndex: 0,
+      privateKey: privateKeyBuffer,
+      chainCode: chainCodeBuffer
+    });
+    var derived = key.derive("m/44'/0'/0'/0/0'");
+    derived.privateKey.toString().should.equal('3348069561d2a0fb925e74bf198762acc47dce7db27372257d2d959a9e6f8aeb');
+  });
+
+  it('should NOT use full 32 bytes for private key data that is hashed with nonCompliant flag', function() {
+    // This is to test that the previously implemented non-compliant to BIP32
+    var privateKeyBuffer = new Buffer('00000055378cf5fafb56c711c674143f9b0ee82ab0ba2924f19b64f5ae7cdbfd', 'hex');
+    var chainCodeBuffer = new Buffer('9c8a5c863e5941f3d99453e6ba66b328bb17cf0b8dec89ed4fc5ace397a1c089', 'hex');
+    var key = HDPrivateKey.fromObject({
+      network: 'testnet',
+      depth: 0,
+      parentFingerPrint: 0,
+      childIndex: 0,
+      privateKey: privateKeyBuffer,
+      chainCode: chainCodeBuffer
+    });
+    var derived = key.deriveNonCompliant("m/44'/0'/0'/0/0'");
+    derived.privateKey.toString().should.equal('4811a079bab267bfdca855b3bddff20231ff7044e648514fa099158472df2836');
+  });
+
   describe('seed', function() {
 
     it('should initialize a new BIP32 correctly from test vector 1 seed', function() {