Commit ae94cd890a7 for nodejs
commit ae94cd890a7039a6d3e6db2373127763c700edac
Author: Tim Perry <pimterry@gmail.com>
Date: Thu Oct 29 18:43:00 2026 +0100
tls: don't allow TLS resumption to change SNI
Signed-off-by: Tim Perry <pimterry@gmail.com>
PR-URL: https://github.com/nodejs-private/node-private/pull/1030
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
CVE-ID: CVE-2026-123872
diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
index f28ce556df3..21decb34e11 100644
--- a/src/crypto/crypto_context.cc
+++ b/src/crypto/crypto_context.cc
@@ -2213,8 +2213,11 @@ void SecureContext::SetSessionIdContext(
reinterpret_cast<const unsigned char*>(*sessionIdContext);
unsigned int sid_ctx_len = sessionIdContext.length();
- if (SSL_CTX_set_session_id_context(sc->ctx_.get(), sid_ctx, sid_ctx_len) == 1)
+ int ok = SSL_CTX_set_session_id_context(sc->ctx_.get(), sid_ctx, sid_ctx_len);
+ if (ok == 1) {
+ sc->session_id_context_.assign(*sessionIdContext, sid_ctx_len);
return;
+ }
Local<String> message;
diff --git a/src/crypto/crypto_context.h b/src/crypto/crypto_context.h
index a762678ea2d..5073897251a 100644
--- a/src/crypto/crypto_context.h
+++ b/src/crypto/crypto_context.h
@@ -55,6 +55,7 @@ class SecureContext final : public BaseObject {
// True if CA certificates were added with addCACert(), as opposed to the
// context relying on the default root store.
bool has_own_ca() const { return has_own_ca_; }
+ const std::string& session_id_context() const { return session_id_context_; }
// Non-const ctx() that allows for non-default initialization of
// the SecureContext.
@@ -192,6 +193,7 @@ class SecureContext final : public BaseObject {
// Non-owning cache for SSL_CTX_get_cert_store(ctx_.get())
X509_STORE* own_cert_store_cache_ = nullptr;
bool has_own_ca_ = false;
+ std::string session_id_context_;
unsigned char ticket_key_name_[16];
unsigned char ticket_key_aes_[16];
diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc
index 333da5d7e72..4c848425771 100644
--- a/src/crypto/crypto_tls.cc
+++ b/src/crypto/crypto_tls.cc
@@ -121,6 +121,21 @@ SSL_SESSION* GetSessionCallback(
// the handshake is suspended here and resumed once JS has answered.
ClientHelloResult EarlyClientHelloCallback(const ClientHelloContext& hello) {
TLSWrap* w = static_cast<TLSWrap*>(SSL_get_app_data(hello.ssl()));
+
+ auto servername = hello.servername();
+ // If we can't parse the hello SNI, reject it explicitly:
+ if (!servername.has_value()) {
+ hello.set_alert(SSL_AD_DECODE_ERROR);
+ return ClientHelloResult::kFail;
+ }
+
+ // Session resumption must only be allowed for sessions with exactly the same
+ // SNI. We use the SNI to derive the session id, so no handshake can resume
+ // from a mismatched SNI value. Mismatches fall back to full handshakes:
+ if (!w->BindSessionsToServerName(*servername)) {
+ return ClientHelloResult::kFail;
+ }
+
if (!w->should_suspend_for_client_hello()) {
return ClientHelloResult::kContinue;
}
@@ -1569,8 +1584,8 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
Local<Value> ctx = p->object()->Get(env->context(), env->sni_context_string())
.FromMaybe(Local<Value>());
- if (ctx.IsEmpty() || !ctx->IsObject()) [[unlikely]]
- return SSL_TLSEXT_ERR_NOACK;
+ // Acknowlege the name so it's correctly stored on the session:
+ if (ctx.IsEmpty() || !ctx->IsObject()) return SSL_TLSEXT_ERR_OK;
if (!env->secure_context_constructor_template()->HasInstance(ctx)) {
// Failure: incorrect SNI context object
@@ -1590,6 +1605,24 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
return SSL_TLSEXT_ERR_OK;
}
+bool TLSWrap::BindSessionsToServerName(std::string_view servername) {
+ if (sc_->session_id_context().empty()) return true;
+
+ // Server names can't contain NULs, so a NUL keeps the parts apart.
+ std::string input = sc_->session_id_context();
+ input += '\0';
+ input += servername;
+ // Hash this to guarantee the value fits in 32 bytes while keeping
+ // values distinct:
+ auto digest = ncrypto::hashDigest(
+ {reinterpret_cast<const unsigned char*>(input.data()), input.size()},
+ ncrypto::Digest::SHA256);
+ return digest && SSL_set_session_id_context(
+ ssl_.get(),
+ static_cast<const unsigned char*>(digest.get()),
+ digest.size()) == 1;
+}
+
int TLSWrap::SetCACerts(SecureContext* sc) {
int err = SSL_set1_verify_cert_store(ssl_.get(),
SSL_CTX_get_cert_store(sc->ctx().get()));
diff --git a/src/crypto/crypto_tls.h b/src/crypto/crypto_tls.h
index f3e0dfff9bc..cb57b7346eb 100644
--- a/src/crypto/crypto_tls.h
+++ b/src/crypto/crypto_tls.h
@@ -33,6 +33,7 @@
#include <openssl/ssl.h>
#include <string>
+#include <string_view>
#include <vector>
namespace node {
@@ -94,6 +95,7 @@ class TLSWrap : public AsyncWrap,
flags_.awaiting_new_session = on;
}
inline void enable_session_callbacks() { flags_.session_callbacks = true; }
+ bool BindSessionsToServerName(std::string_view servername);
inline bool is_server() const { return kind_ == Kind::kServer; }
inline bool is_client() const { return kind_ == Kind::kClient; }
inline bool is_awaiting_new_session() const {
diff --git a/test/parallel/test-tls-resume-servername.js b/test/parallel/test-tls-resume-servername.js
new file mode 100644
index 00000000000..927ee597891
--- /dev/null
+++ b/test/parallel/test-tls-resume-servername.js
@@ -0,0 +1,103 @@
+'use strict';
+
+// A resumed session skips authentication, so it may only be resumed for the
+// server name it was authenticated for. Otherwise a client trusted by one host
+// could resume into another. Asking for a mismatched name gets a full
+// handshake, which authenticates the client from scratch like any other.
+
+const common = require('../common');
+
+if (!common.hasCrypto)
+ common.skip('missing crypto');
+
+const assert = require('node:assert');
+const { isBoringSSL } = require('../common/crypto');
+const tls = require('node:tls');
+const fixtures = require('../common/fixtures');
+
+// What the server reported for each connection it accepted, in order.
+const seen = [];
+
+const server = tls.createServer({
+ key: fixtures.readKey('agent2-key.pem'),
+ cert: fixtures.readKey('agent2-cert.pem'),
+}, (socket) => {
+ seen.push({ servername: socket.servername, reused: socket.isSessionReused() });
+ socket.end();
+});
+
+// Connects TLS with options, resolves with the raw issued session when the
+// conn closes. We need the raw internal session so we can trick Node into
+// (ab)using this for the wrong hostname on resumed connections later.
+function connect(options) {
+ return new Promise((resolve) => {
+ let session;
+ const socket = tls.connect({
+ port: server.address().port,
+ rejectUnauthorized: false,
+ ...options,
+ });
+ const { onnewsession } = socket._handle;
+ socket._handle.onnewsession = function(id, raw) {
+ session ??= raw;
+ return onnewsession.call(this, id, raw);
+ };
+ socket.on('session', () => {});
+ socket.on('close', () => resolve(session));
+ socket.resume();
+ });
+}
+
+// Connecting by IP sends no server name.
+const noName = { host: '127.0.0.1' };
+
+server.listen(0, common.mustCall(async () => {
+ for (const maxVersion of ['TLSv1.2', 'TLSv1.3']) {
+ seen.length = 0;
+ const named = await connect({ servername: 'agent1.example', maxVersion });
+ const unnamed = await connect({ ...noName, maxVersion });
+
+ // Resuming as the same name, or with no name both times, is fine.
+ await connect({ servername: 'agent1.example', maxVersion, session: named });
+ await connect({ ...noName, maxVersion, session: unnamed });
+
+ // Asking for a different name gets a full handshake instead.
+ await connect({ servername: 'agent3.example', maxVersion, session: named });
+ // Going from no SNI to any name gets a full handshake instead.
+ await connect({ servername: 'agent1.example', maxVersion, session: unnamed });
+
+ assert.deepStrictEqual(seen, [
+ { servername: 'agent1.example', reused: false },
+ { servername: false, reused: false },
+ { servername: 'agent1.example', reused: true },
+ { servername: false, reused: true },
+ { servername: 'agent3.example', reused: false },
+ { servername: 'agent1.example', reused: false },
+ ], maxVersion);
+ }
+
+ // BoringSSL doesn't support TLS 1.3 external PSK.
+ if (!isBoringSSL) {
+ // Make sure fresh PSK & PSK resumption work correctly too - this is risky as
+ // TLS1.3 PSK works by pretending to be resumption (but it's not).
+ const secret = Buffer.alloc(32, 1);
+ const ciphers = 'TLS_AES_128_GCM_SHA256';
+ const psk = tls.createServer({ ciphers, pskCallback: () => secret },
+ common.mustCall((socket) => {
+ assert.strictEqual(socket.servername, 'psk.example');
+ socket.end();
+ }, 2));
+ await new Promise((resolve) => psk.listen(0, resolve));
+ const pskOptions = {
+ port: psk.address().port,
+ servername: 'psk.example',
+ ciphers,
+ pskCallback: () => ({ psk: secret, identity: 'client' }),
+ };
+ const ticket = await connect(pskOptions);
+ assert.ok(ticket);
+ await connect({ ...pskOptions, session: ticket });
+ psk.close();
+ }
+ server.close();
+}));