Skip to content

feat(s2n-quic-tls) Retrieve SNI when the 1-RTT keys are retrieved - #2480

Merged
camshaft merged 5 commits into
mainfrom
server_name
Feb 27, 2025
Merged

feat(s2n-quic-tls) Retrieve SNI when the 1-RTT keys are retrieved#2480
camshaft merged 5 commits into
mainfrom
server_name

Conversation

@maddeleine

@maddeleine maddeleine commented Feb 22, 2025

Copy link
Copy Markdown
Contributor

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.

@maddeleine
maddeleine requested a review from camshaft February 24, 2025 22:31
Comment thread quic/s2n-quic-tls/src/session.rs

@camshaft camshaft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@camshaft
camshaft merged commit 8e7f14a into main Feb 27, 2025
@camshaft
camshaft deleted the server_name branch February 27, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants