Skip to content

Commit f9715fc

Browse files
authored
quic: defer server session emit until TLS ClientHello is processed
This ensures we don't fire session events for totally invalid TLS handshakes - fundamental errors, bad SNI/ALPN values, or anything else that our TLS config would reject. Instead, it means servers can access servername & alpnProtocol synchronously as soon as the event is fired - all key session data is available and it's immediately usable. We don't want to defer further to handshake completed, since that'd be an extra RT, and defeat 0RTT benefits entirely. ClientHello processed without errors is sufficient for now. This isn't a security mechanism. Existing structures will defer actually sending & receiving anything that's not marked explicitly as early data until the handshake completes anyway. Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: #64132 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent aedf2a4 commit f9715fc

10 files changed

Lines changed: 302 additions & 43 deletions

File tree

doc/api/quic.md

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1425,6 +1425,32 @@ will be silently dropped and `0n` returned. The local
14251425
`maxDatagramFrameSize` transport parameter (default: `1200` bytes) controls
14261426
what this endpoint advertises to the peer as its own maximum.
14271427

1428+
### `session.servername`
1429+
1430+
<!-- YAML
1431+
added: REPLACEME
1432+
-->
1433+
1434+
* Type: {string|boolean|null}
1435+
1436+
The SNI (Server Name Indication) host name associated with the session. This is
1437+
`null` before the client hello is processed. Once the hello has been
1438+
processed, this is either the host name string or `false` if the handshake
1439+
had no SNI.
1440+
1441+
### `session.alpnProtocol`
1442+
1443+
<!-- YAML
1444+
added: REPLACEME
1445+
-->
1446+
1447+
* Type: {string|null}
1448+
1449+
The negotiated ALPN protocol. This is `null` before the client hello is
1450+
processed. Once ALPN has been negotiated, this is the protocol string. ALPN
1451+
is mandatory in QUIC so this is never `false` on successful connections,
1452+
unlike `node:tls` where this is optional.
1453+
14281454
### `session.certificate`
14291455

14301456
<!-- YAML
@@ -3599,7 +3625,11 @@ added: v23.8.0
35993625
* `this` {quic.QuicEndpoint}
36003626
* `session` {quic.QuicSession}
36013627

3602-
The callback function that is invoked when a new session is initiated by a remote peer.
3628+
The callback function that is invoked when a new server session is initiated by
3629+
a remote peer. It is called once the peer's TLS `ClientHello` has been
3630+
processed, so the negotiated TLS parameters are immediately available when
3631+
the callback runs. Sessions whose handshake is rejected before this point are
3632+
never surfaced.
36033633

36043634
### Callback: `OnStreamCallback`
36053635

