Repository: stripe/stripe-node
Pull request #1237 (2021-09-10 20:45:48)

Add a CryptoProvider interface and NodeCryptoProvider implementation.
### Notify

r? @richardm-stripe

### Summary

Adds an `CryptoProvider` interface and moves out the Node `crypto` logic into a `NodeCryptoProvider` implementation. 

This initially only supports computing HMAC signatures, but lets us support this in a pluggable way. The Webhooks functions are updated to support passing in a custom provider. This can be used in environments where no 'crypto' package is available (removing another Node dependency for https://github.com/stripe/stripe-node/issues/997).

### Test plan

Added a small common set of test cases which can be re-used by any implementation to ensure they meet the contract.

### Looking forwards

Some crypto APIs (eg. the [Web Crypto API](https://developer.mozilla.org/en-US/docs/Web/API/Web_Crypto_API)) only provide async functions for computing HMACs (and other primitives). In the future, we may want to extend this interface with an `asyncComputeHMACSignature` function that returns a promise alongside `computeHMACSignature`. We would need to update the Webhooks code to support both sync and async computations (eg. providing `asyncConstructEvent` and `asyncVerifyHeader`).


Patch:
diff --git a/lib/Webhooks.js b/lib/Webhooks.js
index ca362e59..9a2e7b1d 100644
--- a/lib/Webhooks.js
+++ b/lib/Webhooks.js
@@ -1,19 +1,18 @@
 'use strict';
 
-const crypto = require('crypto');
-
 const utils = require('./utils');
 const {StripeError, StripeSignatureVerificationError} = require('./Error');
 
 const Webhook = {
   DEFAULT_TOLERANCE: 300, // 5 minutes
 
-  constructEvent(payload, header, secret, tolerance) {
+  constructEvent(payload, header, secret, tolerance, cryptoProvider) {
     this.signature.verifyHeader(
       payload,
       header,
       secret,
-      tolerance || Webhook.DEFAULT_TOLERANCE
+      tolerance || Webhook.DEFAULT_TOLERANCE,
+      cryptoProvider
     );
 
     const jsonPayload = JSON.parse(payload);
@@ -29,6 +28,7 @@ const Webhook = {
    * @property {string} secret - Stripe webhook secret 'whsec_...'
    * @property {string} scheme - Version of API to hit. Defaults to 'v1'.
    * @property {string} signature - Computed webhook signature
+   * @property {CryptoProvider} cryptoProvider - Crypto provider to use for computing the signature if none was provided. Defaults to NodeCryptoProvider.
    */
   generateTestHeaderString: function(opts) {
     if (!opts) {
@@ -41,9 +41,11 @@ const Webhook = {
       Math.floor(opts.timestamp) || Math.floor(Date.now() / 1000);
     opts.scheme = opts.scheme || signature.EXPECTED_SCHEME;
 
+    opts.cryptoProvider = opts.cryptoProvider || getNodeCryptoProvider();
+
     opts.signature =
       opts.signature ||
-      signature._computeSignature(
+      opts.cryptoProvider.computeHMACSignature(
         opts.timestamp + '.' + opts.payload,
         opts.secret
       );
@@ -60,14 +62,7 @@ const Webhook = {
 const signature = {
   EXPECTED_SCHEME: 'v1',
 
-  _computeSignature: (payload, secret) => {
-    return crypto
-      .createHmac('sha256', secret)
-      .update(payload, 'utf8')
-      .digest('hex');
-  },
-
-  verifyHeader(payload, header, secret, tolerance) {
+  verifyHeader(payload, header, secret, tolerance, cryptoProvider) {
     payload = Buffer.isBuffer(payload) ? payload.toString('utf8') : payload;
 
     // Express's type for `Request#headers` is `string | []string`
@@ -104,7 +99,8 @@ const signature = {
       });
     }
 
-    const expectedSignature = this._computeSignature(
+    cryptoProvider = cryptoProvider || getNodeCryptoProvider();
+    const expectedSignature = cryptoProvider.computeHMACSignature(
       `${details.timestamp}.${payload}`,
       secret
     );
@@ -168,6 +164,20 @@ function parseHeader(header, scheme) {
   );
 }
 
+let webhooksNodeCryptoProviderInstance = null;
+
+/**
+ * Lazily instantiate a NodeCryptoProvider instance. This is a stateless object
+ * so a singleton can be used here.
+ */
+function getNodeCryptoProvider() {
+  if (!webhooksNodeCryptoProviderInstance) {
+    const NodeCryptoProvider = require('./crypto/NodeCryptoProvider');
+    webhooksNodeCryptoProviderInstance = new NodeCryptoProvider();
+  }
+  return webhooksNodeCryptoProviderInstance;
+}
+
 Webhook.signature = signature;
 
 module.exports = Webhook;
diff --git a/lib/crypto/CryptoProvider.js b/lib/crypto/CryptoProvider.js
new file mode 100644
index 00000000..8c9015a4
--- /dev/null
+++ b/lib/crypto/CryptoProvider.js
@@ -0,0 +1,21 @@
+'use strict';
+
+/**
+ * Interface encapsulating the various crypto computations used by the library,
+ * allowing pluggable underlying crypto implementations.
+ */
+class CryptoProvider {
+  /**
+   * Computes a SHA-256 HMAC given a secret and a payload (encoded in UTF-8).
+   * The output HMAC should be encoded in hexadecimal.
+   *
+   * Sample values for implementations:
+   * - computeHMACSignature('', 'test_secret') => 'f7f9bd47fb987337b5796fdc1fdb9ba221d0d5396814bfcaf9521f43fd8927fd'
+   * - computeHMACSignature('\ud83d\ude00', 'test_secret') => '837da296d05c4fe31f61d5d7ead035099d9585a5bcde87de952012a78f0b0c43
+   */
+  computeHMACSignature(payload, secret) {
+    throw new Error('computeHMACSignature not implemented.');
+  }
+}
+
+module.exports = CryptoProvider;
diff --git a/lib/crypto/NodeCryptoProvider.js b/lib/crypto/NodeCryptoProvider.js
new file mode 100644
index 00000000..dfbee490
--- /dev/null
+++ b/lib/crypto/NodeCryptoProvider.js
@@ -0,0 +1,20 @@
+'use strict';
+
+const crypto = require('crypto');
+
+const CryptoProvider = require('./CryptoProvider');
+
+/**
+ * `CryptoProvider which uses the Node `crypto` package for its computations.
+ */
+class NodeCryptoProvider extends CryptoProvider {
+  /** @override */
+  computeHMACSignature(payload, secret) {
+    return crypto
+      .createHmac('sha256', secret)
+      .update(payload, 'utf8')
+      .digest('hex');
+  }
+}
+
+module.exports = NodeCryptoProvider;
diff --git a/lib/stripe.js b/lib/stripe.js
index 19ae1b07..9cefc1e7 100644
--- a/lib/stripe.js
+++ b/lib/stripe.js
@@ -54,6 +54,9 @@ const {HttpClient, HttpClientResponse} = require('./net/HttpClient');
 Stripe.HttpClient = HttpClient;
 Stripe.HttpClientResponse = HttpClientResponse;
 
+const CryptoProvider = require('./crypto/CryptoProvider');
+Stripe.CryptoProvider = CryptoProvider;
+
 function Stripe(key, config = {}) {
   if (!(this instanceof Stripe)) {
     return new Stripe(key, config);
diff --git a/types/2020-08-27/index.d.ts b/types/2020-08-27/index.d.ts
index 94dd9093..dd3f1ebc 100644
--- a/types/2020-08-27/index.d.ts
+++ b/types/2020-08-27/index.d.ts
@@ -1,6 +1,7 @@
 // File generated from our OpenAPI spec
 
 ///<reference path='../lib.d.ts' />
+///<reference path='../crypto/crypto.d.ts' />
 ///<reference path='../net/net.d.ts' />
 ///<reference path='../shared.d.ts' />
 ///<reference path='../Errors.d.ts' />
diff --git a/types/Webhooks.d.ts b/types/Webhooks.d.ts
index ab713531..1ef2a303 100644
--- a/types/Webhooks.d.ts
+++ b/types/Webhooks.d.ts
@@ -35,7 +35,12 @@ declare module 'stripe' {
         /**
          * Seconds of tolerance on timestamps.
          */
-        tolerance?: number
+        tolerance?: number,
+
+        /**
+         * Optional CryptoProvider to use for computing HMAC signatures.
+         */
+        cryptoProvider?: CryptoProvider
       ): Stripe.Event;
 
       /**
@@ -66,6 +71,12 @@ declare module 'stripe' {
          * Computed webhook signature.
          */
         signature?: string;
+
+        /**
+         * Optional CryptoProvider to use for computing HMAC signatures, if no
+         * signature is given.
+         */
+        cryptoProvider?: CryptoProvider;
       }): string;
 
       signature: Signature;
@@ -79,7 +90,8 @@ declare module 'stripe' {
         payload: string,
         header: string,
         secret: string,
-        tolerance?: number
+        tolerance?: number,
+        cryptoProvider?: CryptoProvider
       ): void;
       parseHeader(
         header: string,
diff --git a/types/crypto/crypto.d.ts b/types/crypto/crypto.d.ts
new file mode 100644
index 00000000..9a988fcb
--- /dev/null
+++ b/types/crypto/crypto.d.ts
@@ -0,0 +1,19 @@
+declare module 'stripe' {
+  namespace Stripe {
+    /**
+     * Interface encapsulating the various crypto computations used by the library,
+     * allowing pluggable underlying crypto implementations.
+     */
+    export interface CryptoProvider {
+      /**
+       * Computes a SHA-256 HMAC given a secret and a payload (encoded in UTF-8).
+       * The output HMAC should be encoded in hexadecimal.
+       *
+       * Sample values for implementations:
+       * - computeHMACSignature('', 'test_secret') => 'f7f9bd47fb987337b5796fdc1fdb9ba221d0d5396814bfcaf9521f43fd8927fd'
+       * - computeHMACSignature('\ud83d\ude00', 'test_secret') => '837da296d05c4fe31f61d5d7ead035099d9585a5bcde87de952012a78f0b0c43
+       */
+      computeHMACSignature: (payload: string, secret: string) => string;
+    }
+  }
+}


Test patch:
diff --git a/test/Webhook.spec.js b/test/Webhook.spec.js
index 5d4dcd50..0668171a 100644
--- a/test/Webhook.spec.js
+++ b/test/Webhook.spec.js
@@ -1,6 +1,7 @@
 'use strict';
 
-const stripe = require('../testUtils').getSpyableStripe();
+const {getSpyableStripe, FakeCryptoProvider} = require('../testUtils');
+const stripe = getSpyableStripe();
 const expect = require('chai').expect;
 
 const EVENT_PAYLOAD = {
@@ -86,6 +87,24 @@ describe('Webhooks', () => {
         'Unexpected: An array was passed as a header, which should not be possible for the stripe-signature header.'
       );
     });
+
+    it('should invoke a custom CryptoProvider', () => {
+      const header = stripe.webhooks.generateTestHeaderString({
+        payload: EVENT_PAYLOAD_STRING,
+        secret: SECRET,
+        signature: 'fake signature',
+      });
+
+      const event = stripe.webhooks.constructEvent(
+        EVENT_PAYLOAD_STRING,
+        header,
+        SECRET,
+        undefined,
+        new FakeCryptoProvider()
+      );
+
+      expect(event.id).to.equal(EVENT_PAYLOAD.id);
+    });
   });
 
   describe('.verifySignatureHeader', () => {
@@ -254,5 +273,48 @@ describe('Webhooks', () => {
         )
       ).to.equal(true);
     });
+
+    describe('custom CryptoProvider', () => {
+      const cryptoProvider = new FakeCryptoProvider();
+
+      it('should use the provider to compute a signature (mismatch)', () => {
+        const header = stripe.webhooks.generateTestHeaderString({
+          payload: EVENT_PAYLOAD_STRING,
+          secret: SECRET,
+          signature: 'different fake signature',
+          timestamp: 123,
+        });
+
+        expect(() => {
+          stripe.webhooks.signature.verifyHeader(
+            EVENT_PAYLOAD_STRING,
+            header,
+            SECRET,
+            undefined,
+            cryptoProvider
+          );
+        }).to.throw(
+          /No signatures found matching the expected signature for payload/
+        );
+      });
+      it('should use the provider to compute a signature (success)', () => {
+        const header = stripe.webhooks.generateTestHeaderString({
+          payload: EVENT_PAYLOAD_STRING,
+          secret: SECRET,
+          signature: 'fake signature',
+          timestamp: 123,
+        });
+
+        expect(
+          stripe.webhooks.signature.verifyHeader(
+            EVENT_PAYLOAD_STRING,
+            header,
+            SECRET,
+            undefined,
+            cryptoProvider
+          )
+        ).to.equal(true);
+      });
+    });
   });
 });
diff --git a/test/crypto/NodeCryptoProvider.spec.js b/test/crypto/NodeCryptoProvider.spec.js
new file mode 100644
index 00000000..5b3e1f49
--- /dev/null
+++ b/test/crypto/NodeCryptoProvider.spec.js
@@ -0,0 +1,9 @@
+'use strict';
+
+const NodeCryptoProvider = require('../../lib/crypto/NodeCryptoProvider');
+
+const {createCryptoProviderTestSuite} = require('./helpers');
+
+describe('NodeCryptoProvider', () => {
+  createCryptoProviderTestSuite(new NodeCryptoProvider());
+});
diff --git a/test/crypto/helpers.js b/test/crypto/helpers.js
new file mode 100644
index 00000000..22529720
--- /dev/null
+++ b/test/crypto/helpers.js
@@ -0,0 +1,42 @@
+'use strict';
+
+const expect = require('chai').expect;
+
+const SECRET = 'test_secret';
+
+/**
+ * Test runner which runs a common set of tests for a given CryptoProvider to
+ * make sure it satisfies the expected contract.
+ */
+const createCryptoProviderTestSuite = (cryptoProvider) => {
+  describe('CryptoProviderTestSuite', () => {
+    describe('computeHMACSignature', () => {
+      it('empty payload', () => {
+        expect(cryptoProvider.computeHMACSignature('', SECRET)).to.equal(
+          'f7f9bd47fb987337b5796fdc1fdb9ba221d0d5396814bfcaf9521f43fd8927fd'
+        );
+      });
+
+      it('sample payload', () => {
+        expect(
+          cryptoProvider.computeHMACSignature(
+            JSON.stringify({obj1: 'hello', obj2: 'world'}),
+            SECRET
+          )
+        ).to.equal(
+          'bebb1a643997f419b315ddba19e6f5411e1ce7f810ba6d3617ce72823092f363'
+        );
+      });
+
+      it('payload with utf-8', () => {
+        expect(
+          cryptoProvider.computeHMACSignature('\ud83d\ude00', SECRET)
+        ).to.equal(
+          '837da296d05c4fe31f61d5d7ead035099d9585a5bcde87de952012a78f0b0c43'
+        );
+      });
+    });
+  });
+};
+
+module.exports = {createCryptoProviderTestSuite};
diff --git a/test/net/FetchHttpClient.spec.js b/test/net/FetchHttpClient.spec.js
index d5f60620..8c726395 100644
--- a/test/net/FetchHttpClient.spec.js
+++ b/test/net/FetchHttpClient.spec.js
@@ -11,10 +11,8 @@ const createFetchHttpClient = () => {
 
 const {createHttpClientTestSuite, ArrayReadable} = require('./helpers');
 
-createHttpClientTestSuite(
-  'FetchHttpClient',
-  createFetchHttpClient,
-  (setupNock, sendRequest) => {
+describe('FetchHttpClient', () => {
+  createHttpClientTestSuite(createFetchHttpClient, (setupNock, sendRequest) => {
     describe('raw stream', () => {
       it('getRawResponse()', async () => {
         setupNock().reply(200);
@@ -57,5 +55,5 @@ createHttpClientTestSuite(
         });
       });
     });
-  }
-);
+  });
+});
diff --git a/test/net/NodeHttpClient.spec.js b/test/net/NodeHttpClient.spec.js
index 1f5a574c..620fab49 100644
--- a/test/net/NodeHttpClient.spec.js
+++ b/test/net/NodeHttpClient.spec.js
@@ -7,10 +7,8 @@ const {createNodeHttpClient} = require('../../lib/Stripe');
 
 const {createHttpClientTestSuite, ArrayReadable} = require('./helpers');
 
-createHttpClientTestSuite(
-  'NodeHttpClient',
-  createNodeHttpClient,
-  (setupNock, sendRequest) => {
+describe('NodeHttpClient', () => {
+  createHttpClientTestSuite(createNodeHttpClient, (setupNock, sendRequest) => {
     describe('raw stream', () => {
       it('getRawResponse()', async () => {
         setupNock().reply(200);
@@ -60,5 +58,5 @@ createHttpClientTestSuite(
         });
       });
     });
-  }
-);
+  });
+});
diff --git a/test/net/helpers.js b/test/net/helpers.js
index 139081aa..b49fefb2 100644
--- a/test/net/helpers.js
+++ b/test/net/helpers.js
@@ -34,18 +34,13 @@ class ArrayReadable extends Readable {
  * Test runner which runs a common set of tests for a given HTTP client to make
  * sure the client meets the interface expectations.
  *
- * This takes in a client name (for the test description) and function to create
- * a client.
+ * This takes in a function to create the client.
  *
  * This can be configured to run extra tests, providing the nock setup function
  * and request function for those tests.
  */
-const createHttpClientTestSuite = (
-  httpClientName,
-  createHttpClientFn,
-  extraTestsFn
-) => {
-  describe(`${httpClientName}`, () => {
+const createHttpClientTestSuite = (createHttpClientFn, extraTestsFn) => {
+  describe('HttpClientTestSuite', () => {
     const setupNock = () => {
       return nock('http://stripe.com').get('/test');
     };
diff --git a/testUtils/index.js b/testUtils/index.js
index 145fc2f4..2aa03459 100644
--- a/testUtils/index.js
+++ b/testUtils/index.js
@@ -8,6 +8,7 @@ require('chai').use(require('chai-as-promised'));
 
 const http = require('http');
 
+const CryptoProvider = require('../lib/crypto/CryptoProvider');
 const ResourceNamespace = require('../lib/ResourceNamespace').ResourceNamespace;
 
 const testingHttpAgent = new http.Agent({keepAlive: false});
@@ -210,4 +211,10 @@ const utils = (module.exports = {
       return false;
     }
   },
+
+  FakeCryptoProvider: class extends CryptoProvider {
+    computeHMACSignature(payload, secret) {
+      return 'fake signature';
+    }
+  },
 });