Skip to content

Commit 3fa5ee2

Browse files
pimterryaduh95
authored andcommitted
tls: disable resumption for mTLS with no session id context
Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: nodejs-private/node-private#1030 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> CVE-ID: CVE-2026-123872
1 parent 1ddeffb commit 3fa5ee2

3 files changed

Lines changed: 94 additions & 1 deletion

File tree

‎src/crypto/crypto_tls.cc‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ SSL_SESSION* GetSessionCallback(
113113
int* copy) {
114114
TLSWrap* w = static_cast<TLSWrap*>(SSL_get_app_data(s));
115115
*copy = 0;
116+
if (w->is_resumption_disabled()) return nullptr;
116117
return w->ReleaseSession();
117118
}
118119

@@ -1604,7 +1605,15 @@ bool TLSWrap::PinServerName(std::string_view servername) {
16041605
}
16051606

16061607
bool TLSWrap::BindSessionsToServerName(std::string_view servername) {
1607-
if (sc_->session_id_context().empty()) return true;
1608+
// If we have no session id context and we need to verify peers, we reject
1609+
// all resumption since we can't do so securely.
1610+
if (sc_->session_id_context().empty()) {
1611+
if (SSL_get_verify_mode(ssl_.get()) & SSL_VERIFY_PEER) {
1612+
SSL_set_options(ssl_.get(), SSL_OP_NO_TICKET);
1613+
flags_.resumption_disabled = true;
1614+
}
1615+
return true;
1616+
}
16081617

16091618
// Server names can't contain NULs, so a NUL keeps the parts apart.
16101619
std::string input = sc_->session_id_context();

‎src/crypto/crypto_tls.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ class TLSWrap : public AsyncWrap,
9595
flags_.awaiting_new_session = on;
9696
}
9797
inline void enable_session_callbacks() { flags_.session_callbacks = true; }
98+
inline bool is_resumption_disabled() const {
99+
return flags_.resumption_disabled;
100+
}
98101
bool BindSessionsToServerName(std::string_view servername);
99102
bool PinServerName(std::string_view servername);
100103
inline bool is_server() const { return kind_ == Kind::kServer; }
@@ -339,6 +342,7 @@ class TLSWrap : public AsyncWrap,
339342
struct Flags {
340343
bool session_callbacks : 1;
341344
bool awaiting_new_session : 1;
345+
bool resumption_disabled : 1;
342346
// 'onclienthello' has been emitted for this connection.
343347
bool hello_emitted : 1;
344348
// JS has answered it by calling clientHelloDone().
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict';
2+
3+
// A server with no session id context of its own - possible when creating a
4+
// TLSSocket directly - can't tell which application a session belongs to. One
5+
// that verifies client certificates mustn't resume any, or a session from an
6+
// application sharing its secure context (and so its ticket keys) would skip
7+
// the client's certificate check. It gets a full handshake instead.
8+
9+
const common = require('../common');
10+
11+
if (!common.hasCrypto)
12+
common.skip('missing crypto');
13+
14+
const assert = require('node:assert');
15+
const net = require('node:net');
16+
const tls = require('node:tls');
17+
const fixtures = require('../common/fixtures');
18+
19+
const secureContext = tls.createSecureContext({
20+
key: fixtures.readKey('agent2-key.pem'),
21+
cert: fixtures.readKey('agent2-cert.pem'),
22+
});
23+
24+
// Whether the server resumed each connection it accepted, in order.
25+
const reused = [];
26+
27+
async function listen(requestCert) {
28+
const server = net.createServer((socket) => {
29+
const tlsSocket = new tls.TLSSocket(socket, {
30+
isServer: true,
31+
secureContext,
32+
requestCert,
33+
});
34+
tlsSocket.on('secure', () => {
35+
reused.push(tlsSocket.isSessionReused());
36+
tlsSocket.end();
37+
});
38+
});
39+
await new Promise((resolve) => server.listen(0, resolve));
40+
return server;
41+
}
42+
43+
// Resolves with the first session issued, once the connection closes. This is
44+
// the raw session from the TLS library: on BoringSSL the 'session' event's
45+
// TLS 1.3 session can't be resumed.
46+
function connect(server, options) {
47+
return new Promise((resolve) => {
48+
let session;
49+
const socket = tls.connect({
50+
port: server.address().port,
51+
rejectUnauthorized: false,
52+
...options,
53+
});
54+
const { onnewsession } = socket._handle;
55+
socket._handle.onnewsession = function(id, raw) {
56+
session ??= raw;
57+
return onnewsession.call(this, id, raw);
58+
};
59+
socket.on('session', () => {});
60+
socket.on('close', () => resolve(session));
61+
socket.resume();
62+
});
63+
}
64+
65+
(async () => {
66+
const open = await listen(false);
67+
const verifying = await listen(true);
68+
69+
for (const maxVersion of ['TLSv1.2', 'TLSv1.3']) {
70+
reused.length = 0;
71+
const session = await connect(open, { maxVersion });
72+
assert.ok(session);
73+
await connect(open, { maxVersion, session });
74+
await connect(verifying, { maxVersion, session });
75+
assert.deepStrictEqual(reused, [false, true, false], maxVersion);
76+
}
77+
78+
open.close();
79+
verifying.close();
80+
})().then(common.mustCall());

0 commit comments

Comments
 (0)