Update everything regarding new flow

Author: unusualbob

.gitignore

@@ -1,2 +1,3 @@
 node_modules
-.idea
+.idea
+examples/config.js

bip70.md

@@ -9,5 +9,63 @@ Since RBF payments can be modified after they are broadcast, they will also be r
 * `required_fee_rate` - The minimum fee per byte required on your transaction. Bitcoin Cash payments will be rejected if fee rate included for the transaction is not at least this value.  _May be fractional value_ ie 0.123 sat/byte
 
 ## Application Logic
-Please note that you should **NOT** broadcast a payment to the P2P network if we respond with an http status code other than `200`. Broadcasting a payment before getting a success notification back from the server will
-lead to a failed payment for the sender. The sender will bear the cost of paying transaction fees yet again to get their money back.
+
+Since rejecting invalid payments before they are broadcast to the network is a primary goal of payment protocol, we recommend following this
+flow when submitting a payment.
+
+1. Fetch payment from url (standard BIP 70)
+2. Create unsigned, funded transaction (standard BIP 70)
+3. Sign transaction, keep unsigned transaction (**bitpay specific**)
+4. Send the unsigned transaction and weighed size of the signed transaction to the server (**bitpay specific**)
+5. If server rejects at this point, do not continue. Otherwise, send via standard payment protocol and broadcast to p2p in parallel
+
+Please note that you should **NOT** broadcast a payment to the P2P network if we respond with an http status code other than `200` in
+the verification step. Broadcasting a payment before getting a success notification back from the server will lead to a failed payment
+for the sender. The sender will bear the cost of paying transaction fees yet again to get their money back.
+
+## Payment Request
+
+### Request
+A GET request should be made to the payment protocol url.
+
+### Response
+The response will payload identical to the BIP70 format with one additional field for `required_fee_rate`.
+
+#### Headers
+On a successful request, the response will contain the standard BIP-70 headers.
+
+
+## Payment Verification
+
+### Request
+A POST request should be made to the payment protocol url with the header `application/bitcoin-verify-payment` or `application/bitcoincash-verify-payment`.
+
+### Request Body
+The body should contain the **unsigned** transaction as well as the weighted size in vbytes of the fully signed transaction. Weighted size
+really only applies for transactions with segwit inputs, if you have a non-segwit transaction the byte size is the correct value to send.
+With bitcoin core this is simply the vsize value of a transaction. If you're not certain about calculating the weighted size, please see
+the [bitcoin documentation about it](https://en.bitcoin.it/wiki/Weight_units). The format of this request should be based on this protobuf
+proto:
+```
+message PaymentVerification {
+    required bytes unsigned_transaction = 1;
+    required uint64 weighted_size = 2 [default = 0];
+}
+```
+
+### Response
+A 200 status code will be returned for valid payments, all other status codes will return with an error message stating why the payment was rejected.
+
+
+## Payment
+
+### Request
+A POST request should be made to the payment protocol url with the standard BIP-70 payment header (`application/bitcoin-payment` or `application/bitcoincash-payment`)
+
+### Request Body
+The body should contain the **signed** transaction in BIP-70 format
+
+### Response
+A 200 status code will be returned for valid payments, all other status codes will return with an error message stating why the payment was rejected.
+
+

examples/bitcoinRpc.js

@@ -6,36 +6,12 @@ const promptly = require('promptly');
 const JsonPaymentProtocol = require('../index'); //or require('json-payment-protocol')
 const paymentProtocol = new JsonPaymentProtocol({strictSSL: false});
 
-let config = {
-  network: 'test',
-  currency: 'BTC',
-  rpcServer: {
-    username: 'fakeUser',
-    password: 'fakePassword',
-    ipAddress: '127.0.0.1',
-    port: '18332'
-  },
-  trustedKeys: {
-    // The idea is that you or the wallet provider will populate this with keys that are trusted, we have provided a few possible approaches
-    // in the specification.md document within the 'key-storing suggestions' section
+let config;
 
-    // Each key here is the pubkey hash so that we can do quick look-ups using the x-identity header sent in the payment request
-    'mh65MN7drqmwpCRZcEeBEE9ceQCQ95HtZc': {
-      // This is displayed to the user, somewhat like the organization field on an SSL certificate
-      owner: 'BitPay (TESTNET ONLY - DO NOT TRUST FOR ACTUAL BITCOIN)',
-      // Which bitcoin networks is this key valid for (regtest, test, main)
-      networks: ['test'],
-      // Which domains this key is valid for
-      domains: ['test.bitpay.com'],
-      // The actual public key which should be used to validate the signatures
-      publicKey: '03159069584176096f1c89763488b94dbc8d5e1fa7bf91f50b42f4befe4e45295a',
-    }
-  }
-};
-
-if (config.rpcServer.username === 'fakeUser') {
-  return console.log('You should update the config in this file to match the actual configuration of your bitcoind' +
-    ' RPC interface');
+try {
+  config = require('./config');
+} catch(e) {
+  return console.log('You need to create a config.js file in examples based on the config.example.js file');
 }
 
 /**
@@ -200,6 +176,7 @@ async.waterfall([
         console.log('Bitcoind did not decode the transaction');
         return cb(new Error('Missing decoded tx'));
       }
+
       // `vsize` for bitcoin core w/ segwit support, `size` for other clients
       let signedTransactionSize = decodedTransaction.vsize || decodedTransaction.size;
       cb(null, fundedRawTransaction, signedRawTransaction,  signedTransactionSize);
@@ -250,7 +227,7 @@ async.waterfall([
   function broadcastPayment(signedRawTransaction, cb) {
     async.parallel([
       function sendToServer(cb) {
-        paymentProtocol.broadcastPayment(config.currency, signedRawTransaction, paymentUrl, function(err) {
+        paymentProtocol.sendSignedPayment(config.currency, signedRawTransaction, paymentUrl, function(err) {
           if (err) {
             console.log('Error sending payment to server', err);
             return cb(err);

examples/config.example.js

@@ -0,0 +1,27 @@
+module.exports = {
+  network: 'test',
+  currency: 'BTC',
+  rpcServer: {
+    // should set this to match your own bitcoin rpc settings
+    username: 'fakeUser',
+    password: 'fakePassword',
+    ipAddress: '127.0.0.1',
+    port: '18332'
+  },
+  trustedKeys: {
+    // The idea is that you or the wallet provider will populate this with keys that are trusted, we have provided a few possible approaches
+    // in the specification.md document within the 'key-storing suggestions' section
+
+    // Each key here is the pubkey hash so that we can do quick look-ups using the x-identity header sent in the payment request
+    'mh65MN7drqmwpCRZcEeBEE9ceQCQ95HtZc': {
+      // This is displayed to the user, somewhat like the organization field on an SSL certificate
+      owner: 'BitPay (TESTNET ONLY - DO NOT TRUST FOR ACTUAL BITCOIN)',
+      // Which bitcoin networks is this key valid for (regtest, test, main)
+      networks: ['test'],
+      // Which domains this key is valid for
+      domains: ['test.bitpay.com'],
+      // The actual public key which should be used to validate the signatures
+      publicKey: '03159069584176096f1c89763488b94dbc8d5e1fa7bf91f50b42f4befe4e45295a',
+    }
+  }
+};

index.js

@@ -196,7 +196,7 @@ PaymentProtocol.prototype.parsePaymentRequestAsync = util.promisify(PaymentProto
  * Sends a given payment to the server for validation
  * @param currency {string} Three letter currency code of proposed transaction (ie BTC, BCH)
  * @param unsignedRawTransaction {string} Hexadecimal format raw unsigned transaction
- * @param weightedSize {number} Weighted size of the transaction
+ * @param weightedSize {number} Weighted size of the transaction in bytes
  * @param url {string} the payment protocol specific url (https)
  * @param callback {function} (err, response)
  */
@@ -221,7 +221,7 @@ PaymentProtocol.prototype.sendPaymentForVerification = function sendPayment(curr
     },
     body: JSON.stringify({
       currency: currency,
-      transactions: [unsignedRawTransaction],
+      unsignedTransaction: unsignedRawTransaction,
       weightedSize: weightedSize
     })
   });
