diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 3a26cfbdcab52e..af84dc31b076f4 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -2941,8 +2941,21 @@ std::optional SSLPointer::GetServerName( const SSL* ssl) { if (ssl == nullptr) return std::nullopt; auto res = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); - if (res == nullptr) return std::nullopt; - return res; + if (res != nullptr) return res; + +#ifndef OPENSSL_IS_BORINGSSL + // SSL_get_servername() returns NULL on server-side TLS 1.3 resumed sessions + // because it reads from ssl->ext.hostname rather than the session. Fall back + // to the session hostname which OpenSSL persists during the original + // handshake and serializes into tickets. + const SSL_SESSION* sess = SSL_get_session(ssl); + if (sess != nullptr) { + res = SSL_SESSION_get0_hostname(sess); + if (res != nullptr) return res; + } +#endif + + return std::nullopt; } std::optional SSLPointer::getServerName() const { diff --git a/src/crypto/crypto_context.h b/src/crypto/crypto_context.h index b6801fc0b40708..1f5ed2a9d21a0d 100644 --- a/src/crypto/crypto_context.h +++ b/src/crypto/crypto_context.h @@ -53,7 +53,6 @@ class SecureContext final : public BaseObject { void SetKeylogCallback(KeylogCb cb); void SetNewSessionCallback(NewSessionCb cb); void SetSelectSNIContextCallback(SelectSNIContextCb cb); - inline const ncrypto::X509Pointer& issuer() const { return issuer_; } inline const ncrypto::X509Pointer& cert() const { return cert_; } diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 5c80184ffb5eae..e67d2d608d9edb 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -1398,6 +1398,19 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) { !Set(env, p->GetOwner(), env->servername_string(), servername.value())) return SSL_TLSEXT_ERR_NOACK; +#ifndef OPENSSL_IS_BORINGSSL + // OpenSSL only persists the hostname in the session when this callback + // returns SSL_TLSEXT_ERR_OK (i.e. when an SNI context switch occurs). + // Explicitly store it so that SSL_SESSION_get0_hostname() works on resumed + // sessions regardless of the callback return value. + { + SSL_SESSION* sess = SSL_get_session(s); + const char* sni = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name); + if (sess != nullptr && sni != nullptr) + SSL_SESSION_set1_hostname(sess, sni); + } +#endif + Local ctx = p->object()->Get(env->context(), env->sni_context_string()) .FromMaybe(Local()); diff --git a/test/parallel/test-tls-servername-session-resumption.js b/test/parallel/test-tls-servername-session-resumption.js new file mode 100644 index 00000000000000..e490860952d8c4 --- /dev/null +++ b/test/parallel/test-tls-servername-session-resumption.js @@ -0,0 +1,80 @@ +'use strict'; + +// Verify that servername (SNI) is preserved on resumed TLS sessions. +// Regression test for https://github.com/nodejs/node/issues/59202 + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// The fix relies on SSL_SESSION_get0_hostname which BoringSSL may not support. +if (process.features.openssl_is_boringssl) + common.skip('BoringSSL does not support SSL_SESSION_get0_hostname'); + +const assert = require('assert'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const SERVERNAME = 'agent1.example.com'; + +function test(minVersion, maxVersion) { + const serverOptions = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + minVersion, + maxVersion, + }; + + let serverConns = 0; + const server = tls.createServer(serverOptions, common.mustCall((socket) => { + assert.strictEqual(socket.servername, SERVERNAME); + serverConns++; + // Don't end the socket immediately on the first connection — the client + // needs time to receive the TLS 1.3 NewSessionTicket message. + if (serverConns === 2) + socket.end(); + }, 2)); + + server.listen(0, common.mustCall(function() { + const port = server.address().port; + + // First connection: establish a session with an SNI servername. + const client1 = tls.connect({ + port, + servername: SERVERNAME, + rejectUnauthorized: false, + minVersion, + maxVersion, + }, common.mustCall()); + + client1.once('session', common.mustCall((session) => { + client1.end(); + + // Second connection: resume the session and verify the servername is + // preserved on the server side. + const client2 = tls.connect({ + port, + servername: SERVERNAME, + rejectUnauthorized: false, + session, + minVersion, + maxVersion, + }, common.mustCall(() => { + assert.strictEqual(client2.isSessionReused(), true); + client2.end(); + })); + + client2.on('close', common.mustCall(() => server.close())); + })); + + client1.resume(); + })); +} + +// TLS 1.3 — the primary bug: SSL_get_servername() returns NULL on +// server-side resumed sessions. +test('TLSv1.3', 'TLSv1.3'); + +// TLS 1.2 — OpenSSL handles this natively, but verify the fallback path +// doesn't break it. +test('TLSv1.2', 'TLSv1.2');