Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions deps/ncrypto/ncrypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2941,8 +2941,21 @@ std::optional<const std::string_view> 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<const std::string_view> SSLPointer::getServerName() const {
Expand Down
1 change: 0 additions & 1 deletion src/crypto/crypto_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_; }

Expand Down
13 changes: 13 additions & 0 deletions src/crypto/crypto_tls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Value> ctx = p->object()->Get(env->context(), env->sni_context_string())
.FromMaybe(Local<Value>());

Expand Down
80 changes: 80 additions & 0 deletions test/parallel/test-tls-servername-session-resumption.js
Original file line number Diff line number Diff line change
@@ -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');