Revert changes to HttpClient/SslStream certificate revocation check mode #118456
Conversation
…eck mode to Online (dotnet#116098)" This reverts commit 6a4b7e3.
There was a problem hiding this comment.
Pull Request Overview
This PR reverts changes that made X509RevocationMode.Online the default certificate revocation check mode for SSL/TLS connections in HttpClient and SslStream. The revert restores the previous default of X509RevocationMode.NoCheck to address compatibility issues with certificates that lack proper revocation checking infrastructure (such as HTTPS proxy certificates).
Key Changes:
- Reverted default revocation check mode from
Onlineback toNoCheck - Removed AppContext switch infrastructure that was added to control the default behavior
- Updated tests to reflect the reverted default behavior
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
SslClientAuthenticationOptions.cs |
Reverted default _checkCertificateRevocation from SslAuthenticationOptions.DefaultRevocationMode to X509RevocationMode.NoCheck |
SslServerAuthenticationOptions.cs |
Reverted default _checkCertificateRevocation from SslAuthenticationOptions.DefaultRevocationMode to X509RevocationMode.NoCheck |
SslAuthenticationOptions.cs |
Removed DefaultRevocationMode property that used AppContext switch |
WinHttpHandler.cs |
Reverted default _checkCertificateRevocationList from DefaultCertificateRevocationCheck to false |
| Various test files | Updated test expectations to reflect NoCheck as default, removed AppContext switch tests, and updated certificate handling patterns |
| Project files | Removed references to AppContextSwitchHelper.cs which is no longer needed |
src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs
Outdated
Show resolved
Hide resolved
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
|
do we need to revert everything? It seems like we can flip just the default back - especially if we pan to move forward eventually. |
|
I agree with Tomas... rather than "revert" the change, which had some good test cleanup in it, it's really revert the intent by just changing the default (and presumably adjusting a few tests to account for it). |
…heck mode to Online (dotnet#116098)" This reverts commit defd136.
yeah was too tired yesterday and didn't remember that that PR had those |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
Outdated
Show resolved
Hide resolved
…tp/WinHttpHandler.cs
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #117681.
We don't have a good story for X509 certificates without CRL Distribution Points and AIA:OCSP extensions (i.e. certificates for which the revocation cannot be checked online). Such as those used by HTTPS proxies (Fiddler, corporate HTTPS proxies, ...).
We may try again in .NET 11 timeline if we figure out a soluiton for these cases.