From 34b750bb3de98fab453d3b733a0495e93f3b3a69 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 29 Jun 2023 10:08:23 -0700 Subject: [PATCH 1/6] fix TLS resume with HttpHandler --- .../System/Net/Http/GenericLoopbackServer.cs | 2 + .../tests/System/Net/Http/LoopbackServer.cs | 20 +++++---- .../src/System/Net/Http/HttpClientHandler.cs | 9 +++- .../SocketsHttpHandler/HttpConnectionPool.cs | 17 +++++++ .../FunctionalTests/SocketsHttpHandlerTest.cs | 45 +++++++++++++++++++ 5 files changed, 84 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs index adc60fdf4a89a..5b31ca36a4e7c 100644 --- a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -8,6 +8,7 @@ using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using System.IO; +using System.Net.Security; using System.Net.Sockets; using System.Net.WebSockets; using System.Threading; @@ -174,6 +175,7 @@ public class GenericLoopbackOptions SslProtocols.Tls12; public int ListenBacklog { get; set; } = 1; + public SslStreamCertificateContext? CertificateContext { get; set; } } public struct HttpHeaderData diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index 980c88b6587df..fdad8d753b45c 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -33,6 +33,10 @@ public sealed partial class LoopbackServer : GenericLoopbackServer, IDisposable public LoopbackServer(Options options = null) { _options = options ??= new Options(); + if (_options.UseSsl && _options.CertificateContext == null) + { + _options.CertificateContext = SslStreamCertificateContext.Create(_options.Certificate ?? Configuration.Certificates.GetServerCertificate(), null); + } } #pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously @@ -470,15 +474,15 @@ public static async Task CreateAsync(SocketWrapper socket, Stream st { if (httpOptions.UseSsl) { - var sslStream = new SslStream(stream, false, delegate { return true; }); - using (X509Certificate2 cert = httpOptions.Certificate ?? Configuration.Certificates.GetServerCertificate()) + SslServerAuthenticationOptions sslOptions = new SslServerAuthenticationOptions() { - await sslStream.AuthenticateAsServerAsync( - cert, - clientCertificateRequired: true, // allowed but not required - enabledSslProtocols: httpOptions.SslProtocols, - checkCertificateRevocation: false).ConfigureAwait(false); - } + EnabledSslProtocols = httpOptions.SslProtocols, + ServerCertificateContext = httpOptions.CertificateContext, + ClientCertificateRequired = true, + }; + + var sslStream = new SslStream(stream, false, delegate { return true; }); + await sslStream.AuthenticateAsServerAsync(sslOptions, default).ConfigureAwait(false); stream = sslStream; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs index 9e4bf892ceb1a..34dfe7ca7b55e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs @@ -205,6 +205,13 @@ public int MaxResponseHeadersLength set => _underlyingHandler.MaxResponseHeadersLength = value; } +#if TARGET_BROWSER +#else + private X509Certificate2 GetEligibleClientCertificate(object _, string _2, X509CertificateCollection _3, X509Certificate? _4, string[] _5) + { + return CertificateHelper.GetEligibleClientCertificate(_underlyingHandler.SslOptions.ClientCertificates)!; + } +#endif public ClientCertificateOption ClientCertificateOptions { get => _clientCertificateOptions; @@ -218,7 +225,7 @@ public ClientCertificateOption ClientCertificateOptions #else ThrowForModifiedManagedSslOptionsIfStarted(); _clientCertificateOptions = value; - _underlyingHandler.SslOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => CertificateHelper.GetEligibleClientCertificate(_underlyingHandler.SslOptions.ClientCertificates)!; + _underlyingHandler.SslOptions.LocalCertificateSelectionCallback = GetEligibleClientCertificate; #endif break; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index c8ea7b6148410..a2bf4ca78bd64 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -12,6 +12,7 @@ using System.Net.Quic; using System.Net.Security; using System.Net.Sockets; +using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using System.Runtime.Versioning; @@ -25,6 +26,8 @@ namespace System.Net.Http /// Provides a pool of connections to the same endpoint. internal sealed class HttpConnectionPool : IDisposable { + private static readonly MethodInfo? internalCertificateSelectionCallback = typeof(HttpClientHandler).GetMethod("GetEligibleClientCertificate", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + private static readonly bool s_isWindows7Or2008R2 = GetIsWindows7Or2008R2(); private readonly HttpConnectionPoolManager _poolManager; @@ -327,6 +330,20 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection SslClientAuthenticationOptions sslOptions = poolManager.Settings._sslOptions?.ShallowClone() ?? new SslClientAuthenticationOptions(); + if (sslOptions.LocalCertificateSelectionCallback != null && (sslOptions.ClientCertificates == null || sslOptions.ClientCertificates.Count == 0)) + { + // If we have no client certificates do not set callback when internal selection is used. + // It breaks TLS resume on Linux + if (sslOptions.LocalCertificateSelectionCallback.Method.Equals(internalCertificateSelectionCallback)) + { + sslOptions.LocalCertificateSelectionCallback = null; + if (poolManager.Settings._sslOptions != null) + { + poolManager.Settings._sslOptions.LocalCertificateSelectionCallback = null; + } + } + } + // Set TargetHost for SNI sslOptions.TargetHost = sslHostName; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index e365c7a72006f..56c7376c017c3 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -11,6 +11,7 @@ using System.Net.Sockets; using System.Net.Test.Common; using System.Numerics; +using System.Reflection; using System.Runtime.CompilerServices; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; @@ -4288,6 +4289,50 @@ public sealed class SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http11 : { public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http11(ITestOutputHelper output) : base(output) { } protected override Version UseVersion => HttpVersion.Version11; + +#if DEBUG + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Https_MultipleRequests_Resume(bool useSocketHandler) + { + await LoopbackServer.CreateClientAndServerAsync(async uri => + { + HttpMessageHandler handler = useSocketHandler ? CreateSocketsHttpHandler(allowAllCertificates: true) : CreateHttpClientHandler(); + using (HttpClient client = CreateHttpClient(handler)) + { + HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get,uri); + request.Headers.Add("Host", "foo.bar"); + request.Headers.Add("Connection", "close"); + + HttpResponseMessage response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + request = new HttpRequestMessage(HttpMethod.Get,uri); + request.Headers.Add("Host", "foo.bar"); + response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + }, + async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + await server.AcceptConnectionAsync(async connection => + { + SslStream ssl = (SslStream)connection.Stream; + object connectionInfo = typeof(SslStream).GetField( + "_connectionInfo", + BindingFlags.Instance | BindingFlags.NonPublic).GetValue(ssl); + + bool resumed = (bool)connectionInfo.GetType().GetProperty("TlsResumed").GetValue(connectionInfo); + Assert.True(resumed); + + await connection.ReadRequestHeaderAndSendResponseAsync(); + }); + }, + new LoopbackServer.Options { UseSsl = true, SslProtocols = SslProtocols.Tls12 }); + } +#endif } [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] From e3f89b795cb65544d715030507b2fc8d9b8de717 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 29 Jun 2023 11:12:19 -0700 Subject: [PATCH 2/6] update --- .../src/System/Net/Http/HttpClientHandler.cs | 2 ++ .../Http/SocketsHttpHandler/HttpConnectionPool.cs | 15 +++------------ .../SocketsHttpHandler/HttpConnectionSettings.cs | 3 +++ .../Http/SocketsHttpHandler/SocketsHttpHandler.cs | 11 +++++++++++ 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs index 34dfe7ca7b55e..540de1da555c7 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs @@ -42,6 +42,8 @@ public HttpClientHandler() { Handler = new DiagnosticsHandler(Handler, DistributedContextPropagator.Current); } +#else + _underlyingHandler.HttpClientHandlerCompat = true; #endif ClientCertificateOptions = ClientCertificateOption.Manual; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index a2bf4ca78bd64..7af741439c30e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -12,7 +12,6 @@ using System.Net.Quic; using System.Net.Security; using System.Net.Sockets; -using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using System.Runtime.Versioning; @@ -26,8 +25,6 @@ namespace System.Net.Http /// Provides a pool of connections to the same endpoint. internal sealed class HttpConnectionPool : IDisposable { - private static readonly MethodInfo? internalCertificateSelectionCallback = typeof(HttpClientHandler).GetMethod("GetEligibleClientCertificate", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); - private static readonly bool s_isWindows7Or2008R2 = GetIsWindows7Or2008R2(); private readonly HttpConnectionPoolManager _poolManager; @@ -330,18 +327,12 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection SslClientAuthenticationOptions sslOptions = poolManager.Settings._sslOptions?.ShallowClone() ?? new SslClientAuthenticationOptions(); - if (sslOptions.LocalCertificateSelectionCallback != null && (sslOptions.ClientCertificates == null || sslOptions.ClientCertificates.Count == 0)) + if (poolManager.Settings._httpClientHandlerCompat && sslOptions.LocalCertificateSelectionCallback != null && + (sslOptions.ClientCertificates == null || sslOptions.ClientCertificates.Count == 0)) { // If we have no client certificates do not set callback when internal selection is used. // It breaks TLS resume on Linux - if (sslOptions.LocalCertificateSelectionCallback.Method.Equals(internalCertificateSelectionCallback)) - { - sslOptions.LocalCertificateSelectionCallback = null; - if (poolManager.Settings._sslOptions != null) - { - poolManager.Settings._sslOptions.LocalCertificateSelectionCallback = null; - } - } + sslOptions.LocalCertificateSelectionCallback = null; } // Set TargetHost for SNI diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs index f651dca784b35..4e80299564f57 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs @@ -63,6 +63,8 @@ internal sealed class HttpConnectionSettings // Http2 flow control settings: internal int _initialHttp2StreamWindowSize = HttpHandlerDefaults.DefaultInitialHttp2StreamWindowSize; + internal bool _httpClientHandlerCompat; + public HttpConnectionSettings() { bool allowHttp2 = GlobalHttpSettings.SocketsHttpHandler.AllowHttp2; @@ -117,6 +119,7 @@ public HttpConnectionSettings CloneAndNormalize() _activityHeadersPropagator = _activityHeadersPropagator, _defaultCredentialsUsedForProxy = _proxy != null && (_proxy.Credentials == CredentialCache.DefaultCredentials || _defaultProxyCredentials == CredentialCache.DefaultCredentials), _defaultCredentialsUsedForServer = _credentials == CredentialCache.DefaultCredentials, + _httpClientHandlerCompat = _httpClientHandlerCompat }; return settings; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs index 7bc6673276d31..bd2e733123389 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs @@ -450,6 +450,17 @@ public DistributedContextPropagator? ActivityHeadersPropagator } } + /// + /// Indicates that we are wrapped by HttpClientHandler and we want compatible behavior + /// + internal bool HttpClientHandlerCompat + { + set + { + _settings._httpClientHandlerCompat = value; + } + } + protected override void Dispose(bool disposing) { if (disposing && !_disposed) From a3006183ed83d62fbf0f4da502b1fc18cd6bde9e Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 29 Jun 2023 11:30:17 -0700 Subject: [PATCH 3/6] cleanup --- .../Common/tests/System/Net/Http/LoopbackServer.cs | 2 +- .../src/System/Net/Http/HttpClientHandler.cs | 9 +-------- .../tests/FunctionalTests/SocketsHttpHandlerTest.cs | 3 ++- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index fdad8d753b45c..2f1842f19bbae 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -477,7 +477,7 @@ public static async Task CreateAsync(SocketWrapper socket, Stream st SslServerAuthenticationOptions sslOptions = new SslServerAuthenticationOptions() { EnabledSslProtocols = httpOptions.SslProtocols, - ServerCertificateContext = httpOptions.CertificateContext, + ServerCertificateContext = httpOptions.CertificateContext ?? SslStreamCertificateContext.Create(Configuration.Certificates.GetServerCertificate(), null), ClientCertificateRequired = true, }; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs index 540de1da555c7..5d93294a107a0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs @@ -207,13 +207,6 @@ public int MaxResponseHeadersLength set => _underlyingHandler.MaxResponseHeadersLength = value; } -#if TARGET_BROWSER -#else - private X509Certificate2 GetEligibleClientCertificate(object _, string _2, X509CertificateCollection _3, X509Certificate? _4, string[] _5) - { - return CertificateHelper.GetEligibleClientCertificate(_underlyingHandler.SslOptions.ClientCertificates)!; - } -#endif public ClientCertificateOption ClientCertificateOptions { get => _clientCertificateOptions; @@ -227,7 +220,7 @@ public ClientCertificateOption ClientCertificateOptions #else ThrowForModifiedManagedSslOptionsIfStarted(); _clientCertificateOptions = value; - _underlyingHandler.SslOptions.LocalCertificateSelectionCallback = GetEligibleClientCertificate; + _underlyingHandler.SslOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => CertificateHelper.GetEligibleClientCertificate(_underlyingHandler.SslOptions.ClientCertificates)!; #endif break; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index 56c7376c017c3..9ec410e7c8605 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -4294,7 +4294,8 @@ public SocketsHttpHandler_SocketsHttpHandler_SecurityTest_Http11(ITestOutputHelp [Theory] [InlineData(true)] [InlineData(false)] - public async Task Https_MultipleRequests_Resume(bool useSocketHandler) + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] + public async Task Https_MultipleRequests_TlsResumed(bool useSocketHandler) { await LoopbackServer.CreateClientAndServerAsync(async uri => { From d376db2dfd9d7cf59e97c28609556513adaeb357 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 29 Jun 2023 13:46:14 -0700 Subject: [PATCH 4/6] update --- .../BrowserHttpHandler/BrowserHttpHandler.cs | 2 ++ .../src/System/Net/Http/HttpClientHandler.cs | 17 ++++------------- .../SocketsHttpHandler/HttpConnectionPool.cs | 3 ++- .../HttpConnectionSettings.cs | 6 ++++-- .../SocketsHttpHandler/SocketsHttpHandler.cs | 10 ++++------ 5 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs index 411aade42be6f..8b9919906076e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs @@ -107,6 +107,8 @@ public bool AllowAutoRedirect } } + internal ClientCertificateOption ClientCertificateOptions; + public const bool SupportsAutomaticDecompression = false; public const bool SupportsProxy = false; public const bool SupportsRedirectConfiguration = true; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs index 5d93294a107a0..cd88e7930918d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs @@ -28,8 +28,6 @@ public partial class HttpClientHandler : HttpMessageHandler private HttpHandlerType Handler => _underlyingHandler; #endif - private ClientCertificateOption _clientCertificateOptions; - private volatile bool _disposed; public HttpClientHandler() @@ -42,8 +40,6 @@ public HttpClientHandler() { Handler = new DiagnosticsHandler(Handler, DistributedContextPropagator.Current); } -#else - _underlyingHandler.HttpClientHandlerCompat = true; #endif ClientCertificateOptions = ClientCertificateOption.Manual; @@ -209,27 +205,21 @@ public int MaxResponseHeadersLength public ClientCertificateOption ClientCertificateOptions { - get => _clientCertificateOptions; + get => _underlyingHandler.ClientCertificateOptions; set { switch (value) { case ClientCertificateOption.Manual: -#if TARGET_BROWSER - _clientCertificateOptions = value; -#else +#if !TARGET_BROWSER ThrowForModifiedManagedSslOptionsIfStarted(); - _clientCertificateOptions = value; _underlyingHandler.SslOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => CertificateHelper.GetEligibleClientCertificate(_underlyingHandler.SslOptions.ClientCertificates)!; #endif break; case ClientCertificateOption.Automatic: -#if TARGET_BROWSER - _clientCertificateOptions = value; -#else +#if !TARGET_BROWSER ThrowForModifiedManagedSslOptionsIfStarted(); - _clientCertificateOptions = value; _underlyingHandler.SslOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => CertificateHelper.GetEligibleClientCertificate()!; #endif break; @@ -237,6 +227,7 @@ public ClientCertificateOption ClientCertificateOptions default: throw new ArgumentOutOfRangeException(nameof(value)); } + _underlyingHandler.ClientCertificateOptions = value; } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index 7af741439c30e..a460307b99c04 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -327,7 +327,8 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection SslClientAuthenticationOptions sslOptions = poolManager.Settings._sslOptions?.ShallowClone() ?? new SslClientAuthenticationOptions(); - if (poolManager.Settings._httpClientHandlerCompat && sslOptions.LocalCertificateSelectionCallback != null && + // This is only set if we are underlying handler for HttpClientHandler + if (poolManager.Settings._clientCertificateOptions == ClientCertificateOption.Manual && sslOptions.LocalCertificateSelectionCallback != null && (sslOptions.ClientCertificates == null || sslOptions.ClientCertificates.Count == 0)) { // If we have no client certificates do not set callback when internal selection is used. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs index 4e80299564f57..eaedf244ce4d4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs @@ -63,7 +63,7 @@ internal sealed class HttpConnectionSettings // Http2 flow control settings: internal int _initialHttp2StreamWindowSize = HttpHandlerDefaults.DefaultInitialHttp2StreamWindowSize; - internal bool _httpClientHandlerCompat; + internal ClientCertificateOption _clientCertificateOptions; public HttpConnectionSettings() { @@ -73,6 +73,8 @@ public HttpConnectionSettings() allowHttp3 && allowHttp2 ? HttpVersion.Version30 : allowHttp2 ? HttpVersion.Version20 : HttpVersion.Version11; + + _clientCertificateOptions = ClientCertificateOption.Automatic; } /// Creates a copy of the settings but with some values normalized to suit the implementation. @@ -119,7 +121,7 @@ public HttpConnectionSettings CloneAndNormalize() _activityHeadersPropagator = _activityHeadersPropagator, _defaultCredentialsUsedForProxy = _proxy != null && (_proxy.Credentials == CredentialCache.DefaultCredentials || _defaultProxyCredentials == CredentialCache.DefaultCredentials), _defaultCredentialsUsedForServer = _credentials == CredentialCache.DefaultCredentials, - _httpClientHandlerCompat = _httpClientHandlerCompat + _clientCertificateOptions = _clientCertificateOptions, }; return settings; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs index bd2e733123389..eae4cb5245717 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs @@ -450,17 +450,15 @@ public DistributedContextPropagator? ActivityHeadersPropagator } } - /// - /// Indicates that we are wrapped by HttpClientHandler and we want compatible behavior - /// - internal bool HttpClientHandlerCompat + internal ClientCertificateOption ClientCertificateOptions { + get => _settings._clientCertificateOptions; set { - _settings._httpClientHandlerCompat = value; + CheckDisposedOrStarted(); + _settings._clientCertificateOptions = value; } } - protected override void Dispose(bool disposing) { if (disposing && !_disposed) From d2091358e66ffd8de7cce32a1f97a6175745cc0b Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 30 Jun 2023 15:44:37 +0200 Subject: [PATCH 5/6] net48 --- .../System/Net/Http/GenericLoopbackServer.cs | 2 ++ .../tests/System/Net/Http/LoopbackServer.cs | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs index 5b31ca36a4e7c..054985f64b8e6 100644 --- a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -175,7 +175,9 @@ public class GenericLoopbackOptions SslProtocols.Tls12; public int ListenBacklog { get; set; } = 1; +#if !NETSTANDARD2_0 && !NETFRAMEWORK public SslStreamCertificateContext? CertificateContext { get; set; } +#endif } public struct HttpHeaderData diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index 2f1842f19bbae..d182f0fdddc8b 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -33,10 +33,12 @@ public sealed partial class LoopbackServer : GenericLoopbackServer, IDisposable public LoopbackServer(Options options = null) { _options = options ??= new Options(); +#if !NETSTANDARD2_0 && !NETFRAMEWORK if (_options.UseSsl && _options.CertificateContext == null) { _options.CertificateContext = SslStreamCertificateContext.Create(_options.Certificate ?? Configuration.Certificates.GetServerCertificate(), null); } +#endif } #pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously @@ -474,6 +476,8 @@ public static async Task CreateAsync(SocketWrapper socket, Stream st { if (httpOptions.UseSsl) { + var sslStream = new SslStream(stream, false, delegate { return true; }); +#if !NETFRAMEWORK SslServerAuthenticationOptions sslOptions = new SslServerAuthenticationOptions() { EnabledSslProtocols = httpOptions.SslProtocols, @@ -481,8 +485,17 @@ public static async Task CreateAsync(SocketWrapper socket, Stream st ClientCertificateRequired = true, }; - var sslStream = new SslStream(stream, false, delegate { return true; }); await sslStream.AuthenticateAsServerAsync(sslOptions, default).ConfigureAwait(false); +#else + using (X509Certificate2 cert = httpOptions.Certificate ?? Configuration.Certificates.GetServerCertificate()) + { + await sslStream.AuthenticateAsServerAsync( + cert, + clientCertificateRequired: true, // allowed but not required + enabledSslProtocols: httpOptions.SslProtocols, + checkCertificateRevocation: false).ConfigureAwait(false); + } +#endif stream = sslStream; } From 26b416e98abc5c065fc6af9e7f882ef0609f0ebd Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 5 Jul 2023 14:16:31 -0700 Subject: [PATCH 6/6] fix spacing --- .../Common/tests/System/Net/Http/LoopbackServer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index d182f0fdddc8b..3506102194551 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -489,11 +489,11 @@ public static async Task CreateAsync(SocketWrapper socket, Stream st #else using (X509Certificate2 cert = httpOptions.Certificate ?? Configuration.Certificates.GetServerCertificate()) { - await sslStream.AuthenticateAsServerAsync( - cert, - clientCertificateRequired: true, // allowed but not required - enabledSslProtocols: httpOptions.SslProtocols, - checkCertificateRevocation: false).ConfigureAwait(false); + await sslStream.AuthenticateAsServerAsync( + cert, + clientCertificateRequired: true, // allowed but not required + enabledSslProtocols: httpOptions.SslProtocols, + checkCertificateRevocation: false).ConfigureAwait(false); } #endif stream = sslStream;