Skip to content

Change HttpClient/SslStream default certificate revocation check mode to Online #116098

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Jun 16, 2025
Merged

Change HttpClient/SslStream default certificate revocation check mode to Online #116098

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c784242
Change default CertificateRevocationMode to Online
rzikm May 29, 2025
e8da401
Change in WinHttpHandler
rzikm May 30, 2025
c8a8486
Fix Tests
rzikm May 30, 2025
74d3607
fixup! Change in WinHttpHandler
rzikm Jun 2, 2025
8916b1a
Make sure RevocationResponder is alive during tests
rzikm Jun 2, 2025
a2cfc53
Fix SMTP test compilation
rzikm Jun 5, 2025
7e09de0
fixup! Fix Tests
rzikm Jun 6, 2025
f4da81f
fixup! Fix Tests
rzikm Jun 6, 2025
ca55e6c
Add RemoteExecutor tests
rzikm Jun 6, 2025
dfa7ad8
AppContextSwitch also covers WinHttpHandler
rzikm Jun 6, 2025
3f9fb43
fixup! AppContextSwitch also covers WinHttpHandler
rzikm Jun 9, 2025
b70c830
Update src/libraries/Common/tests/System/Net/Configuration.Certificat…
rzikm Jun 13, 2025
2152d2d
Update src/libraries/Common/tests/System/Net/Configuration.Certificat…
rzikm Jun 13, 2025
4d3e7c2
Merge remote-tracking branch 'upstream/main' into 111704-certificate-…
rzikm Jun 16, 2025
0976295
Merge remote-tracking branch 'upstream/main' into 111704-certificate-…
rzikm Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 70 additions & 47 deletions src/libraries/Common/tests/System/Net/Configuration.Certificates.Dynamic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Net.Security;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Security.Cryptography;
Expand Down Expand Up @@ -37,29 +38,6 @@ public static partial class Certificates
private static readonly X509BasicConstraintsExtension s_eeConstraints =
new X509BasicConstraintsExtension(false, false, 0, false);

private static X509Certificate2 s_dynamicServerCertificate;
private static X509Certificate2Collection s_dynamicCaCertificates;
private static object certLock = new object();


// These Get* methods make a copy of the certificates so that consumers own the lifetime of the
// certificates handed back. Consumers are expected to dispose of their certs when done with them.

public static X509Certificate2 GetDynamicServerCerttificate(X509Certificate2Collection? chainCertificates)
{
lock (certLock)
{
if (s_dynamicServerCertificate == null)
{
CleanupCertificates();
(s_dynamicServerCertificate, s_dynamicCaCertificates) = GenerateCertificates("localhost", nameof(Configuration) + nameof(Certificates));
}

chainCertificates?.AddRange(s_dynamicCaCertificates);
return new X509Certificate2(s_dynamicServerCertificate);
}
}

public static void CleanupCertificates([CallerMemberName] string? testName = null, StoreName storeName = StoreName.CertificateAuthority)
{
string caName = $"O={testName}";
Expand All @@ -78,7 +56,9 @@ public static void CleanupCertificates([CallerMemberName] string? testName = nul
}
}
}
catch { };
catch
{
}

try
{
Expand All @@ -95,7 +75,8 @@ public static void CleanupCertificates([CallerMemberName] string? testName = nul
}
}
}
catch { };
catch { }
;
}

internal static X509ExtensionCollection BuildTlsServerCertExtensions(string serverName)
Expand All @@ -119,7 +100,68 @@ private static X509ExtensionCollection BuildTlsCertExtensions(string targetName,
return extensions;
}

public static (X509Certificate2 certificate, X509Certificate2Collection) GenerateCertificates(string targetName, [CallerMemberName] string? testName = null, bool longChain = false, bool serverCertificate = true, bool ephemeralKey = false)
internal class PkiHolder : IDisposable
{
internal CertificateAuthority Root { get; }
internal CertificateAuthority[] Intermediates { get; }
public X509Certificate2 EndEntity { get; }
public X509Certificate2Collection IssuerChain { get; }
internal RevocationResponder Responder { get; }

private readonly string? _testName;

public PkiHolder(string? testName, CertificateAuthority root, CertificateAuthority[] intermediates, X509Certificate2 endEntity, RevocationResponder responder)
{
_testName = testName;
Root = root;
Intermediates = intermediates;
EndEntity = endEntity;
Responder = responder;

// Walk the intermediates backwards so we build the chain collection as
// Issuer3
// Issuer2
// Issuer1
// Root
IssuerChain = new X509Certificate2Collection();
for (int i = intermediates.Length - 1; i >= 0; i--)
{
CertificateAuthority authority = intermediates[i];

IssuerChain.Add(authority.CloneIssuerCert());
}

IssuerChain.Add(root.CloneIssuerCert());
}

public SslStreamCertificateContext CreateSslStreamCertificateContext()
{
return SslStreamCertificateContext.Create(EndEntity, IssuerChain);
}

public void Dispose()
{
foreach (CertificateAuthority authority in Intermediates)
{
authority.Dispose();
}
Root.Dispose();
EndEntity.Dispose();
Responder.Dispose();

foreach (X509Certificate2 authority in IssuerChain)
{
authority.Dispose();
}

if (PlatformDetection.IsWindows && _testName != null)
{
CleanupCertificates(_testName);
}
}
}

internal static PkiHolder GenerateCertificates(string targetName, [CallerMemberName] string? testName = null, bool longChain = false, bool serverCertificate = true, bool ephemeralKey = false)
{
const int keySize = 2048;
if (PlatformDetection.IsWindows && testName != null)
Expand All @@ -131,7 +173,7 @@ public static (X509Certificate2 certificate, X509Certificate2Collection) Generat
X509ExtensionCollection extensions = BuildTlsCertExtensions(targetName, serverCertificate);

CertificateAuthority.BuildPrivatePki(
PkiOptions.IssuerRevocationViaCrl,
PkiOptions.AllRevocation,
out RevocationResponder responder,
out CertificateAuthority root,
out CertificateAuthority[] intermediates,
Expand All @@ -142,34 +184,15 @@ public static (X509Certificate2 certificate, X509Certificate2Collection) Generat
keyFactory: CertificateAuthority.KeyFactory.RSASize(keySize),
extensions: extensions);

// Walk the intermediates backwards so we build the chain collection as
// Issuer3
// Issuer2
// Issuer1
// Root
for (int i = intermediates.Length - 1; i >= 0; i--)
{
CertificateAuthority authority = intermediates[i];

chain.Add(authority.CloneIssuerCert());
authority.Dispose();
}

chain.Add(root.CloneIssuerCert());

responder.Dispose();
root.Dispose();

if (!ephemeralKey && PlatformDetection.IsWindows)
{
X509Certificate2 ephemeral = endEntity;
endEntity = X509CertificateLoader.LoadPkcs12(endEntity.Export(X509ContentType.Pfx), (string?)null, X509KeyStorageFlags.Exportable);
ephemeral.Dispose();
}

return (endEntity, chain);
return new PkiHolder(testName, root, intermediates, endEntity, responder);
}

}
}
}
2 changes: 1 addition & 1 deletion src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ServerCertificates.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void Ctor_ExpectedDefaultValues()
using (HttpClientHandler handler = CreateHttpClientHandler())
{
Assert.Null(handler.ServerCertificateCustomValidationCallback);
Assert.False(handler.CheckCertificateRevocationList);
Assert.True(handler.CheckCertificateRevocationList);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void Ctor_ExpectedDefaultPropertyValues()
Assert.True(handler.SupportsRedirectConfiguration);

// Changes from .NET Framework.
Assert.False(handler.CheckCertificateRevocationList);
Assert.True(handler.CheckCertificateRevocationList);
Assert.Equal(0, handler.MaxRequestContentBufferSize);
Assert.Equal(SslProtocols.None, handler.SslProtocols);
}
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private Func<
X509Chain,
SslPolicyErrors,
bool>? _serverCertificateValidationCallback;
private bool _checkCertificateRevocationList;
private bool _checkCertificateRevocationList = true;
Copy link
Member

@bartonjs bartonjs Jun 5, 2025

Choose a reason for hiding this comment

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

Should this be true, or should it inspect the appcontext-overridable default mode value?

Copy link
Member

@bartonjs bartonjs Jun 5, 2025

Choose a reason for hiding this comment

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

I can't see from the PR, so: Ensure you have tests where this gets forced to false, instead of only tests that assumed it was false. Ideally tests where the cert is revoked (or the revocation responder is set to explode, or time out, or whatever), so you can tell it worked.

