From 092c3ff37e5d2333e54384248c14ea921594794d Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Thu, 26 Nov 2015 15:16:05 -0300 Subject: [PATCH] make self contained signature keys optional --- lib/server.js | 29 +++++++++----- test/integration/helpers.js | 4 -- test/integration/server.js | 75 ++++++++++++++++++++++++++++++++----- 3 files changed, 85 insertions(+), 23 deletions(-) diff --git a/lib/server.js b/lib/server.js index 4874759..a23c2e3 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1425,13 +1425,18 @@ WalletService.prototype.createTx = function(opts, cb) { }); }; +WalletService.prototype._verifyRequestPubKey = function(requestPubKey, signature, xPubKey) { + var pub = (new Bitcore.HDPublicKey(xPubKey)).derive(Constants.PATHS.REQUEST_KEY_AUTH).publicKey; + return Utils.verifyMessage(requestPubKey, signature, pub.toString()); +}; + /** * Send an already created tx proposal to other copayers in the wallet. * @param {Object} opts * @param {string} opts.txProposalId - The tx id. * @param {string} opts.proposalSignature - S(raw tx). Used by other copayers to verify the proposal. - * @param {string} opts.proposalSignaturePubKey - The public key needed to verify the proposal signature. - * @param {string} opts.proposalSignaturePubKeySig - A signature of the public key used to validate that the public key belongs to the creator. + * @param {string} opts.proposalSignaturePubKey - (Optional) An alternative public key used to verify the proposal signature. + * @param {string} opts.proposalSignaturePubKeySig - (Optional) A signature used to validate the opts.proposalSignaturePubKey. */ WalletService.prototype.sendTx = function(opts, cb) { var self = this; @@ -1440,7 +1445,7 @@ WalletService.prototype.sendTx = function(opts, cb) { return utxo.txid + '|' + utxo.vout }; - if (!Utils.checkRequired(opts, ['txProposalId', 'proposalSignature', 'proposalSignaturePubKey', 'proposalSignaturePubKeySig'])) + if (!Utils.checkRequired(opts, ['txProposalId', 'proposalSignature'])) return cb(new ClientError('Required argument missing')); self._runLocked(cb, function(cb) { @@ -1452,19 +1457,23 @@ WalletService.prototype.sendTx = function(opts, cb) { if (!txp) return cb(Errors.TX_NOT_FOUND); if (!txp.isTemporary()) return cb(); - // Validate that the pubkey used to sign the proposal actually belongs to the copayer var copayer = wallet.getCopayer(self.copayerId); - var validPubKey = !!self._getSigningKey(opts.proposalSignaturePubKey, opts.proposalSignaturePubKeySig, copayer.requestPubKeys); - if (!validPubKey) { - return cb(new ClientError('Invalid proposal signing key')); - } var raw = txp.getRawTx(); - var validSignature = self._verifySignature(raw, opts.proposalSignature, opts.proposalSignaturePubKey); - if (!validSignature) { + var signingKey = self._getSigningKey(raw, opts.proposalSignature, copayer.requestPubKeys); + if (!signingKey) { return cb(new ClientError('Invalid proposal signature')); } + // Verify signingKey + if (opts.proposalSignaturePubKey) { + if (opts.proposalSignaturePubKey != signingKey || + !self._verifyRequestPubKey(opts.proposalSignaturePubKey, opts.proposalSignaturePubKeySig, copayer.xPubKey) + ) { + return cb(new ClientError('Invalid proposal signing key')); + } + } + // Verify UTXOs are still available self.getUtxos({}, function(err, utxos) { if (err) return cb(err); diff --git a/test/integration/helpers.js b/test/integration/helpers.js index 508c579..c717e5f 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -397,14 +397,10 @@ helpers.createProposalOpts2 = function(outputs, moreOpts, inputs) { helpers.getProposalSignatureOpts = function(txp, signingKey) { var raw = txp.getRawTx(); var proposalSignature = helpers.signMessage(raw, signingKey); - var pubKey = new Bitcore.PrivateKey(signingKey).toPublicKey().toString(); - var pubKeySig = helpers.signMessage(pubKey, signingKey); return { txProposalId: txp.id, proposalSignature: proposalSignature, - proposalSignaturePubKey: pubKey, - proposalSignaturePubKeySig: pubKeySig, } }; diff --git a/test/integration/server.js b/test/integration/server.js index 888580c..ceb74ca 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1638,7 +1638,7 @@ describe('Wallet service', function() { }); }); - describe('#createTxLegacy', function() { + describe('createTxLegacy', function() { var server, wallet; beforeEach(function(done) { helpers.createAndJoinWallet(2, 3, function(s, w) { @@ -2390,12 +2390,11 @@ describe('Wallet service', function() { }); }); }); + it('should fail to send non-existent tx proposal', function(done) { server.sendTx({ txProposalId: 'wrong-id', proposalSignature: 'dummy', - proposalSignaturePubKey: 'dummy', - proposalSignaturePubKeySig: 'dummy', }, function(err) { should.exist(err); server.getPendingTxs({}, function(err, txs) { @@ -2405,6 +2404,7 @@ describe('Wallet service', function() { }); }); }); + it('should fail to send tx proposal with wrong signature', function(done) { helpers.stubUtxos(server, wallet, [1, 2], function() { var txOpts = helpers.createProposalOpts2([{ @@ -2412,14 +2412,14 @@ describe('Wallet service', function() { amount: 0.8 }], { message: 'some message', - customData: 'some custom data', }); server.createTx(txOpts, function(err, txp) { should.not.exist(err); should.exist(txp); - var sendOpts = helpers.getProposalSignatureOpts(txp, TestData.copayers[0].privKey_1H_0); - sendOpts.proposalSignature = 'dummy'; - server.sendTx(sendOpts, function(err) { + server.sendTx({ + txProposalId: txp.id, + proposalSignature: 'dummy' + }, function(err) { should.exist(err); err.message.should.contain('Invalid proposal signature'); done(); @@ -2427,6 +2427,7 @@ describe('Wallet service', function() { }); }); }); + it('should fail to send tx proposal not signed by the creator', function(done) { helpers.stubUtxos(server, wallet, [1, 2], function() { var txOpts = helpers.createProposalOpts2([{ @@ -2434,12 +2435,23 @@ describe('Wallet service', function() { amount: 0.8 }], { message: 'some message', - customData: 'some custom data', }); server.createTx(txOpts, function(err, txp) { should.not.exist(err); should.exist(txp); - var sendOpts = helpers.getProposalSignatureOpts(txp, TestData.copayers[1].privKey_1H_0); + + var raw = txp.getRawTx(); + var proposalSignature = helpers.signMessage(raw, TestData.copayers[0].privKey_1H_0); + var pubKey = new Bitcore.PrivateKey(TestData.copayers[0].privKey_1H_0).toPublicKey().toString(); + var pubKeySig = helpers.signMessage(pubKey, TestData.copayers[1].privKey_1H_0); + + var sendOpts = { + txProposalId: txp.id, + proposalSignature: proposalSignature, + proposalSignaturePubKey: pubKey, + proposalSignaturePubKeySig: pubKeySig, + } + server.sendTx(sendOpts, function(err) { should.exist(err); err.message.should.contain('Invalid proposal signing key'); @@ -2448,6 +2460,51 @@ describe('Wallet service', function() { }); }); }); + + it('should accept a tx proposal signed with a custom key', function(done) { + var reqPrivKey = new Bitcore.PrivateKey(); + var reqPubKey = reqPrivKey.toPublicKey(); + + var xPrivKey = TestData.copayers[0].xPrivKey_44H_0H_0H; + var sig = helpers.signRequestPubKey(reqPubKey, xPrivKey); + + var opts = { + copayerId: TestData.copayers[0].id44, + requestPubKey: reqPubKey, + signature: sig, + }; + + server.addAccess(opts, function(err) { + should.not.exist(err); + + helpers.stubUtxos(server, wallet, [1, 2], function() { + var txOpts = helpers.createProposalOpts2([{ + toAddress: '18PzpUFkFZE8zKWUPvfykkTxmB9oMR8qP7', + amount: 0.8 + }], { + message: 'some message', + }); + server.createTx(txOpts, function(err, txp) { + should.not.exist(err); + should.exist(txp); + + var sendOpts = { + txProposalId: txp.id, + proposalSignature: helpers.signMessage(txp.getRawTx(), reqPrivKey), + proposalSignaturePubKey: reqPubKey, + proposalSignaturePubKeySig: sig, + } + + server.sendTx(sendOpts, function(err) { + should.exist(err); + err.message.should.contain('Invalid proposal signing key'); + done(); + }); + }); + }); + }); + }); + it('should fail to send a temporary tx proposal if utxos are unavailable', function(done) { var txp1, txp2; var txOpts = helpers.createProposalOpts2([{