https: honor per-request TLS options over agent options

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);
 

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());