Skip to content

Commit cf03ae0

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 cf03ae0

2 files changed

Lines changed: 122 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: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
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 }, (res) => {
25+
res.resume();
26+
res.on('end', resolve);
27+
});
28+
req.on('error', reject);
29+
});
30+
}
31+
32+
(async function main() {
33+
server.listen(0);
34+
await once(server, 'listening');
35+
const port = server.address().port;
36+
37+
// A per-request `rejectUnauthorized: true` must override an agent that
38+
// disables verification.
39+
{
40+
const agent = new https.Agent({ keepAlive: true, rejectUnauthorized: false });
41+
await request(port, { agent });
42+
await assert.rejects(
43+
request(port, { agent, rejectUnauthorized: true, servername: 'agent1' }),
44+
{ code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' },
45+
);
46+
agent.destroy();
47+
}
48+
49+
// A per-request narrowed `ca` must override an agent that trusts a broader
50+
// set of CAs.
51+
{
52+
const agent = new https.Agent({ keepAlive: true, ca: [ca1] });
53+
await request(port, { agent, servername: 'agent1' });
54+
await assert.rejects(
55+
request(port, { agent, servername: 'agent1', ca: [ca2] }),
56+
{ code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE' },
57+
);
58+
agent.destroy();
59+
}
60+
61+
// A per-request `servername` must override an agent that pins a different
62+
// servername.
63+
{
64+
const agent = new https.Agent({ keepAlive: true, ca: [ca1], servername: 'agent1' });
65+
await request(port, { agent });
66+
await assert.rejects(
67+
request(port, { agent, servername: 'wronghost' }),
68+
{ code: 'ERR_TLS_CERT_ALTNAME_INVALID' },
69+
);
70+
agent.destroy();
71+
}
72+
73+
server.close();
74+
await once(server, 'close');
75+
})().then(common.mustCall());

0 commit comments

Comments
 (0)