Merge pull request #24 from isocolsky/check_proposal_signature

Check proposal signature

lib/bitcoinutils.js

@@ -1,25 +0,0 @@
-
-var _ = require('lodash');
-
-var Bitcore = require('bitcore');
-var BitcoreAddress = Bitcore.Address;
-
-function BitcoinUtils () {};
-
-BitcoinUtils.deriveAddress = function(publicKeyRing, path, m, network) {
-
-  var publicKeys = _.map(publicKeyRing, function(xPubKey) {
-    var xpub = new Bitcore.HDPublicKey(xPubKey);
-    return xpub.derive(path).publicKey;
-  });
-
-  var bitcoreAddress = BitcoreAddress.createMultisig(publicKeys, m, network);
-
-  return {
-    address: bitcoreAddress.toString(),
-    path: path,
-    publicKeys: _.invoke(publicKeys, 'toString'),
-  };
-};
-
-module.exports = BitcoinUtils;

lib/client/Verifier.js

@@ -3,8 +3,7 @@ var _ = require('lodash');
 var log = require('npmlog');
 
 var Bitcore = require('bitcore');
-var BitcoinUtils = require('../bitcoinutils')
+var WalletUtils = require('../walletutils')
-var SignUtils = require('../signutils');
 
 /* 
  * Checks data given by the server
@@ -13,13 +12,10 @@ var SignUtils = require('../signutils');
 function Verifier(opts) {};
 
 Verifier.checkAddress = function(data, address) {
-  var local = BitcoinUtils.deriveAddress(data.publicKeyRing, address.path, data.m, data.network);
+  var local = WalletUtils.deriveAddress(data.publicKeyRing, address.path, data.m, data.network);
   return (local.address == address.address && JSON.stringify(local.publicKeys) == JSON.stringify(address.publicKeys));
 };
 
-
-//
-
 Verifier.checkCopayers = function(copayers, walletPrivKey, myXPrivKey, n) {
 
   var walletPubKey = Bitcore.PrivateKey.fromString(walletPrivKey).toPublicKey().toString();
@@ -39,7 +35,7 @@ Verifier.checkCopayers = function(copayers, walletPrivKey, myXPrivKey, n) {
     }
 
     // Not signed pub keys
-    if (!SignUtils.verify(copayer.xPubKey, copayer.xPubKeySignature, walletPubKey)) {
+    if (!WalletUtils.verifyMessage(copayer.xPubKey, copayer.xPubKeySignature, walletPubKey)) {
       log.error('Invalid signatures in server response');
       error = true;
     }
@@ -56,4 +52,12 @@ Verifier.checkCopayers = function(copayers, walletPrivKey, myXPrivKey, n) {
 };
 
 
+Verifier.checkTxProposal = function(data, txp) {
+  var hash = WalletUtils.getProposalHash(txp.toAddress, txp.amount, txp.message);
+  var signingPubKey = Bitcore.PrivateKey.fromString(data.signingPrivKey).toPublicKey().toString();
+  if (!WalletUtils.verifyMessage(hash, txp.proposalSignature, signingPubKey)) return false;
+
+  return Verifier.checkAddress(data, txp.changeAddress);
+};
+
 module.exports = Verifier;

lib/client/api.js

@@ -9,7 +9,7 @@ var request = require('request')
 log.debug = log.verbose;
 
 var Bitcore = require('bitcore')
-var SignUtils = require('../signutils');
+var WalletUtils = require('../walletutils');
 var Verifier = require('./verifier');
 var ServerCompromisedError = require('./servercompromisederror')
 
@@ -17,7 +17,7 @@ var BASE_URL = 'http://localhost:3001/copay/api';
 
 function _createProposalOpts(opts, signingKey) {
   var msg = opts.toAddress + '|' + opts.amount + '|' + opts.message;
-  opts.proposalSignature = SignUtils.sign(msg, signingKey);
+  opts.proposalSignature = WalletUtils.signMessage(msg, signingKey);
   return opts;
 };
 
@@ -42,7 +42,7 @@ function _parseError(body) {
 
 function _signRequest(method, url, args, privKey) {
   var message = method.toLowerCase() + '|' + url + '|' + JSON.stringify(args);
-  return SignUtils.sign(message, privKey);
+  return WalletUtils.signMessage(message, privKey);
 };
 
 function _createXPrivKey(network) {
@@ -62,7 +62,6 @@ function API(opts) {
 };
 
 
-
 API.prototype._tryToComplete = function(data, cb) {
   var self = this;
 
@@ -209,7 +208,7 @@ API.prototype._joinWallet = function(data, secret, copayerName, cb) {
   data.xPrivKey = _createXPrivKey(network);
 
   var xPubKey = new Bitcore.HDPublicKey(data.xPrivKey);
-  var xPubKeySignature = SignUtils.sign(xPubKey.toString(), walletPrivKey);
+  var xPubKeySignature = WalletUtils.signMessage(xPubKey.toString(), walletPrivKey);
 
   var signingPrivKey = (new Bitcore.HDPrivateKey(data.xPrivKey)).derive('m/1/0').privateKey;
   var args = {
@@ -329,6 +328,10 @@ API.prototype.signTxProposal = function(txp, cb) {
     function(err, data) {
       if (err) return cb(err);
 
+      if (!Verifier.checkTxProposal(data, txp)) {
+        return cb(new ServerCompromisedError('Server sent fake transaction proposal'));
+      }
+
 
       //Derive proper key to sign, for each input
       var privs = [],

lib/model/wallet.js

@@ -8,7 +8,7 @@ var Uuid = require('uuid');
 var Address = require('./address');
 var Copayer = require('./copayer');
 var AddressManager = require('./addressmanager');
-var BitcoinUtils = require('../bitcoinutils');
+var WalletUtils = require('../walletutils');
 
 var VERSION = '1.0.0';
 
@@ -117,7 +117,7 @@ Wallet.prototype.createAddress = function(isChange) {
   $.checkState(this.isComplete());
 
   var path = this.addressManager.getNewAddressPath(isChange);
-  return new Address(BitcoinUtils.deriveAddress(this.publicKeyRing, path, this.m, this.network));
+  return new Address(WalletUtils.deriveAddress(this.publicKeyRing, path, this.m, this.network));
 };
 
 

lib/server.js

@@ -17,7 +17,7 @@ var Explorers = require('bitcore-explorers');
 var ClientError = require('./clienterror');
 var Utils = require('./utils');
 var Storage = require('./storage');
-var SignUtils = require('./signutils');
+var WalletUtils = require('./walletutils');
 
 var Wallet = require('./model/wallet');
 var Copayer = require('./model/copayer');
@@ -151,7 +151,7 @@ CopayServer.prototype.getWallet = function(opts, cb) {
  * @param pubKey
  */
 CopayServer.prototype._verifySignature = function(text, signature, pubKey) {
-  return SignUtils.verify(text, signature, pubKey);
+  return WalletUtils.verifyMessage(text, signature, pubKey);
 };
 
 
