镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Commit e750e1f

Browse files
pimterryaduh95
authored andcommitted
tls: don't allow renegotiation or HRR to change SNI
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 97b1357 commit e750e1f

5 files changed

Lines changed: 338 additions & 39 deletions

File tree

‎lib/internal/tls/wrap.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,7 @@ TLSSocket.prototype._finishInit = function() {
11981198
return;
11991199

12001200
this.alpnProtocol = this._handle.getALPNNegotiatedProtocol();
1201-
// The servername could be set by TLSWrap::SelectSNIContextCallback().
1201+
// On a server, TLSWrap::PinServerName() sets it from the first ClientHello.
12021202
if (this.servername === null) {
12031203
this.servername = this._handle.getServername();
12041204
}

‎src/crypto/crypto_tls.cc‎

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ SSL_SESSION* GetSessionCallback(
102102
return w->ReleaseSession();
103103
}
104104

105+
// The server name is handled in EarlyClientHelloCallback, so this only
106+
// acknowledges it, which also has the TLS library store it in new sessions.
107+
int AcknowledgeServerName(SSL* s, int* ad, void* arg) {
108+
return SSL_TLSEXT_ERR_OK;
109+
}
110+
105111
// The TLS library invokes this before version negotiation and before session
106112
// or ticket resumption, which makes async session lookup possible. If required
107113
// the handshake is suspended here and resumed once JS has answered.
@@ -115,10 +121,8 @@ ClientHelloResult EarlyClientHelloCallback(const ClientHelloContext& hello) {
115121
return ClientHelloResult::kFail;
116122
}
117123

118-
// Session resumption must only be allowed for sessions with exactly the same
119-
// SNI. We use the SNI to derive the session id, so no handshake can resume
120-
// from a mismatched SNI value. Mismatches fall back to full handshakes:
121-
if (!w->BindSessionsToServerName(*servername)) {
124+
if (!w->PinServerName(*servername)) {
125+
hello.set_alert(SSL_AD_ILLEGAL_PARAMETER);
122126
return ClientHelloResult::kFail;
123127
}
124128

@@ -579,8 +583,7 @@ void TLSWrap::InitSSL() {
579583
// https://github.com/openssl/openssl/issues/7199#issuecomment-420670544
580584
SSL_set_info_callback(ssl_.get(), SSLInfoCallback);
581585

582-
if (is_server())
583-
sc_->SetSelectSNIContextCallback(SelectSNIContextCallback);
586+
if (is_server()) sc_->SetSelectSNIContextCallback(AcknowledgeServerName);
584587

585588
ConfigureSecureContext(sc_.get());
586589

@@ -1516,39 +1519,34 @@ void TLSWrap::SetServername(const FunctionCallbackInfo<Value>& args) {
15161519
SSL_set_tlsext_host_name(wrap->ssl_.get(), *servername);
15171520
}
15181521

1519-
int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
1520-
TLSWrap* p = static_cast<TLSWrap*>(SSL_get_app_data(s));
1521-
Environment* env = p->env();
1522-
HandleScope handle_scope(env->isolate());
1523-
Context::Scope context_scope(env->context());
1524-
1525-
auto servername = SSLPointer::GetServerName(s);
1526-
if (!servername.has_value() ||
1527-
!Set(env, p->GetOwner(), env->servername_string(), servername.value()))
1528-
return SSL_TLSEXT_ERR_NOACK;
1529-
1530-
Local<Value> ctx = p->object()->Get(env->context(), env->sni_context_string())
1531-
.FromMaybe(Local<Value>());
1532-
1533-
// Acknowlege the name so it's correctly stored on the session:
1534-
if (ctx.IsEmpty() || !ctx->IsObject()) return SSL_TLSEXT_ERR_OK;
1535-
1536-
if (!env->secure_context_constructor_template()->HasInstance(ctx)) {
1537-
// Failure: incorrect SNI context object
1538-
Local<Value> err = Exception::TypeError(env->sni_context_err_string());
1539-
p->MakeCallback(env->onerror_string(), 1, &err);
1540-
return SSL_TLSEXT_ERR_NOACK;
1522+
// The connection's server name is fixed by its first ClientHello: later ones (a
1523+
// renegotiation, or the second of a HelloRetryRequest) must ask for exactly the
1524+
// same, as the connection's secure context was chosen for it.
1525+
bool TLSWrap::PinServerName(std::string_view servername) {
1526+
if (flags_.servername_pinned) {
1527+
if (servername == servername_) return true;
1528+
Debug(this,
1529+
"ClientHello asked for server name '%s', connection pinned to '%s'",
1530+
servername,
1531+
servername_);
1532+
return false;
15411533
}
1534+
flags_.servername_pinned = true;
1535+
servername_ = servername;
15421536

1543-
SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
1544-
CHECK_NOT_NULL(sc);
1545-
p->sni_context_ = BaseObjectPtr<SecureContext>(sc);
1546-
1547-
ConfigureSecureContext(sc);
1548-
CHECK_EQ(SSL_set_SSL_CTX(p->ssl_.get(), sc->ctx().get()), sc->ctx().get());
1549-
p->SetCACerts(sc);
1537+
if (!servername.empty()) {
1538+
HandleScope handle_scope(env()->isolate());
1539+
Context::Scope context_scope(env()->context());
1540+
if (!Set(env(), GetOwner(), env()->servername_string(), servername_)) {
1541+
return false;
1542+
}
1543+
}
15501544

1551-
return SSL_TLSEXT_ERR_OK;
1545+
// Session resumption must only be allowed for sessions with exactly the same
1546+
// SNI. We use the SNI to derive the session id context, so no handshake can
1547+
// resume from a mismatched SNI value. Mismatches fall back to full
1548+
// handshakes:
1549+
return BindSessionsToServerName(servername);
15521550
}
15531551

15541552
bool TLSWrap::BindSessionsToServerName(std::string_view servername) {
@@ -1727,6 +1725,7 @@ void TLSWrap::MemoryInfo(MemoryTracker* tracker) const {
17271725
tracker->TrackField("ocsp_response", ocsp_response_);
17281726
tracker->TrackField("sni_context", sni_context_);
17291727
tracker->TrackField("error", error_);
1728+
tracker->TrackField("servername", servername_);
17301729
if (pending_cleartext_input_)
17311730
tracker->TrackField("pending_cleartext_input", pending_cleartext_input_);
17321731
if (enc_in_ != nullptr)

‎src/crypto/crypto_tls.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ class TLSWrap : public AsyncWrap,
9696
}
9797
inline void enable_session_callbacks() { flags_.session_callbacks = true; }
9898
bool BindSessionsToServerName(std::string_view servername);
99+
bool PinServerName(std::string_view servername);
99100
inline bool is_server() const { return kind_ == Kind::kServer; }
100101
inline bool is_client() const { return kind_ == Kind::kClient; }
101102
inline bool is_awaiting_new_session() const {
@@ -231,8 +232,6 @@ class TLSWrap : public AsyncWrap,
231232

232233
int SetCACerts(SecureContext* sc);
233234

234-
static int SelectSNIContextCallback(SSL* s, int* ad, void* arg);
235-
236235
static void CertCbDone(const v8::FunctionCallbackInfo<v8::Value>& args);
237236
static void ClientHelloDone(const v8::FunctionCallbackInfo<v8::Value>& args);
238237
static void DestroySSL(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -316,6 +315,7 @@ class TLSWrap : public AsyncWrap,
316315
v8::Global<v8::ArrayBufferView> ocsp_response_;
317316
BaseObjectPtr<SecureContext> sni_context_;
318317
BaseObjectPtr<SecureContext> sc_;
318+
std::string servername_;
319319

320320
// BIO buffers hold encrypted data.
321321
BIO* enc_in_ = nullptr; // StreamListener fills this for SSL_read().
@@ -353,6 +353,7 @@ class TLSWrap : public AsyncWrap,
353353
bool write_callback_scheduled : 1;
354354
bool has_active_write_issued_by_prev_listener : 1;
355355
bool alpn_callback_enabled : 1;
356+
bool servername_pinned : 1;
356357
};
357358
Flags flags_{};
358359

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
'use strict';
2+
3+
// A connection's secure context - its certificate, and the CAs it checks client
4+
// certificates against - is chosen from the server name of its first client
5+
// hello, but a connection can do multiple hellos: a TLS 1.2 renegotiation, or
6+
// a TLS 1.3 HelloRetryRequest. These later client hellos must not be allowed
7+
// to subsequently change the server name, since it can associate state and
8+
// authorization (mTLS) from one SNI with another, unexpectedly.
9+
//
10+
// Node's own client can't change the name mid-connection, so the client here
11+
// drives the TLS library directly over memory BIOs, leaving the socket I/O to
12+
// node:net.
13+
14+
const common = require('../common');
15+
16+
if (!common.hasCrypto)
17+
common.skip('missing crypto');
18+
19+
common.skipIfFFIMissing();
20+
21+
if (common.isWindows)
22+
common.skip('dlopen(null) is not supported on Windows');
23+
24+
// SSL_ctrl() and SSL_CTX_ctrl() take and return a C `long`, which node:ffi has
25+
// no type for, so they're declared below as i64, which it only is on 64-bit.
26+
if (['ia32', 'arm'].includes(process.arch))
27+
common.skip('a C long is not 64 bits here');
28+
29+
// The SSL_ctrl() command numbers below are OpenSSL's, and BoringSSL neither
30+
// renegotiates nor reads the server name of a second ClientHello.
31+
// BoringSSL guards against the whole class of errors internally anyway, and
32+
// the FFI implementation for testing below is OpenSSL specific:
33+
if (require('../common/crypto').isBoringSSL)
34+
common.skip('OpenSSL-specific');
35+
36+
const assert = require('node:assert');
37+
const net = require('node:net');
38+
const tls = require('node:tls');
39+
const fixtures = require('../common/fixtures');
40+
const { dlopen } = require('node:ffi');
41+
42+
const SSL_CTRL_SET_TLSEXT_HOSTNAME = 55;
43+
const SSL_CTRL_SET_MAX_PROTO_VERSION = 124;
44+
const TLSEXT_NAMETYPE_host_name = 0n;
45+
const TLS1_2_VERSION = 0x0303n;
46+
const SSL_ERROR_WANT_READ = 2;
47+
const SSL_ERROR_WANT_WRITE = 3;
48+
49+
// Node's own OpenSSL, resolved out of the running process rather than loaded
50+
// from the system.
51+
const { functions: ssl } = dlopen(null, {
52+
TLS_client_method: { arguments: [], return: 'pointer' },
53+
SSL_CTX_new: { arguments: ['pointer'], return: 'pointer' },
54+
SSL_CTX_free: { arguments: ['pointer'], return: 'void' },
55+
SSL_CTX_ctrl: { arguments: ['pointer', 'i32', 'i64', 'pointer'], return: 'i64' },
56+
SSL_new: { arguments: ['pointer'], return: 'pointer' },
57+
SSL_free: { arguments: ['pointer'], return: 'void' },
58+
SSL_ctrl: { arguments: ['pointer', 'i32', 'i64', 'string'], return: 'i64' },
59+
SSL_set_bio: { arguments: ['pointer', 'pointer', 'pointer'], return: 'void' },
60+
SSL_set_connect_state: { arguments: ['pointer'], return: 'void' },
61+
SSL_do_handshake: { arguments: ['pointer'], return: 'i32' },
62+
SSL_renegotiate: { arguments: ['pointer'], return: 'i32' },
63+
SSL_get_error: { arguments: ['pointer', 'i32'], return: 'i32' },
64+
SSL_get1_session: { arguments: ['pointer'], return: 'pointer' },
65+
SSL_set_session: { arguments: ['pointer', 'pointer'], return: 'i32' },
66+
SSL_session_reused: { arguments: ['pointer'], return: 'i32' },
67+
SSL_SESSION_free: { arguments: ['pointer'], return: 'void' },
68+
BIO_s_mem: { arguments: [], return: 'pointer' },
69+
BIO_new: { arguments: ['pointer'], return: 'pointer' },
70+
BIO_read: { arguments: ['pointer', 'buffer', 'i32'], return: 'i32' },
71+
BIO_write: { arguments: ['pointer', 'buffer', 'i32'], return: 'i32' },
72+
});
73+
74+
// A client whose records are carried by a plain socket. The server name is
75+
// settable at any point, which is the whole reason for this class.
76+
class RawClient {
77+
constructor({ tls12Only = false, session } = {}) {
78+
this.ctx = ssl.SSL_CTX_new(ssl.TLS_client_method());
79+
if (tls12Only) {
80+
ssl.SSL_CTX_ctrl(this.ctx, SSL_CTRL_SET_MAX_PROTO_VERSION, TLS1_2_VERSION, null);
81+
}
82+
this.ssl = ssl.SSL_new(this.ctx);
83+
if (session) ssl.SSL_set_session(this.ssl, session);
84+
this.rbio = ssl.BIO_new(ssl.BIO_s_mem());
85+
this.wbio = ssl.BIO_new(ssl.BIO_s_mem());
86+
// SSL_set_bio() takes ownership of both, so SSL_free() releases them.
87+
ssl.SSL_set_bio(this.ssl, this.rbio, this.wbio);
88+
ssl.SSL_set_connect_state(this.ssl);
89+
90+
this.buf = Buffer.alloc(16 * 1024);
91+
this.incoming = [];
92+
this.notify = null;
93+
this.ended = false;
94+
this.socket = net.connect(server.address().port, '127.0.0.1');
95+
this.socket.on('data', (chunk) => {
96+
this.incoming.push(chunk);
97+
this.wake();
98+
});
99+
this.socket.on('close', () => {
100+
this.ended = true;
101+
this.wake();
102+
});
103+
this.socket.on('error', () => {});
104+
this.connected = new Promise((resolve) => this.socket.once('connect', resolve));
105+
}
106+
107+
wake() {
108+
const notify = this.notify;
109+
this.notify = null;
110+
notify?.();
111+
}
112+
113+
// A null name clears it, so the next ClientHello carries no extension.
114+
setServername(name) {
115+
ssl.SSL_ctrl(this.ssl, SSL_CTRL_SET_TLSEXT_HOSTNAME, TLSEXT_NAMETYPE_host_name, name);
116+
}
117+
118+
// Everything the TLS library has produced goes out on the socket.
119+
flush() {
120+
for (;;) {
121+
const read = ssl.BIO_read(this.wbio, this.buf, this.buf.length);
122+
if (read <= 0) break;
123+
this.socket.write(Buffer.from(this.buf.subarray(0, read)));
124+
}
125+
}
126+
127+
// Hands the library the next thing to arrive, false once nothing more will.
128+
async feed() {
129+
while (this.incoming.length === 0) {
130+
if (this.ended) return false;
131+
await new Promise((resolve) => { this.notify = resolve; });
132+
}
133+
const chunk = this.incoming.shift();
134+
ssl.BIO_write(this.rbio, chunk, chunk.length);
135+
return true;
136+
}
137+
138+
// `afterFirstRead` runs once the library has been handed the server's first
139+
// flight but before it has written anything in reply. For a TLS 1.3
140+
// HelloRetryRequest that is the gap between the two ClientHellos.
141+
async handshake(afterFirstRead) {
142+
let reads = 0;
143+
for (;;) {
144+
const result = ssl.SSL_do_handshake(this.ssl);
145+
this.flush();
146+
if (result === 1) return true;
147+
const error = ssl.SSL_get_error(this.ssl, result);
148+
if (error !== SSL_ERROR_WANT_READ && error !== SSL_ERROR_WANT_WRITE) return false;
149+
if (!await this.feed()) return false;
150+
if (++reads === 1) afterFirstRead?.();
151+
}
152+
}
153+
154+
async renegotiate() {
155+
if (ssl.SSL_renegotiate(this.ssl) !== 1) return false;
156+
return this.handshake();
157+
}
158+
159+
close() {
160+
this.socket.destroy();
161+
ssl.SSL_free(this.ssl);
162+
ssl.SSL_CTX_free(this.ctx);
163+
}
164+
}
165+
166+
const server = tls.createServer({
167+
key: fixtures.readKey('agent2-key.pem'),
168+
cert: fixtures.readKey('agent2-cert.pem'),
169+
// Offering only this group makes the server answer a TLS 1.3 client, whose
170+
// first key share is for a different curve, with a HelloRetryRequest.
171+
ecdhCurve: 'P-521',
172+
}, (socket) => {
173+
socket.on('error', () => {});
174+
socket.resume();
175+
});
176+
server.on('tlsClientError', () => {});
177+
178+
// Do a set of TLS 1.2 handshakes, asking for SNI `first` (or null) then
179+
// renegotiating for SNI `second` (or null), and returning whether that
180+
// was accepted. If `resume` is set, the `first` session is a resumption.
181+
async function renegotiating(first, second, { resume = false } = {}) {
182+
let session = null;
183+
let issuer = null;
184+
if (resume) {
185+
// The session must be used before the client it came from is closed:
186+
// freeing that client's context makes its sessions unresumable.
187+
issuer = new RawClient({ tls12Only: true });
188+
await issuer.connected;
189+
issuer.setServername(first);
190+
assert.strictEqual(await issuer.handshake(), true);
191+
session = ssl.SSL_get1_session(issuer.ssl);
192+
}
193+
const client = new RawClient({ tls12Only: true, session });
194+
await client.connected;
195+
client.setServername(first);
196+
assert.strictEqual(await client.handshake(), true);
197+
assert.strictEqual(ssl.SSL_session_reused(client.ssl) === 1, resume);
198+
client.setServername(second);
199+
const accepted = await client.renegotiate();
200+
client.close();
201+
if (resume) {
202+
issuer.close();
203+
ssl.SSL_SESSION_free(session);
204+
}
205+
return accepted;
206+
}
207+
208+
// One TLS 1.3 handshake, asking for `first` in the ClientHello and `second` in
209+
// the one that answers the HelloRetryRequest, returning whether it succeeded.
210+
async function retrying(first, second) {
211+
const client = new RawClient();
212+
await client.connected;
213+
client.setServername(first);
214+
const accepted = await client.handshake(() => client.setServername(second));
215+
client.close();
216+
return accepted;
217+
}
218+
219+
server.listen(0, common.mustCall(async () => {
220+
// Renegotiating must ask for exactly the name the first handshake did, or
221+
// none if that had none. Changing, dropping or adding a name is refused.
222+
for (const resume of [false, true]) {
223+
for (const [first, second, accepted] of [
224+
['agent1.example', 'agent1.example', true],
225+
[null, null, true],
226+
['agent1.example', 'agent3.example', false],
227+
['agent1.example', null, false],
228+
[null, 'agent1.example', false],
229+
]) {
230+
assert.strictEqual(await renegotiating(first, second, { resume }),
231+
accepted,
232+
`${resume ? 'resuming then ' : ''}renegotiating ` +
233+
`from ${first} to ${second}`);
234+
}
235+
}
236+
237+
// TLS1.3 HelloRetryRequest can't change SNI either:
238+
assert.strictEqual(await retrying('agent1.example', 'agent1.example'), true);
239+
assert.strictEqual(await retrying('agent1.example', 'agent3.example'), false);
240+
241+
server.close();
242+
}));

0 commit comments

Comments
 (0)