Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WinHTTP] Let OS chose SSL/TLS protocol if not set to WinHttpHandler #113525

Merged
merged 3 commits into from
Mar 17, 2025

Conversation

ManickaP
Copy link
Member

@ManickaP ManickaP commented Mar 14, 2025

If WinHTTPHandler.SslProtocols remains untouched (SslProtocols.None), leave the protocol selection up to the WinHTTP.dll / OS.

Also remove shared SecurityProtocol.cs with constants, now that the only relevant constant is SslProtocols.None (replaced all occurrences of SystemDefaultSecurityProtocols).

@ManickaP ManickaP force-pushed the winhttp-sslprotocols branch from 7007533 to 01b9a79 Compare March 14, 2025 17:12
@ManickaP ManickaP changed the title [WinHTTP] Do not set SslProtocols and log ClientHello on the server. [WinHTTP] Let OS chose SSL/TLS protocol if not set to WinHttpHandler Mar 14, 2025
@ManickaP ManickaP added area-System.Net.Security and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it area-System.Net.Http labels Mar 14, 2025
@ManickaP ManickaP marked this pull request as ready for review March 14, 2025 17:18
@ManickaP ManickaP requested review from Copilot and a team March 14, 2025 17:18

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 enables the OS to select the SSL/TLS protocol when WinHTTPHandler.SslProtocols is left as SslProtocols.None and removes the now‑redundant shared SecurityProtocol constants.

  • Updated authentication methods in SslStream to pass SslProtocols.None.
  • Changed defaults in SslClientAuthenticationOptions and SslServerAuthenticationOptions to use SslProtocols.None.
  • Refactored WinHttpHandler TLS options handling and removed legacy Windows 7/2008R2 fallback logic, along with deleting the obsolete SecurityProtocol.cs.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs Updated client/server authentication methods to use SslProtocols.None for OS-managed protocol selection.
src/libraries/System.Net.Security/src/System/Net/Security/SslClientAuthenticationOptions.cs Modified default protocol from the shared constant to SslProtocols.None.
src/libraries/System.Net.Security/src/System/Net/Security/SslServerAuthenticationOptions.cs Modified default protocol from the shared constant to SslProtocols.None.
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs Adjusted TLS options configuration to early exit when SslProtocols.None is set, letting the OS handle protocol selection.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs Removed legacy fallback for Windows 7/2008R2 for default TLS protocols.
src/libraries/Common/src/System/Net/SecurityProtocol.cs Removed the obsolete SecurityProtocol constants file.
Copy link
Contributor

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

Copy link
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, thanks!

@ManickaP ManickaP merged commit 066ee97 into dotnet:main Mar 17, 2025
84 checks passed
@ManickaP ManickaP deleted the winhttp-sslprotocols branch March 17, 2025 19:01
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.

3 participants