diff --git a/lib/https.js b/lib/https.js index dc9df0da184..e751cddc118 100644 --- a/lib/https.js +++ b/lib/https.js @@ -50,6 +50,7 @@ assertCrypto(); const tls = require('tls'); const kPerRequestCheckServerIdentity = Symbol('per-request checkServerIdentity'); +const kPerRequestTLSOptions = Symbol('per-request TLS options'); let perRequestCheckServerIdentityIndex = 0; const { kProxyConfig, @@ -376,6 +377,14 @@ function createConnection(...args) { debug('createConnection', options); + const perRequestTLSOptions = options[kPerRequestTLSOptions]; + if (perRequestTLSOptions !== undefined) { + options = { + ...options, + ...perRequestTLSOptions, + }; + } + const reuseSession = options._agentKey && !options[kPerRequestCheckServerIdentity]; if (reuseSession) { @@ -660,18 +669,45 @@ Agent.prototype._evictSession = function _evictSession(key) { const globalAgent = getGlobalAgent(getOptionValue('--use-env-proxy') ? process.env : undefined, Agent); -function hasAgentCheckServerIdentity(options) { +function getAgent(options) { let { agent } = options; if (agent === false) - return false; + return undefined; if (agent === null || agent === undefined) { if (typeof options.createConnection === 'function') - return false; + return undefined; agent = module.exports.globalAgent; } - return agent?.options?.checkServerIdentity !== undefined; + return agent; +} + +function hasAgentCheckServerIdentity(options) { + return getAgent(options)?.options?.checkServerIdentity !== undefined; +} + +const kPerRequestTLSOptionKeys = ['rejectUnauthorized', 'ca', 'servername']; + +// When an https.Agent is constructed with TLS options, those agent options take +// precedence over the same options passed per-request (the http.Agent merges the +// request options over the agent's own, letting the agent win). That is +// undesirable for security-relevant TLS options: a per-request stricter value +// (e.g. `rejectUnauthorized: true`) would otherwise be silently ignored. Capture +// the per-request overrides here so createConnection() can re-apply them. +function getPerRequestTLSOptions(options) { + const agentOptions = getAgent(options)?.options; + const overrides = {}; + let hasOverride = false; + for (const key of kPerRequestTLSOptionKeys) { + if (options[key] !== undefined && + agentOptions?.[key] !== undefined && + options[key] !== agentOptions[key]) { + overrides[key] = options[key]; + hasOverride = true; + } + } + return hasOverride ? overrides : undefined; } /** @@ -700,6 +736,13 @@ function request(...args) { ++perRequestCheckServerIdentityIndex; } + const perRequestTLSOptions = getPerRequestTLSOptions(options); + if (perRequestTLSOptions !== undefined) { + options[kPerRequestTLSOptions] = perRequestTLSOptions; + options[kPerRequestCheckServerIdentity] = + ++perRequestCheckServerIdentityIndex; + } + options._defaultAgent = module.exports.globalAgent; ArrayPrototypeUnshift(args, options); diff --git a/test/parallel/test-https-agent-per-request-tls-override.js b/test/parallel/test-https-agent-per-request-tls-override.js new file mode 100644 index 00000000000..f1dde280035 --- /dev/null +++ b/test/parallel/test-https-agent-per-request-tls-override.js @@ -0,0 +1,73 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const https = require('https'); +const { once } = require('events'); + +const key = fixtures.readKey('agent1-key.pem'); +const cert = fixtures.readKey('agent1-cert.pem'); +const ca1 = fixtures.readKey('ca1-cert.pem'); +const ca2 = fixtures.readKey('ca2-cert.pem'); + +const server = https.createServer( + { key, cert, minVersion: 'TLSv1.2', maxVersion: 'TLSv1.2' }, + (req, res) => res.end('ok'), +); + +function request(port, options) { + return new Promise((resolve, reject) => { + const req = https.get({ host: '127.0.0.1', port, ...options }, + (res) => { res.resume(); res.on('end', resolve); }); + req.on('error', reject); + }); +} + +(async function main() { + server.listen(0); + await once(server, 'listening'); + const port = server.address().port; + + // A per-request `rejectUnauthorized: true` must override an agent that + // disables verification. + { + const agent = new https.Agent({ keepAlive: true, rejectUnauthorized: false }); + await request(port, { agent }); + await assert.rejects( + request(port, { agent, rejectUnauthorized: true, servername: 'agent1' }), + { code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' }, + ); + agent.destroy(); + } + + // A per-request narrowed `ca` must override an agent that trusts a broader + // set of CAs. + { + const agent = new https.Agent({ keepAlive: true, ca: [ca1] }); + await request(port, { agent, servername: 'agent1' }); + await assert.rejects( + request(port, { agent, servername: 'agent1', ca: [ca2] }), + { code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' }, + ); + agent.destroy(); + } + + // A per-request `servername` must override an agent that pins a different + // servername. + { + const agent = new https.Agent({ keepAlive: true, ca: [ca1], servername: 'agent1' }); + await request(port, { agent }); + await assert.rejects( + request(port, { agent, servername: 'wronghost' }), + { code: 'ERR_TLS_CERT_ALTNAME_INVALID' }, + ); + agent.destroy(); + } + + server.close(); + await once(server, 'close'); +})().then(common.mustCall());