Skip to content

quic: defer server session emit until TLS ClientHello is processed#64132

Open
pimterry wants to merge 1 commit into
nodejs:mainfrom
pimterry:defer-quic-session-emit
Open

quic: defer server session emit until TLS ClientHello is processed#64132
pimterry wants to merge 1 commit into
nodejs:mainfrom
pimterry:defer-quic-session-emit

Conversation

@pimterry

@pimterry pimterry commented Jun 25, 2026

Copy link
Copy Markdown
Member

Extracted from #63995. It's not identical to the implementation there, I did a little more cleanup and refactoring here, and dropped some parts that aren't relevant without the latest changes in that PR, but it's reviewable standalone.

This change 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. New getters are exposed for that. This also 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.

It does so without waiting for any more round trips that before, in any normal flow: it just defers the session event to the end of the client hello processing which should be momentarily after the previous behaviour. In very strange flows (if the first flight from the client doesn't contain the full client hello) then this could introduce a more significant delay, but AFAICT no normal client should ever do that (and we'd have to do some kind of wait regardless eventually if they did - we can't really do anything useful without a complete client hello).

This is just intended to improve UX and smooth the path towards dynamic Application selection (as used in #63995). A few things this doesn't do:

  • It doesn't wait for handshake completion. It's just processing of the client hello. Completion would imply an extra round trip, and thereby defeat 0RTT benefits entirely.
  • It doesn't improve security (though it helps avoid some footguns). Existing mechanisms already ensure we can't read or write data that isn't correctly within the TLS handshake (deferred data on new connections until the handshake completes, or allowing early data that's explicitly marked as such on resumed connections)
  • It doesn't change client behaviour. In the client case, the session is available on creation, and you always have to wait for opened. Dynamic app selection there would wait for full handshake completion anyway, since that's the only way to know the confirmed ALPN, which you'll want in most cases (though we don't currently support multiple ALPN for clients anyway). All deferred for later.
  • It doesn't expose these failed TLS sessions in any way. In future we will probably want a tlsClientError analogue, but imo that was true before as well and we can continue to defer it for now. Most servers aren't that interested in clients who fail to connect, it's not a top priority.

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>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants