From 7c682f2fd085c9844ff90c07a3566ba0483c10a9 Mon Sep 17 00:00:00 2001 From: Jeff Principe Date: Mon, 16 Oct 2017 21:23:29 -0700 Subject: [PATCH] https: add extra options to Agent#getName() Adds the remaining options from tls.createSecureContext() to the string generated by Agent#getName(). This allows https.request() to accept the options and generate unique sockets appropriately. PR-URL: https://github.com/nodejs/node/pull/16402 Reviewed-By: Luigi Pinca Reviewed-By: Ruben Bridgewater Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- doc/api/https.md | 11 +-- lib/https.js | 24 +++++ .../test-https-agent-additional-options.js | 87 +++++++++++++++++++ test/parallel/test-https-agent-getname.js | 12 ++- .../test-https-agent-secure-protocol.js | 57 ------------ 5 files changed, 127 insertions(+), 64 deletions(-) create mode 100644 test/parallel/test-https-agent-additional-options.js delete mode 100644 test/parallel/test-https-agent-secure-protocol.js diff --git a/doc/api/https.md b/doc/api/https.md index da536d7ac84cad..4ac82220c85cef 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -12,7 +12,7 @@ separate module. added: v0.4.5 --> -An Agent object for HTTPS similar to [`http.Agent`][]. See [`https.request()`][] +An [`Agent`][] object for HTTPS similar to [`http.Agent`][]. See [`https.request()`][] for more information. ## Class: https.Server @@ -168,9 +168,10 @@ changes: Makes a request to a secure web server. -The following additional `options` from [`tls.connect()`][] are also accepted -when using a custom [`Agent`][]: `ca`, `cert`, `ciphers`, `clientCertEngine`, -`key`, `passphrase`, `pfx`, `rejectUnauthorized`, `secureProtocol`, `servername` +The following additional `options` from [`tls.connect()`][] are also accepted: +`ca`, `cert`, `ciphers`, `clientCertEngine`, `crl`, `dhparam`, `ecdhCurve`, +`honorCipherOrder`, `key`, `passphrase`, `pfx`, `rejectUnauthorized`, +`secureOptions`, `secureProtocol`, `servername`, `sessionIdContext` `options` can be an object, a string, or a [`URL`][] object. If `options` is a string, it is automatically parsed with [`url.parse()`][]. If it is a [`URL`][] @@ -220,7 +221,7 @@ const req = https.request(options, (res) => { }); ``` -Alternatively, opt out of connection pooling by not using an `Agent`. +Alternatively, opt out of connection pooling by not using an [`Agent`][]. Example: diff --git a/lib/https.js b/lib/https.js index 741ce84d2f8820..84ddeb5036a900 100644 --- a/lib/https.js +++ b/lib/https.js @@ -194,6 +194,30 @@ Agent.prototype.getName = function getName(options) { if (options.secureProtocol) name += options.secureProtocol; + name += ':'; + if (options.crl) + name += options.crl; + + name += ':'; + if (options.honorCipherOrder !== undefined) + name += options.honorCipherOrder; + + name += ':'; + if (options.ecdhCurve) + name += options.ecdhCurve; + + name += ':'; + if (options.dhparam) + name += options.dhparam; + + name += ':'; + if (options.secureOptions !== undefined) + name += options.secureOptions; + + name += ':'; + if (options.sessionIdContext) + name += options.sessionIdContext; + return name; }; diff --git a/test/parallel/test-https-agent-additional-options.js b/test/parallel/test-https-agent-additional-options.js new file mode 100644 index 00000000000000..8d10524d902ca7 --- /dev/null +++ b/test/parallel/test-https-agent-additional-options.js @@ -0,0 +1,87 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const crypto = require('crypto'); +const https = require('https'); +const fixtures = require('../common/fixtures'); + +const options = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + ca: fixtures.readKey('ca1-cert.pem') +}; + +const server = https.Server(options, function(req, res) { + res.writeHead(200); + res.end('hello world\n'); +}); + +function getBaseOptions(port) { + return { + path: '/', + port: port, + ca: options.ca, + rejectUnautorized: true, + servername: 'agent1', + }; +} + +const updatedValues = new Map([ + ['dhparam', fixtures.readKey('dh2048.pem')], + ['ecdhCurve', 'secp384r1'], + ['honorCipherOrder', true], + ['secureOptions', crypto.constants.SSL_OP_CIPHER_SERVER_PREFERENCE], + ['secureProtocol', 'TLSv1_method'], + ['sessionIdContext', 'sessionIdContext'], +]); + +function variations(iter, port, cb) { + const { done, value } = iter.next(); + if (done) { + return common.mustCall(cb); + } else { + const [key, val] = value; + return common.mustCall(function(res) { + res.resume(); + https.globalAgent.once('free', common.mustCall(function() { + https.get( + Object.assign({}, getBaseOptions(port), { [key]: val }), + variations(iter, port, cb) + ); + })); + }); + } +} + +server.listen(0, common.mustCall(function() { + const port = this.address().port; + const globalAgent = https.globalAgent; + globalAgent.keepAlive = true; + https.get(getBaseOptions(port), variations( + updatedValues.entries(), + port, + common.mustCall(function(res) { + res.resume(); + globalAgent.once('free', common.mustCall(function() { + // Verify that different keep-alived connections are created + // for the base call and each variation + const keys = Object.keys(globalAgent.freeSockets); + assert.strictEqual(keys.length, 1 + updatedValues.size); + let i = 1; + for (const [, value] of updatedValues) { + assert.ok( + keys[i].startsWith(value.toString() + ':') || + keys[i].endsWith(':' + value.toString()) || + keys[i].includes(':' + value.toString() + ':') + ); + i++; + } + globalAgent.destroy(); + server.close(); + })); + }) + )); +})); diff --git a/test/parallel/test-https-agent-getname.js b/test/parallel/test-https-agent-getname.js index 0cdc9568d84470..c29e09731df0b2 100644 --- a/test/parallel/test-https-agent-getname.js +++ b/test/parallel/test-https-agent-getname.js @@ -12,7 +12,7 @@ const agent = new https.Agent(); // empty options assert.strictEqual( agent.getName({}), - 'localhost:::::::::::' + 'localhost:::::::::::::::::' ); // pass all options arguments @@ -23,13 +23,21 @@ const options = { ca: 'ca', cert: 'cert', ciphers: 'ciphers', + crl: [Buffer.from('c'), Buffer.from('r'), Buffer.from('l')], + dhparam: 'dhparam', + ecdhCurve: 'ecdhCurve', + honorCipherOrder: false, key: 'key', pfx: 'pfx', rejectUnauthorized: false, + secureOptions: 0, + secureProtocol: 'secureProtocol', servername: 'localhost', + sessionIdContext: 'sessionIdContext' }; assert.strictEqual( agent.getName(options), - '0.0.0.0:443:192.168.1.1:ca:cert::ciphers:key:pfx:false:localhost:' + '0.0.0.0:443:192.168.1.1:ca:cert::ciphers:key:pfx:false:localhost:' + + 'secureProtocol:c,r,l:false:ecdhCurve:dhparam:0:sessionIdContext' ); diff --git a/test/parallel/test-https-agent-secure-protocol.js b/test/parallel/test-https-agent-secure-protocol.js deleted file mode 100644 index 82554952e8446b..00000000000000 --- a/test/parallel/test-https-agent-secure-protocol.js +++ /dev/null @@ -1,57 +0,0 @@ -'use strict'; -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); - -const assert = require('assert'); -const https = require('https'); -const fixtures = require('../common/fixtures'); - -const options = { - key: fixtures.readKey('agent1-key.pem'), - cert: fixtures.readKey('agent1-cert.pem'), - ca: fixtures.readKey('ca1-cert.pem') -}; - -const server = https.Server(options, function(req, res) { - res.writeHead(200); - res.end('hello world\n'); -}); - -server.listen(0, common.mustCall(function() { - const port = this.address().port; - const globalAgent = https.globalAgent; - globalAgent.keepAlive = true; - https.get({ - path: '/', - port: port, - ca: options.ca, - rejectUnauthorized: true, - servername: 'agent1', - secureProtocol: 'SSLv23_method' - }, common.mustCall(function(res) { - res.resume(); - globalAgent.once('free', common.mustCall(function() { - https.get({ - path: '/', - port: port, - ca: options.ca, - rejectUnauthorized: true, - servername: 'agent1', - secureProtocol: 'TLSv1_method' - }, common.mustCall(function(res) { - res.resume(); - globalAgent.once('free', common.mustCall(function() { - // Verify that two keep-alive connections are created - // due to the different secureProtocol settings: - const keys = Object.keys(globalAgent.freeSockets); - assert.strictEqual(keys.length, 2); - assert.ok(keys[0].includes(':SSLv23_method')); - assert.ok(keys[1].includes(':TLSv1_method')); - globalAgent.destroy(); - server.close(); - })); - })); - })); - })); -}));