Skip to content

Commit c7ec3dc

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 6122200 commit c7ec3dc

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
@@ -100,6 +100,10 @@ changes:
100100

101101
See [`Session Resumption`][] for information about TLS session reuse.
102102

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

105109
<!-- 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
consttls=require('tls');
52+
constkPerRequestCheckServerIdentity=Symbol('per-request checkServerIdentity');
53+
letperRequestCheckServerIdentityIndex=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+
constreuseSession=options._agentKey&&
358+
!options[kPerRequestCheckServerIdentity];
359+
if(reuseSession){
353360
constsession=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=functionkeepSocketAlive(socket){
485+
if(socket[kPerRequestCheckServerIdentity])
486+
returnfalse;
487+
488+
returnFunctionPrototypeCall(HttpAgent.prototype.keepSocketAlive,this,socket);
489+
};
474490

475491
functiongetPfxAgentKey(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
returnname;
583602
};
584603

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

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

641+
functionhasAgentCheckServerIdentity(options){
642+
let{ agent }=options;
643+
if(agent===false)
644+
returnfalse;
645+
646+
if(agent===null||agent===undefined){
647+
if(typeofoptions.createConnection==='function')
648+
returnfalse;
649+
agent=module.exports.globalAgent;
650+
}
651+
652+
returnagent?.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+
constcommon=require('../common');
3+
if(!common.hasCrypto)
4+
common.skip('missing crypto');
5+
6+
constassert=require('assert');
7+
constfixtures=require('../common/fixtures');
8+
consthttps=require('https');
9+
const{ once }=require('events');
10+
11+
constkey=fixtures.readKey('agent1-key.pem');
12+
constcert=fixtures.readKey('agent1-cert.pem');
13+
constca=fixtures.readKey('ca1-cert.pem');
14+
constexpectedError=/rejectedbycallback/;
15+
16+
functionrequest(options){
17+
returnnewPromise((resolve,reject)=>{
18+
constreq=https.get({
19+
host: '127.0.0.1',
20+
servername: 'agent1',
21+
ca: [ca],
22+
...options,
23+
},(res)=>{
24+
constsocket=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+
constserver=https.createServer({
37+
key,
38+
cert,
39+
minVersion: 'TLSv1.2',
40+
maxVersion: 'TLSv1.2',
41+
},(req,res)=>{
42+
res.end('ok');
43+
});
44+
45+
(asyncfunction(){
46+
server.listen(0);
47+
awaitonce(server,'listening');
48+
49+
constport=server.address().port;
50+
letacceptCalls=0;
51+
letrejectCalls=0;
52+
constacceptingCheck=()=>{
53+
acceptCalls++;
54+
};
55+
constrejectingCheck=()=>{
56+
rejectCalls++;
57+
returnnewError('rejected by callback');
58+
};
59+
60+
constsessionAgent=newhttps.Agent();
61+
constkeepAliveAgent=newhttps.Agent({
62+
keepAlive: true,
63+
maxCachedSessions: 0,
64+
});
65+
constagentLevelAgent=newhttps.Agent({
66+
checkServerIdentity: acceptingCheck,
67+
});
68+
69+
try{
70+
awaitrequest({
71+
port,
72+
agent: sessionAgent,
73+
checkServerIdentity: acceptingCheck,
74+
});
75+
assert.deepStrictEqual(sessionAgent._sessionCache.map,{});
76+
awaitassert.rejects(request({
77+
port,
78+
agent: sessionAgent,
79+
checkServerIdentity: rejectingCheck,
80+
}),expectedError);
81+
82+
awaitrequest({
83+
port,
84+
agent: keepAliveAgent,
85+
checkServerIdentity: acceptingCheck,
86+
});
87+
awaitassert.rejects(request({
88+
port,
89+
agent: keepAliveAgent,
90+
checkServerIdentity: rejectingCheck,
91+
}),expectedError);
92+
93+
constfirst=awaitrequest({
94+
port,
95+
agent: agentLevelAgent,
96+
});
97+
assert.strictEqual(first.socket.isSessionReused(),false);
98+
constsecond=awaitrequest({
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+
awaitonce(server,'close');
112+
}
113+
})().then(common.mustCall());

0 commit comments

Comments
 (0)