Skip to content

Commit 95ba2cf

Browse files
mcollinaRafaelGSS
authored andcommitted
https: bind identity checks to session reuse
PR-URL: nodejs-private/node-private#904 Refs: https://hackerone.com/reports/3811980 CVE-ID: CVE-2026-58040
1 parent 9a6b7e3 commit 95ba2cf

3 files changed

Lines changed: 159 additions & 2 deletions

File tree

doc/api/https.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ changes:
9898

9999
See [`Session Resumption`][] for information about TLS session reuse.
100100

101+
Requests that specify a custom `checkServerIdentity` option are not eligible
102+
for connection reuse or TLS session reuse by an `https.Agent`, unless the
103+
`checkServerIdentity` option was specified when constructing the Agent.
104+
101105
#### Event: `'keylog'`
102106

103107
<!-- YAML

lib/https.js

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const {
3535
ObjectSetPrototypeOf,
3636
ReflectApply,
3737
ReflectConstruct,
38+
Symbol,
3839
SymbolAsyncDispose,
3940
} = primordials;
4041

@@ -48,6 +49,8 @@ const { ERR_PROXY_TUNNEL } = require('internal/errors').codes;
4849
assertCrypto();
4950

5051
const tls = require('tls');
52+
const kPerRequestCheckServerIdentity = Symbol('per-request checkServerIdentity');
53+
let perRequestCheckServerIdentityIndex = 0;
5154
const {
5255
kProxyConfig,
5356
checkShouldUseProxy,
@@ -281,6 +284,8 @@ function establishTunnel(agent, socket, options, tunnelConfig, afterSocket) {
281284
tunneldSocket.removeListener('error', onTLSHandshakeError);
282285
afterSocket(null, tunneldSocket);
283286
});
287+
if (requestOptions[kPerRequestCheckServerIdentity])
288+
tunneldSocket[kPerRequestCheckServerIdentity] = true;
284289
tunneldSocket.on('free', () => {
285290
debug('Propagate free event from tunneled socket to tunnel socket');
286291
socket.emit('free');
@@ -349,7 +354,9 @@ function createConnection(...args) {
349354

350355
debug('createConnection', options);
351356

352-
if (options._agentKey) {
357+
const reuseSession = options._agentKey &&
358+
!options[kPerRequestCheckServerIdentity];
359+
if (reuseSession) {
353360
const session = this._getSession(options._agentKey);
354361
if (session) {
355362
debug('reuse session for %j', options._agentKey);
@@ -416,7 +423,10 @@ function createConnection(...args) {
416423
socket[kWaitForProxyTunnel] = true;
417424
}
418425

419-
if (options._agentKey) {
426+
if (options[kPerRequestCheckServerIdentity])
427+
socket[kPerRequestCheckServerIdentity] = true;
428+
429+
if (reuseSession) {
420430
// Cache new session for reuse
421431
socket.on('session', (session) => {
422432
this._cacheSession(options._agentKey, session);
@@ -471,6 +481,12 @@ function Agent(options) {
471481
ObjectSetPrototypeOf(Agent.prototype, HttpAgent.prototype);
472482
ObjectSetPrototypeOf(Agent, HttpAgent);
473483
Agent.prototype.createConnection = createConnection;
484+
Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
485+
if (socket[kPerRequestCheckServerIdentity])
486+
return false;
487+
488+
return FunctionPrototypeCall(HttpAgent.prototype.keepSocketAlive, this, socket);
489+
};
474490

475491
function getPfxAgentKey(pfx, passphrase) {
476492
if (!ArrayIsArray(pfx))
@@ -579,6 +595,9 @@ Agent.prototype.getName = function getName(options = kEmptyObject) {
579595
if (options.privateKeyEngine)
580596
name += options.privateKeyEngine;
581597

598+
if (options[kPerRequestCheckServerIdentity])
599+
name += `:${options[kPerRequestCheckServerIdentity]}`;
600+
582601
return name;
583602
};
584603

@@ -619,6 +638,20 @@ Agent.prototype._evictSession = function _evictSession(key) {
619638

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

641+
function hasAgentCheckServerIdentity(options) {
642+
let { agent } = options;
643+
if (agent === false)
644+
return false;
645+
646+
if (agent === null || agent === undefined) {
647+
if (typeof options.createConnection === 'function')
648+
return false;
649+
agent = module.exports.globalAgent;
650+
}
651+
652+
return agent?.options?.checkServerIdentity !== undefined;
653+
}
654+
622655
/**
623656
* Makes a request to a secure web server.
624657
* @param {...any} args
@@ -638,6 +671,13 @@ function request(...args) {
638671
ObjectAssign(options, ArrayPrototypeShift(args));
639672
}
640673

674+
if (options.checkServerIdentity !== undefined &&
675+
options.checkServerIdentity !== tls.checkServerIdentity &&
676+
!hasAgentCheckServerIdentity(options)) {
677+
options[kPerRequestCheckServerIdentity] =
678+
++perRequestCheckServerIdentityIndex;
679+
}
680+
641681
options._defaultAgent = module.exports.globalAgent;
642682
ArrayPrototypeUnshift(args, options);
643683

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto)
4+
common.skip('missing crypto');
5+
6+
const assert = require('assert');
7+
const fixtures = require('../common/fixtures');
8+
const https = require('https');
9+
const { once } = require('events');
10+
11+
const key = fixtures.readKey('agent1-key.pem');
12+
const cert = fixtures.readKey('agent1-cert.pem');
13+
const ca = fixtures.readKey('ca1-cert.pem');
14+
const expectedError = /rejected by callback/;
15+
16+
function request(options) {
17+
return new Promise((resolve, reject) => {
18+
const req = https.get({
19+
host: '127.0.0.1',
20+
servername: 'agent1',
21+
ca: [ca],
22+
...options,
23+
}, (res) => {
24+
const socket = res.socket;
25+
res.resume();
26+
res.on('end', () => resolve({
27+
socket,
28+
reusedSocket: req.reusedSocket,
29+
}));
30+
});
31+
32+
req.on('error', reject);
33+
});
34+
}
35+
36+
const server = https.createServer({
37+
key,
38+
cert,
39+
minVersion: 'TLSv1.2',
40+
maxVersion: 'TLSv1.2',
41+
}, (req, res) => {
42+
res.end('ok');
43+
});
44+
45+
(async function() {
46+
server.listen(0);
47+
await once(server, 'listening');
48+
49+
const port = server.address().port;
50+
let acceptCalls = 0;
51+
let rejectCalls = 0;
52+
const acceptingCheck = () => {
53+
acceptCalls++;
54+
};
55+
const rejectingCheck = () => {
56+
rejectCalls++;
57+
return new Error('rejected by callback');
58+
};
59+
60+
const sessionAgent = new https.Agent();
61+
const keepAliveAgent = new https.Agent({
62+
keepAlive: true,
63+
maxCachedSessions: 0,
64+
});
65+
const agentLevelAgent = new https.Agent({
66+
checkServerIdentity: acceptingCheck,
67+
});
68+
69+
try {
70+
await request({
71+
port,
72+
agent: sessionAgent,
73+
checkServerIdentity: acceptingCheck,
74+
});
75+
assert.deepStrictEqual(sessionAgent._sessionCache.map, {});
76+
await assert.rejects(request({
77+
port,
78+
agent: sessionAgent,
79+
checkServerIdentity: rejectingCheck,
80+
}), expectedError);
81+
82+
await request({
83+
port,
84+
agent: keepAliveAgent,
85+
checkServerIdentity: acceptingCheck,
86+
});
87+
await assert.rejects(request({
88+
port,
89+
agent: keepAliveAgent,
90+
checkServerIdentity: rejectingCheck,
91+
}), expectedError);
92+
93+
const first = await request({
94+
port,
95+
agent: agentLevelAgent,
96+
});
97+
assert.strictEqual(first.socket.isSessionReused(), false);
98+
const second = await request({
99+
port,
100+
agent: agentLevelAgent,
101+
});
102+
assert.strictEqual(second.socket.isSessionReused(), true);
103+
104+
assert.strictEqual(acceptCalls, 3);
105+
assert.strictEqual(rejectCalls, 2);
106+
} finally {
107+
sessionAgent.destroy();
108+
keepAliveAgent.destroy();
109+
agentLevelAgent.destroy();
110+
server.close();
111+
await once(server, 'close');
112+
}
113+
})().then(common.mustCall());

0 commit comments

Comments
 (0)