Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
Merge pull request #286 from isocolsky/ref/validation
Browse files Browse the repository at this point in the history 
Refactor key derivation validation
  • Loading branch information
@matiu
matiu committed Jun 14, 2016
2 parents 0e9b891 + d518d12 commit 8169736
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 74 deletions.
73 changes: 36 additions & 37 deletions lib/api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ function API(opts) {
this.doNotVerifyPayPro = opts.doNotVerifyPayPro;
this.timeout = opts.timeout || 50000;


if (this.verbose) {
log.setLevel('debug');
} else {
Expand Down Expand Up @@ -297,10 +296,22 @@ API.prototype.seedFromRandom = function(opts) {
this.credentials = Credentials.create(opts.network || 'livenet');
};

var _hardcodedOk;
API.prototype._validateKeys = function(passphrase) {
var c = this.credentials;
if (!c.canSign()) return;

var _deviceValidated;

/**
* Seed from random
*
* @param {Object} opts
* @param {String} opts.passphrase
* @param {String} opts.skipDeviceValidation
*/
API.prototype.validateKeyDerivation = function(opts, cb) {
var self = this;

opts = opts || {};

var c = self.credentials;

function testMessageSigning(xpriv, xpub) {
var nonHardenedPath = 'm/0/0';
Expand Down Expand Up @@ -331,9 +342,9 @@ API.prototype._validateKeys = function(passphrase) {
} catch (ex) {}

var xpriv;
if (words && (!c.mnemonicHasPassphrase || passphrase)) {
if (words && (!c.mnemonicHasPassphrase || opts.passphrase)) {
var m = new Mnemonic(words);
xpriv = m.toHDPrivateKey(passphrase, c.network);
xpriv = m.toHDPrivateKey(opts.passphrase, c.network);
}
if (!xpriv) {
xpriv = new Bitcore.HDPrivateKey(c.xPrivKey);
Expand All @@ -344,18 +355,17 @@ API.prototype._validateKeys = function(passphrase) {
return testMessageSigning(xpriv, xpub);
};

if (_.isUndefined(_hardcodedOk)) {
_hardcodedOk = testHardcodedKeys();
var hardcodedOk = true;
if (!_deviceValidated && !opts.skipDeviceValidation) {
hardcodedOk = testHardcodedKeys();
_deviceValidated = true;
}
var liveOk = !c.isPrivKeyEncrypted() ? testLiveKeys() : true;

this.incorrectDerivation = !_hardcodedOk || !liveOk;
var liveOk = c.canSign() ? testLiveKeys() : true;

if (this.incorrectDerivation) {
this.emit('derivation-error');
}
self.keyDerivationOk = hardcodedOk && liveOk;

return !this.incorrectDerivation;
return cb(null, self.keyDerivationOk);
};

/**
Expand All @@ -373,9 +383,6 @@ API.prototype.seedFromRandomWithMnemonic = function(opts) {

opts = opts || {};
this.credentials = Credentials.createWithMnemonic(opts.network || 'livenet', opts.passphrase, opts.language || 'en', opts.account || 0);
if (!this._validateKeys(opts.passphrase)) {
this.credentials = null;
}
};

API.prototype.getMnemonic = function() {
Expand Down Expand Up @@ -422,7 +429,6 @@ API.prototype.seedFromMnemonic = function(words, opts) {

opts = opts || {};
this.credentials = Credentials.fromMnemonic(opts.network || 'livenet', words, opts.passphrase, opts.account || 0, opts.derivationStrategy || Constants.DERIVATION_STRATEGIES.BIP44);
this._validateKeys(opts.passphrase);
};

/**
Expand Down Expand Up @@ -476,9 +482,6 @@ API.prototype.export = function(opts) {
* @param {Object} opts
* @param {String} opts.password If the source has the private key encrypted, the password
* will be needed for derive credentials fields.
* @param {Boolean} opts.skipKeyValidation Skip extended key validation
*
*
*/
API.prototype.import = function(str, opts) {
opts = opts || {};
Expand All @@ -488,9 +491,6 @@ API.prototype.import = function(str, opts) {
} catch (ex) {
throw new Errors.INVALID_BACKUP;
}

if (!opts.skipKeyValidation)
this._validateKeys();
};

API.prototype._import = function(cb) {
Expand Down Expand Up @@ -1151,6 +1151,14 @@ API.prototype.getVersion = function(cb) {
this._doGetRequest('/v1/version/', cb);
};

API.prototype._checkKeyDerivation = function() {
var isInvalid = (this.keyDerivationOk === false);
if (isInvalid) {
log.error('Key derivation for this device is not working as expected');
}
return !isInvalid;
};

/**
*
* Create a wallet.
Expand All @@ -1169,10 +1177,7 @@ API.prototype.getVersion = function(cb) {
API.prototype.createWallet = function(walletName, copayerName, m, n, opts, cb) {
var self = this;

if (self.incorrectDerivation) {
log.error('Key derivation for this device is not working as expected');
return cb(new Error('Cannot create new wallet'));
}
if (!self._checkKeyDerivation()) return cb(new Error('Cannot create new wallet'));

if (opts) $.shouldBeObject(opts);
opts = opts || {};
Expand Down Expand Up @@ -1242,10 +1247,7 @@ API.prototype.joinWallet = function(secret, copayerName, opts, cb) {
log.warn('DEPRECATED WARN: joinWallet should receive 4 parameters.');
}

if (self.incorrectDerivation) {
log.error('Key derivation for this device is not working as expected');
return cb(new Error('Cannot join wallet'));
}
if (!self._checkKeyDerivation()) return cb(new Error('Cannot join wallet'));

opts = opts || {};

Expand Down Expand Up @@ -1744,10 +1746,7 @@ API.prototype.createAddress = function(opts, cb) {
log.warn('DEPRECATED WARN: createAddress should receive 2 parameters.')
}

if (self.incorrectDerivation) {
log.error('Key derivation for this device is not working as expected');
return cb(new Error('Cannot create new address for this wallet'));
}
if (!self._checkKeyDerivation()) return cb(new Error('Cannot create new address for this wallet'));

opts = opts || {};

Expand Down
71 changes: 34 additions & 37 deletions test/client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,7 @@ describe('client API', function() {
describe('Wallet Creation', function() {
it('should fail to create wallet in bogus device', function(done) {
clients[0].seedFromRandomWithMnemonic();
clients[0].incorrectDerivation = true;
clients[0].keyDerivationOk = false;
clients[0].createWallet('mywallet', 'pepe', 1, 1, {}, function(err, secret) {
should.exist(err);
should.not.exist(secret);
Expand Down Expand Up @@ -1166,7 +1166,7 @@ describe('client API', function() {
network: 'testnet'
}, function(err, secret) {
should.not.exist(err);
clients[1].incorrectDerivation = true;
clients[1].keyDerivationOk = false;
clients[1].joinWallet(secret, 'guest', {}, function(err, wallet) {
should.exist(err);
done();
Expand Down Expand Up @@ -1637,7 +1637,7 @@ describe('client API', function() {
});
it('should fail if key derivation is not ok', function(done) {
helpers.createAndJoinWallet(clients, 1, 1, function() {
clients[0].incorrectDerivation = true;
clients[0].keyDerivationOk = false;
clients[0].createAddress(function(err, address) {
should.exist(err);
should.not.exist(address);
Expand Down Expand Up @@ -3627,42 +3627,8 @@ describe('client API', function() {

importedClient = helpers.newClient(app);
importedClient.import(exported);
importedClient.incorrectDerivation.should.equal(false);
});


it('should handle wrong derivation', function(done) {
var exported = JSON.parse(clients[0].export());

// Tamper export with a wrong xpub
exported.xPubKey = 'tpubD6NzVbkrYhZ4XJEQQWBgysPKJcBv8zLhHpfhcw4RyhakMxmffNRRRFDUe1Zh7fxvjt1FdNJcaxHgqxyKLL8XiZug7C8KJFLFtGfPVBcY6Nb';

importedClient = helpers.newClient(app);
should.not.exist(importedClient.incorrectDerivation);

importedClient.on('derivation-error', function() {
importedClient.incorrectDerivation.should.equal(true);
done();
});

importedClient.import(JSON.stringify(exported));
});


it('should skip derivation check if commanded', function() {
var exported = JSON.parse(clients[0].export());

// Tamper export with a wrong xpub
exported.xPubKey = 'tpubD6NzVbkrYhZ4XJEQQWBgysPKJcBv8zLhHpfhcw4RyhakMxmffNRRRFDUe1Zh7fxvjt1FdNJcaxHgqxyKLL8XiZug7C8KJFLFtGfPVBcY6Nb';

importedClient = helpers.newClient(app);
importedClient.import(JSON.stringify(exported), {
skipKeyValidation: true
});
should.not.exist(importedClient.incorrectDerivation);
});


it('should export without signing rights', function() {
clients[0].canSign().should.be.true;
var exported = clients[0].export({
Expand Down Expand Up @@ -3719,6 +3685,37 @@ describe('client API', function() {
});
});

describe('#validateKeyDerivation', function() {
beforeEach(function(done) {
helpers.createAndJoinWallet(clients, 1, 1, function() {
done();
});
});
it('should validate key derivation', function(done) {
clients[0].validateKeyDerivation({}, function(err, isValid) {
should.not.exist(err);
isValid.should.be.true;
clients[0].keyDerivationOk.should.be.true;

var exported = JSON.parse(clients[0].export());

// Tamper export with a wrong xpub
exported.xPubKey = 'tpubD6NzVbkrYhZ4XJEQQWBgysPKJcBv8zLhHpfhcw4RyhakMxmffNRRRFDUe1Zh7fxvjt1FdNJcaxHgqxyKLL8XiZug7C8KJFLFtGfPVBcY6Nb';

var importedClient = helpers.newClient(app);
should.not.exist(importedClient.keyDerivationOk);

importedClient.import(JSON.stringify(exported));
importedClient.validateKeyDerivation({}, function(err, isValid) {
should.not.exist(err);
isValid.should.be.false;
importedClient.keyDerivationOk.should.be.false;
done();
});
});
});
});

describe('Mnemonic related tests', function() {
var importedClient;

Expand Down

0 comments on commit 8169736

Please sign in to comment.