@@ -249,7 +249,7 @@ PaymentProtocol.prototype.sendPaymentForVerification = function sendPayment(curr
  * Sends a given payment to the server for validation
  * @param currency {string} Three letter currency code of proposed transaction (ie BTC, BCH)
  * @param unsignedRawTransaction {string} Hexadecimal format raw unsigned transaction
- * @param weightedSize {number} Weighted size of the transaction
+ * @param weightedSize {number} Weighted size of the transaction in bytes
  * @param url {string} the payment protocol specific url (https)
  */
 PaymentProtocol.prototype.sendPaymentForVerificationAsync = util.promisify(PaymentProtocol.prototype.sendPaymentForVerification);
@@ -261,7 +261,7 @@ PaymentProtocol.prototype.sendPaymentForVerificationAsync = util.promisify(Payme
  * @param url {string} the payment protocol specific url (https)
  * @param callback {function} (err, response)
  */
-PaymentProtocol.prototype.broadcastPayment = function broadcastPayment(currency, signedRawTransaction, url, callback) {
+PaymentProtocol.prototype.sendSignedPayment = function sendSignedPayment(currency, signedRawTransaction, url, callback) {
   let paymentResponse;
 
   //Basic sanity checks
@@ -303,12 +303,12 @@ PaymentProtocol.prototype.broadcastPayment = function broadcastPayment(currency,
 };
 
 /**
- * Sends a given payment to the server for validation
+ * Sends actual payment to server
  * @param currency {string} Three letter currency code of proposed transaction (ie BTC, BCH)
  * @param signedRawTransaction {string} Hexadecimal format raw signed transaction
  * @param url {string} the payment protocol specific url (https)
  */
-PaymentProtocol.prototype.broadcastPaymentAsync = util.promisify(PaymentProtocol.prototype.broadcastPayment);
+PaymentProtocol.prototype.sendSignedPaymentAsync = util.promisify(PaymentProtocol.prototype.sendSignedPayment);
 
 module.exports = PaymentProtocol;
 

paymentFlow.png

@@ -30,16 +30,28 @@ paymentProtocol.getRawPaymentRequest(requestUrl, function (err, response) {
     console.log(paymentRequest);
 
     //TODO: Create the rawTransaction and sign it in your wallet instead of this, do NOT broadcast yet
-    let currency = 'BTC'
-    let signedRawTransaction = '02000000010c2b0d60448d5cdfebe222014407bdb408b8427f837447484911efddea700323000000006a47304402201d3ed3117f1968c3b0a078f15f8462408c745ff555b173eff3dfe0a25e063c0c02200551572ec33d45ece8e64275970bd1b1694621f0ed8fac2f7e18095f170fe3fe012102d4edb773e3bd94e1251790f5cc543cbfa76c2b0abad14898674b1c4e27176ef2ffffffff02c44e0100000000001976a914dd826377dcf2075e5065713453cfad675ba9434f88aca070002a010000001976a914e7d0344ba970301e93cd7b505c7ae1b5bcf5639288ac00000000';
-
-    paymentProtocol.sendPayment(currency, signedRawTransaction, paymentRequest.paymentUrl, function(err, response) {
+    let currency = 'BTC';
+    
+    // Funded unsigned raw transaction
+    let unsignedRawTransaction = '02000000016b7bceefa3ff3bf6f3ad39a99cf6def9126a6edf8f49462bd06e4cb74366dab00100000000feffffff0248590095000000001976a9141b4f4e0c5354ce950ea702cc79be34885e7a60af88ac0c430100000000001976a914072053b485736e002f665d5fc65c443fb379256e88ac00000000'
+    // Signed version of that transaction
+    let signedRawTransaction = '02000000016b7bceefa3ff3bf6f3ad39a99cf6def9126a6edf8f49462bd06e4cb74366dab0010000006b4830450221008d8852576eb8e505832a53569dd756a1d0c304606c27e81d0ac1a83e78250969022058b2bde3f2e1ea7e6a62e69d99f7219e846f04c1c58ff163e2996669a935c31501210206e855c3cfd24a5e154cf94ff7a214d598dfc2d62966011fd83c360cf229777ffeffffff0248590095000000001976a9141b4f4e0c5354ce950ea702cc79be34885e7a60af88ac0c430100000000001976a914072053b485736e002f665d5fc65c443fb379256e88ac00000000';
+    // total size of the signed transaction (note the way shown here is incorrect for segwit, see the code in /examples for getting vsize from RPC)
+    let signedRawTransactionSize = Buffer.from(signedRawTransaction, 'hex').byteLength;
+
+    paymentProtocol.sendPaymentForVerification(currency, unsignedRawTransaction, signedRawTransactionSize, paymentRequest.paymentUrl, function(err, response) {
       if (err) {
-        //DO NOT BROADCAST PAYMENT
-        return console.log('Error sending payment to server');
+        // If server rejects, stop, don't broadcast, show user the error
+        return console.log('Error verifying payment with server', err);
       }
-      console.log('Payment sent successfully');
-      //TODO: Broadcast payment to network here
+
+      // Execute these in parallel
+      // Sending payment to server via payment protocol
+      paymentProtocol.broadcastPayment(currency, signedRawTransaction, paymentRequest.paymentUrl, function(err, response) {
+        console.log('Ignore any errors here if you already received verified above');
+      });
+      // Sending payment to bitcoin p2p network
+      myWallet.broadcastp2p(signedRawTransaction);
     });
   });
 });
@@ -51,29 +63,37 @@ const JsonPaymentProtocol = require('json-payment-protocol');
 const paymentProtocol = new JsonPaymentProtocol();
 
 let requestUrl = 'bitcoin:?r=https://test.bitpay.com/i/Jr629pwsXKdTCneLyZja4t';
-paymentProtocol
-  .getRawPaymentRequestAsync(requestUrl)
-  .then((response) => {
-    return paymentProtocol.parsePaymentRequestAsync(response.rawBody, response.headers);
-  })
-  .then((paymentRequest) => {
-    console.log('Payment request retrieved');
-    console.log(paymentRequest);
-    
-    //TODO: Create the rawTransaction and sign it in your wallet instead of this, do NOT broadcast yet
-    let currency = 'BTC'
-    let signedRawTransaction = '02000000010c2b0d60448d5cdfebe222014407bdb408b8427f837447484911efddea700323000000006a47304402201d3ed3117f1968c3b0a078f15f8462408c745ff555b173eff3dfe0a25e063c0c02200551572ec33d45ece8e64275970bd1b1694621f0ed8fac2f7e18095f170fe3fe012102d4edb773e3bd94e1251790f5cc543cbfa76c2b0abad14898674b1c4e27176ef2ffffffff02c44e0100000000001976a914dd826377dcf2075e5065713453cfad675ba9434f88aca070002a010000001976a914e7d0344ba970301e93cd7b505c7ae1b5bcf5639288ac00000000';
-
-    return paymentProtocol.sendPaymentAsync(currency, signedRawTransaction, paymentRequest.paymentUrl);
-  })
-  .then((response) => {
-    console.log('Payment sent successfully');
-    //TODO: Broadcast payment to network here
-  })
-  .catch((err) => {
-    //DO NOT BROADCAST PAYMENT
-    return console.log('Error processing payment request', err);
-  });
+let response = await paymentProtocol.getRawPaymentRequestAsync(requestUrl);
+let paymentRequest = await paymentProtocol.parsePaymentRequestAsync(response.rawBody, response.headers);
+
+console.log('Payment request retrieved');
+console.log(paymentRequest);
+
+//TODO: Create the rawTransaction and sign it in your wallet instead of this example, do NOT broadcast yet
+// Funded unsigned raw transaction
+let unsignedRawTransaction = '02000000016b7bceefa3ff3bf6f3ad39a99cf6def9126a6edf8f49462bd06e4cb74366dab00100000000feffffff0248590095000000001976a9141b4f4e0c5354ce950ea702cc79be34885e7a60af88ac0c430100000000001976a914072053b485736e002f665d5fc65c443fb379256e88ac00000000'
+// Signed version of that transaction
+let signedRawTransaction = '02000000016b7bceefa3ff3bf6f3ad39a99cf6def9126a6edf8f49462bd06e4cb74366dab0010000006b4830450221008d8852576eb8e505832a53569dd756a1d0c304606c27e81d0ac1a83e78250969022058b2bde3f2e1ea7e6a62e69d99f7219e846f04c1c58ff163e2996669a935c31501210206e855c3cfd24a5e154cf94ff7a214d598dfc2d62966011fd83c360cf229777ffeffffff0248590095000000001976a9141b4f4e0c5354ce950ea702cc79be34885e7a60af88ac0c430100000000001976a914072053b485736e002f665d5fc65c443fb379256e88ac00000000';
+// total size of the signed transaction (note the way shown here is incorrect for segwit, see the code in /examples for getting vsize from RPC)
+let signedRawTransactionSize = Buffer.from(signedRawTransaction, 'hex').byteLength;
+
+// This sends the proposed unsigned transaction to the server
+try {
+  await paymentProtocol.sendPaymentForVerificationAsync(currency, unsignedRawTransaction, signedRawTransactionSize, paymentRequest.paymentUrl);
+} catch (e) {
+  // Payment was rejected, do not continue, tell the user why they were rejected
+  return console.log('Proposed payment rejected', e);
+}
+
+// This sends the fully signed transaction
+try {
+  await paymentProtocol.sendSignedPaymentAsync(currency, signedRawTransaction, paymentRequest.paymentUrl);
+} catch (e) {
+  // ignore errors here 
+}
+
+// Broadcast from your wallet to p2p
+myWallet.broadcastp2p(signedRawTransaction);
 ```
 
 ### Options

readme.md

@@ -0,0 +1,49 @@
+# Security Updates
+
+### 2019-02-01
+Recently a bitcoin community member discussed a potential bug in our modified BIP-70 and JSON payment protocol flows when implemented by a
+malicious server. This bug would not affect BitPay itself, but has the potential to affect wallets using our payment process
+recommendations. This was due to an implicit trust by the client that the server is not malicious. 
+
+A proposed malicious flow is as follows:
+
+1. In person Eve asks Adam if she can buy his bitcoin, hands him cash provides a payment protocol url
+2. Adams wallet interacts with the url, and sends signed transaction to the server for verification
+3. Server rejects signed transaction, but secretly stores it
+4. Wallet notifies Adam that the transaction was rejected
+5. Eve asks for her money back, Adam complies since transaction was rejected
+6. Later Eve has the server broadcast the signed transaction, Eve now has both the cash and the crypto
+
+To resolve this we're advising a change to the payment protocol flow to protect users.
+
+#### Existing flow:
+
+1. Wallet requests payment data
+2. Wallet creates unsigned transaction
+2. Wallet signs transaction
+3. Wallet sends **SIGNED** transaction to server for verification
+4. Server verifies or rejects payment and notifies wallet
+5. Wallet broadcasts if server accepts, stops if rejected 
+
+#### Updated flow:
+
+1. Wallet requests payment data
+2. Wallet creates unsigned transaction
+3. Wallet signs transaction
+4. Wallet sends **UNSIGNED** transaction and weighted size of signed transaction to server for verification
+5. Server verifies or rejects payment and notifies wallet
+6. Wallet sends **SIGNED** transaction to server and broadcasts to p2p at the same time
+
+This new flow prevents a malicious server from being able to broadcast without the user's knowledge as it now only ever has access to the
+unsigned transaction before accepting or rejecting. This is also why we recommend broadcasting to the p2p network at the same time as the
+sending the payment via payment protocol to the server. Since you've already gotten approval you should be free to broadcast and ignore
+any rejections by the server.
+
+While JSON payment protocol is vulnerable, trying to use a malicious server is less viable than the modified BIP-70 flow due to the need
+for each wallet to whitelist ECC keys. Since BIP-70 only requires a valid x509 certificate, anyone with a domain could run a malicious
+server. If your wallet uses the modified BIP-70 flow you should update as soon as possible.
+
+Our servers will remain backwards compatible with the old flow for the interim, however to protect users we highly recommend wallets update
+to use the more secure flow.
+
+More details of *what* exactly needs to be sent *where* can be found in the updated specification document.