Skip to content

Commit defd136

Browse files
committed
Revert "Change HttpClient/SslStream default certificate revocation check mode to Online (#116098)"
This reverts commit 6a4b7e3.
1 parent d34cad0 commit defd136

23 files changed

+427
-450
lines changed

src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs

Lines changed: 47 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Net.Security;
54
using System.Runtime.CompilerServices;
65
using System.Runtime.InteropServices;
76
using System.Security.Cryptography;
@@ -38,6 +37,29 @@ public static partial class Certificates
3837
private static readonly X509BasicConstraintsExtension s_eeConstraints =
3938
new X509BasicConstraintsExtension(false, false, 0, false);
4039

40+
private static X509Certificate2 s_dynamicServerCertificate;
41+
private static X509Certificate2Collection s_dynamicCaCertificates;
42+
private static object certLock = new object();
43+
44+
45+
// These Get* methods make a copy of the certificates so that consumers own the lifetime of the
46+
// certificates handed back. Consumers are expected to dispose of their certs when done with them.
47+
48+
public static X509Certificate2 GetDynamicServerCerttificate(X509Certificate2Collection? chainCertificates)
49+
{
50+
lock (certLock)
51+
{
52+
if (s_dynamicServerCertificate == null)
53+
{
54+
CleanupCertificates();
55+
(s_dynamicServerCertificate, s_dynamicCaCertificates) = GenerateCertificates("localhost", nameof(Configuration) + nameof(Certificates));
56+
}
57+
58+
chainCertificates?.AddRange(s_dynamicCaCertificates);
59+
return new X509Certificate2(s_dynamicServerCertificate);
60+
}
61+
}
62+
4163
public static void CleanupCertificates([CallerMemberName] string? testName = null, StoreName storeName = StoreName.CertificateAuthority)
4264
{
4365
string caName = $"O={testName}";
@@ -56,9 +78,7 @@ public static void CleanupCertificates([CallerMemberName] string? testName = nul
5678
}
5779
}
5880
}
59-
catch
60-
{
61-
}
81+
catch { };
6282

6383
try
6484
{
@@ -75,9 +95,7 @@ public static void CleanupCertificates([CallerMemberName] string? testName = nul
7595
}
7696
}
7797
}
78-
catch
79-
{
80-
}
98+
catch { };
8199
}
82100

83101
internal static X509ExtensionCollection BuildTlsServerCertExtensions(string serverName)
@@ -101,68 +119,7 @@ private static X509ExtensionCollection BuildTlsCertExtensions(string targetName,
101119
return extensions;
102120
}
103121