@@ -464,8 +464,8 @@ CopayServer.prototype.createTx = function(opts, cb) {
       if (!wallet.isComplete()) return cb(new ClientError('Wallet is not complete'));
 
       var copayer = wallet.getCopayer(self.copayerId);
-      var msg = opts.toAddress + '|' + opts.amount + '|' + opts.message;
+      var hash = WalletUtils.getProposalHash(opts.toAddress, opts.amount, opts.message);
-      if (!self._verifySignature(msg, opts.proposalSignature, copayer.signingPubKey))
+      if (!self._verifySignature(hash, opts.proposalSignature, copayer.signingPubKey))
         return cb(new ClientError('Invalid proposal signature'));
 
       var toAddress;

lib/signutils.js

@@ -1,43 +0,0 @@
-var _ = require('lodash');
-var Bitcore = require('bitcore');
-var PrivateKey = Bitcore.PrivateKey;
-var PublicKey = Bitcore.PublicKey;
-var Signature = Bitcore.crypto.Signature;
-var ECDSA = Bitcore.crypto.ECDSA;
-var Hash = Bitcore.crypto.Hash;
-var BufferReader = Bitcore.encoding.BufferReader;
-
-
-
-var SignUtils = function() {};
-
-/* TODO: It would be nice to be compatible with bitcoind signmessage. How
- * the hash is calculated there? */
-SignUtils.hash = function(text) {
-  var buf = new Buffer(text);
-  var ret = Hash.sha256sha256(buf);
-  ret = new BufferReader(ret).readReverse();
-  return ret;
-};
-
-
-SignUtils.sign = function(text, privKey) {
-  var priv = new PrivateKey(privKey);
-  var hash = SignUtils.hash(text);
-  return ECDSA.sign(hash, priv, 'little').toString();
-};
-
-
-SignUtils.verify = function(text, signature, pubKey) {
-  var pub = new PublicKey(pubKey);
-  var hash = SignUtils.hash(text);
-
-  try {
-    var sig = new Signature.fromString(signature);
-    return ECDSA.verify(hash, sig, pub, 'little');
-  } catch (e) {
-    return false;
-  }
-};
-
-module.exports = SignUtils;

lib/walletutils.js

@@ -0,0 +1,59 @@
+var _ = require('lodash');
+var Bitcore = require('bitcore');
+var Address = Bitcore.Address;
+var PrivateKey = Bitcore.PrivateKey;
+var PublicKey = Bitcore.PublicKey;
+var crypto = Bitcore.crypto;
+
+function WalletUtils() {};
+
+/* TODO: It would be nice to be compatible with bitcoind signmessage. How
+ * the hash is calculated there? */
+WalletUtils.hashMessage = function(text) {
+  var buf = new Buffer(text);
+  var ret = crypto.Hash.sha256sha256(buf);
+  ret = new Bitcore.encoding.BufferReader(ret).readReverse();
+  return ret;
+};
+
+
+WalletUtils.signMessage = function(text, privKey) {
+  var priv = new PrivateKey(privKey);
+  var hash = WalletUtils.hashMessage(text);
+  return crypto.ECDSA.sign(hash, priv, 'little').toString();
+};
+
+
+WalletUtils.verifyMessage = function(text, signature, pubKey) {
+  var pub = new PublicKey(pubKey);
+  var hash = WalletUtils.hashMessage(text);
+
+  try {
+    var sig = new crypto.Signature.fromString(signature);
+    return crypto.ECDSA.verify(hash, sig, pub, 'little');
+  } catch (e) {
+    return false;
+  }
+};
+
+WalletUtils.deriveAddress = function(publicKeyRing, path, m, network) {
+
+  var publicKeys = _.map(publicKeyRing, function(xPubKey) {
+    var xpub = new Bitcore.HDPublicKey(xPubKey);
+    return xpub.derive(path).publicKey;
+  });
+
+  var bitcoreAddress = Address.createMultisig(publicKeys, m, network);
+
+  return {
+    address: bitcoreAddress.toString(),
+    path: path,
+    publicKeys: _.invoke(publicKeys, 'toString'),
+  };
+};
+
+WalletUtils.getProposalHash = function(toAddress, amount, message) {
+  return toAddress + '|' + amount + '|' + (message || '');
+};
+
+module.exports = WalletUtils;

test/integration/clientApi.js

@@ -9,12 +9,10 @@ var API = Client.API;
 var Bitcore = require('bitcore');
 var TestData = require('./clienttestdata');
 
-describe(' client API ', function() {
+describe('client API ', function() {
-
   var client;
 
   beforeEach(function() {
-
     var fsmock = {};;
     fsmock.readFile = sinon.mock().yields(null, JSON.stringify(TestData.storage.wallet11));
     fsmock.writeFile = sinon.mock().yields();
@@ -27,7 +25,7 @@ describe(' client API ', function() {
     });
   });
 
-  describe(' _tryToComplete ', function() {
+  describe('#_tryToComplete ', function() {
     it('should complete a wallet ', function(done) {
       var request = sinon.stub();
 
@@ -45,7 +43,7 @@ describe(' client API ', function() {
         should.not.exist(err);
         done();
       });
-    })
+    });
 
 
     it('should handle incomple wallets', function(done) {
@@ -62,7 +60,7 @@ describe(' client API ', function() {
         err.should.contain('Incomplete');
         done();
       });
-    })
+    });
 
     it('should reject wallets with bad signatures', function(done) {
       var request = sinon.stub();
@@ -77,7 +75,8 @@ describe(' client API ', function() {
         err.should.contain('verified');
         done();
       });
-    })
+    });
+
     it('should reject wallets with missing signatures ', function(done) {
       var request = sinon.stub();
       // Wallet request
@@ -91,7 +90,7 @@ describe(' client API ', function() {
         err.should.contain('verified');
         done();
       });
-    })
+    });
 
     it('should reject wallets missing caller"s pubkey', function(done) {
       var request = sinon.stub();
@@ -106,13 +105,11 @@ describe(' client API ', function() {
         err.should.contain('verified');
         done();
       });
-    })
+    });
-
-
   });
 