private ClientCertificateOption _clientCertificateOption = ClientCertificateOption.Manual;
private X509Certificate2Collection? _clientCertificates; // Only create collection when required.
private ICredentials? _serverCredentials;
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/WinHttpHandlerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void Ctor_ExpectedDefaultPropertyValues()
Assert.Equal(CookieUsePolicy.UseInternalCookieStoreOnly, handler.CookieUsePolicy);
Assert.Null(handler.CookieContainer);
Assert.Null(handler.ServerCertificateValidationCallback);
Assert.False(handler.CheckCertificateRevocationList);
Assert.True(handler.CheckCertificateRevocationList);
Assert.Equal(ClientCertificateOption.Manual, handler.ClientCertificateOption);
X509Certificate2Collection certs = handler.ClientCertificates;
Assert.True(certs.Count == 0);
Expand Down
78 changes: 53 additions & 25 deletions src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,26 @@ namespace System.Net.Http.Functional.Tests
{
using Configuration = System.Net.Test.Common.Configuration;

public class CertificateSetup : IDisposable
{
public X509Certificate2 ServerCert => _pkiHolder.EndEntity;
public X509Certificate2Collection ServerChain => _pkiHolder.IssuerChain;

Configuration.Certificates.PkiHolder _pkiHolder;

public CertificateSetup()
{
_pkiHolder = Configuration.Certificates.GenerateCertificates("localhost", nameof(HttpClientHandlerTestBase), longChain: true);
}

public SslStreamCertificateContext CreateSslStreamCertificateContext() => _pkiHolder.CreateSslStreamCertificateContext();

public void Dispose()
{
_pkiHolder.Dispose();
}
}

public sealed class SocketsHttpHandler_HttpClientHandler_Asynchrony_Test_Http11 : SocketsHttpHandler_HttpClientHandler_Asynchrony_Test
{
public SocketsHttpHandler_HttpClientHandler_Asynchrony_Test_Http11(ITestOutputHelper output) : base(output) { }
Expand Down Expand Up @@ -2475,7 +2495,7 @@ public void SslOptions_GetSet_Roundtrips()

Assert.True(options.AllowRenegotiation);
Assert.Null(options.ApplicationProtocols);
Assert.Equal(X509RevocationMode.NoCheck, options.CertificateRevocationCheckMode);
Assert.Equal(X509RevocationMode.Online, options.CertificateRevocationCheckMode);
Assert.Null(options.ClientCertificates);
Assert.Equal(SslProtocols.None, options.EnabledSslProtocols);
Assert.Equal(EncryptionPolicy.RequireEncryption, options.EncryptionPolicy);
Expand Down Expand Up @@ -4339,15 +4359,17 @@ public SocketsHttpHandler_RequestContentLengthMismatchTest_Http3(ITestOutputHelp
[ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
public abstract class SocketsHttpHandler_SecurityTest : HttpClientHandlerTestBase
{
public SocketsHttpHandler_SecurityTest(ITestOutputHelper output) : base(output) { }
private readonly CertificateSetup _certificateSetup;

public SocketsHttpHandler_SecurityTest(ITestOutputHelper output, CertificateSetup certificateSetup) : base(output)
{
_certificateSetup = certificateSetup;
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))]
public async Task SslOptions_CustomTrust_Ok()
{
X509Certificate2Collection caCerts = new X509Certificate2Collection();
X509Certificate2 certificate = Configuration.Certificates.GetDynamicServerCerttificate(caCerts);

GenericLoopbackOptions options = new GenericLoopbackOptions() { UseSsl = true, Certificate = certificate };
GenericLoopbackOptions options = new GenericLoopbackOptions() { UseSsl = true, Certificate = new X509Certificate2(_certificateSetup.ServerCert) };
await LoopbackServerFactory.CreateClientAndServerAsync(
async uri =>
{
Expand All @@ -4360,8 +4382,8 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
TrustMode = X509ChainTrustMode.CustomRootTrust,
};

policy.ExtraStore.AddRange(caCerts);
policy.CustomTrustStore.Add(caCerts[caCerts.Count - 1]);
policy.ExtraStore.AddRange(_certificateSetup.ServerChain);
policy.CustomTrustStore.Add(_certificateSetup.ServerChain[^1]);
socketsHandler.SslOptions = new SslClientAuthenticationOptions() { CertificateChainPolicy = policy };
using HttpClient client = CreateHttpClient(handler);
client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact;
Expand All @@ -4380,15 +4402,22 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
[Fact]
public async Task SslOptions_InvalidName_Throws()
{
X509Certificate2Collection caCerts = new X509Certificate2Collection();
using X509Certificate2 certificate = Configuration.Certificates.GetDynamicServerCerttificate(caCerts);

GenericLoopbackOptions options = new GenericLoopbackOptions() { UseSsl = true, Certificate = certificate };
GenericLoopbackOptions options = new GenericLoopbackOptions() { UseSsl = true, Certificate = new X509Certificate2(_certificateSetup.ServerCert) };
await LoopbackServerFactory.CreateClientAndServerAsync(
async uri =>
{
using HttpClientHandler handler = CreateHttpClientHandler(allowAllCertificates: false);
var socketsHandler = (SocketsHttpHandler)GetUnderlyingSocketsHttpHandler(handler);

var policy = new X509ChainPolicy()
{
RevocationMode = X509RevocationMode.NoCheck,
TrustMode = X509ChainTrustMode.CustomRootTrust,
};

policy.ExtraStore.AddRange(_certificateSetup.ServerChain);
policy.CustomTrustStore.Add(_certificateSetup.ServerChain[^1]);
socketsHandler.SslOptions = new SslClientAuthenticationOptions() { CertificateChainPolicy = policy };
using HttpClient client = CreateHttpClient(handler);
client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact;
HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, uri) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact };
Expand All @@ -4403,17 +4432,15 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
{
await server.AcceptConnectionSendResponseAndCloseAsync(content: "foo");
}
catch { };
catch { }
;
}, options: options);
}

[Fact]
public async Task SslOptions_CustomPolicy_IgnoresNameMismatch()
{
X509Certificate2Collection caCerts = new X509Certificate2Collection();
X509Certificate2 certificate = Configuration.Certificates.GetDynamicServerCerttificate(caCerts);

GenericLoopbackOptions options = new GenericLoopbackOptions() { UseSsl = true, Certificate = certificate };
GenericLoopbackOptions options = new GenericLoopbackOptions() { UseSsl = true, Certificate = new X509Certificate2(_certificateSetup.ServerCert) };
await LoopbackServerFactory.CreateClientAndServerAsync(
async uri =>
{
Expand All @@ -4427,8 +4454,8 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
VerificationFlags = X509VerificationFlags.IgnoreInvalidName,
};

policy.ExtraStore.AddRange(caCerts);
policy.CustomTrustStore.Add(caCerts[caCerts.Count - 1]);
policy.ExtraStore.AddRange(_certificateSetup.ServerChain);
policy.CustomTrustStore.Add(_certificateSetup.ServerChain[^1]);
socketsHandler.SslOptions = new SslClientAuthenticationOptions() { CertificateChainPolicy = policy };

using HttpClient client = CreateHttpClient(handler);
Expand All @@ -4447,9 +4474,9 @@ await LoopbackServerFactory.CreateClientAndServerAsync(
}
}

public sealed class SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http11 : SocketsHttpHandler_SecurityTest
public sealed class SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http11 : SocketsHttpHandler_SecurityTest, IClassFixture<CertificateSetup>
{
public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http11(ITestOutputHelper output) : base(output) { }
public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http11(ITestOutputHelper output, CertificateSetup certificateSetup) : base(output, certificateSetup) { }
protected override Version UseVersion => HttpVersion.Version11;

#if DEBUG
Expand Down Expand Up @@ -4499,16 +4526,17 @@ await server.AcceptConnectionAsync(async connection =>
}

[ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))]
public sealed class SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http2 : SocketsHttpHandler_SecurityTest
public sealed class SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http2 : SocketsHttpHandler_SecurityTest, IClassFixture<CertificateSetup>
{
public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http2(ITestOutputHelper output) : base(output) { }
public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http2(ITestOutputHelper output, CertificateSetup certificateSetup) : base(output, certificateSetup) { }

protected override Version UseVersion => HttpVersion.Version20;
}

[ConditionalClass(typeof(HttpClientHandlerTestBase), nameof(IsQuicSupported))]
public sealed class SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http3 : SocketsHttpHandler_SecurityTest
public sealed class SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http3 : SocketsHttpHandler_SecurityTest, IClassFixture<CertificateSetup>
{
public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http3(ITestOutputHelper output) : base(output) { }
public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http3(ITestOutputHelper output, CertificateSetup certificateSetup) : base(output, certificateSetup) { }
protected override Version UseVersion => HttpVersion.Version30;
}

Expand Down
Loading
Loading