104-
internal class PkiHolder : IDisposable
105-
{
106-
internal CertificateAuthority Root { get; }
107-
internal CertificateAuthority[] Intermediates { get; }
108-
public X509Certificate2 EndEntity { get; }
109-
public X509Certificate2Collection IssuerChain { get; }
110-
internal RevocationResponder Responder { get; }
111-
112-
private readonly string? _testName;
113-
114-
public PkiHolder(string? testName, CertificateAuthority root, CertificateAuthority[] intermediates, X509Certificate2 endEntity, RevocationResponder responder)
115-
{
116-
_testName = testName;
117-
Root = root;
118-
Intermediates = intermediates;
119-
EndEntity = endEntity;
120-
Responder = responder;
121-
122-
// Walk the intermediates backwards so we build the chain collection as
123-
// Issuer3
124-
// Issuer2
125-
// Issuer1
126-
// Root
127-
IssuerChain = new X509Certificate2Collection();
128-
for (int i = intermediates.Length - 1; i >= 0; i--)
129-
{
130-
CertificateAuthority authority = intermediates[i];
131-
132-
IssuerChain.Add(authority.CloneIssuerCert());
133-
}
134-
135-
IssuerChain.Add(root.CloneIssuerCert());
136-
}
137-
138-
public SslStreamCertificateContext CreateSslStreamCertificateContext()
139-
{
140-
return SslStreamCertificateContext.Create(EndEntity, IssuerChain);
141-
}
142-
143-
public void Dispose()
144-
{
145-
foreach (CertificateAuthority authority in Intermediates)
146-
{
147-
authority.Dispose();
148-
}
149-
Root.Dispose();
150-
EndEntity.Dispose();
151-
Responder.Dispose();
152-
153-
foreach (X509Certificate2 authority in IssuerChain)
154-
{
155-
authority.Dispose();
156-
}
157-
158-
if (PlatformDetection.IsWindows && _testName != null)
159-
{
160-
CleanupCertificates(_testName);
161-
}
162-
}
163-
}
164-
165-
internal static PkiHolder GenerateCertificates(string targetName, [CallerMemberName] string? testName = null, bool longChain = false, bool serverCertificate = true, bool ephemeralKey = false)
122+
public static (X509Certificate2 certificate, X509Certificate2Collection) GenerateCertificates(string targetName, [CallerMemberName] string? testName = null, bool longChain = false, bool serverCertificate = true, bool ephemeralKey = false)
166123
{
167124
const int keySize = 2048;
168125
if (PlatformDetection.IsWindows && testName != null)
@@ -174,7 +131,7 @@ internal static PkiHolder GenerateCertificates(string targetName, [CallerMemberN
174131
X509ExtensionCollection extensions = BuildTlsCertExtensions(targetName, serverCertificate);
175132

176133
CertificateAuthority.BuildPrivatePki(
177-
PkiOptions.AllRevocation,
134+
PkiOptions.IssuerRevocationViaCrl,
178135
out RevocationResponder responder,
179136
out CertificateAuthority root,
180137
out CertificateAuthority[] intermediates,
@@ -185,15 +142,34 @@ internal static PkiHolder GenerateCertificates(string targetName, [CallerMemberN
185142
keyFactory: CertificateAuthority.KeyFactory.RSASize(keySize),
186143
extensions: extensions);
187144

145+
// Walk the intermediates backwards so we build the chain collection as
146+
// Issuer3
147+
// Issuer2
148+
// Issuer1
149+
// Root
150+
for (int i = intermediates.Length - 1; i >= 0; i--)
151+
{
152+
CertificateAuthority authority = intermediates[i];
153+
154+
chain.Add(authority.CloneIssuerCert());
155+
authority.Dispose();
156+
}
157+
158+
chain.Add(root.CloneIssuerCert());
159+
160+
responder.Dispose();
161+
root.Dispose();
162+
188163
if (!ephemeralKey && PlatformDetection.IsWindows)
189164
{
190165
X509Certificate2 ephemeral = endEntity;
191166
endEntity = X509CertificateLoader.LoadPkcs12(endEntity.Export(X509ContentType.Pfx), (string?)null, X509KeyStorageFlags.Exportable);
192167
ephemeral.Dispose();
193168
}
194169

195-
return new PkiHolder(testName, root, intermediates, endEntity, responder);
170+
return (endEntity, chain);
196171
}
172+
197173
}
198174
}
199175
}

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ServerCertificates.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public void Ctor_ExpectedDefaultValues()
4848
using (HttpClientHandler handler = CreateHttpClientHandler())
4949
{
5050
Assert.Null(handler.ServerCertificateCustomValidationCallback);
51-
Assert.True(handler.CheckCertificateRevocationList);
51+
Assert.False(handler.CheckCertificateRevocationList);
5252
}
5353
}
5454