lib/internal/quic/quic.js

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,9 @@ setCallbacks({
747747
this[kOwner][kFinishClose](context, status);
748748
},
749749
/**
750-
* Called when the QuicEndpoint C++ handle receives a new server-side session
750+
* Called when a new server session is surfaced. The emit happens once the
751+
* session's ClientHello has been processed, so its servername/protocol
752+
* getters are already readable.
751753
* @param {object} session The QuicSession C++ handle
752754
*/
753755
onSessionNew(session) {
@@ -2730,6 +2732,8 @@ class QuicSession {
27302732
certificate: undefined,
27312733
peerCertificate: undefined,
27322734
ephemeralKeyInfo: undefined,
2735+
servername: undefined,
2736+
alpnProtocol: undefined,
27332737
localTransportParams: undefined,
27342738
remoteTransportParams: undefined,
27352739
};
@@ -2880,6 +2884,38 @@ class QuicSession {
28802884
}
28812885
}
28822886

2887+
/**
2888+
* The SNI servername: `null` until known, then the host name string, or
2889+
* `false` if the handshake produced no SNI.
2890+
* @type {string|boolean|null}
2891+
*/
2892+
get servername() {
2893+
assertIsQuicSession(this);
2894+
const inner = this.#inner;
2895+
if (inner.servername !== undefined) return inner.servername;
2896+
if (this.destroyed) return null;
2897+
// The handle returns null until the value is final; cache it only once it
2898+
// settles (to a string or `false`) so earlier reads re-query.
2899+
const value = this.#handle.getServername();
2900+
if (value !== null) inner.servername = value;
2901+
return value;
2902+
}
2903+
2904+
/**
2905+
* The negotiated ALPN protocol: `null` until known, then the protocol
2906+
* string. ALPN is mandatory for QUIC, so there is no "no ALPN" case.
2907+
* @type {string|null}
2908+
*/
2909+
get alpnProtocol() {
2910+
assertIsQuicSession(this);
2911+
const inner = this.#inner;
2912+
if (inner.alpnProtocol !== undefined) return inner.alpnProtocol;
2913+
if (this.destroyed) return null;
2914+
const value = this.#handle.getAlpnProtocol();
2915+
if (value !== null) inner.alpnProtocol = value;
2916+
return value;
2917+
}
2918+
28832919
/** @type {OnDatagramCallback} */
28842920
get ondatagram() {
28852921
assertIsQuicSession(this);

src/quic/endpoint.cc

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,8 @@ void Endpoint::AddSession(const CID& cid, BaseObjectPtr<Session> session) {
814814
// For server sessions, associate the client's original DCID (ocid) so
815815
// that 0-RTT packets arriving in a separate UDP datagram can be routed
816816
// to this session. This must happen after the session is added (so
817-
// FindSession can resolve the mapping) but before EmitNewSession (which
818-
// runs JS and may yield to libuv, allowing the 0-RTT packet to arrive).
817+
// FindSession can resolve the mapping) and before any JS runs (which
818+
// may yield to libuv, allowing the 0-RTT packet to arrive).
819819
if (session->is_server() && session->config().ocid) {
820820
AssociateCID(session->config().ocid, session->config().scid);
821821
}
@@ -825,22 +825,22 @@ void Endpoint::AddSession(const CID& cid, BaseObjectPtr<Session> session) {
825825
if (session->is_server() && session->config().retry_scid) {
826826
AssociateCID(session->config().retry_scid, session->config().scid);
827827
}
828-
// Increment the primary session count and ref the handle BEFORE
829-
// EmitNewSession. EmitNewSession calls into JS, which may close/destroy
830-
// the session synchronously. The session's ~Impl calls RemoveSession
831-
// which decrements the count. If we increment after EmitNewSession,
832-
// RemoveSession would see count=0 and the count would be permanently
833-
// off by one.
828+
// Increment the primary session count and ref the handle BEFORE any
829+
// JS can run for this session (the deferred EmitNewSession, or packet
830+
// processing callbacks). JS may close/destroy the session
831+
// synchronously; the session's ~Impl calls RemoveSession which
832+
// decrements the count. If we incremented after, RemoveSession would
833+
// see count=0 and the count would be permanently off by one.
834834
if (primary_session_count_++ == 0) {
835835
idle_timer_.Stop();
836836
udp_.Ref();
837837
}
838838
if (session->is_server()) {
839839
STAT_INCREMENT(Stats, server_sessions);
840-
// We only emit the new session event for server sessions.
841-
EmitNewSession(session);
842-
// It is important to note that the session may be closed/destroyed
843-
// when it is emitted here.
840+
// Note that we don't emit new sessions here - that's deferred until the
841+
// ClientHello has been processed (see Session::ReadPacket), so the
842+
// session is exposed to JS only once its SNI/ALPN are known and invalid
843+
// handshakes never surface.
844844
} else {
845845
STAT_INCREMENT(Stats, client_sessions);
846846
}
@@ -1998,6 +1998,12 @@ void Endpoint::EmitNewSession(const BaseObjectPtr<Session>& session) {
19981998
// the call to MakeCallback. If that's the case, the session object still
19991999
// exists but it is in a destroyed state. Care should be taken accessing
20002000
// session after this point.
2001+
2002+
// Deliver any stream events that were held until the stream was setup,
2003+
// e.g. 0-RTT streams from the first flight.
2004+
if (!session->is_destroyed()) {
2005+
session->ReplayDeferredEmits();
2006+
}
20012007
}
20022008

20032009
void Endpoint::EmitClose(CloseContext context, int status) {

src/quic/session.cc

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ uint64_t MaxDatagramPayload(uint64_t max_frame_size) {
184184
#define SESSION_JS_METHODS(V) \
185185
V(Destroy, destroy, SIDE_EFFECT) \
186186
V(GetRemoteAddress, getRemoteAddress, NO_SIDE_EFFECT) \
187+
V(GetServername, getServername, NO_SIDE_EFFECT) \
188+
V(GetAlpnProtocol, getAlpnProtocol, NO_SIDE_EFFECT) \
187189
V(GetLocalAddress, getLocalAddress, NO_SIDE_EFFECT) \
188190
V(GetCertificate, getCertificate, NO_SIDE_EFFECT) \
189191
V(GetEphemeralKeyInfo, getEphemeralKey, NO_SIDE_EFFECT) \
@@ -781,6 +783,8 @@ struct Session::Impl final : public MemoryRetainer {
781783
SocketAddress remote_address_;
782784
std::unique_ptr<Application> application_;
783785
StreamsMap streams_;
786+
// Emits deferred until after session setup is completed
787+
std::vector<std::function<void()>> deferred_emits_;
784788
TimerWrapHandle timer_;
785789
size_t send_scope_depth_ = 0;
786790
QuicError last_error_;
@@ -1001,6 +1005,41 @@ struct Session::Impl final : public MemoryRetainer {
10011005
session->Destroy();
10021006
}
10031007

1008+
// The SNI servername: null until the TLS parameters are final, then the
1009+
// host name string, or false if the handshake produced no SNI.
1010+
JS_METHOD(GetServername) {
1011+
auto env = Environment::GetCurrent(args);
1012+
Session* session;
1013+
ASSIGN_OR_RETURN_UNWRAP(&session, args.This());
1014+
if (session->is_destroyed() || !session->tls_info_ready()) {
1015+
return args.GetReturnValue().SetNull();
1016+
}
1017+
auto sn = session->tls_session().servername();
1018+
if (sn.empty()) return args.GetReturnValue().Set(false);
1019+
Local<Value> ret;
1020+
if (ToV8Value(env->context(), sn).ToLocal(&ret)) {
1021+
args.GetReturnValue().Set(ret);
1022+
}
1023+
}
1024+
1025+
// The negotiated ALPN protocol: null until the TLS parameters are final,
1026+
// then the protocol string.
1027+
JS_METHOD(GetAlpnProtocol) {
1028+
auto env = Environment::GetCurrent(args);
1029+
Session* session;
1030+
ASSIGN_OR_RETURN_UNWRAP(&session, args.This());
1031+
if (session->is_destroyed() || !session->tls_info_ready()) {
1032+
return args.GetReturnValue().SetNull();
1033+
}
1034+
auto proto = session->tls_session().protocol();
1035+
// QUIC requires ALPN
1036+
DCHECK(!proto.empty());
1037+
Local<Value> ret;
1038+
if (ToV8Value(env->context(), proto).ToLocal(&ret)) {
1039+
args.GetReturnValue().Set(ret);
1040+
}
1041+
}
1042+
10041043
JS_METHOD(GetRemoteAddress) {
10051044
auto env = Environment::GetCurrent(args);
10061045
Session* session;
@@ -2649,6 +2688,12 @@ const Session::Options& Session::options() const {
26492688
void Session::EmitQlog(uint32_t flags, std::string_view data) {
26502689
if (!env()->can_call_into_js()) return;
26512690

2691+
if (!is_destroyed() && must_defer_emits()) {
2692+
QueueDeferredEmit(
2693+
[this, flags, held = std::string(data)]() { EmitQlog(flags, held); });
2694+
return;
2695+
}
2696+
26522697
bool fin = (flags & NGTCP2_QLOG_WRITE_FLAG_FIN) != 0;
26532698

26542699
// Fun fact... ngtcp2 does not emit the final qlog statement until the
@@ -2771,6 +2816,16 @@ bool Session::ReadPacket(const uint8_t* data,
27712816
// Process deferred operations that couldn't run inside callback
27722817
// scopes (e.g., HTTP/3 GOAWAY handling that calls into JS).
27732818
application().PostReceive();
2819+
// Surface a server session to JS once its ClientHello has been
2820+
// processed (OnSelectAlpn fired: SNI + ALPN are known and reliable).
2821+
// Held first-flight events - including 0-RTT request streams - replay
2822+
// at emit. The !wrapped guard makes this fire exactly once, on
2823+
// whichever packet completes the ClientHello (so a multi-datagram
2824+
// ClientHello is handled correctly).
2825+
if (is_server() && hello_processed_ && !impl_->state()->wrapped &&
2826+
!is_destroyed()) {
2827+
endpoint().EmitNewSession(BaseObjectPtr<Session>(this));
2828+
}
27742829
}
27752830
return true;
27762831
}
@@ -3460,6 +3515,36 @@ void Session::set_wrapped() {
34603515
impl_->state()->wrapped = 1;
34613516
}
34623517

3518+
bool Session::must_defer_emits() const {
3519+
// Server sessions are surfaced to JS (via the deferred new-session emit)
3520+
// only after the ClientHello has been processed and wrapped; anything
3521+
// emitted before then has no JS wrapper to receive it and must be held
3522+
// for replay.
3523+
return is_server() && !impl_->state()->wrapped;
3524+
}
3525+
3526+
bool Session::tls_info_ready() const {
3527+
// hello_processed_ is set server-side, handshake_completed covers
3528+
// the client. Together they mark the point when SNI/ALPN are final.
3529+
return hello_processed_ || impl_->state()->handshake_completed;
3530+
}
3531+
3532+
void Session::QueueDeferredEmit(std::function<void()> fn) {
3533+
impl_->deferred_emits_.emplace_back(std::move(fn));
3534+
}
3535+
3536+
void Session::ReplayDeferredEmits() {
3537+
if (is_destroyed()) return;
3538+
DCHECK(impl_->state()->wrapped);
3539+
// Runs synchronously immediately after the new-session callback
3540+
// returns (still within first-flight processing).
3541+
auto emits = std::move(impl_->deferred_emits_);
3542+
for (auto& emit : emits) {
3543+
if (is_destroyed()) return;
3544+
emit();
3545+
}
3546+
}
3547+
34633548
void Session::set_priority_supported(bool on) {
34643549
DCHECK(!is_destroyed());
34653550
impl_->state()->priority_supported = on ? 1 : 0;
@@ -3769,6 +3854,10 @@ bool Session::HandshakeCompleted() {
37693854

37703855
Debug(this, "Session handshake completed");
37713856
impl_->state()->handshake_completed = 1;
3857+
// This implies fully completing a handshake without setting hello_processed
3858+
// (set during ALPN negotiation). Should be impossible unless ALPN flow is
3859+
// changed drastically, but good to check as it'd lose sessions.
3860+
DCHECK(!is_server() || hello_processed_);
37723861

37733862
STAT_RECORD_TIMESTAMP(Stats, handshake_completed_at);
37743863
SetStreamOpenAllowed();
@@ -3967,6 +4056,7 @@ void Session::set_max_datagram_size(uint16_t size) {
39674056

39684057
void Session::EmitGoaway(stream_id last_stream_id) {
39694058
if (is_destroyed()) return;
4059+
if (DeferEmit([this, last_stream_id] { EmitGoaway(last_stream_id); })) return;
39704060
if (!env()->can_call_into_js()) return;
39714061

39724062
CallbackScope<Session> cb_scope(this);
@@ -3981,6 +4071,14 @@ void Session::EmitGoaway(stream_id last_stream_id) {
39814071

39824072
void Session::EmitDatagram(Store&& datagram, DatagramReceivedFlags flag) {
39834073
DCHECK(!is_destroyed());
4074+
4075+
if (must_defer_emits()) {
4076+
QueueDeferredEmit([this, datagram = std::move(datagram), flag]() mutable {
4077+
EmitDatagram(std::move(datagram), flag);
4078+
});
4079+
return;
4080+
}
4081+
39844082
if (!env()->can_call_into_js()) return;
39854083

39864084
CallbackScope<Session> cbv_scope(this);
@@ -3996,6 +4094,8 @@ void Session::EmitDatagram(Store&& datagram, DatagramReceivedFlags flag) {
39964094
void Session::EmitDatagramStatus(datagram_id id, quic::DatagramStatus status) {
39974095
DCHECK(!is_destroyed());
39984096

4097+
if (DeferEmit([this, id, status] { EmitDatagramStatus(id, status); })) return;
4098+
39994099
if (!env()->can_call_into_js()) return;
40004100

40014101
CallbackScope<Session> cb_scope(this);
@@ -4157,6 +4257,7 @@ void Session::EmitSessionTicket(Store&& ticket) {
41574257

41584258
void Session::EmitApplication() {
41594259
if (is_destroyed()) return;
4260+
if (DeferEmit([this] { EmitApplication(); })) return;
41604261
if (!env()->can_call_into_js()) return;
41614262

41624263
if (!has_application()) {
@@ -4227,6 +4328,10 @@ void Session::EmitNewToken(const uint8_t* token, size_t len) {
42274328
void Session::EmitStream(const BaseObjectWeakPtr<Stream>& stream) {
42284329
DCHECK(!is_destroyed());
42294330

4331+
if (DeferEmit([this, stream] { EmitStream(stream); })) return;
4332+
4333+
if (!stream) return;
4334+
42304335
if (!env()->can_call_into_js()) return;
42314336
CallbackScope<Session> cb_scope(this);
42324337

@@ -4282,6 +4387,14 @@ void Session::EmitVersionNegotiation(const ngtcp2_pkt_hd& hd,
42824387

42834388
void Session::EmitOrigins(std::vector<std::string>&& origins) {
42844389
DCHECK(!is_destroyed());
4390+
4391+
if (must_defer_emits()) {
4392+
QueueDeferredEmit([this, origins = std::move(origins)]() mutable {
4393+
EmitOrigins(std::move(origins));
4394+
});
4395+
return;
4396+
}
4397+
42854398
if (!HasListenerFlag(impl_->state()->listener_flags,
42864399
SessionListenerFlags::ORIGIN))
42874400
return;
@@ -4307,11 +4420,17 @@ void Session::EmitOrigins(std::vector<std::string>&& origins) {
43074420

43084421
void Session::EmitKeylog(const char* line) {
43094422
DCHECK(!is_destroyed());
4423+
4424+
if (must_defer_emits()) {
4425+
QueueDeferredEmit(
4426+
[this, str = std::string(line)]() { EmitKeylog(str.c_str()); });
4427+
return;
4428+
}
4429+
43104430
if (!env()->can_call_into_js()) return;
43114431

4312-
auto str = std::string(line);
43134432
Local<Value> argv[] = {Undefined(env()->isolate())};
4314-
if (!ToV8Value(env()->context(), str).ToLocal(&argv[0])) {
4433+
if (!ToV8Value(env()->context(), std::string(line)).ToLocal(&argv[0])) {
43154434
Debug(this, "Failed to convert keylog line to V8 string");
43164435
return;
43174436
}

0 commit comments

Comments
 (0)