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

[release/8.0] Don't unset ALPN list pointer during ALPN selection callback. #99670

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 13, 2024

Backport of #99357 to release/8.0-staging
Fixes #99289

/cc @rzikm

Customer Impact

The issue prevents adoption of .NET 8 in scenarios where FIPS conformance is required on Linux. Clients attempting to connect to FIPS .NET 8 Linux server will see TLS protocol errors.

The only workarounds are disabling TLS 1.3 or allowing algorithms not approved by FIPS on the server running the application. These workarounds go against security best-practices and may require system-wide configuration settings which may not be accessible in e.g. cloud deployment scenarios.

Reported by 2 customers:

Technical details

The issue in .NET is not related to FIPS specifically. When connecting to server via TLS 1.3, clients take a guess at an algorithm to be used when negotiating the shared secret needed for deriving the encryption keys. If server does not agree with the clients guess, it sends a HelloRetryRequest message with a different algorithm suggestion. The client then retries the handshake process.

The typical default guess of clients (x25519) is not a FIPS approved algorithm, and thus servers configured to use only FIPS-approved algorithms would always reply with HelloRetryRequest.

Another part of the TLS handshake process is Application Layer Protocol Negotiation (ALPN), which allows negotiating e.g. the HTTP version as during the TLS handshake (client sends a list of protocols it is willing to negotiate, server picks one of these). The bug manifests because .NET discards the list of accepted ALPN protocols on the server after receiving the first ClientHello. However, due to server sending a HelloRetryRequest, client sends another ClientHello message and at this point, the server behaves as if there was no commonly understood application layer protocol and terminates the handshake.

Regression

Yes, regression against .NET 6.0, introduced in PR #57079 (.NET 7).

Testing

Tested manually. It is not possible to implement automated test (the API on OpenSSL is not exposed via .NET APIs). Editing system-wide configuration is necessary for reproducing the issue.

The issue was missed previously because the situation where server replies with HelloRetryRequest is rare.

Risk

Low, the issue is well understood and tested and the fix is very localized to the rare HelloRetryRequest code path and does not affect other code paths.

Copy link
Contributor

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

@rzikm rzikm changed the title [release/8.0-staging] Don't unset ALPN list pointer during ALPN selection callback. [release/8.0] Don't unset ALPN list pointer during ALPN selection callback. Mar 13, 2024
@michaelwildvarian
Copy link

@rzikm, any chance of this backport making it into the April patch release of .NET 8?

@rzikm
Copy link
Member

rzikm commented Mar 13, 2024

@rzikm, any chance of this backport making it into the April patch release of .NET 8?

Unfortunately no, the deadline for that one was on Monday.

@rzikm rzikm requested review from wfurt and karelz March 13, 2024 12:22
@rzikm rzikm added the Servicing-consider Issue for next servicing release review label Mar 18, 2024
@rzikm
Copy link
Member

rzikm commented Mar 18, 2024

The CI failure is unrelated

@michaelwildvarian
Copy link

@rzikm any news on this making it into the May patch release?

@rzikm
Copy link
Member

rzikm commented Mar 25, 2024

@rzikm any news on this making it into the May patch release?

The deadline for code changes for that release is mid April, I am keeping an eye on this so that it makes it there.

@karelz karelz removed the Servicing-consider Issue for next servicing release review label Mar 26, 2024
@karelz karelz added this to the 8.0.x milestone Mar 26, 2024
@karelz karelz added the os-linux Linux OS (any supported distro) label Mar 27, 2024
@karelz
Copy link
Member

karelz commented Mar 27, 2024

FIPS on Linux is important scenario. It is a regression against .NET 6.0. We should fix it.

@karelz karelz added the Servicing-consider Issue for next servicing release review label Mar 27, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 28, 2024
@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.5 Mar 28, 2024
@rzikm
Copy link
Member

rzikm commented Apr 2, 2024

CI failure is unrelated. System.Net.Security-related pipeliens pass

@rzikm rzikm merged commit 3ea18f2 into release/8.0-staging Apr 2, 2024
106 of 123 checks passed
@jkotas jkotas deleted the backport/pr-99357-to-release/8.0-staging branch April 17, 2024 01:26
@github-actions github-actions bot locked and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Security os-linux Linux OS (any supported distro) Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants