From 067ef9102deaad9721ace8fe9cf343b12a1ae388 Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Mon, 12 Sep 2016 18:14:30 -0400 Subject: [PATCH 1/2] 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/hdprivatekey.js | 10 +++++- lib/hdpublickey.js | 7 ++++- test/hdkeys.js | 74 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 2 deletions(-) diff --git a/lib/hdprivatekey.js b/lib/hdprivatekey.js index 3826e0b40..e6f1867c2 100644 --- a/lib/hdprivatekey.js +++ b/lib/hdprivatekey.js @@ -180,7 +180,10 @@ HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) { var indexBuffer = BufferUtil.integerAsBuffer(index); var data; if (hardened) { - data = BufferUtil.concat([new buffer.Buffer([0]), this.privateKey.toBuffer(), indexBuffer]); + // 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]); } @@ -194,6 +197,11 @@ HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) { size: 32 }); + if (!PrivateKey.isValid(privateKey)) { + // Index at this point is already hardened, we do not need to pass the hardened arg + return this._deriveWithNumber(index + 1); + } + var derived = new HDPrivateKey({ network: this.network, depth: this.depth + 1, diff --git a/lib/hdpublickey.js b/lib/hdpublickey.js index 198947d06..04aad48d9 100644 --- a/lib/hdpublickey.js +++ b/lib/hdpublickey.js @@ -136,7 +136,12 @@ HDPublicKey.prototype._deriveWithNumber = function(index, hardened) { var leftPart = BN.fromBuffer(hash.slice(0, 32), {size: 32}); var chainCode = hash.slice(32, 64); - var publicKey = PublicKey.fromPoint(Point.getG().mul(leftPart).add(this.publicKey.point)); + var publicKey; + try { + publicKey = PublicKey.fromPoint(Point.getG().mul(leftPart).add(this.publicKey.point)); + } catch (e) { + return this._deriveWithNumber(index + 1); + } var derived = new HDPublicKey({ network: this.network, diff --git a/test/hdkeys.js b/test/hdkeys.js index f6e838d34..cfa91efc4 100644 --- a/test/hdkeys.js +++ b/test/hdkeys.js @@ -13,6 +13,7 @@ var _ = require('lodash'); var should = require('chai').should(); var expect = require('chai').expect; +var sinon = require('sinon'); var bitcore = require('..'); var Networks = bitcore.Networks; var HDPrivateKey = bitcore.HDPrivateKey; @@ -221,6 +222,79 @@ 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'); + }); + + describe('edge cases', function() { + var sandbox = sinon.sandbox.create(); + afterEach(function() { + sandbox.restore(); + }); + it('will handle edge case that derived private key is invalid', function() { + var invalid = new Buffer('0000000000000000000000000000000000000000000000000000000000000000', 'hex'); + var privateKeyBuffer = new Buffer('5f72914c48581fc7ddeb944a9616389200a9560177d24f458258e5b04527bcd1', 'hex'); + var chainCodeBuffer = new Buffer('39816057bba9d952fe87fe998b7fd4d690a1bb58c2ff69141469e4d1dffb4b91', 'hex'); + var unstubbed = bitcore.crypto.BN.prototype.toBuffer; + var count = 0; + var stub = sandbox.stub(bitcore.crypto.BN.prototype, 'toBuffer', function(args) { + // On the fourth call to the function give back an invalid private key + // otherwise use the normal behavior. + count++; + if (count === 4) { + return invalid; + } + var ret = unstubbed.apply(this, arguments); + return ret; + }); + sandbox.spy(bitcore.PrivateKey, 'isValid'); + var key = HDPrivateKey.fromObject({ + network: 'testnet', + depth: 0, + parentFingerPrint: 0, + childIndex: 0, + privateKey: privateKeyBuffer, + chainCode: chainCodeBuffer + }); + var derived = key.derive("m/44'"); + derived.privateKey.toString().should.equal('b15bce3608d607ee3a49069197732c656bca942ee59f3e29b4d56914c1de6825'); + bitcore.PrivateKey.isValid.callCount.should.equal(2); + }); + it('will handle edge case that a derive public key is invalid', function() { + var publicKeyBuffer = new Buffer('029e58b241790284ef56502667b15157b3fc58c567f044ddc35653860f9455d099', 'hex'); + var chainCodeBuffer = new Buffer('39816057bba9d952fe87fe998b7fd4d690a1bb58c2ff69141469e4d1dffb4b91', 'hex'); + var key = new HDPublicKey({ + network: 'testnet', + depth: 0, + parentFingerPrint: 0, + childIndex: 0, + chainCode: chainCodeBuffer, + publicKey: publicKeyBuffer + }); + var unstubbed = bitcore.PublicKey.fromPoint; + bitcore.PublicKey.fromPoint = function() { + bitcore.PublicKey.fromPoint = unstubbed; + throw new Error('Point cannot be equal to Infinity'); + }; + sandbox.spy(key, '_deriveWithNumber'); + var derived = key.derive("m/44"); + key._deriveWithNumber.callCount.should.equal(2); + key.publicKey.toString().should.equal('029e58b241790284ef56502667b15157b3fc58c567f044ddc35653860f9455d099'); + }); + }); + describe('seed', function() { it('should initialize a new BIP32 correctly from test vector 1 seed', function() { From db466f79d958f16bf651ab7a2938f3db5518debf Mon Sep 17 00:00:00 2001 From: Kirill Fomichev Date: Thu, 25 Feb 2016 19:03:39 +0300 Subject: [PATCH 2/2] Fix PrivateKey.toBuffer --- lib/privatekey.js | 2 +- test/privatekey.js | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/privatekey.js b/lib/privatekey.js index 9697bbfba..636eb2d52 100644 --- a/lib/privatekey.js +++ b/lib/privatekey.js @@ -336,7 +336,7 @@ PrivateKey.prototype.toBigNumber = function(){ * @returns {Buffer} A buffer of the private key */ PrivateKey.prototype.toBuffer = function(){ - return this.bn.toBuffer(); + return this.bn.toBuffer({ size: 32 }); }; /** diff --git a/test/privatekey.js b/test/privatekey.js index 320b85865..0f0c4c0b8 100644 --- a/test/privatekey.js +++ b/test/privatekey.js @@ -330,6 +330,13 @@ describe('PrivateKey', function() { var fromBuffer = PrivateKey.fromBuffer(toBuffer.toBuffer()); fromBuffer.toString().should.equal(privkey.toString()); }); + + it('should return buffer with length equal 32', function() { + var bn = BN.fromBuffer(buf.slice(0, 31)); + var privkey = new PrivateKey(bn, 'livenet'); + var expected = Buffer.concat([ new Buffer([0]), buf.slice(0, 31) ]); + privkey.toBuffer().toString('hex').should.equal(expected.toString('hex')); + }); }); describe('#toBigNumber', function() {