feat(s2n-quic-tls) Retrieve SNI when the 1-RTT keys are retrieved #2480
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maddeleine
commented
Feb 24, 2025
quic/s2n-quic/src/tests/snapshots/tests__pto__handshake_pto_timer_is_armed__events.snap
Outdated
Show resolved
Hide resolved
camshaft
reviewed
Feb 24, 2025
maddeleine
force-pushed
the
server_name
branch
from
3d622fb to
0d61e48
Compare
maddeleine
force-pushed
the
server_name
branch
from
af6ac1a to
e9d9aad
Compare
camshaft
approved these changes
Feb 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release Summary:
Resolved issues:
N/A
Description of changes:
Previously, when using s2n-tls as a TLS provider and operating as a server, we were emitting the server_name after a call to s2n_negotiate. Essentially we would read the Client Hello, generate 1-RTT keys, finish the call to s2n_negotiate and only then try to emit the server name.
Technically the server name can be emitted earlier, in the same place as where we get the 1-RTT keys. So that's what this PR does.
We historically emitted the server name in the place where we get the 1-RTT keys, only moving it later in this PR: #1238. I have no idea why we made that change. I know that s2n-tls didn't always get the 1-RTT keys as early as possible, so maybe originally 1-RTT was triggered later?
Call-outs:
I'm removing the emitted_server_name variable. That seems to be only useful if you are attempting to get the server name multiple times per handshake(we call s2n_negotiate multiple times per handshake.) The callback that provides the 1-RTT keys only triggers once.
Testing:
You can basically just see in the snapshot tests that now we're emitting the server name earlier.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.