-  describe(' createAddress ', function() {
+  describe('#createAddress ', function() {
-    it(' should check address ', function(done) {
+    it('should check address ', function(done) {
 
       var response = {
         createdOn: 1424105995,
@@ -131,8 +128,9 @@ describe(' client API ', function() {
         x.address.should.equal('2N3fA6wDtnebzywPkGuNK9KkFaEzgbPRRTq');
         done();
       });
-    })
+    });
-    it(' should detect fake addresses ', function(done) {
+
+    it('should detect fake addresses ', function(done) {
       var response = {
         createdOn: 1424105995,
         address: '2N3fA6wDtnebzywPkGuNK9KkFaEzgbPRRTq',
@@ -148,6 +146,48 @@ describe(' client API ', function() {
         err.message.should.contain('fake address');
         done();
       });
-    })
+    });
-  })
+  });
+
+  describe('#signTxProposal ', function() {
+    it.skip('should sign tx proposal', function(done) {});
+
+    it('should detect fake tx proposal signature', function(done) {
+      var txp = {
+        toAddress: '2N3fA6wDtnebzywPkGuNK9KkFaEzgbPRRTq',
+        amount: 100000,
+        message: 'some message',
+        proposalSignature: 'dummy',
+        changeAddress: {
+          address: '2N3fA6wDtnebzywPkGuNK9KkFaEzgbPRRTq',
+          path: 'm/2147483647/0/7',
+          publicKeys: ['03f6a5fe8db51bfbaf26ece22a3e3bc242891a47d3048fc70bc0e8c03a071ad76f']
+        },
+      };
+      client.signTxProposal(txp, function(err) {
+        err.code.should.equal('SERVERCOMPROMISED');
+        err.message.should.contain('fake transaction proposal');
+        done();
+      });
+    });
+
+    it('should detect fake tx proposal change address', function(done) {
+      var txp = {
+        toAddress: '2N3fA6wDtnebzywPkGuNK9KkFaEzgbPRRTq',
+        amount: 100000,
+        message: 'some message',
+        proposalSignature: '3045022100e2d9ef7ed592217ab2256fdcf9627075f35ecdf431dde8c9a9c9422b7b1fb00f02202bc8ce066db4401bdbafb2492c3138debbc69c4c01db50d8c22a227e744c8906',
+        changeAddress: {
+          address: '2N3fA6wDtnebzywPkGuNK9KkFaEzgbPRRTq',
+          path: 'm/2147483647/0/8',
+          publicKeys: ['03f6a5fe8db51bfbaf26ece22a3e3bc242891a47d3048fc70bc0e8c03a071ad76f']
+        },
+      };
+      client.signTxProposal(txp, function(err) {
+        err.code.should.equal('SERVERCOMPROMISED');
+        err.message.should.contain('fake transaction proposal');
+        done();
+      });
+    });
+  });
 });

test/integration/server.js

@@ -11,7 +11,7 @@ var memdown = require('memdown');
 var Bitcore = require('bitcore');
 
 var Utils = require('../../lib/utils');
-var SignUtils = require('../../lib/signutils');
+var WalletUtils = require('../../lib/walletutils');
 var Storage = require('../../lib/storage');
 
 var Wallet = require('../../lib/model/wallet');
@@ -176,9 +176,9 @@ helpers.createProposalOpts = function(toAddress, amount, message, signingKey) {
     message: message,
     proposalSignature: null,
   };
-  var msg = opts.toAddress + '|' + opts.amount + '|' + opts.message;
+  var hash = WalletUtils.getProposalHash(opts.toAddress, opts.amount, opts.message);
   try {
-    opts.proposalSignature = SignUtils.sign(msg, signingKey);
+    opts.proposalSignature = WalletUtils.signMessage(hash, signingKey);
   } catch (ex) {}
 
   return opts;
@@ -213,7 +213,7 @@ describe('Copay server', function() {
           .toString();
 
         var message = 'hola';
-        var sig = SignUtils.sign(message, priv);
+        var sig = WalletUtils.signMessage(message, priv);
 
         CopayServer.getInstanceWithAuth({
           copayerId: wallet.copayers[0].id,
@@ -706,7 +706,7 @@ describe('Copay server', function() {
       });
     });
 
-    it('should fail to create tx for address invalid address', function(done) {
+    it('should fail to create tx for invalid address', function(done) {
       helpers.createUtxos(server, wallet, [100, 200], function(utxos) {
         helpers.stubBlockExplorer(server, utxos);
         var txOpts = helpers.createProposalOpts('invalid address', 80, null, TestData.copayers[0].privKey);

test/walletutils.js

@@ -4,7 +4,7 @@ var _ = require('lodash');
 var chai = require('chai');
 var sinon = require('sinon');
 var should = chai.should();
-var SignUtils = require('../lib/signutils');
+var WalletUtils = require('../lib/walletutils');
 
 
 var aText = 'hola';
@@ -14,56 +14,55 @@ var aSignature = '3045022100d6186930e4cd9984e3168e15535e2297988555838ad10126d6c2
 
 var otherPubKey = '02555a2d45e309c00cc8c5090b6ec533c6880ab2d3bc970b3943def989b3373f16';
 
-describe('SignUtils', function() {
+describe('WalletUtils', function() {
 
-  describe('#hash', function() {
+  describe('#hashMessage', function() {
     it('Should create a hash', function() {
-      var res = SignUtils.hash(aText);
+      var res = WalletUtils.hashMessage(aText);
       res.toString('hex').should.equal('4102b8a140ec642feaa1c645345f714bc7132d4fd2f7f6202db8db305a96172f');
     });
   });
 
-  describe('#sign', function() {
+  describe('#signMessage', function() {
-    it('Should sign', function() {
+    it('Should sign a message', function() {
-      var sig = SignUtils.sign(aText, aPrivKey);
+      var sig = WalletUtils.signMessage(aText, aPrivKey);
       should.exist(sig);
       sig.should.equal(aSignature);
     });
     it('Should fail to sign with wrong args', function() {
       (function() {
-        SignUtils.sign(aText, aPubKey);
+        WalletUtils.signMessage(aText, aPubKey);
       }).should.throw('Number');
     });
   });
 
-  describe('#verify', function() {
+  describe('#verifyMessage', function() {
     it('Should fail to verify a malformed signature', function() {
-      var res = SignUtils.verify(aText, 'badsignature', otherPubKey);
+      var res = WalletUtils.verifyMessage(aText, 'badsignature', otherPubKey);
       should.exist(res);
       res.should.equal(false);
     });
     it('Should fail to verify a null signature', function() {
-      var res = SignUtils.verify(aText, null, otherPubKey);
+      var res = WalletUtils.verifyMessage(aText, null, otherPubKey);
       should.exist(res);
       res.should.equal(false);
     });
     it('Should fail to verify with wrong pubkey', function() {
-      var res = SignUtils.verify(aText, aSignature, otherPubKey);
+      var res = WalletUtils.verifyMessage(aText, aSignature, otherPubKey);
       should.exist(res);
       res.should.equal(false);
     });
     it('Should verify', function() {
-      var res = SignUtils.verify(aText, aSignature, aPubKey);
+      var res = WalletUtils.verifyMessage(aText, aSignature, aPubKey);
       should.exist(res);
       res.should.equal(true);
     });
   });
-  describe('#sign #verify round trip', function() {
+  describe('#signMessage #verifyMessage round trip', function() {
     it('Should sign and verify', function() {
       var aLongerText = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.";
-      var sig = SignUtils.sign(aLongerText, aPrivKey);
+      var sig = WalletUtils.signMessage(aLongerText, aPrivKey);
-      SignUtils.verify(aLongerText, sig, aPubKey).should.equal(true);
+      WalletUtils.verifyMessage(aLongerText, sig, aPubKey).should.equal(true);
     });
   });
-
 });