diff --git a/README.md b/README.md index 08c942c47..7cbfbcda6 100644 --- a/README.md +++ b/README.md @@ -809,23 +809,16 @@ request({url: 'https://www.google.com', verbose: true}, function (error, respons ### Extending root CAs -When this feature is enabled, the root CAs can be extended using the `extraCA` option. The file should consist of one or more trusted certificates in PEM format. +The root CAs can be extended using the `extraCA` option. The file should consist of one or more trusted certificates in PEM format. This is similar to [NODE_EXTRA_CA_CERTS](https://nodejs.org/api/cli.html#cli_node_extra_ca_certs_file). But, if `options.ca` is specified, those will be extended as well. ```js -// enable extending CAs -request.enableNodeExtraCACerts(); - // request with extra CA certs request.get({ url: 'https://api.some-server.com/', extraCA: fs.readFileSync('Extra CA Certificates .pem') }); - -// disable this feature -request.disableNodeExtraCACerts() - ``` [back to top](#table-of-contents) diff --git a/index.js b/index.js index b2846d0ff..738432b26 100755 --- a/index.js +++ b/index.js @@ -14,7 +14,6 @@ 'use strict' -var tls = require('tls') var extend = require('extend') var cookies = require('./lib/cookies') @@ -131,92 +130,6 @@ request.forever = function (agentOptions, optionsArg) { return request.defaults(options) } -// As of now (Node v10.x LTS), the only way to extend the well known "root" CA -// is by using an environment variable called `NODE_EXTRA_CA_CERTS`. -// This function enables the same functionality and provides a programmatic way -// to extend the CA certificates. -// Refer: https://nodejs.org/docs/latest-v10.x/api/cli.html#cli_node_extra_ca_certs_file -// -// @note Unlike NODE_EXTRA_CA_CERTS, this method extends the CA for every -// request sent and since its an expensive operation its advised to use a -// keepAlive agent(agentOptions.keepAlive: true) when this is enabled. -// -// Benchmarks using a local server: -// NODE_EXTRA_CA_CERTS (keepAlive: false) : 422 ops/sec ±1.73% (77 runs sampled) -// NODE_EXTRA_CA_CERTS (keepAlive: true) : 2,096 ops/sec ±4.23% (69 runs sampled) -// -// enableNodeExtraCACerts (keepAlive: false) : 331 ops/sec ±5.64% (77 runs sampled) -// enableNodeExtraCACerts (keepAlive: true) : 2,045 ops/sec ±5.20% (69 runs sampled) -// -// @note Enabling this will override the singleton `tls.createSecureContext` method -// which will be affected for every request sent (using native HTTPS etc.) on the -// same process. BUT, this will only be effective when `extraCA` option is -// passed to `tls.createSecureContext`, which is limited to this library. -request.enableNodeExtraCACerts = function (callback) { - // @note callback is optional to catch missing tls method - !callback && (callback = function () {}) - - // bail out if already enabled - if (tls.__createSecureContext) { - return callback() - } - - // enable only if `SecureContext.addCACert` is present - // otherwise return callback with error. - // @note try-catch is used to make sure testing this will not break - // the main process due to OpenSSL error. - try { - var testContext = tls.createSecureContext() - - if (!(testContext && testContext.context && - typeof testContext.context.addCACert === 'function')) { - return callback(new Error('SecureContext.addCACert is not a function')) - } - } catch (err) { - return callback(err) - } - - // store the original tls.createSecureContext method. - // used to extend existing functionality as well as restore later. - tls.__createSecureContext = tls.createSecureContext - - // override tls.createSecureContext with extraCA support - // @note if agent is keepAlive, same context will be reused. - tls.createSecureContext = function () { - // call original createSecureContext and store the context - var secureContext = tls.__createSecureContext.apply(this, arguments) - - // if `extraCA` is present in options, extend CA certs - // @note this request option is available here because all the - // Request properties are passed to HTTPS Agent. - if (arguments[0] && arguments[0].extraCA) { - // extend root CA with specified CA certificates - // @note `addCACert` is an undocumented API and performs an expensive operations - // Refer: https://github.com/nodejs/node/blob/v10.15.1/lib/_tls_common.js#L97 - secureContext.context.addCACert(arguments[0].extraCA) - } - - return secureContext - } - - // enabled extra CA support - return callback() -} - -// disable the extended CA certificates feature -request.disableNodeExtraCACerts = function () { - // bail out if not enabled - if (typeof tls.__createSecureContext !== 'function') { - return - } - - // reset `tls.createSecureContext` with the original method - tls.createSecureContext = tls.__createSecureContext - - // delete the reference of original method - delete tls.__createSecureContext -} - // Exports module.exports = request diff --git a/request.js b/request.js index 85c8d384b..b93cfbd38 100644 --- a/request.js +++ b/request.js @@ -639,7 +639,12 @@ Request.prototype.init = function (options) { if (self.pool === false) { self.agent = false } else { - self.agent = self.agent || self.getNewAgent() + try { + self.agent = self.agent || self.getNewAgent() + } catch (error) { + // tls.createSecureContext() throws on bad options + return self.emit('error', error) + } } self.on('pipe', function (src) { @@ -824,11 +829,16 @@ Request.prototype.getNewAgent = function () { } // only add when NodeExtraCACerts is enabled - if (tls.__createSecureContext && options.extraCA) { + if (options.extraCA) { if (poolKey) { poolKey += ':' } poolKey += options.extraCA + + // Create a new secure context to add the extra CA + var secureContext = tls.createSecureContext(options) + secureContext.context.addCACert(options.extraCA) + options.secureContext = secureContext } if (typeof options.rejectUnauthorized !== 'undefined') { diff --git a/tests/test-api.js b/tests/test-api.js index 0ee8c27dc..541c9460b 100644 --- a/tests/test-api.js +++ b/tests/test-api.js @@ -1,6 +1,5 @@ 'use strict' -var tls = require('tls') var http = require('http') var tape = require('tape') @@ -8,7 +7,6 @@ var tape = require('tape') var request = require('../index') var server -var origCreateSecureContext = tls.createSecureContext // fixture tape('setup', function (t) { server = http.createServer() @@ -33,111 +31,6 @@ tape('callback option', function (t) { }) }) -tape('enableNodeExtraCACerts', function (t) { - request.enableNodeExtraCACerts(function (err) { - t.error(err) - t.equal(typeof tls.createSecureContext, 'function') - t.equal(typeof tls.__createSecureContext, 'function') - t.equal(tls.__createSecureContext, origCreateSecureContext) // backup - request.disableNodeExtraCACerts() // RESET - t.end() - }) -}) - -tape('enableNodeExtraCACerts: without callback', function (t) { - request.enableNodeExtraCACerts() - - t.equal(typeof tls.createSecureContext, 'function') - t.equal(typeof tls.__createSecureContext, 'function') - t.equal(tls.__createSecureContext, origCreateSecureContext) // backup - request.disableNodeExtraCACerts() // RESET - t.end() -}) - -tape('enableNodeExtraCACerts: with missing addCACert', function (t) { - // override createSecureContext - tls.createSecureContext = function () { - return { - context: { - addCACert: undefined - } - } - } - - request.enableNodeExtraCACerts(function (err) { - t.ok(err) - t.equal(err.message, 'SecureContext.addCACert is not a function') - t.equal(typeof tls.__createSecureContext, 'undefined') - tls.createSecureContext = origCreateSecureContext // RESET - t.end() - }) -}) - -tape('enableNodeExtraCACerts: on createSecureContext error', function (t) { - // override createSecureContext - tls.createSecureContext = function () { - throw new Error('something went wrong') - } - - request.enableNodeExtraCACerts(function (err) { - t.ok(err) - t.equal(err.message, 'something went wrong') - t.equal(typeof tls.__createSecureContext, 'undefined') - tls.createSecureContext = origCreateSecureContext // RESET - t.end() - }) -}) - -tape('enableNodeExtraCACerts: called twice', function (t) { - request.enableNodeExtraCACerts(function (err) { - t.error(err) - t.equal(typeof tls.createSecureContext, 'function') - t.equal(typeof tls.__createSecureContext, 'function') - t.equal(tls.__createSecureContext, origCreateSecureContext) - - // called twice - request.enableNodeExtraCACerts(function (err) { - t.error(err) - t.equal(typeof tls.createSecureContext, 'function') - t.equal(typeof tls.__createSecureContext, 'function') - t.equal(tls.__createSecureContext, origCreateSecureContext) - request.disableNodeExtraCACerts() // RESET - t.end() - }) - }) -}) - -tape('disableNodeExtraCACerts', function (t) { - // enable first - request.enableNodeExtraCACerts(function (err) { - t.error(err) - - // disable - request.disableNodeExtraCACerts() - - t.equal(typeof tls.createSecureContext, 'function') - t.equal(typeof tls.__createSecureContext, 'undefined') - t.equal(tls.createSecureContext, origCreateSecureContext) // restored - t.end() - }) -}) - -tape('disableNodeExtraCACerts: called twice', function (t) { - // enable first - request.enableNodeExtraCACerts(function (err) { - t.error(err) - - // disable - request.disableNodeExtraCACerts() - request.disableNodeExtraCACerts() - - t.equal(typeof tls.createSecureContext, 'function') - t.equal(typeof tls.__createSecureContext, 'undefined') - t.equal(tls.createSecureContext, origCreateSecureContext) // restored - t.end() - }) -}) - tape('cleanup', function (t) { server.close(t.end) }) diff --git a/tests/test-ssl.js b/tests/test-ssl.js index dd0e3f4d8..1182d55a1 100644 --- a/tests/test-ssl.js +++ b/tests/test-ssl.js @@ -106,10 +106,7 @@ tape('pfx + passphrase(invalid)', function (t) { }) }) -tape('extraCA(NodeExtraCACerts: enabled)', function (t) { - // enable extraCA support - request.enableNodeExtraCACerts() - +tape('extraCA', function (t) { request({ url: sslServer.url, extraCA: ca, @@ -118,27 +115,11 @@ tape('extraCA(NodeExtraCACerts: enabled)', function (t) { }, function (err, res, body) { t.equal(err, null) t.equal(body.toString(), 'authorized') - request.disableNodeExtraCACerts() // RESET t.end() }) }) -tape('extraCA(NodeExtraCACerts: disabled)', function (t) { - request({ - url: sslServer.url, - extraCA: ca, - key: clientKey, - cert: clientCert - }, function (err, res, body) { - t.ok(err) - t.end() - }) -}) - -tape('ca + extraCA(NodeExtraCACerts: enabled)', function (t) { - // enable extraCA support - request.enableNodeExtraCACerts() - +tape('ca + extraCA', function (t) { request({ url: sslServer.url, ca: ca, @@ -148,7 +129,6 @@ tape('ca + extraCA(NodeExtraCACerts: enabled)', function (t) { }, function (err, res, body) { t.equal(err, null) t.equal(body.toString(), 'authorized') - request.disableNodeExtraCACerts() // RESET t.end() }) }) diff --git a/tests/test-tunnel.js b/tests/test-tunnel.js index d1c402832..e34b06c6d 100644 --- a/tests/test-tunnel.js +++ b/tests/test-tunnel.js @@ -494,7 +494,6 @@ tape('setup', function (t) { ss.listen(0, function () { ss2.listen(0, 'localhost', function () { ss3.listen(0, 'localhost', function () { - request.enableNodeExtraCACerts() addTests() tape('cleanup', function (t) { s.destroy(function () { @@ -502,7 +501,6 @@ tape('setup', function (t) { ss2.destroy(function () { ss3.destroy(function () { t.end() - request.disableNodeExtraCACerts() }) }) })