@@ -232,9 +232,7 @@ public async Task NoCallback_BadCertificate_ThrowsException(string url)
232232
[ActiveIssue("https://github.com/dotnet/runtime/issues/106634", typeof(PlatformDetection), nameof(PlatformDetection.IsAlpine))]
233233
public async Task NoCallback_RevokedCertificate_NoRevocationChecking_Succeeds()
234234
{
235-
HttpClientHandler handler = CreateHttpClientHandler();
236-
handler.CheckCertificateRevocationList = false;
237-
using (HttpClient client = CreateHttpClient(handler))
235+
using (HttpClient client = CreateHttpClient())
238236
using (HttpResponseMessage response = await client.GetAsync(Configuration.Http.RevokedCertRemoteServer))
239237
{
240238
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
@@ -246,6 +244,7 @@ public async Task NoCallback_RevokedCertificate_NoRevocationChecking_Succeeds()
246244
public async Task NoCallback_RevokedCertificate_RevocationChecking_Fails()
247245
{
248246
HttpClientHandler handler = CreateHttpClientHandler();
247+
handler.CheckCertificateRevocationList = true;
249248
using (HttpClient client = CreateHttpClient(handler))
250249
{
251250
await Assert.ThrowsAsync<HttpRequestException>(() => client.GetAsync(Configuration.Http.RevokedCertRemoteServer));

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public void Ctor_ExpectedDefaultPropertyValues()
7878
Assert.True(handler.SupportsRedirectConfiguration);
7979

8080
// Changes from .NET Framework.
81-
Assert.True(handler.CheckCertificateRevocationList);
81+
Assert.False(handler.CheckCertificateRevocationList);
8282
Assert.Equal(0, handler.MaxRequestContentBufferSize);
8383
Assert.Equal(SslProtocols.None, handler.SslProtocols);
8484
}

src/libraries/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.csproj

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,6 @@ System.Net.Http.WinHttpHandler</PackageDescription>
9696
<Compile Include="System\Net\Http\WinHttpTraceHelper.cs" />
9797
<Compile Include="System\Net\Http\WinHttpTrailersHelper.cs" />
9898
<Compile Include="System\Net\Http\WinHttpTransportContext.cs" />
99-
<Compile Include="$(CommonPath)System\AppContextSwitchHelper.cs"
100-
Link="Common\System\AppContextSwitchHelper.cs" />
10199
<Compile Include="$(CommonPath)System\IO\StreamHelpers.CopyValidation.cs"
102100
Link="Common\System\IO\StreamHelpers.CopyValidation.cs" />
103101
<Compile Include="$(CommonPath)System\Net\Logging\NetEventSource.Common.cs"

src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,6 @@ public class WinHttpHandler : HttpMessageHandler
4343
internal static readonly Version HttpVersion20 = new Version(2, 0);
4444
internal static readonly Version HttpVersion30 = new Version(3, 0);
4545
internal static readonly Version HttpVersionUnknown = new Version(0, 0);
46-
internal static bool DefaultCertificateRevocationCheck { get; } =
47-
AppContextSwitchHelper.GetBooleanConfig(
48-
"System.Net.Security.NoRevocationCheckByDefault",
49-
"DOTNET_SYSTEM_NET_SECURITY_NOREVOCATIONCHECKBYDEFAULT") ? false : true;
50-
5146
internal static bool CertificateCachingAppContextSwitchEnabled { get; } = AppContext.TryGetSwitch("System.Net.Http.UseWinHttpCertificateCaching", out bool enabled) && enabled;
5247
private static readonly TimeSpan s_maxTimeout = TimeSpan.FromMilliseconds(int.MaxValue);
5348

@@ -73,7 +68,7 @@ private Func<
7368
X509Chain,
7469
SslPolicyErrors,
7570
bool>? _serverCertificateValidationCallback;
76-
private bool _checkCertificateRevocationList = DefaultCertificateRevocationCheck;
71+
private bool _checkCertificateRevocationList;
7772
private ClientCertificateOption _clientCertificateOption = ClientCertificateOption.Manual;
7873
private X509Certificate2Collection? _clientCertificates; // Only create collection when required.
7974
private ICredentials? _serverCredentials;

src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
Link="Common\Interop\Windows\WinHttp\Interop.SafeWinHttpHandle.cs" />
2525
<Compile Include="$(CommonPath)Interop\Windows\WinHttp\Interop.winhttp_types.cs"
2626
Link="Common\Interop\Windows\WinHttp\Interop.winhttp_types.cs" />
27-
<Compile Include="$(CommonPath)System\AppContextSwitchHelper.cs"
28-
Link="Common\System\AppContextSwitchHelper.cs" />
2927
<Compile Include="$(CommonPath)System\CharArrayHelpers.cs"
3028
Link="Common\System\CharArrayHelpers.cs" />
3129
<Compile Include="$(CommonPath)System\Obsoletions.cs"

src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public void Ctor_ExpectedDefaultPropertyValues()
4545
Assert.Equal(CookieUsePolicy.UseInternalCookieStoreOnly, handler.CookieUsePolicy);
4646
Assert.Null(handler.CookieContainer);
4747
Assert.Null(handler.ServerCertificateValidationCallback);
48-
Assert.True(handler.CheckCertificateRevocationList);
48+
Assert.False(handler.CheckCertificateRevocationList);
4949
Assert.Equal(ClientCertificateOption.Manual, handler.ClientCertificateOption);
5050
X509Certificate2Collection certs = handler.ClientCertificates;
5151
Assert.True(certs.Count == 0);

0 commit comments

Comments
 (0)