Skip to content

improve accuracy of SslClientHelloInfo#128244

Merged
rzikm merged 4 commits into
dotnet:mainfrom
wfurt:tlsInfo
May 19, 2026
Merged

improve accuracy of SslClientHelloInfo#128244
rzikm merged 4 commits into
dotnet:mainfrom
wfurt:tlsInfo

Conversation

@wfurt
Copy link
Copy Markdown
Member

@wfurt wfurt commented May 15, 2026

We construct SslClientHelloInfo when using ServerOptionsSelectionCallback delegate. That was primarily meant to SNI but is also have bit mask of client supported versions. The change makes sure we process all of them from the client hello.

@wfurt wfurt added this to the 11.0.0 milestone May 15, 2026
@wfurt wfurt requested a review from rzikm May 15, 2026 04:24
@wfurt wfurt self-assigned this May 15, 2026
Copilot AI review requested due to automatic review settings May 15, 2026 04:24
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves SslStream server-side ClientHello handling so ServerOptionsSelectionCallback receives TLS versions advertised via the supported_versions extension, especially for TLS 1.3.

Changes:

  • Enables ClientHello supported-version parsing when a server options callback is configured.
  • Adds a TLS 1.3 regression test verifying the callback sees Tls13.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs Adds supported-version parsing for the server options callback path.
src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs Adds regression coverage for TLS 1.3 version visibility in the SNI/options callback.

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs Outdated
Copilot AI review requested due to automatic review settings May 15, 2026 05:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/libraries/System.Net.Security/src/System/Net/Security/TlsFrameHelper.cs:531

  • Now that Versions is enabled for the SNI callback path, this parser should reject malformed supported_versions vectors before exposing them to user code. TryGetSupportedVersionsFromExtension currently returns true for a zero-length vector and for odd byte counts (it just drops the trailing byte), even though the TLS vector must contain one or more 2-byte ProtocolVersion values. That can make SslClientHelloInfo.SslProtocols look valid but incomplete for malformed ClientHello messages.
                else if (extensionType == ExtensionType.SupportedVersions && (options & ProcessingOptions.Versions) != 0)
                {
                    if (!TryGetSupportedVersionsFromExtension(extensionData, out SslProtocols versions))
                    {
                        return false;

Comment thread src/libraries/System.Net.Security/src/System/Net/Security/TlsFrameHelper.cs Outdated
Copy link
Copy Markdown
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM

@rzikm
Copy link
Copy Markdown
Member

rzikm commented May 15, 2026

cc @MihaZupan

@rzikm
Copy link
Copy Markdown
Member

rzikm commented May 19, 2026

/ba-g test failure is unrelated

@rzikm rzikm merged commit 86db03a into dotnet:main May 19, 2026
62 of 93 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot modified the milestones: 11.0.0, 11.0-preview6 May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants