From 90a4b819bfd5e6ec2830c095519389f694db1b6a Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 10 Jun 2019 22:49:34 -0700 Subject: [PATCH 1/5] Move the HTTPS connection adapter to connection middleware --- .../src/Adapter/Internal/AdaptedPipeline.cs | 4 +- .../Core/src/HttpsConnectionAdapterOptions.cs | 7 + .../Core/src/Internal/HttpConnection.cs | 74 ++-- ...dapter.cs => HttpsConnectionMiddleware.cs} | 223 ++++++------ .../Core/src/KestrelConfigurationLoader.cs | 2 +- src/Servers/Kestrel/Core/src/KestrelServer.cs | 1 + src/Servers/Kestrel/Core/src/ListenOptions.cs | 26 +- .../Core/src/ListenOptionsHttpsExtensions.cs | 17 +- .../Core/src/LocalhostListenOptions.cs | 7 +- .../Kestrel/Core/test/KestrelServerTests.cs | 2 +- .../test/KestrelConfigurationBuilderTests.cs | 8 +- .../test/TransportTestHelpers/TestServer.cs | 14 +- .../FunctionalTests/Http2/HandshakeTests.cs | 4 +- .../FunctionalTests/Http2/ShutdownTests.cs | 5 +- .../test/FunctionalTests/ResponseTests.cs | 14 +- .../Http2/TlsTests.cs | 4 +- ...s.cs => HttpsConnectionMiddlewareTests.cs} | 341 ++++++++---------- .../InMemory.FunctionalTests/HttpsTests.cs | 27 +- .../LoggingConnectionAdapterTests.cs | 2 +- 19 files changed, 411 insertions(+), 371 deletions(-) rename src/Servers/Kestrel/Core/src/Internal/{HttpsConnectionAdapter.cs => HttpsConnectionMiddleware.cs} (51%) rename src/Servers/Kestrel/test/InMemory.FunctionalTests/{HttpsConnectionAdapterTests.cs => HttpsConnectionMiddlewareTests.cs} (69%) diff --git a/src/Servers/Kestrel/Core/src/Adapter/Internal/AdaptedPipeline.cs b/src/Servers/Kestrel/Core/src/Adapter/Internal/AdaptedPipeline.cs index 337f11135f76..eafdef55c162 100644 --- a/src/Servers/Kestrel/Core/src/Adapter/Internal/AdaptedPipeline.cs +++ b/src/Servers/Kestrel/Core/src/Adapter/Internal/AdaptedPipeline.cs @@ -21,7 +21,7 @@ internal class AdaptedPipeline : IDuplexPipe public AdaptedPipeline(IDuplexPipe transport, Pipe inputPipe, Pipe outputPipe, - IKestrelTrace log, + ILogger log, int minAllocBufferSize) { TransportStream = new RawStream(transport.Input, transport.Output, throwOnCancelled: true); @@ -37,7 +37,7 @@ public AdaptedPipeline(IDuplexPipe transport, public Pipe Output { get; } - public IKestrelTrace Log { get; } + public ILogger Log { get; } PipeReader IDuplexPipe.Input => Input.Reader; diff --git a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs index baf7785773ce..99084fdc5263 100644 --- a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs +++ b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO.Pipelines; using System.Net.Security; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; @@ -96,5 +97,11 @@ public TimeSpan HandshakeTimeout _handshakeTimeout = value != Timeout.InfiniteTimeSpan ? value : TimeSpan.MaxValue; } } + + internal PipeScheduler Scheduler { get; set; } = PipeScheduler.ThreadPool; + + internal long? MaxInputBufferSize { get; set; } + + internal long? MaxOutputBufferSize { get; set; } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs b/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs index 6d25e2b14109..58fe6b8de8c8 100644 --- a/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs @@ -215,64 +215,90 @@ private HttpConnectionContext CreateDerivedContext(IDuplexPipe transport) private void StopProcessingNextRequest() { + ProtocolSelectionState previousState; lock (_protocolSelectionLock) { + previousState = _protocolSelectionState; + switch (_protocolSelectionState) { case ProtocolSelectionState.Initializing: - CloseUninitializedConnection(new ConnectionAbortedException(CoreStrings.ServerShutdownDuringConnectionInitialization)); _protocolSelectionState = ProtocolSelectionState.Aborted; break; case ProtocolSelectionState.Selected: - _requestProcessor.StopProcessingNextRequest(); - break; case ProtocolSelectionState.Aborted: break; } } + + switch (previousState) + { + case ProtocolSelectionState.Initializing: + CloseUninitializedConnection(new ConnectionAbortedException(CoreStrings.ServerShutdownDuringConnectionInitialization)); + break; + case ProtocolSelectionState.Selected: + _requestProcessor.StopProcessingNextRequest(); + break; + case ProtocolSelectionState.Aborted: + break; + } } private void OnInputOrOutputCompleted() { + ProtocolSelectionState previousState; lock (_protocolSelectionLock) { + previousState = _protocolSelectionState; + switch (_protocolSelectionState) { case ProtocolSelectionState.Initializing: - // OnReader/WriterCompleted callbacks are not wired until after leaving the Initializing state. - Debug.Assert(false); - - CloseUninitializedConnection(new ConnectionAbortedException("HttpConnection.OnInputOrOutputCompleted() called while in the ProtocolSelectionState.Initializing state!?")); _protocolSelectionState = ProtocolSelectionState.Aborted; break; case ProtocolSelectionState.Selected: - _requestProcessor.OnInputOrOutputCompleted(); - break; case ProtocolSelectionState.Aborted: break; } + } + + switch (previousState) + { + case ProtocolSelectionState.Initializing: + // OnReader/WriterCompleted callbacks are not wired until after leaving the Initializing state. + Debug.Assert(false); + CloseUninitializedConnection(new ConnectionAbortedException("HttpConnection.OnInputOrOutputCompleted() called while in the ProtocolSelectionState.Initializing state!?")); + break; + case ProtocolSelectionState.Selected: + _requestProcessor.OnInputOrOutputCompleted(); + break; + case ProtocolSelectionState.Aborted: + break; } } private void Abort(ConnectionAbortedException ex) { + ProtocolSelectionState previousState; + lock (_protocolSelectionLock) { - switch (_protocolSelectionState) - { - case ProtocolSelectionState.Initializing: - CloseUninitializedConnection(ex); - break; - case ProtocolSelectionState.Selected: - _requestProcessor.Abort(ex); - break; - case ProtocolSelectionState.Aborted: - break; - } - + previousState = _protocolSelectionState; _protocolSelectionState = ProtocolSelectionState.Aborted; } + + switch (previousState) + { + case ProtocolSelectionState.Initializing: + CloseUninitializedConnection(ex); + break; + case ProtocolSelectionState.Selected: + _requestProcessor.Abort(ex); + break; + case ProtocolSelectionState.Aborted: + break; + } } private async Task ApplyConnectionAdaptersAsync(RawStream stream) @@ -365,6 +391,12 @@ private void Tick() private void CloseUninitializedConnection(ConnectionAbortedException abortReason) { _context.ConnectionContext.Abort(abortReason); + + if (_context.ConnectionAdapters.Count > 0) + { + _adaptedTransport.Input.Complete(); + _adaptedTransport.Output.Complete(); + } } public void OnTimeout(TimeoutReason reason) diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionAdapter.cs b/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs similarity index 51% rename from src/Servers/Kestrel/Core/src/Internal/HttpsConnectionAdapter.cs rename to src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs index 46ad0ae1322e..6a62913c62fa 100644 --- a/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionAdapter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs @@ -5,50 +5,45 @@ using System.Collections.Generic; using System.IO; using System.Net.Security; -using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.Extensions.Logging; +using Microsoft.AspNetCore.Http.Features; +using System.IO.Pipelines; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.Extensions.Logging.Abstractions; namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal { - internal class HttpsConnectionAdapter : IConnectionAdapter + internal class HttpsConnectionMiddleware { - private static readonly ClosedAdaptedConnection _closedAdaptedConnection = new ClosedAdaptedConnection(); + private readonly ConnectionDelegate _next; private readonly HttpsConnectionAdapterOptions _options; + private readonly ILogger _logger; private readonly X509Certificate2 _serverCertificate; private readonly Func _serverCertificateSelector; - private readonly ILogger _logger; - - public HttpsConnectionAdapter(HttpsConnectionAdapterOptions options) - : this(options, loggerFactory: null) + public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options) + : this(next, options, loggerFactory: NullLoggerFactory.Instance) { } - public HttpsConnectionAdapter(HttpsConnectionAdapterOptions options, ILoggerFactory loggerFactory) + public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options, ILoggerFactory loggerFactory) { if (options == null) { throw new ArgumentNullException(nameof(options)); } - // This configuration will always fail per-request, preemptively fail it here. See HttpConnection.SelectProtocol(). - if (options.HttpProtocols == HttpProtocols.Http2 && RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - throw new NotSupportedException(CoreStrings.HTTP2NoTlsOsx); - } - + _next = next; // capture the certificate now so it can't be switched after validation _serverCertificate = options.ServerCertificate; _serverCertificateSelector = options.ServerCertificateSelector; @@ -69,18 +64,14 @@ public HttpsConnectionAdapter(HttpsConnectionAdapterOptions options, ILoggerFact } _options = options; - _logger = loggerFactory?.CreateLogger() ?? (ILogger)NullLogger.Instance; + _logger = loggerFactory?.CreateLogger(); } - - public bool IsHttps => true; - - public Task OnConnectionAsync(ConnectionAdapterContext context) + public Task OnConnectionAsync(ConnectionContext context) { - // Don't trust SslStream not to block. return Task.Run(() => InnerOnConnectionAsync(context)); } - private async Task InnerOnConnectionAsync(ConnectionAdapterContext context) + private async Task InnerOnConnectionAsync(ConnectionContext context) { SslStream sslStream; bool certificateRequired; @@ -88,14 +79,43 @@ private async Task InnerOnConnectionAsync(ConnectionAdapterC context.Features.Set(feature); context.Features.Set(feature); + // TODO: Handle the cases where this can be null + var memoryPoolFeature = context.Features.Get(); + + var inputPipeOptions = new PipeOptions + ( + pool: memoryPoolFeature.MemoryPool, + readerScheduler: _options.Scheduler, + writerScheduler: PipeScheduler.Inline, + pauseWriterThreshold: _options.MaxInputBufferSize ?? 0, + resumeWriterThreshold: _options.MaxInputBufferSize / 2 ?? 0, + useSynchronizationContext: false, + minimumSegmentSize: memoryPoolFeature.MemoryPool.GetMinimumSegmentSize() + ); + + var outputPipeOptions = new PipeOptions + ( + pool: memoryPoolFeature.MemoryPool, + readerScheduler: PipeScheduler.Inline, + writerScheduler: PipeScheduler.Inline, + pauseWriterThreshold: _options.MaxOutputBufferSize ?? 0, + resumeWriterThreshold: _options.MaxOutputBufferSize / 2 ?? 0, + useSynchronizationContext: false, + minimumSegmentSize: memoryPoolFeature.MemoryPool.GetMinimumSegmentSize() + ); + + // TODO: eventually make SslDuplexStream : Stream, IDuplexPipe to avoid RawStream allocation and pipe allocations + var adaptedPipeline = new AdaptedPipeline(context.Transport, new Pipe(inputPipeOptions), new Pipe(outputPipeOptions), _logger, memoryPoolFeature.MemoryPool.GetMinimumAllocSize()); + var transportStream = adaptedPipeline.TransportStream; + if (_options.ClientCertificateMode == ClientCertificateMode.NoCertificate) { - sslStream = new SslStream(context.ConnectionStream); + sslStream = new SslStream(transportStream); certificateRequired = false; } else { - sslStream = new SslStream(context.ConnectionStream, + sslStream = new SslStream(transportStream, leaveInnerStreamOpen: false, userCertificateValidationCallback: (sender, certificate, chain, sslPolicyErrors) => { @@ -132,69 +152,66 @@ private async Task InnerOnConnectionAsync(ConnectionAdapterC certificateRequired = true; } - var timeoutFeature = context.Features.Get(); - timeoutFeature.SetTimeout(_options.HandshakeTimeout); - - try + using (var cancellationTokeSource = new CancellationTokenSource(_options.HandshakeTimeout)) + using (cancellationTokeSource.Token.UnsafeRegister(state => ((ConnectionContext)state).Abort(), context)) { - // Adapt to the SslStream signature - ServerCertificateSelectionCallback selector = null; - if (_serverCertificateSelector != null) + try { - selector = (sender, name) => + // Adapt to the SslStream signature + ServerCertificateSelectionCallback selector = null; + if (_serverCertificateSelector != null) { - context.Features.Set(sslStream); - var cert = _serverCertificateSelector(context.ConnectionContext, name); - if (cert != null) + selector = (sender, name) => { - EnsureCertificateIsAllowedForServerAuth(cert); - } - return cert; + context.Features.Set(sslStream); + var cert = _serverCertificateSelector(context, name); + if (cert != null) + { + EnsureCertificateIsAllowedForServerAuth(cert); + } + return cert; + }; + } + + var sslOptions = new SslServerAuthenticationOptions + { + ServerCertificate = _serverCertificate, + ServerCertificateSelectionCallback = selector, + ClientCertificateRequired = certificateRequired, + EnabledSslProtocols = _options.SslProtocols, + CertificateRevocationCheckMode = _options.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, + ApplicationProtocols = new List() }; - } - var sslOptions = new SslServerAuthenticationOptions - { - ServerCertificate = _serverCertificate, - ServerCertificateSelectionCallback = selector, - ClientCertificateRequired = certificateRequired, - EnabledSslProtocols = _options.SslProtocols, - CertificateRevocationCheckMode = _options.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, - ApplicationProtocols = new List() - }; - - // This is order sensitive - if ((_options.HttpProtocols & HttpProtocols.Http2) != 0) + // This is order sensitive + if ((_options.HttpProtocols & HttpProtocols.Http2) != 0) + { + sslOptions.ApplicationProtocols.Add(SslApplicationProtocol.Http2); + // https://tools.ietf.org/html/rfc7540#section-9.2.1 + sslOptions.AllowRenegotiation = false; + } + + if ((_options.HttpProtocols & HttpProtocols.Http1) != 0) + { + sslOptions.ApplicationProtocols.Add(SslApplicationProtocol.Http11); + } + + _options.OnAuthenticate?.Invoke(context, sslOptions); + + await sslStream.AuthenticateAsServerAsync(sslOptions, CancellationToken.None); + } + catch (OperationCanceledException) { - sslOptions.ApplicationProtocols.Add(SslApplicationProtocol.Http2); - // https://tools.ietf.org/html/rfc7540#section-9.2.1 - sslOptions.AllowRenegotiation = false; + _logger?.LogDebug(2, CoreStrings.AuthenticationTimedOut); + sslStream.Dispose(); + return; } - - if ((_options.HttpProtocols & HttpProtocols.Http1) != 0) + catch (Exception ex) when (ex is IOException || ex is AuthenticationException) { - sslOptions.ApplicationProtocols.Add(SslApplicationProtocol.Http11); + _logger?.LogDebug(1, ex, CoreStrings.AuthenticationFailed); + sslStream.Dispose(); + return; } - - _options.OnAuthenticate?.Invoke(context.ConnectionContext, sslOptions); - - await sslStream.AuthenticateAsServerAsync(sslOptions, CancellationToken.None); - } - catch (OperationCanceledException) - { - _logger.LogDebug(2, CoreStrings.AuthenticationTimedOut); - sslStream.Dispose(); - return _closedAdaptedConnection; - } - catch (Exception ex) when (ex is IOException || ex is AuthenticationException) - { - _logger.LogDebug(1, ex, CoreStrings.AuthenticationFailed); - sslStream.Dispose(); - return _closedAdaptedConnection; - } - finally - { - timeoutFeature.CancelTimeout(); } feature.ApplicationProtocol = sslStream.NegotiatedApplicationProtocol.Protocol; @@ -208,7 +225,31 @@ private async Task InnerOnConnectionAsync(ConnectionAdapterC feature.KeyExchangeStrength = sslStream.KeyExchangeStrength; feature.Protocol = sslStream.SslProtocol; - return new HttpsAdaptedConnection(sslStream); + var original = context.Transport; + + try + { + context.Transport = adaptedPipeline; + + using (sslStream) + { + try + { + adaptedPipeline.RunAsync(sslStream); + + await _next(context); + } + finally + { + await adaptedPipeline.CompleteAsync(); + } + } + } + finally + { + // Restore the original so that it gets closed appropriately + context.Transport = original; + } } private static void EnsureCertificateIsAllowedForServerAuth(X509Certificate2 certificate) @@ -233,31 +274,5 @@ private static X509Certificate2 ConvertToX509Certificate2(X509Certificate certif return new X509Certificate2(certificate); } - - private class HttpsAdaptedConnection : IAdaptedConnection - { - private readonly SslStream _sslStream; - - public HttpsAdaptedConnection(SslStream sslStream) - { - _sslStream = sslStream; - } - - public Stream ConnectionStream => _sslStream; - - public void Dispose() - { - _sslStream.Dispose(); - } - } - - private class ClosedAdaptedConnection : IAdaptedConnection - { - public Stream ConnectionStream { get; } = new ClosedStream(); - - public void Dispose() - { - } - } } } diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index df024e8750f6..4522fedd6293 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -256,7 +256,7 @@ public void Load() } // EndpointDefaults or configureEndpoint may have added an https adapter. - if (https && !listenOptions.ConnectionAdapters.Any(f => f.IsHttps)) + if (https && !listenOptions.IsTls) { if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) { diff --git a/src/Servers/Kestrel/Core/src/KestrelServer.cs b/src/Servers/Kestrel/Core/src/KestrelServer.cs index 0481df8fad71..868594b8f45e 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServer.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServer.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO.Pipelines; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; diff --git a/src/Servers/Kestrel/Core/src/ListenOptions.cs b/src/Servers/Kestrel/Core/src/ListenOptions.cs index 85651e1956ba..f3153bae1477 100644 --- a/src/Servers/Kestrel/Core/src/ListenOptions.cs +++ b/src/Servers/Kestrel/Core/src/ListenOptions.cs @@ -89,23 +89,35 @@ internal ListenOptions(ulong fileHandle, FileHandleType handleType) public IServiceProvider ApplicationServices => KestrelServerOptions?.ApplicationServices; + internal string Scheme + { + get + { + if (IsHttp) + { + return IsTls ? "https" : "http"; + } + return "tcp"; + } + } + + internal bool IsHttp { get; set; } = true; + + internal bool IsTls { get; set; } + /// /// Gets the name of this endpoint to display on command-line when the web server starts. /// internal virtual string GetDisplayName() { - var scheme = ConnectionAdapters.Any(f => f.IsHttps) - ? "https" - : "http"; - switch (EndPoint) { case IPEndPoint _: - return $"{scheme}://{IPEndPoint}"; + return $"{Scheme}://{IPEndPoint}"; case UnixDomainSocketEndPoint _: - return $"{scheme}://unix:{EndPoint}"; + return $"{Scheme}://unix:{EndPoint}"; case FileHandleEndPoint _: - return $"{scheme}://"; + return $"{Scheme}://"; default: throw new InvalidOperationException(); } diff --git a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs index e1950bd52735..fd119bd7851b 100644 --- a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs +++ b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs @@ -10,6 +10,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; namespace Microsoft.AspNetCore.Hosting { @@ -185,6 +186,7 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, ActionThe . public static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsConnectionAdapterOptions httpsOptions) { - var loggerFactory = listenOptions.KestrelServerOptions.ApplicationServices.GetRequiredService(); + var loggerFactory = listenOptions.KestrelServerOptions?.ApplicationServices.GetRequiredService() ?? NullLoggerFactory.Instance; + // Set the list of protocols from listen options httpsOptions.HttpProtocols = listenOptions.Protocols; - listenOptions.ConnectionAdapters.Add(new HttpsConnectionAdapter(httpsOptions, loggerFactory)); + httpsOptions.MaxInputBufferSize = listenOptions.KestrelServerOptions?.Limits.MaxRequestBufferSize; + httpsOptions.MaxOutputBufferSize = listenOptions.KestrelServerOptions?.Limits.MaxResponseBufferSize; + + listenOptions.IsTls = true; + + listenOptions.Use(next => + { + var middleware = new HttpsConnectionMiddleware(next, httpsOptions, loggerFactory); + return middleware.OnConnectionAsync; + }); return listenOptions; } } diff --git a/src/Servers/Kestrel/Core/src/LocalhostListenOptions.cs b/src/Servers/Kestrel/Core/src/LocalhostListenOptions.cs index c7e5f47cadb5..ba0a82736d1b 100644 --- a/src/Servers/Kestrel/Core/src/LocalhostListenOptions.cs +++ b/src/Servers/Kestrel/Core/src/LocalhostListenOptions.cs @@ -28,11 +28,7 @@ internal LocalhostListenOptions(int port) /// internal override string GetDisplayName() { - var scheme = ConnectionAdapters.Any(f => f.IsHttps) - ? "https" - : "http"; - - return $"{scheme}://localhost:{IPEndPoint.Port}"; + return $"{Scheme}://localhost:{IPEndPoint.Port}"; } internal override async Task BindAsync(AddressBindContext context) @@ -78,6 +74,7 @@ internal ListenOptions Clone(IPAddress address) { KestrelServerOptions = KestrelServerOptions, Protocols = Protocols, + IsTls = IsTls }; options._middleware.AddRange(_middleware); diff --git a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs index 8af4397809be..36add68a21a1 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs @@ -61,7 +61,7 @@ public void StartWithHttpsAddressConfiguresHttpsEndpoints() StartDummyApplication(server); Assert.True(server.Options.ListenOptions.Any()); - Assert.Contains(server.Options.ListenOptions[0].ConnectionAdapters, adapter => adapter.IsHttps); + Assert.True(server.Options.ListenOptions[0].IsTls); } } diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs index f3bedad135c4..fbcf3b568ea6 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs @@ -168,8 +168,8 @@ public void ConfigureDefaultsAppliesToNewConfigureEndpoints() Assert.True(ran1); Assert.True(ran2); - Assert.NotNull(serverOptions.ListenOptions[0].ConnectionAdapters.Where(adapter => adapter.IsHttps).SingleOrDefault()); - Assert.Null(serverOptions.ListenOptions[1].ConnectionAdapters.Where(adapter => adapter.IsHttps).SingleOrDefault()); + Assert.True(serverOptions.ListenOptions[0].IsTls); + Assert.False(serverOptions.ListenOptions[1].IsTls); } [Fact] @@ -210,8 +210,8 @@ public void ConfigureEndpointDefaultCanEnableHttps() Assert.True(ran2); // You only get Https once per endpoint. - Assert.NotNull(serverOptions.ListenOptions[0].ConnectionAdapters.Where(adapter => adapter.IsHttps).SingleOrDefault()); - Assert.NotNull(serverOptions.ListenOptions[1].ConnectionAdapters.Where(adapter => adapter.IsHttps).SingleOrDefault()); + Assert.True(serverOptions.ListenOptions[0].IsTls); + Assert.True(serverOptions.ListenOptions[1].IsTls); } [Fact] diff --git a/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs b/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs index 1382a7017599..3db27b32ea7f 100644 --- a/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs +++ b/src/Servers/Kestrel/shared/test/TransportTestHelpers/TestServer.cs @@ -42,12 +42,20 @@ public TestServer(RequestDelegate app, TestServiceContext context) } public TestServer(RequestDelegate app, TestServiceContext context, ListenOptions listenOptions) - : this(app, context, listenOptions, _ => { }) + : this(app, context, options => options.ListenOptions.Add(listenOptions), _ => { }) { } - public TestServer(RequestDelegate app, TestServiceContext context, ListenOptions listenOptions, Action configureServices) - : this(app, context, options => options.ListenOptions.Add(listenOptions), configureServices) + public TestServer(RequestDelegate app, TestServiceContext context, Action configureListenOptions) + : this(app, context, options => + { + var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + { + KestrelServerOptions = options + }; + configureListenOptions(listenOptions); + options.ListenOptions.Add(listenOptions); + }, _ => { }) { } diff --git a/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs b/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs index eccfa17c2e0e..26ffd760a9fa 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs @@ -71,7 +71,7 @@ public async Task TlsAlpnHandshakeSelectsHttp2From1and2() "ALPN: " + tlsFeature.ApplicationProtocol.Length); return context.Response.WriteAsync("hello world " + context.Request.Protocol); - }, new TestServiceContext(LoggerFactory), + }, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, kestrelOptions => { kestrelOptions.Listen(IPAddress.Loopback, 0, listenOptions => @@ -102,7 +102,7 @@ public async Task TlsAlpnHandshakeSelectsHttp2() "ALPN: " + tlsFeature.ApplicationProtocol.Length); return context.Response.WriteAsync("hello world " + context.Request.Protocol); - }, new TestServiceContext(LoggerFactory), + }, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, kestrelOptions => { kestrelOptions.Listen(IPAddress.Loopback, 0, listenOptions => diff --git a/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs b/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs index 6020455633b1..cd9582a9697b 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/Http2/ShutdownTests.cs @@ -60,7 +60,7 @@ public async Task GracefulShutdownWaitsForRequestsToFinish() .Setup(m => m.Http2ConnectionClosing(It.IsAny())) .Callback(() => requestStopping.SetResult(null)); - var testContext = new TestServiceContext(LoggerFactory, mockKestrelTrace.Object); + var testContext = new TestServiceContext(LoggerFactory, mockKestrelTrace.Object) { ExpectedConnectionMiddlewareCount = 1 }; testContext.InitializeHeartbeat(); @@ -111,7 +111,8 @@ public async Task GracefulTurnsAbortiveIfRequestsDoNotFinish() var testContext = new TestServiceContext(LoggerFactory) { - MemoryPoolFactory = memoryPoolFactory.Create + MemoryPoolFactory = memoryPoolFactory.Create, + ExpectedConnectionMiddlewareCount = 1 }; TestApplicationErrorLogger.ThrowOnUngracefulShutdown = false; diff --git a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs index bb1dc036d7a8..5604f402297f 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs @@ -583,18 +583,16 @@ public async Task HttpsConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate { MinResponseDataRate = new MinDataRate(bytesPerSecond: 1024 * 1024, gracePeriod: TimeSpan.FromSeconds(2)) } - } + }, + ExpectedConnectionMiddlewareCount = 1 }; testContext.InitializeHeartbeat(); - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = - { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions { ServerCertificate = certificate }) - } - }; + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { ServerCertificate = certificate }); + } using (var server = new TestServer(async context => { @@ -621,7 +619,7 @@ public async Task HttpsConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate { await aborted.Task.DefaultTimeout(); } - }, testContext, listenOptions)) + }, testContext, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/TlsTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/TlsTests.cs index 283d0c2e2def..275fcb999767 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/TlsTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/TlsTests.cs @@ -30,7 +30,7 @@ public class TlsTests : LoggedTest private static X509Certificate2 _x509Certificate2 = TestResources.GetTestCertificate(); [ConditionalFact] - [SkipOnHelix("https://github.com/aspnet/AspNetCore/issues/7000 ")] + [SkipOnHelix("https://github.com/aspnet/AspNetCore/issues/7000")] public async Task TlsHandshakeRejectsTlsLessThan12() { using (var server = new TestServer(context => @@ -41,7 +41,7 @@ public async Task TlsHandshakeRejectsTlsLessThan12() return context.Response.WriteAsync("hello world " + context.Request.Protocol); }, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1}, listenOptions => { listenOptions.Protocols = HttpProtocols.Http2; diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionAdapterTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs similarity index 69% rename from src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionAdapterTests.cs rename to src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs index f2a4d4113634..f0da9d9b7db9 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionAdapterTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs @@ -13,6 +13,7 @@ using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections.Features; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; @@ -34,15 +35,12 @@ public class HttpsConnectionAdapterTests : LoggedTest [Fact] public async Task CanReadAndWriteWithHttpsConnectionAdapter() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = - { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }) - } + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }); }; - await using (var server = new TestServer(App, new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(App, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { var result = await server.HttpClientSlim.PostAsync($"https://localhost:{server.Port}/", new FormUrlEncodedContent(new[] { @@ -57,12 +55,9 @@ public async Task CanReadAndWriteWithHttpsConnectionAdapter() [Fact] public async Task HandshakeDetailsAreAvailable() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = - { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }) - } + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }); }; await using (var server = new TestServer(context => @@ -78,7 +73,7 @@ public async Task HandshakeDetailsAreAvailable() Assert.True(tlsFeature.KeyExchangeStrength >= 0, "KeyExchangeStrength"); // May be 0 on mac return context.Response.WriteAsync("hello world"); - }, new TestServiceContext(LoggerFactory), listenOptions)) + }, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { var result = await server.HttpClientSlim.GetStringAsync($"https://localhost:{server.Port}/", validateCertificate: false); Assert.Equal("hello world", result); @@ -88,20 +83,14 @@ public async Task HandshakeDetailsAreAvailable() [Fact] public async Task RequireCertificateFailsWhenNoCertificate() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)); + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - ConnectionAdapters = - { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions - { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = ClientCertificateMode.RequireCertificate - }) - } - }; - + ServerCertificate = _x509Certificate2, + ClientCertificateMode = ClientCertificateMode.RequireCertificate + }); - await using (var server = new TestServer(App, new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(App, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions)) { await Assert.ThrowsAnyAsync( () => server.HttpClientSlim.GetStringAsync($"https://localhost:{server.Port}/")); @@ -111,17 +100,14 @@ await Assert.ThrowsAnyAsync( [Fact] public async Task AllowCertificateContinuesWhenNoCertificate() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions - { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = ClientCertificateMode.AllowCertificate - }) - } - }; + ServerCertificate = _x509Certificate2, + ClientCertificateMode = ClientCertificateMode.AllowCertificate + }); + } await using (var server = new TestServer(context => { @@ -129,7 +115,7 @@ public async Task AllowCertificateContinuesWhenNoCertificate() Assert.NotNull(tlsFeature); Assert.Null(tlsFeature.ClientCertificate); return context.Response.WriteAsync("hello world"); - }, new TestServiceContext(LoggerFactory), listenOptions)) + }, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { var result = await server.HttpClientSlim.GetStringAsync($"https://localhost:{server.Port}/", validateCertificate: false); Assert.Equal("hello world", result); @@ -139,7 +125,7 @@ public async Task AllowCertificateContinuesWhenNoCertificate() [Fact] public void ThrowsWhenNoServerCertificateIsProvided() { - Assert.Throws(() => new HttpsConnectionAdapter( + Assert.Throws(() => new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions()) ); } @@ -147,15 +133,12 @@ public void ThrowsWhenNoServerCertificateIsProvided() [Fact] public async Task UsesProvidedServerCertificate() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = - { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }) - } + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }); }; - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -173,24 +156,22 @@ public async Task UsesProvidedServerCertificate() public async Task UsesProvidedServerCertificateSelector() { var selectorCalled = 0; - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + ServerCertificateSelector = (connection, name) => { - ServerCertificateSelector = (connection, name) => - { - Assert.NotNull(connection); - Assert.NotNull(connection.Features.Get()); - Assert.Equal("localhost", name); - selectorCalled++; - return _x509Certificate2; - } - }) - } - }; - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + Assert.NotNull(connection); + Assert.NotNull(connection.Features.Get()); + Assert.Equal("localhost", name); + selectorCalled++; + return _x509Certificate2; + } + }); + } + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -209,28 +190,26 @@ public async Task UsesProvidedServerCertificateSelector() public async Task UsesProvidedServerCertificateSelectorEachTime() { var selectorCalled = 0; - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + ServerCertificateSelector = (connection, name) => { - ServerCertificateSelector = (connection, name) => + Assert.NotNull(connection); + Assert.NotNull(connection.Features.Get()); + Assert.Equal("localhost", name); + selectorCalled++; + if (selectorCalled == 1) { - Assert.NotNull(connection); - Assert.NotNull(connection.Features.Get()); - Assert.Equal("localhost", name); - selectorCalled++; - if (selectorCalled == 1) - { - return _x509Certificate2; - } - return _x509Certificate2NoExt; + return _x509Certificate2; } - }) - } - }; - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + return _x509Certificate2NoExt; + } + }); + } + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -259,21 +238,19 @@ public async Task UsesProvidedServerCertificateSelectorEachTime() public async Task UsesProvidedServerCertificateSelectorValidatesEkus() { var selectorCalled = 0; - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + ServerCertificateSelector = (features, name) => { - ServerCertificateSelector = (features, name) => - { - selectorCalled++; - return TestResources.GetTestCertificate("eku.code_signing.pfx"); - } - }) - } - }; - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + selectorCalled++; + return TestResources.GetTestCertificate("eku.code_signing.pfx"); + } + }); + } + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -292,25 +269,23 @@ await Assert.ThrowsAsync(() => public async Task UsesProvidedServerCertificateSelectorOverridesServerCertificate() { var selectorCalled = 0; - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + ServerCertificate = _x509Certificate2NoExt, + ServerCertificateSelector = (connection, name) => { - ServerCertificate = _x509Certificate2NoExt, - ServerCertificateSelector = (connection, name) => - { - Assert.NotNull(connection); - Assert.NotNull(connection.Features.Get()); - Assert.Equal("localhost", name); - selectorCalled++; - return _x509Certificate2; - } - }) - } - }; - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + Assert.NotNull(connection); + Assert.NotNull(connection.Features.Get()); + Assert.Equal("localhost", name); + selectorCalled++; + return _x509Certificate2; + } + }); + } + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -329,21 +304,19 @@ public async Task UsesProvidedServerCertificateSelectorOverridesServerCertificat public async Task UsesProvidedServerCertificateSelectorFailsIfYouReturnNull() { var selectorCalled = 0; - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + ServerCertificateSelector = (features, name) => { - ServerCertificateSelector = (features, name) => - { - selectorCalled++; - return null; - } - }) - } - }; - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + selectorCalled++; + return null; + } + }); + } + + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -363,19 +336,16 @@ await Assert.ThrowsAsync(() => [InlineData(HttpProtocols.Http1AndHttp2)] // Make sure Http/1.1 doesn't regress with Http/2 enabled. public async Task CertificatePassedToHttpContext(HttpProtocols httpProtocols) { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - Protocols = httpProtocols, - ConnectionAdapters = + listenOptions.Protocols = httpProtocols; + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions - { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = ClientCertificateMode.RequireCertificate, - ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => true - }) - } - }; + ServerCertificate = _x509Certificate2, + ClientCertificateMode = ClientCertificateMode.RequireCertificate, + ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => true + }); + } await using (var server = new TestServer(context => { @@ -384,7 +354,7 @@ public async Task CertificatePassedToHttpContext(HttpProtocols httpProtocols) Assert.NotNull(tlsFeature.ClientCertificate); Assert.NotNull(context.Connection.ClientCertificate); return context.Response.WriteAsync("hello world"); - }, new TestServiceContext(LoggerFactory), listenOptions)) + }, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -401,15 +371,12 @@ public async Task CertificatePassedToHttpContext(HttpProtocols httpProtocols) [Fact] public async Task HttpsSchemePassedToRequestFeature() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = - { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }) - } - }; + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { ServerCertificate = _x509Certificate2 }); + } - await using (var server = new TestServer(context => context.Response.WriteAsync(context.Request.Scheme), new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(context => context.Response.WriteAsync(context.Request.Scheme), new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { var result = await server.HttpClientSlim.GetStringAsync($"https://localhost:{server.Port}/", validateCertificate: false); Assert.Equal("https", result); @@ -419,20 +386,18 @@ public async Task HttpsSchemePassedToRequestFeature() [Fact] public async Task DoesNotSupportTls10() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions - { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = ClientCertificateMode.RequireCertificate, - ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => true - }) - } - }; + ServerCertificate = _x509Certificate2, + ClientCertificateMode = ClientCertificateMode.RequireCertificate, + ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => true + }); + } + - await using (var server = new TestServer(context => context.Response.WriteAsync("hello world"), new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(context => context.Response.WriteAsync("hello world"), new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { // SslStream is used to ensure the certificate is actually passed to the server // HttpClient might not send the certificate because it is invalid or it doesn't match any @@ -453,26 +418,23 @@ public async Task DoesNotSupportTls10() public async Task ClientCertificateValidationGetsCalledWithNotNullParameters(ClientCertificateMode mode) { var clientCertificateValidationCalled = false; - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + ServerCertificate = _x509Certificate2, + ClientCertificateMode = mode, + ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = mode, - ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => - { - clientCertificateValidationCalled = true; - Assert.NotNull(certificate); - Assert.NotNull(chain); - return true; - } - }) - } - }; + clientCertificateValidationCalled = true; + Assert.NotNull(certificate); + Assert.NotNull(chain); + return true; + } + }); + } - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -490,20 +452,17 @@ public async Task ClientCertificateValidationGetsCalledWithNotNullParameters(Cli [InlineData(ClientCertificateMode.RequireCertificate)] public async Task ValidationFailureRejectsConnection(ClientCertificateMode mode) { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions - { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = mode, - ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => false - }) - } - }; + ServerCertificate = _x509Certificate2, + ClientCertificateMode = mode, + ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => false + }); + } - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -519,19 +478,16 @@ public async Task ValidationFailureRejectsConnection(ClientCertificateMode mode) [InlineData(ClientCertificateMode.RequireCertificate)] public async Task RejectsConnectionOnSslPolicyErrorsWhenNoValidation(ClientCertificateMode mode) { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions - { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = mode - }) - } - }; + ServerCertificate = _x509Certificate2, + ClientCertificateMode = mode + }); + } - await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { using (var connection = server.CreateConnection()) { @@ -545,18 +501,15 @@ public async Task RejectsConnectionOnSslPolicyErrorsWhenNoValidation(ClientCerti [Fact] public async Task CertificatePassedToHttpContextIsNotDisposed() { - var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + void ConfigureListenOptions(ListenOptions listenOptions) { - ConnectionAdapters = + listenOptions.UseHttps(new HttpsConnectionAdapterOptions { - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions - { - ServerCertificate = _x509Certificate2, - ClientCertificateMode = ClientCertificateMode.RequireCertificate, - ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => true - }) - } - }; + ServerCertificate = _x509Certificate2, + ClientCertificateMode = ClientCertificateMode.RequireCertificate, + ClientCertificateValidation = (certificate, chain, sslPolicyErrors) => true + }); + } RequestDelegate app = context => { @@ -568,7 +521,7 @@ public async Task CertificatePassedToHttpContextIsNotDisposed() return context.Response.WriteAsync("hello world"); }; - await using (var server = new TestServer(app, new TestServiceContext(LoggerFactory), listenOptions)) + await using (var server = new TestServer(app, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, ConfigureListenOptions)) { // SslStream is used to ensure the certificate is actually passed to the server // HttpClient might not send the certificate because it is invalid or it doesn't match any @@ -591,7 +544,7 @@ public void AcceptsCertificateWithoutExtensions(string testCertName) var cert = new X509Certificate2(certPath, "testPassword"); Assert.Empty(cert.Extensions.OfType()); - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions { ServerCertificate = cert, }); @@ -609,7 +562,7 @@ public void ValidatesEnhancedKeyUsageOnCertificate(string testCertName) var eku = Assert.Single(cert.Extensions.OfType()); Assert.NotEmpty(eku.EnhancedKeyUsages); - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions { ServerCertificate = cert, }); @@ -628,7 +581,7 @@ public void ThrowsForCertificatesMissingServerEku(string testCertName) Assert.NotEmpty(eku.EnhancedKeyUsages); var ex = Assert.Throws(() => - new HttpsConnectionAdapter(new HttpsConnectionAdapterOptions + new HttpsConnectionMiddleware(context => Task.CompletedTask, new HttpsConnectionAdapterOptions { ServerCertificate = cert, })); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs index 00602983e4b9..87d0316d7aa9 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs @@ -122,7 +122,7 @@ public async Task EmptyRequestLoggedAsDebug() LoggerFactory.AddProvider(loggerProvider); await using (var server = new TestServer(context => Task.CompletedTask, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(TestResources.GetTestCertificate()); @@ -149,7 +149,7 @@ public async Task ClientHandshakeFailureLoggedAsDebug() LoggerFactory.AddProvider(loggerProvider); await using (var server = new TestServer(context => Task.CompletedTask, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(TestResources.GetTestCertificate()); @@ -193,7 +193,7 @@ public async Task DoesNotThrowObjectDisposedExceptionOnConnectionAbort() } } }, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(TestResources.GetTestCertificate()); @@ -237,7 +237,7 @@ public async Task DoesNotThrowObjectDisposedExceptionFromWriteAsyncAfterConnecti tcs.SetException(ex); } }, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(TestResources.GetTestCertificate()); @@ -268,7 +268,7 @@ public async Task DoesNotThrowObjectDisposedExceptionOnEmptyConnection() LoggerFactory.AddProvider(loggerProvider); await using (var server = new TestServer(context => Task.CompletedTask, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(TestResources.GetTestCertificate()); @@ -294,7 +294,7 @@ public async Task ConnectionFilterDoesNotLeakBlock() LoggerFactory.AddProvider(loggerProvider); await using (var server = new TestServer(context => Task.CompletedTask, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(TestResources.GetTestCertificate()); @@ -313,7 +313,7 @@ public async Task HandshakeTimesOutAndIsLoggedAsDebug() var loggerProvider = new HandshakeErrorLoggerProvider(); LoggerFactory.AddProvider(loggerProvider); - var testContext = new TestServiceContext(LoggerFactory); + var testContext = new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }; var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var handshakeStartedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -326,7 +326,10 @@ public async Task HandshakeTimesOutAndIsLoggedAsDebug() listenOptions.UseHttps(o => { o.ServerCertificate = new X509Certificate2(TestResources.GetTestCertificate()); - o.OnAuthenticate = (_, __) => handshakeStartedTcs.SetResult(null); + o.OnAuthenticate = (_, __) => + { + handshakeStartedTcs.SetResult(null); + }; handshakeTimeout = o.HandshakeTimeout; }); @@ -358,7 +361,7 @@ public async Task ClientAttemptingToUseUnsupportedProtocolIsLoggedAsDebug() LoggerFactory.AddProvider(loggerProvider); await using (var server = new TestServer(context => Task.CompletedTask, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(TestResources.GetTestCertificate()); @@ -390,7 +393,7 @@ public async Task OnAuthenticate_SeesOtherSettings() var onAuthenticateCalled = false; await using (var server = new TestServer(context => Task.CompletedTask, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(httpsOptions => @@ -426,7 +429,7 @@ public async Task OnAuthenticate_CanSetSettings() var onAuthenticateCalled = false; await using (var server = new TestServer(context => Task.CompletedTask, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, listenOptions => { listenOptions.UseHttps(httpsOptions => @@ -462,7 +465,7 @@ private class HandshakeErrorLoggerProvider : ILoggerProvider public ILogger CreateLogger(string categoryName) { - if (categoryName == TypeNameHelper.GetTypeDisplayName(typeof(HttpsConnectionAdapter))) + if (categoryName == TypeNameHelper.GetTypeDisplayName(typeof(HttpsConnectionMiddleware))) { return FilterLogger; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/LoggingConnectionAdapterTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/LoggingConnectionAdapterTests.cs index 4fe77ba7d347..c7ae1d3d5d0e 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/LoggingConnectionAdapterTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/LoggingConnectionAdapterTests.cs @@ -23,7 +23,7 @@ public async Task LoggingConnectionAdapterCanBeAddedBeforeAndAfterHttpsAdapter() context.Response.ContentLength = 12; return context.Response.WriteAsync("Hello World!"); }, - new TestServiceContext(LoggerFactory), + new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1}, listenOptions => { listenOptions.UseConnectionLogging(); From 283591362def1c8b648049aada0cc0f99c7fe3ba Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 20 Jun 2019 01:09:07 +0200 Subject: [PATCH 2/5] Fix the number of expected middleware --- .../Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs b/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs index 26ffd760a9fa..7d7717ba1337 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/Http2/HandshakeTests.cs @@ -44,7 +44,7 @@ public void TlsAndHttp2NotSupportedOnMac() var ex = Assert.Throws(() => new TestServer(context => { throw new NotImplementedException(); - }, new TestServiceContext(LoggerFactory), + }, new TestServiceContext(LoggerFactory) { ExpectedConnectionMiddlewareCount = 1 }, kestrelOptions => { kestrelOptions.Listen(IPAddress.Loopback, 0, listenOptions => From ef7555fec774fc322f810c01c2365ec6e67e797e Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 20 Jun 2019 01:10:27 +0200 Subject: [PATCH 3/5] Fixed comment --- src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs b/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs index 58fe6b8de8c8..26a9d7b86348 100644 --- a/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs @@ -265,7 +265,7 @@ private void OnInputOrOutputCompleted() switch (previousState) { case ProtocolSelectionState.Initializing: - // OnReader/WriterCompleted callbacks are not wired until after leaving the Initializing state. + // ConnectionClosed callback is not wired up until after leaving the Initializing state. Debug.Assert(false); CloseUninitializedConnection(new ConnectionAbortedException("HttpConnection.OnInputOrOutputCompleted() called while in the ProtocolSelectionState.Initializing state!?")); From 5117b181a04de4b1cccd60196ab42591e089fb81 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 20 Jun 2019 01:14:05 +0200 Subject: [PATCH 4/5] Sort usings --- .../Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs index 6a62913c62fa..8a2900916ca0 100644 --- a/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.IO.Pipelines; using System.Net.Security; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; @@ -11,13 +12,12 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; -using Microsoft.Extensions.Logging; -using Microsoft.AspNetCore.Http.Features; -using System.IO.Pipelines; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal From 86a17b9ed80ebe6978d3a034f5021113f47c88dc Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 20 Jun 2019 01:15:50 +0200 Subject: [PATCH 5/5] Add check that got removed in merge --- .../Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs index 8a2900916ca0..5cd60b720c10 100644 --- a/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Internal/HttpsConnectionMiddleware.cs @@ -6,6 +6,7 @@ using System.IO; using System.IO.Pipelines; using System.Net.Security; +using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using System.Threading; @@ -43,6 +44,12 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter throw new ArgumentNullException(nameof(options)); } + // This configuration will always fail per-request, preemptively fail it here. See HttpConnection.SelectProtocol(). + if (options.HttpProtocols == HttpProtocols.Http2 && RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + throw new NotSupportedException(CoreStrings.HTTP2NoTlsOsx); + } + _next = next; // capture the certificate now so it can't be switched after validation _serverCertificate = options.ServerCertificate;