Skip to content

Commit 7145186

Browse files
committed
https: honor per-request TLS options over agent options
Per-request rejectUnauthorized/ca/servername were silently ignored when the https.Agent set the same option, because http.Agent merges request options over the agent's own, letting the agent win. Capture the per-request overrides and re-apply them in createConnection(), mirroring the checkServerIdentity handling from CVE-2026-58040. Refs: 52a8ace880d Signed-off-by: axedos <acceleratingssoul@proton.me>
1 parent bc6e1ad commit 7145186

2 files changed

Lines changed: 120 additions & 4 deletions

File tree

‎lib/https.js‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ assertCrypto();
5050

5151
const tls = require('tls');
5252
const kPerRequestCheckServerIdentity = Symbol('per-request checkServerIdentity');
53+
const kPerRequestTLSOptions = Symbol('per-request TLS options');
5354
let perRequestCheckServerIdentityIndex = 0;
5455
const {
5556
kProxyConfig,
@@ -376,6 +377,14 @@ function createConnection(...args) {
376377

377378
debug('createConnection', options);
378379

380+
const perRequestTLSOptions = options[kPerRequestTLSOptions];
381+
if (perRequestTLSOptions !== undefined) {
382+
options = {
383+
...options,
384+
...perRequestTLSOptions,
385+
};
386+
}
387+
379388
const reuseSession = options._agentKey &&
380389
!options[kPerRequestCheckServerIdentity];
381390
if (reuseSession) {
@@ -660,18 +669,45 @@ Agent.prototype._evictSession = function _evictSession(key) {
660669

661670
const globalAgent = getGlobalAgent(getOptionValue('--use-env-proxy') ? process.env : undefined, Agent);
662671

663-
function hasAgentCheckServerIdentity(options) {
672+
function getAgent(options) {
664673
let { agent } = options;
665674
if (agent === false)
666-
return false;
675+
return undefined;
667676

668677
if (agent === null || agent === undefined) {
669678
if (typeof options.createConnection === 'function')
670-
return false;
679+
return undefined;
671680
agent = module.exports.globalAgent;
672681
}
673682

674-
return agent?.options?.checkServerIdentity !== undefined;
683+
return agent;
684+
}
685+
686+
function hasAgentCheckServerIdentity(options) {
687+
return getAgent(options)?.options?.checkServerIdentity !== undefined;
688+
}
689+
690+
const kPerRequestTLSOptionKeys = ['rejectUnauthorized', 'ca', 'servername'];
691+
692+
// When an https.Agent is constructed with TLS options, those agent options take
693+
// precedence over the same options passed per-request (the http.Agent merges the
694+
// request options over the agent's own, letting the agent win). That is
695+
// undesirable for security-relevant TLS options: a per-request stricter value
696+
// (e.g. `rejectUnauthorized: true`) would otherwise be silently ignored. Capture
697+
// the per-request overrides here so createConnection() can re-apply them.
698+
function getPerRequestTLSOptions(options) {
699+
const agentOptions = getAgent(options)?.options;
700+
const overrides = {};
701+
let hasOverride = false;
702+
for (const key of kPerRequestTLSOptionKeys) {
703+
if (options[key] !== undefined &&
704+
agentOptions?.[key] !== undefined &&
705+
options[key] !== agentOptions[key]) {
706+
overrides[key] = options[key];
707+
hasOverride = true;
708+
}
709+
}
710+
return hasOverride ? overrides : undefined;
675711
}
676712

677713
/**
@@ -700,6 +736,13 @@ function request(...args) {
700736
++perRequestCheckServerIdentityIndex;
701737
}
702738

739+
const perRequestTLSOptions = getPerRequestTLSOptions(options);
740+
if (perRequestTLSOptions !== undefined) {
741+
options[kPerRequestTLSOptions] = perRequestTLSOptions;
742+
options[kPerRequestCheckServerIdentity] =
743+
++perRequestCheckServerIdentityIndex;
744+
}
745+
703746
options._defaultAgent = module.exports.globalAgent;
704747
ArrayPrototypeUnshift(args, options);
705748

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const assert = require('assert');
8+
const fixtures = require('../common/fixtures');
9+
const https = require('https');
10+
const { once } = require('events');
11+
12+
const key = fixtures.readKey('agent1-key.pem');
13+
const cert = fixtures.readKey('agent1-cert.pem');
14+
const ca1 = fixtures.readKey('ca1-cert.pem');
15+
const ca2 = fixtures.readKey('ca2-cert.pem');
16+
17+
const server = https.createServer(
18+
{ key, cert, minVersion: 'TLSv1.2', maxVersion: 'TLSv1.2' },
19+
(req, res) => res.end('ok'),
20+
);
21+
22+
function request(port, options) {
23+
return new Promise((resolve, reject) => {
24+
const req = https.get({ host: '127.0.0.1', port, ...options },
25+
(res) => { res.resume(); res.on('end', resolve); });
26+
req.on('error', reject);
27+
});
28+
}
29+
30+
(async function main() {
31+
server.listen(0);
32+
await once(server, 'listening');
33+
const port = server.address().port;
34+
35+
// A per-request `rejectUnauthorized: true` must override an agent that
36+
// disables verification.
37+
{
38+
const agent = new https.Agent({ keepAlive: true, rejectUnauthorized: false });
39+
await request(port, { agent });
40+
await assert.rejects(
41+
request(port, { agent, rejectUnauthorized: true, servername: 'agent1' }),
42+
{ code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' },
43+
);
44+
agent.destroy();
45+
}
46+
47+
// A per-request narrowed `ca` must override an agent that trusts a broader
48+
// set of CAs.
49+
{
50+
const agent = new https.Agent({ keepAlive: true, ca: [ca1] });
51+
await request(port, { agent, servername: 'agent1' });
52+
await assert.rejects(
53+
request(port, { agent, servername: 'agent1', ca: [ca2] }),
54+
{ code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' },
55+
);
56+
agent.destroy();
57+
}
58+
59+
// A per-request `servername` must override an agent that pins a different
60+
// servername.
61+
{
62+
const agent = new https.Agent({ keepAlive: true, ca: [ca1], servername: 'agent1' });
63+
await request(port, { agent });
64+
await assert.rejects(
65+
request(port, { agent, servername: 'wronghost' }),
66+
{ code: 'ERR_TLS_CERT_ALTNAME_INVALID' },
67+
);
68+
agent.destroy();
69+
}
70+
71+
server.close();
72+
await once(server, 'close');
73+
})().then(common.mustCall());

0 commit comments

Comments
 (0)