From 5b737d6f45f5b1ef05aea6df87e0e95c623d5285 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 6 Jul 2020 20:44:25 -0700 Subject: [PATCH 01/24] Kestrel SNI from config --- .../Routing/src/Matching/HostMatcherPolicy.cs | 2 +- .../Core/src/HttpsConnectionAdapterOptions.cs | 6 +- .../Core/src/Internal/ConfigurationReader.cs | 116 +++++- .../Core/src/KestrelConfigurationLoader.cs | 128 ++++++- .../Core/src/ListenOptionsHttpsExtensions.cs | 22 ++ .../Middleware/HttpsConnectionMiddleware.cs | 332 +++++++++++------- 6 files changed, 453 insertions(+), 153 deletions(-) diff --git a/src/Http/Routing/src/Matching/HostMatcherPolicy.cs b/src/Http/Routing/src/Matching/HostMatcherPolicy.cs index d0f29612e469..2a3047489ebf 100644 --- a/src/Http/Routing/src/Matching/HostMatcherPolicy.cs +++ b/src/Http/Routing/src/Matching/HostMatcherPolicy.cs @@ -118,7 +118,7 @@ public Task ApplyAsync(HttpContext httpContext, CandidateSet candidates) else if ( host.StartsWith(WildcardPrefix) && - // Note that we only slice of the `*`. We want to match the leading `.` also. + // Note that we only slice off the `*`. We want to match the leading `.` also. MemoryExtensions.EndsWith(requestHost, host.Slice(WildcardHost.Length), StringComparison.OrdinalIgnoreCase)) { // Matches a suffix wildcard. diff --git a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs index d801316b5f9c..30d06f0c0ed3 100644 --- a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs +++ b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs @@ -8,6 +8,7 @@ using System.Threading; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Https { @@ -24,7 +25,8 @@ public class HttpsConnectionAdapterOptions public HttpsConnectionAdapterOptions() { ClientCertificateMode = ClientCertificateMode.NoCertificate; - HandshakeTimeout = TimeSpan.FromSeconds(10); + // Defaults to 10 seconds + HandshakeTimeout = HttpsConnectionMiddleware.DefaultHandshakeTimeout; } /// @@ -91,7 +93,7 @@ public void AllowAnyClientCertificate() public Action OnAuthenticate { get; set; } /// - /// Specifies the maximum amount of time allowed for the TLS/SSL handshake. This must be positive and finite. + /// Specifies the maximum amount of time allowed for the TLS/SSL handshake. This must be positive and finite. Defaults to 10 seconds. /// public TimeSpan HandshakeTimeout { diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index 894cd0fa01ed..a3f4379c4ea9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -20,6 +20,7 @@ internal class ConfigurationReader private const string EndpointsKey = "Endpoints"; private const string UrlKey = "Url"; private const string ClientCertificateModeKey = "ClientCertificateMode"; + private const string SNIKey = "SNI"; private readonly IConfiguration _configuration; @@ -97,7 +98,8 @@ private IEnumerable ReadEndpoints() ConfigSection = endpointConfig, Certificate = new CertificateConfig(endpointConfig.GetSection(CertificateKey)), SslProtocols = ParseSslProcotols(endpointConfig.GetSection(SslProtocolsKey)), - ClientCertificateMode = ParseClientCertificateMode(endpointConfig[ClientCertificateModeKey]) + ClientCertificateMode = ParseClientCertificateMode(endpointConfig[ClientCertificateModeKey]), + SNI = ReadSni(endpointConfig.GetSection(SNIKey)), }; endpoints.Add(endpoint); @@ -106,6 +108,46 @@ private IEnumerable ReadEndpoints() return endpoints; } + private Dictionary ReadSni(IConfigurationSection sniConfig) + { + var sniDictionary = new Dictionary(0, StringComparer.OrdinalIgnoreCase); + + foreach (var sniChild in sniConfig.GetChildren()) + { + // "SNI": { + // "a.example.org": { + // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], + // "Certificate": { + // "Path": "testCert.pfx", + // "Password": "testPassword" + // }, + // "ClientCertificateMode" : "NoCertificate" + // }, + // "b.example.org": { + // "Certificate": { + // "Path": "testCert2.pfx", + // "Password": "testPassword" + // } + // } + // } + + var sni = new SniConfig + { + Name = sniChild.Key, + Protocols = ParseProtocols(sniChild[ProtocolsKey]), + Certificate = new CertificateConfig(sniChild.GetSection(CertificateKey)), + SslProtocols = ParseSslProcotols(sniChild.GetSection(SslProtocolsKey)), + ClientCertificateMode = ParseClientCertificateMode(sniChild[ClientCertificateModeKey]) + }; + + sniDictionary[sniChild.Key] = sni; + } + + return sniDictionary; + } + + private ClientCertificateMode? ParseClientCertificateMode(string clientCertificateMode) { if (Enum.TryParse(clientCertificateMode, ignoreCase: true, out var result)) @@ -164,6 +206,27 @@ internal class EndpointDefaults // }, // "ClientCertificateMode" : "NoCertificate" // } + // *OR* + // "EndpointName": { + // "Url": "https://*", + // "SNI": { + // "a.example.org": { + // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], + // "Certificate": { + // "Path": "testCert.pfx", + // "Password": "testPassword" + // }, + // "ClientCertificateMode" : "NoCertificate" + // }, + // "b.example.org": { + // "Certificate": { + // "Path": "testCert2.pfx", + // "Password": "testPassword" + // } + // } + // } + // } internal class EndpointConfig { private IConfigurationSection _configSection; @@ -175,6 +238,7 @@ internal class EndpointConfig public SslProtocols? SslProtocols { get; set; } public CertificateConfig Certificate { get; set; } public ClientCertificateMode? ClientCertificateMode { get; set; } + public Dictionary SNI { get; set; } // Compare config sections because it's accessible to app developers via an Action callback. // We cannot rely entirely on comparing config sections for equality, because KestrelConfigurationLoader.Reload() sets @@ -196,14 +260,60 @@ obj is EndpointConfig other && Name == other.Name && Url == other.Url && (Protocols ?? ListenOptions.DefaultHttpProtocols) == (other.Protocols ?? ListenOptions.DefaultHttpProtocols) && - Certificate == other.Certificate && (SslProtocols ?? System.Security.Authentication.SslProtocols.None) == (other.SslProtocols ?? System.Security.Authentication.SslProtocols.None) && + Certificate == other.Certificate && + (ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate) == (other.ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate) && + CompareSniDictionaries(SNI, other.SNI) && _configSectionClone == other._configSectionClone; - public override int GetHashCode() => HashCode.Combine(Name, Url, Protocols ?? ListenOptions.DefaultHttpProtocols, Certificate, _configSectionClone); + public override int GetHashCode() => HashCode.Combine(Name, Url, + Protocols ?? ListenOptions.DefaultHttpProtocols, SslProtocols ?? System.Security.Authentication.SslProtocols.None, + Certificate, ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate, SNI.Count, _configSectionClone); public static bool operator ==(EndpointConfig lhs, EndpointConfig rhs) => lhs is null ? rhs is null : lhs.Equals(rhs); public static bool operator !=(EndpointConfig lhs, EndpointConfig rhs) => !(lhs == rhs); + + private static bool CompareSniDictionaries(Dictionary lhs, Dictionary rhs) + { + if (lhs.Count != rhs.Count) + { + return false; + } + + foreach (var (lhsName, lhsSniConfig) in lhs) + { + if (!rhs.TryGetValue(lhsName, out var rhsSniConfig) || lhsSniConfig != rhsSniConfig) + { + return false; + } + } + + return true; + } + } + + internal class SniConfig + { + public string Name { get; set; } + public HttpProtocols? Protocols { get; set; } + public SslProtocols? SslProtocols { get; set; } + public CertificateConfig Certificate { get; set; } + public ClientCertificateMode? ClientCertificateMode { get; set; } + + public override bool Equals(object obj) => + obj is SniConfig other && + Name == other.Name && + (Protocols ?? ListenOptions.DefaultHttpProtocols) == (other.Protocols ?? ListenOptions.DefaultHttpProtocols) && + Certificate == other.Certificate && + (SslProtocols ?? System.Security.Authentication.SslProtocols.None) == (other.SslProtocols ?? System.Security.Authentication.SslProtocols.None) && + (ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate) == (other.ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate); + + public override int GetHashCode() => HashCode.Combine(Name, + Protocols ?? ListenOptions.DefaultHttpProtocols, SslProtocols ?? System.Security.Authentication.SslProtocols.None, + Certificate, ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate); + + public static bool operator ==(SniConfig lhs, SniConfig rhs) => lhs is null ? rhs is null : lhs.Equals(rhs); + public static bool operator !=(SniConfig lhs, SniConfig rhs) => !(lhs == rhs); } // "CertificateName": { diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 86e63ddc6a73..3813f58a9429 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -6,15 +6,19 @@ using System.IO; using System.Linq; using System.Net; +using System.Net.Security; using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNetCore.Certificates.Generation; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Https; +using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -277,6 +281,8 @@ public void Load() // Compare to UseHttps(httpsOptions => { }) var httpsOptions = new HttpsConnectionAdapterOptions(); + ServerOptionsSelectionCallback serverOptionsCallback = null; + if (https) { httpsOptions.SslProtocols = ConfigurationReader.EndpointDefaults.SslProtocols ?? SslProtocols.None; @@ -289,23 +295,31 @@ public void Load() { httpsOptions.SslProtocols = endpoint.SslProtocols.Value; } + else + { + // Ensure endpoint is reloaded if it used the default protocol and the SslProtocols changed. + endpoint.SslProtocols = ConfigurationReader.EndpointDefaults.SslProtocols; + } if (endpoint.ClientCertificateMode.HasValue) { httpsOptions.ClientCertificateMode = endpoint.ClientCertificateMode.Value; } + else + { + // Ensure endpoint is reloaded if it used the default protocol and the ClientCertificateMode changed. + endpoint.ClientCertificateMode = ConfigurationReader.EndpointDefaults.ClientCertificateMode; + } // Specified - httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) - ?? httpsOptions.ServerCertificate; - - if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) + if (endpoint.SNI.Count == 0) { - // Fallback - Options.ApplyDefaultCert(httpsOptions); - - // Ensure endpoint is reloaded if it used the default certificate and the certificate changed. - endpoint.Certificate = DefaultCertificateConfig; + LoadEndpointOrDefaultCertificate(httpsOptions, endpoint); + } + else + { + serverOptionsCallback = (stream, clientHelloInfo, state, cancellationToken) => + SniCallback(httpsOptions, listenOptions.Protocols, endpoint, clientHelloInfo); } } @@ -329,12 +343,19 @@ public void Load() // EndpointDefaults or configureEndpoint may have added an https adapter. if (https && !listenOptions.IsTls) { - if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) + if (serverOptionsCallback is null) { - throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); - } + if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) + { + throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); + } - listenOptions.UseHttps(httpsOptions); + listenOptions.UseHttps(httpsOptions); + } + else + { + listenOptions.UseHttps(serverOptionsCallback); + } } listenOptions.EndpointConfig = endpoint; @@ -346,6 +367,82 @@ public void Load() return (endpointsToStop, endpointsToStart); } + private ValueTask SniCallback( + HttpsConnectionAdapterOptions httpsOptions, + HttpProtocols endpointDefaultHttpProtocols, + EndpointConfig endpoint, + SslClientHelloInfo clientHelloInfo) + { + const string wildcardHost = "*"; + const string wildcardPrefix = "*."; + + var sslServerOptions = new SslServerAuthenticationOptions(); + var serverName = clientHelloInfo.ServerName; + SniConfig sniConfig = null; + + if (!string.IsNullOrEmpty(serverName)) + { + foreach (var (name, sniConfigCandidate) in endpoint.SNI) + { + if (name.Equals(serverName, StringComparison.OrdinalIgnoreCase)) + { + sniConfig = sniConfigCandidate; + break; + } + // Note that we only slice off the `*`. We want to match the leading `.` also. + else if (name.StartsWith(wildcardPrefix, StringComparison.Ordinal) + && serverName.EndsWith(name.Substring(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) + && name.Length > (sniConfig?.Name.Length ?? 0)) + { + sniConfig = sniConfigCandidate; + } + else if (name.Equals(wildcardHost, StringComparison.Ordinal)) + { + sniConfig ??= sniConfigCandidate; + } + } + } + + sslServerOptions.ServerCertificate = LoadCertificate(sniConfig?.Certificate, endpoint.Name) ?? LoadEndpointOrDefaultCertificate(httpsOptions, endpoint); + + if (sslServerOptions.ServerCertificate is null) + { + throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); + } + + sslServerOptions.EnabledSslProtocols = sniConfig?.SslProtocols ?? httpsOptions.SslProtocols; + HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, sniConfig?.Protocols ?? endpointDefaultHttpProtocols); + + var clientCertificateMode = sniConfig?.ClientCertificateMode ?? httpsOptions.ClientCertificateMode; + + if (clientCertificateMode != ClientCertificateMode.NoCertificate) + { + sslServerOptions.ClientCertificateRequired = true; + sslServerOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => + HttpsConnectionMiddleware.RemoteCertificateValidationCallback(clientCertificateMode, httpsOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors); + } + + return new ValueTask(sslServerOptions); + } + + private X509Certificate2 LoadEndpointOrDefaultCertificate(HttpsConnectionAdapterOptions httpsOptions, EndpointConfig endpoint) + { + // Specified + httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) + ?? httpsOptions.ServerCertificate; + + if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) + { + // Fallback + Options.ApplyDefaultCert(httpsOptions); + + // Ensure endpoint is reloaded if it used the default certificate and the certificate changed. + endpoint.Certificate = DefaultCertificateConfig; + } + + return httpsOptions.ServerCertificate; + } + private void LoadDefaultCert(ConfigurationReader configReader) { if (configReader.Certificates.TryGetValue("Default", out var defaultCertConfig)) @@ -436,6 +533,11 @@ private bool TryGetCertificatePath(out string path) private X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) { + if (certInfo is null) + { + return null; + } + var logger = Options.ApplicationServices.GetRequiredService>(); if (certInfo.IsFileCert && certInfo.IsStoreCert) { diff --git a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs index d664fa5ee0e9..55f36f37240a 100644 --- a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs +++ b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Net.Security; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Https; @@ -227,5 +228,26 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsConn return listenOptions; } + + /// + /// Configure Kestrel to use HTTPS. + /// + /// The to configure. + /// Callback to configure HTTPS options. + /// State for the . + /// The . + internal static ListenOptions UseHttps(this ListenOptions listenOptions, ServerOptionsSelectionCallback serverOptionsCallback, object state = null) + { + var loggerFactory = listenOptions.KestrelServerOptions?.ApplicationServices.GetRequiredService() ?? NullLoggerFactory.Instance; + + listenOptions.IsTls = true; + listenOptions.Use(next => + { + var middleware = new HttpsConnectionMiddleware(next, serverOptionsCallback, state, loggerFactory); + return middleware.OnConnectionAsync; + }); + + return listenOptions; + } } } diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index 3f908091fc50..30a747583398 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.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.Buffers; using System.Collections.Generic; using System.IO; using System.IO.Pipelines; @@ -26,12 +27,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal internal class HttpsConnectionMiddleware { private const string EnableWindows81Http2 = "Microsoft.AspNetCore.Server.Kestrel.EnableWindows81Http2"; + + internal static TimeSpan DefaultHandshakeTimeout = TimeSpan.FromSeconds(10); + private readonly ConnectionDelegate _next; - private readonly HttpsConnectionAdapterOptions _options; private readonly ILogger _logger; + private readonly Func _sslStreamFactory; + + // The following fields are only set by HttpsConnectionAdapterOptions ctor. + private readonly HttpsConnectionAdapterOptions _options; private readonly X509Certificate2 _serverCertificate; private readonly Func _serverCertificateSelector; + // The following fields are only set by ServerOptionsSelectionCallback ctor. + private readonly ServerOptionsSelectionCallback _serverOptionsSelectionCallback; + private readonly object _serverOptionsSelectionCallbackState; + public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options) : this(next, options, loggerFactory: NullLoggerFactory.Instance) { @@ -84,13 +95,30 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter { EnsureCertificateIsAllowedForServerAuth(_serverCertificate); } + + var remoteCertificateValidationCallback = _options.ClientCertificateMode == ClientCertificateMode.NoCertificate ? + (RemoteCertificateValidationCallback)null : RemoteCertificateValidationCallback; + + _sslStreamFactory = s => new SslStream(s, leaveInnerStreamOpen: false, userCertificateValidationCallback: remoteCertificateValidationCallback); + } + + internal HttpsConnectionMiddleware( + ConnectionDelegate next, + ServerOptionsSelectionCallback serverOptionsSelectionCallback, + object serverOptionsSelectionCallbackState, + ILoggerFactory loggerFactory) + { + _next = next; + _logger = loggerFactory.CreateLogger(); + _serverOptionsSelectionCallback = serverOptionsSelectionCallback; + _serverOptionsSelectionCallbackState = serverOptionsSelectionCallbackState; + _sslStreamFactory = s => new SslStream(s); } public async Task OnConnectionAsync(ConnectionContext context) { await Task.Yield(); - bool certificateRequired; if (context.Features.Get() != null) { await _next(context); @@ -101,152 +129,54 @@ public async Task OnConnectionAsync(ConnectionContext context) context.Features.Set(feature); context.Features.Set(feature); - var memoryPool = context.Features.Get()?.MemoryPool; - - var inputPipeOptions = new StreamPipeReaderOptions - ( - pool: memoryPool, - bufferSize: memoryPool.GetMinimumSegmentSize(), - minimumReadSize: memoryPool.GetMinimumAllocSize(), - leaveOpen: true - ); - - var outputPipeOptions = new StreamPipeWriterOptions - ( - pool: memoryPool, - leaveOpen: true - ); - - SslDuplexPipe sslDuplexPipe = null; - - if (_options.ClientCertificateMode == ClientCertificateMode.NoCertificate) - { - sslDuplexPipe = new SslDuplexPipe(context.Transport, inputPipeOptions, outputPipeOptions); - certificateRequired = false; - } - else - { - sslDuplexPipe = new SslDuplexPipe(context.Transport, inputPipeOptions, outputPipeOptions, s => new SslStream(s, - leaveInnerStreamOpen: false, - userCertificateValidationCallback: (sender, certificate, chain, sslPolicyErrors) => - { - if (certificate == null) - { - return _options.ClientCertificateMode != ClientCertificateMode.RequireCertificate; - } - - if (_options.ClientCertificateValidation == null) - { - if (sslPolicyErrors != SslPolicyErrors.None) - { - return false; - } - } - - var certificate2 = ConvertToX509Certificate2(certificate); - if (certificate2 == null) - { - return false; - } - - if (_options.ClientCertificateValidation != null) - { - if (!_options.ClientCertificateValidation(certificate2, chain, sslPolicyErrors)) - { - return false; - } - } - - return true; - })); - - certificateRequired = true; - } - + var sslDuplexPipe = CreateSslDuplexPipe(context.Transport, context.Features.Get()?.MemoryPool); var sslStream = sslDuplexPipe.Stream; - using (var cancellationTokeSource = new CancellationTokenSource(_options.HandshakeTimeout)) + try { - try + using var cancellationTokenSource = new CancellationTokenSource(_options?.HandshakeTimeout ?? DefaultHandshakeTimeout); + if (_serverOptionsSelectionCallback is null) { - // Adapt to the SslStream signature - ServerCertificateSelectionCallback selector = null; - if (_serverCertificateSelector != null) - { - selector = (sender, name) => - { - feature.HostName = name; - 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() - }; - - // 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); - - KestrelEventSource.Log.TlsHandshakeStart(context, sslOptions); - - await sslStream.AuthenticateAsServerAsync(sslOptions, cancellationTokeSource.Token); + await DoOptionsBasedHandshakeAsync(context, sslStream, feature, cancellationTokenSource.Token); } - catch (OperationCanceledException) + else { - KestrelEventSource.Log.TlsHandshakeFailed(context.ConnectionId); - KestrelEventSource.Log.TlsHandshakeStop(context, null); - - _logger.AuthenticationTimedOut(); - await sslStream.DisposeAsync(); - return; + var state = (this, context, feature); + await sslStream.AuthenticateAsServerAsync(ServerOptionsCallback, state, cancellationTokenSource.Token); } - catch (IOException ex) - { - KestrelEventSource.Log.TlsHandshakeFailed(context.ConnectionId); - KestrelEventSource.Log.TlsHandshakeStop(context, null); + } + catch (OperationCanceledException) + { + KestrelEventSource.Log.TlsHandshakeFailed(context.ConnectionId); + KestrelEventSource.Log.TlsHandshakeStop(context, null); - _logger.AuthenticationFailed(ex); - await sslStream.DisposeAsync(); - return; - } - catch (AuthenticationException ex) - { - KestrelEventSource.Log.TlsHandshakeFailed(context.ConnectionId); - KestrelEventSource.Log.TlsHandshakeStop(context, null); + _logger.AuthenticationTimedOut(); + await sslStream.DisposeAsync(); + return; + } + catch (IOException ex) + { + KestrelEventSource.Log.TlsHandshakeFailed(context.ConnectionId); + KestrelEventSource.Log.TlsHandshakeStop(context, null); - _logger.AuthenticationFailed(ex); + _logger.AuthenticationFailed(ex); + await sslStream.DisposeAsync(); + return; + } + catch (AuthenticationException ex) + { + KestrelEventSource.Log.TlsHandshakeFailed(context.ConnectionId); + KestrelEventSource.Log.TlsHandshakeStop(context, null); - await sslStream.DisposeAsync(); - return; - } + _logger.AuthenticationFailed(ex); + + await sslStream.DisposeAsync(); + return; } feature.ApplicationProtocol = sslStream.NegotiatedApplicationProtocol.Protocol; context.Features.Set(feature); + feature.ClientCertificate = ConvertToX509Certificate2(sslStream.RemoteCertificate); feature.CipherAlgorithm = sslStream.CipherAlgorithm; feature.CipherStrength = sslStream.CipherStrength; @@ -282,6 +212,140 @@ public async Task OnConnectionAsync(ConnectionContext context) } } + private Task DoOptionsBasedHandshakeAsync(ConnectionContext context, SslStream sslStream, Core.Internal.TlsConnectionFeature feature, CancellationToken cancellationToken) + { + // Adapt to the SslStream signature + ServerCertificateSelectionCallback selector = null; + if (_serverCertificateSelector != null) + { + selector = (sender, name) => + { + feature.HostName = name; + 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 = _options.ClientCertificateMode != ClientCertificateMode.NoCertificate, + EnabledSslProtocols = _options.SslProtocols, + CertificateRevocationCheckMode = _options.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, + }; + + ConfigureAlpn(sslOptions, _options.HttpProtocols); + + _options.OnAuthenticate?.Invoke(context, sslOptions); + + KestrelEventSource.Log.TlsHandshakeStart(context, sslOptions); + + return sslStream.AuthenticateAsServerAsync(sslOptions, cancellationToken); + } + + internal static void ConfigureAlpn(SslServerAuthenticationOptions serverOptions, HttpProtocols httpProtocols) + { + serverOptions.ApplicationProtocols = new List(); + + // This is order sensitive + if ((httpProtocols & HttpProtocols.Http2) != 0) + { + serverOptions.ApplicationProtocols.Add(SslApplicationProtocol.Http2); + // https://tools.ietf.org/html/rfc7540#section-9.2.1 + serverOptions.AllowRenegotiation = false; + } + + if ((httpProtocols & HttpProtocols.Http1) != 0) + { + serverOptions.ApplicationProtocols.Add(SslApplicationProtocol.Http11); + } + } + + private bool RemoteCertificateValidationCallback(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors) => + RemoteCertificateValidationCallback(_options.ClientCertificateMode, _options.ClientCertificateValidation, certificate, chain, sslPolicyErrors); + + internal static bool RemoteCertificateValidationCallback( + ClientCertificateMode clientCertificateMode, + Func clientCertificateValidation, + X509Certificate certificate, + X509Chain chain, + SslPolicyErrors sslPolicyErrors) + { + if (certificate == null) + { + return clientCertificateMode != ClientCertificateMode.RequireCertificate; + } + + if (clientCertificateValidation == null) + { + if (sslPolicyErrors != SslPolicyErrors.None) + { + return false; + } + } + + var certificate2 = ConvertToX509Certificate2(certificate); + if (certificate2 == null) + { + return false; + } + + if (clientCertificateValidation != null) + { + if (!clientCertificateValidation(certificate2, chain, sslPolicyErrors)) + { + return false; + } + } + + return true; + } + + private SslDuplexPipe CreateSslDuplexPipe(IDuplexPipe transport, MemoryPool memoryPool) + { + var inputPipeOptions = new StreamPipeReaderOptions + ( + pool: memoryPool, + bufferSize: memoryPool.GetMinimumSegmentSize(), + minimumReadSize: memoryPool.GetMinimumAllocSize(), + leaveOpen: true + ); + + var outputPipeOptions = new StreamPipeWriterOptions + ( + pool: memoryPool, + leaveOpen: true + ); + + return new SslDuplexPipe(transport, inputPipeOptions, outputPipeOptions, _sslStreamFactory); + } + + private static async ValueTask ServerOptionsCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) + { + var (middleware, context, feature) = (ValueTuple)state; + + feature.HostName = clientHelloInfo.ServerName; + + var sslOptions = await middleware._serverOptionsSelectionCallback(stream, clientHelloInfo, middleware._serverOptionsSelectionCallbackState, cancellationToken); + + // REVIEW: Cache results? We don't do this for ServerCertificateSelectionCallback. + if (sslOptions.ServerCertificate is X509Certificate2 cert) + { + EnsureCertificateIsAllowedForServerAuth(cert); + } + + // REVIEW: Should we write any event before this? + KestrelEventSource.Log.TlsHandshakeStart(context, sslOptions); + + return sslOptions; + } + private static void EnsureCertificateIsAllowedForServerAuth(X509Certificate2 certificate) { if (!CertificateLoader.IsCertificateAllowedForServerAuth(certificate)) From b66dbc4c6dc82f95577a4661f986750b4711f8a8 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 24 Jul 2020 09:19:21 -0700 Subject: [PATCH 02/24] ConfigurationReaderTests --- .../Kestrel/test/ConfigurationReaderTests.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index 6b24b86b62d2..0709ca149b6d 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using System.Security.Authentication; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Https; @@ -273,6 +274,47 @@ public void ReadEndpointWithNoSslProtocolSettings_ReturnsNull() Assert.Null(endpoint.SslProtocols); } + [Fact] + public void ReadEndpointWithoutSniConfigured_ReturnsEmptyCollection() + { + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + }).Build(); + + var reader = new ConfigurationReader(config); + var endpoint = reader.Endpoints.First(); + Assert.NotNull(endpoint.SNI); + Assert.False(endpoint.SNI.Any()); + } + + [Fact] + public void ReadEndpointWithSniConfigured_ReturnsCorrectValue() + { + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:SNI:*.example.org:Protocols", "Http1"), + new KeyValuePair("Endpoints:End1:SNI:*.example.org:SslProtocols:0", "Tls12"), + new KeyValuePair("Endpoints:End1:SNI:*.example.org:Certificate:Path", "/path/cert.pfx"), + new KeyValuePair("Endpoints:End1:SNI:*.example.org:Certificate:Password", "certpassword"), + new KeyValuePair("Endpoints:End1:SNI:*.example.org:ClientCertificateMode", "AllowCertificate"), + }).Build(); + + var reader = new ConfigurationReader(config); + var endpoint = reader.Endpoints.First(); + var sni = endpoint.SNI["*.EXAMPLE.org"]; + + Assert.NotNull(sni); + + Assert.Equal("*.example.org", sni.Name); + Assert.Equal(HttpProtocols.Http1, sni.Protocols); + Assert.Equal(SslProtocols.Tls12, sni.SslProtocols); + Assert.Equal("/path/cert.pfx", sni.Certificate.Path); + Assert.Equal("certpassword", sni.Certificate.Password); + Assert.Equal(ClientCertificateMode.AllowCertificate, sni.ClientCertificateMode); + } + [Fact] public void ReadEndpointDefaultsWithSingleSslProtocolSet_ReturnsCorrectValue() { From 3d98e442676586dbdd8dd9d47e2794e4bb6386aa Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 24 Jul 2020 17:59:14 -0700 Subject: [PATCH 03/24] Cache SslServerAuthenticationOptions and certs --- src/Servers/Kestrel/Core/src/CoreStrings.resx | 6 + .../Core/src/Internal/SniOptionsSelector.cs | 116 ++++++++++++++++++ .../Core/src/KestrelConfigurationLoader.cs | 106 ++++------------ 3 files changed, 149 insertions(+), 79 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index 49c2f5fb482c..84b9592fdc1a 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -620,4 +620,10 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l Unknown algorithm for certificate with public key type '{0}'. + + Connection refused because no SNI configuration was found for '{serverName}' in '{endpointName}'. To allow all connections, add a wildcard ('*') SNI section. + + + Connection refulsed because the client did not specify a server name, and no wildcard ('*') SNI configuration was found in '{endpointName}'. + \ No newline at end of file diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs new file mode 100644 index 000000000000..e7d20f803a62 --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -0,0 +1,116 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Net.Security; +using System.Security.Authentication; +using Microsoft.AspNetCore.Server.Kestrel.Https; +using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal +{ + internal class SniOptionsSelector + { + private const string wildcardHost = "*"; + private const string wildcardPrefix = "*."; + + private readonly string _endpointName; + private readonly Dictionary _fullNameOptions = new Dictionary(StringComparer.OrdinalIgnoreCase); + private readonly List<(string, SslServerAuthenticationOptions)> _wildcardPrefixOptions = new List<(string, SslServerAuthenticationOptions)>(); + private readonly SslServerAuthenticationOptions _wildcardHostOptions = null; + + public SniOptionsSelector( + KestrelConfigurationLoader configLoader, + EndpointConfig endpointConfig, + HttpsConnectionAdapterOptions fallbackOptions, + HttpProtocols fallbackHttpProtocols) + { + _endpointName = endpointConfig.Name; + + foreach (var (name, sniConfig) in endpointConfig.SNI) + { + var sslServerOptions = new SslServerAuthenticationOptions(); + + sslServerOptions.ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, endpointConfig.Name) + ?? configLoader.LoadEndpointOrDefaultCertificate(fallbackOptions, endpointConfig); + + if (sslServerOptions.ServerCertificate is null) + { + throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); + } + + sslServerOptions.EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols; + HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, sniConfig.Protocols ?? fallbackHttpProtocols); + + var clientCertificateMode = sniConfig.ClientCertificateMode ?? fallbackOptions.ClientCertificateMode; + + if (clientCertificateMode != ClientCertificateMode.NoCertificate) + { + sslServerOptions.ClientCertificateRequired = true; + sslServerOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => + HttpsConnectionMiddleware.RemoteCertificateValidationCallback( + clientCertificateMode, fallbackOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors); + } + + if (name.Equals(wildcardHost, StringComparison.Ordinal)) + { + _wildcardHostOptions = sslServerOptions; + } + else if (name.StartsWith(wildcardPrefix, StringComparison.Ordinal)) + { + _wildcardPrefixOptions.Add((name, sslServerOptions)); + } + else + { + _fullNameOptions[name] = sslServerOptions; + } + } + } + + public SslServerAuthenticationOptions GetOptions(string serverName) + { + SslServerAuthenticationOptions options = null; + + if (!string.IsNullOrEmpty(serverName)) + { + if (_fullNameOptions.TryGetValue(serverName, out options)) + { + return options; + } + + var matchedNameLength = 0; + ReadOnlySpan serverNameSpan = serverName; + + foreach (var (nameCandidate, optionsCandidate) in _wildcardPrefixOptions) + { + ReadOnlySpan nameCandidateSpan = nameCandidate; + + // Note that we only slice off the `*`. We want to match the leading `.` also. + if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) + && nameCandidateSpan.Length > matchedNameLength) + { + matchedNameLength = nameCandidateSpan.Length; + options = optionsCandidate; + } + } + } + + options ??= _wildcardHostOptions; + + if (options is null) + { + if (serverName is null) + { + throw new AuthenticationException(CoreStrings.FormatSniNotConfiguredToAllowNoServerName(_endpointName)); + } + else + { + throw new AuthenticationException(CoreStrings.FormatSniNotConfiguredForServerName(serverName, _endpointName)); + } + } + + return options; + } + } +} diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 3813f58a9429..35c59fa2b47d 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -281,7 +281,7 @@ public void Load() // Compare to UseHttps(httpsOptions => { }) var httpsOptions = new HttpsConnectionAdapterOptions(); - ServerOptionsSelectionCallback serverOptionsCallback = null; + SniOptionsSelector sniOptionsSelector = null; if (https) { @@ -318,8 +318,7 @@ public void Load() } else { - serverOptionsCallback = (stream, clientHelloInfo, state, cancellationToken) => - SniCallback(httpsOptions, listenOptions.Protocols, endpoint, clientHelloInfo); + sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols); } } @@ -343,7 +342,7 @@ public void Load() // EndpointDefaults or configureEndpoint may have added an https adapter. if (https && !listenOptions.IsTls) { - if (serverOptionsCallback is null) + if (sniOptionsSelector is null) { if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) { @@ -354,7 +353,7 @@ public void Load() } else { - listenOptions.UseHttps(serverOptionsCallback); + listenOptions.UseHttps(ServerOptionsSelectionCallback, sniOptionsSelector); } } @@ -367,80 +366,11 @@ public void Load() return (endpointsToStop, endpointsToStart); } - private ValueTask SniCallback( - HttpsConnectionAdapterOptions httpsOptions, - HttpProtocols endpointDefaultHttpProtocols, - EndpointConfig endpoint, - SslClientHelloInfo clientHelloInfo) + private static ValueTask ServerOptionsSelectionCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) { - const string wildcardHost = "*"; - const string wildcardPrefix = "*."; - - var sslServerOptions = new SslServerAuthenticationOptions(); - var serverName = clientHelloInfo.ServerName; - SniConfig sniConfig = null; - - if (!string.IsNullOrEmpty(serverName)) - { - foreach (var (name, sniConfigCandidate) in endpoint.SNI) - { - if (name.Equals(serverName, StringComparison.OrdinalIgnoreCase)) - { - sniConfig = sniConfigCandidate; - break; - } - // Note that we only slice off the `*`. We want to match the leading `.` also. - else if (name.StartsWith(wildcardPrefix, StringComparison.Ordinal) - && serverName.EndsWith(name.Substring(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) - && name.Length > (sniConfig?.Name.Length ?? 0)) - { - sniConfig = sniConfigCandidate; - } - else if (name.Equals(wildcardHost, StringComparison.Ordinal)) - { - sniConfig ??= sniConfigCandidate; - } - } - } - - sslServerOptions.ServerCertificate = LoadCertificate(sniConfig?.Certificate, endpoint.Name) ?? LoadEndpointOrDefaultCertificate(httpsOptions, endpoint); - - if (sslServerOptions.ServerCertificate is null) - { - throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); - } - - sslServerOptions.EnabledSslProtocols = sniConfig?.SslProtocols ?? httpsOptions.SslProtocols; - HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, sniConfig?.Protocols ?? endpointDefaultHttpProtocols); - - var clientCertificateMode = sniConfig?.ClientCertificateMode ?? httpsOptions.ClientCertificateMode; - - if (clientCertificateMode != ClientCertificateMode.NoCertificate) - { - sslServerOptions.ClientCertificateRequired = true; - sslServerOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => - HttpsConnectionMiddleware.RemoteCertificateValidationCallback(clientCertificateMode, httpsOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors); - } - - return new ValueTask(sslServerOptions); - } - - private X509Certificate2 LoadEndpointOrDefaultCertificate(HttpsConnectionAdapterOptions httpsOptions, EndpointConfig endpoint) - { - // Specified - httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) - ?? httpsOptions.ServerCertificate; - - if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) - { - // Fallback - Options.ApplyDefaultCert(httpsOptions); - - // Ensure endpoint is reloaded if it used the default certificate and the certificate changed. - endpoint.Certificate = DefaultCertificateConfig; - } - - return httpsOptions.ServerCertificate; + var sniOptionsSelector = (SniOptionsSelector)state; + var options = sniOptionsSelector.GetOptions(clientHelloInfo.ServerName); + return new ValueTask(options); } private void LoadDefaultCert(ConfigurationReader configReader) @@ -531,7 +461,7 @@ private bool TryGetCertificatePath(out string path) return path != null; } - private X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) + internal X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) { if (certInfo is null) { @@ -622,6 +552,24 @@ static X509Certificate2 GetCertificate(string certificatePath) } } + internal X509Certificate2 LoadEndpointOrDefaultCertificate(HttpsConnectionAdapterOptions httpsOptions, EndpointConfig endpoint) + { + // Specified + httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) + ?? httpsOptions.ServerCertificate; + + if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) + { + // Fallback + Options.ApplyDefaultCert(httpsOptions); + + // Ensure endpoint is reloaded if it used the default certificate and the certificate changed. + endpoint.Certificate = DefaultCertificateConfig; + } + + return httpsOptions.ServerCertificate; + } + private static X509Certificate2 AttachPemRSAKey(X509Certificate2 certificate, string keyText, string password) { using var rsa = RSA.Create(); From 14b20e3aff8d6e9c1f07a647aa10614687c948ff Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 24 Jul 2020 19:08:02 -0700 Subject: [PATCH 04/24] Validate cached options on load --- .../Core/src/Internal/ConfigurationReader.cs | 4 +- .../Core/src/Internal/SniOptionsSelector.cs | 9 +++- .../Core/src/KestrelConfigurationLoader.cs | 3 +- .../Middleware/HttpsConnectionMiddleware.cs | 41 +++++++++++-------- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index a3f4379c4ea9..2c13103c16c9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -124,7 +124,7 @@ private Dictionary ReadSni(IConfigurationSection sniConfig) // }, // "ClientCertificateMode" : "NoCertificate" // }, - // "b.example.org": { + // "*.example.org": { // "Certificate": { // "Path": "testCert2.pfx", // "Password": "testPassword" @@ -219,7 +219,7 @@ internal class EndpointDefaults // }, // "ClientCertificateMode" : "NoCertificate" // }, - // "b.example.org": { + // "*.example.org": { // "Certificate": { // "Path": "testCert2.pfx", // "Password": "testPassword" diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index e7d20f803a62..da3e845dd87a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -7,6 +7,7 @@ using System.Security.Authentication; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { @@ -24,7 +25,8 @@ public SniOptionsSelector( KestrelConfigurationLoader configLoader, EndpointConfig endpointConfig, HttpsConnectionAdapterOptions fallbackOptions, - HttpProtocols fallbackHttpProtocols) + HttpProtocols fallbackHttpProtocols, + ILogger logger) { _endpointName = endpointConfig.Name; @@ -41,7 +43,10 @@ public SniOptionsSelector( } sslServerOptions.EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols; - HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, sniConfig.Protocols ?? fallbackHttpProtocols); + + var httpProtocols = sniConfig.Protocols ?? fallbackHttpProtocols; + httpProtocols = HttpsConnectionMiddleware.ValidateAndNormalizeHttpProtocols(httpProtocols, logger); + HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, httpProtocols); var clientCertificateMode = sniConfig.ClientCertificateMode ?? fallbackOptions.ClientCertificateMode; diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 35c59fa2b47d..d4b16170be68 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -318,7 +318,8 @@ public void Load() } else { - sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols); + var logger = Options.ApplicationServices.GetRequiredService>(); + sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols, logger); } } diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index 30a747583398..2f8fb00022a4 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -58,23 +58,7 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter _options = options; _logger = loggerFactory.CreateLogger(); - // This configuration will always fail per-request, preemptively fail it here. See HttpConnection.SelectProtocol(). - if (options.HttpProtocols == HttpProtocols.Http2) - { - if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - throw new NotSupportedException(CoreStrings.Http2NoTlsOsx); - } - else if (IsWindowsVersionIncompatible()) - { - throw new NotSupportedException(CoreStrings.Http2NoTlsWin81); - } - } - else if (options.HttpProtocols == HttpProtocols.Http1AndHttp2 && IsWindowsVersionIncompatible()) - { - _logger.Http2DefaultCiphersInsufficient(); - options.HttpProtocols = HttpProtocols.Http1; - } + _options.HttpProtocols = ValidateAndNormalizeHttpProtocols(_options.HttpProtocols, _logger); _next = next; // capture the certificate now so it can't be switched after validation @@ -369,6 +353,29 @@ private static X509Certificate2 ConvertToX509Certificate2(X509Certificate certif return new X509Certificate2(certificate); } + internal static HttpProtocols ValidateAndNormalizeHttpProtocols(HttpProtocols httpProtocols, ILogger logger) + { + // This configuration will always fail per-request, preemptively fail it here. See HttpConnection.SelectProtocol(). + if (httpProtocols == HttpProtocols.Http2) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + throw new NotSupportedException(CoreStrings.Http2NoTlsOsx); + } + else if (IsWindowsVersionIncompatible()) + { + throw new NotSupportedException(CoreStrings.Http2NoTlsWin81); + } + } + else if (httpProtocols == HttpProtocols.Http1AndHttp2 && IsWindowsVersionIncompatible()) + { + logger.Http2DefaultCiphersInsufficient(); + return HttpProtocols.Http1; + } + + return httpProtocols; + } + private static bool IsWindowsVersionIncompatible() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) From 246de72dd2edd1f606f59a4a81459049080350da Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 27 Jul 2020 22:54:55 -0700 Subject: [PATCH 05/24] WIP --- .../Core/src/Internal/SniOptionsSelector.cs | 11 ++--- .../Core/src/KestrelConfigurationLoader.cs | 42 +++++++------------ .../SampleApp/appsettings.Production.json | 9 +++- 3 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index da3e845dd87a..18a77570f1e1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -34,10 +34,11 @@ public SniOptionsSelector( { var sslServerOptions = new SslServerAuthenticationOptions(); - sslServerOptions.ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, endpointConfig.Name) - ?? configLoader.LoadEndpointOrDefaultCertificate(fallbackOptions, endpointConfig); + sslServerOptions.ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, endpointConfig.Name); - if (sslServerOptions.ServerCertificate is null) + if (sslServerOptions.ServerCertificate is null && + fallbackOptions.ServerCertificate is null && + fallbackOptions.ServerCertificateSelector is null) { throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); } @@ -92,8 +93,8 @@ public SslServerAuthenticationOptions GetOptions(string serverName) ReadOnlySpan nameCandidateSpan = nameCandidate; // Note that we only slice off the `*`. We want to match the leading `.` also. - if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) - && nameCandidateSpan.Length > matchedNameLength) + if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) && + nameCandidateSpan.Length > matchedNameLength) { matchedNameLength = nameCandidateSpan.Length; options = optionsCandidate; diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index d4b16170be68..f1f4f5d2dfb1 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -281,7 +281,6 @@ public void Load() // Compare to UseHttps(httpsOptions => { }) var httpsOptions = new HttpsConnectionAdapterOptions(); - SniOptionsSelector sniOptionsSelector = null; if (https) { @@ -311,15 +310,17 @@ public void Load() endpoint.ClientCertificateMode = ConfigurationReader.EndpointDefaults.ClientCertificateMode; } - // Specified - if (endpoint.SNI.Count == 0) - { - LoadEndpointOrDefaultCertificate(httpsOptions, endpoint); - } - else + // A cert specified directly on the endpoint overrides any defaults. + httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) + ?? httpsOptions.ServerCertificate; + + if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) { - var logger = Options.ApplicationServices.GetRequiredService>(); - sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols, logger); + // Fallback + Options.ApplyDefaultCert(httpsOptions); + + // Ensure endpoint is reloaded if it used the default certificate and the certificate changed. + endpoint.Certificate = DefaultCertificateConfig; } } @@ -343,7 +344,7 @@ public void Load() // EndpointDefaults or configureEndpoint may have added an https adapter. if (https && !listenOptions.IsTls) { - if (sniOptionsSelector is null) + if (endpoint.SNI.Count == 0) { if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) { @@ -354,6 +355,9 @@ public void Load() } else { + var logger = Options.ApplicationServices.GetRequiredService>(); + var sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols, logger); + listenOptions.UseHttps(ServerOptionsSelectionCallback, sniOptionsSelector); } } @@ -553,24 +557,6 @@ static X509Certificate2 GetCertificate(string certificatePath) } } - internal X509Certificate2 LoadEndpointOrDefaultCertificate(HttpsConnectionAdapterOptions httpsOptions, EndpointConfig endpoint) - { - // Specified - httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) - ?? httpsOptions.ServerCertificate; - - if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) - { - // Fallback - Options.ApplyDefaultCert(httpsOptions); - - // Ensure endpoint is reloaded if it used the default certificate and the certificate changed. - endpoint.Certificate = DefaultCertificateConfig; - } - - return httpsOptions.ServerCertificate; - } - private static X509Certificate2 AttachPemRSAKey(X509Certificate2 certificate, string keyText, string password) { using var rsa = RSA.Create(); diff --git a/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json b/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json index 71c9c03be034..beb07a0c5c86 100644 --- a/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json +++ b/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json @@ -1,9 +1,16 @@ -{ +{ "Kestrel": { "Endpoints": { "NamedEndpoint": { "Url": "http://*:6000" }, "NamedHttpsEndpoint": { "Url": "https://*:6443", + "SNI": { + "localhost": { + "Protocols": "Http1" + }, + "*": { + } + } } } } From 5e49ce1fed1983a391a0d0d6d55ef5465f13dae4 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 30 Jul 2020 10:49:11 -0700 Subject: [PATCH 06/24] Solidify fallback logic --- .../Kestrel/Core/src/Internal/SniOptions.cs | 36 ++++++++ .../Core/src/Internal/SniOptionsSelector.cs | 84 ++++++++++++++----- .../Core/src/KestrelConfigurationLoader.cs | 7 +- .../Core/src/ListenOptionsHttpsExtensions.cs | 6 +- .../Middleware/HttpsConnectionMiddleware.cs | 22 +++-- .../SampleApp/appsettings.Production.json | 1 + 6 files changed, 120 insertions(+), 36 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/SniOptions.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptions.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptions.cs new file mode 100644 index 000000000000..b3f245ea388e --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptions.cs @@ -0,0 +1,36 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Net.Security; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal +{ + internal class SniOptions + { + public SslServerAuthenticationOptions SslOptions { get; set; } + public HttpProtocols HttpProtocols { get; set; } + + // TODO: Reflection based test to ensure we clone everything! + public SniOptions Clone() + { + return new SniOptions + { + SslOptions = new SslServerAuthenticationOptions + { + AllowRenegotiation = SslOptions.AllowRenegotiation, + ApplicationProtocols = SslOptions.ApplicationProtocols, + CertificateRevocationCheckMode = SslOptions.CertificateRevocationCheckMode, + CipherSuitesPolicy = SslOptions.CipherSuitesPolicy, + ClientCertificateRequired = SslOptions.ClientCertificateRequired, + EnabledSslProtocols = SslOptions.EnabledSslProtocols, + EncryptionPolicy = SslOptions.EncryptionPolicy, + RemoteCertificateValidationCallback = SslOptions.RemoteCertificateValidationCallback, + ServerCertificate = SslOptions.ServerCertificate, + ServerCertificateContext = SslOptions.ServerCertificateContext, + ServerCertificateSelectionCallback = SslOptions.ServerCertificateSelectionCallback, + }, + HttpProtocols = HttpProtocols, + }; + } + } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 18a77570f1e1..507d00d3f2cd 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -3,8 +3,11 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Net.Security; using System.Security.Authentication; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.Logging; @@ -17,9 +20,13 @@ internal class SniOptionsSelector private const string wildcardPrefix = "*."; private readonly string _endpointName; - private readonly Dictionary _fullNameOptions = new Dictionary(StringComparer.OrdinalIgnoreCase); - private readonly List<(string, SslServerAuthenticationOptions)> _wildcardPrefixOptions = new List<(string, SslServerAuthenticationOptions)>(); - private readonly SslServerAuthenticationOptions _wildcardHostOptions = null; + + private readonly Func _fallbackServerCertificateSelector; + private readonly Action _onAuthenticateCallback; + + private readonly Dictionary _fullNameOptions = new Dictionary(StringComparer.OrdinalIgnoreCase); + private readonly List<(string, SniOptions)> _wildcardPrefixOptions = new List<(string, SniOptions)>(); + private readonly SniOptions _wildcardHostOptions = null; public SniOptionsSelector( KestrelConfigurationLoader configLoader, @@ -30,24 +37,30 @@ public SniOptionsSelector( { _endpointName = endpointConfig.Name; + _fallbackServerCertificateSelector = fallbackOptions.ServerCertificateSelector; + _onAuthenticateCallback = fallbackOptions.OnAuthenticate; + foreach (var (name, sniConfig) in endpointConfig.SNI) { - var sslServerOptions = new SslServerAuthenticationOptions(); - - sslServerOptions.ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, endpointConfig.Name); - - if (sslServerOptions.ServerCertificate is null && - fallbackOptions.ServerCertificate is null && - fallbackOptions.ServerCertificateSelector is null) + var sslServerOptions = new SslServerAuthenticationOptions { - throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); - } + ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, endpointConfig.Name), + EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols, + }; - sslServerOptions.EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols; + if (sslServerOptions.ServerCertificate is null) + { + if (fallbackOptions.ServerCertificate is null && fallbackOptions.ServerCertificateSelector is null) + { + throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); + } - var httpProtocols = sniConfig.Protocols ?? fallbackHttpProtocols; - httpProtocols = HttpsConnectionMiddleware.ValidateAndNormalizeHttpProtocols(httpProtocols, logger); - HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, httpProtocols); + if (fallbackOptions.ServerCertificateSelector is null) + { + // Cache the fallback ServerCertificate since there's no fallback ServerCertificateSelector taking precedence. + sslServerOptions.ServerCertificate = fallbackOptions.ServerCertificate; + } + } var clientCertificateMode = sniConfig.ClientCertificateMode ?? fallbackOptions.ClientCertificateMode; @@ -59,24 +72,34 @@ fallbackOptions.ServerCertificate is null && clientCertificateMode, fallbackOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors); } + var httpProtocols = sniConfig.Protocols ?? fallbackHttpProtocols; + httpProtocols = HttpsConnectionMiddleware.ValidateAndNormalizeHttpProtocols(httpProtocols, logger); + HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, httpProtocols); + + var sniOptions = new SniOptions + { + SslOptions = sslServerOptions, + HttpProtocols = httpProtocols, + }; + if (name.Equals(wildcardHost, StringComparison.Ordinal)) { - _wildcardHostOptions = sslServerOptions; + _wildcardHostOptions = sniOptions; } else if (name.StartsWith(wildcardPrefix, StringComparison.Ordinal)) { - _wildcardPrefixOptions.Add((name, sslServerOptions)); + _wildcardPrefixOptions.Add((name, sniOptions)); } else { - _fullNameOptions[name] = sslServerOptions; + _fullNameOptions[name] = sniOptions; } } } - public SslServerAuthenticationOptions GetOptions(string serverName) + public SniOptions GetOptions(ConnectionContext connection, string serverName) { - SslServerAuthenticationOptions options = null; + SniOptions options = null; if (!string.IsNullOrEmpty(serverName)) { @@ -116,6 +139,25 @@ public SslServerAuthenticationOptions GetOptions(string serverName) } } + if (options.SslOptions.ServerCertificate is null) + { + Debug.Assert(_fallbackServerCertificateSelector != null, + "The cached SniOptions ServerCertificate can only be null if there's a fallback certificate selector."); + + // If a ServerCertificateSelector passed into HttpsConnectionMiddleware via HttpsConnectionAdapterOptions doesn't return a cert, + // HttpsConnectionMiddleware doesn't fallback to the ServerCertificate, so we don't do that here either. + options = options.Clone(); + options.SslOptions.ServerCertificate = _fallbackServerCertificateSelector(connection, serverName); + } + + if (_onAuthenticateCallback != null) + { + options = options.Clone(); + + // From doc comments: "This is called after all of the other settings have already been applied." + _onAuthenticateCallback(connection, options.SslOptions); + } + return options; } } diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index f1f4f5d2dfb1..b7c3dd0797d4 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -14,6 +14,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Certificates.Generation; +using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; @@ -371,11 +372,11 @@ public void Load() return (endpointsToStop, endpointsToStart); } - private static ValueTask ServerOptionsSelectionCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) + private static ValueTask ServerOptionsSelectionCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) { var sniOptionsSelector = (SniOptionsSelector)state; - var options = sniOptionsSelector.GetOptions(clientHelloInfo.ServerName); - return new ValueTask(options); + var options = sniOptionsSelector.GetOptions(connection, clientHelloInfo.ServerName); + return new ValueTask(options.SslOptions); } private void LoadDefaultCert(ConfigurationReader configReader) diff --git a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs index 55f36f37240a..41080eebcc96 100644 --- a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs +++ b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs @@ -233,17 +233,17 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsConn /// Configure Kestrel to use HTTPS. /// /// The to configure. - /// Callback to configure HTTPS options. + /// Callback to configure HTTPS options. /// State for the . /// The . - internal static ListenOptions UseHttps(this ListenOptions listenOptions, ServerOptionsSelectionCallback serverOptionsCallback, object state = null) + internal static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsOptionsCallback httpsOptionsCallback, object state = null) { var loggerFactory = listenOptions.KestrelServerOptions?.ApplicationServices.GetRequiredService() ?? NullLoggerFactory.Instance; listenOptions.IsTls = true; listenOptions.Use(next => { - var middleware = new HttpsConnectionMiddleware(next, serverOptionsCallback, state, loggerFactory); + var middleware = new HttpsConnectionMiddleware(next, httpsOptionsCallback, state, loggerFactory); return middleware.OnConnectionAsync; }); diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index 2f8fb00022a4..ab2338ee2cf9 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -24,11 +24,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal { + + internal delegate ValueTask HttpsOptionsCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken); + internal class HttpsConnectionMiddleware { private const string EnableWindows81Http2 = "Microsoft.AspNetCore.Server.Kestrel.EnableWindows81Http2"; - internal static TimeSpan DefaultHandshakeTimeout = TimeSpan.FromSeconds(10); + internal static readonly TimeSpan DefaultHandshakeTimeout = TimeSpan.FromSeconds(10); private readonly ConnectionDelegate _next; private readonly ILogger _logger; @@ -40,8 +43,9 @@ internal class HttpsConnectionMiddleware private readonly Func _serverCertificateSelector; // The following fields are only set by ServerOptionsSelectionCallback ctor. - private readonly ServerOptionsSelectionCallback _serverOptionsSelectionCallback; - private readonly object _serverOptionsSelectionCallbackState; + // If we ever expose this via a public API, we should really create a delegate type. + private readonly HttpsOptionsCallback _httpsOptionsCallback; + private readonly object _httpsOptionsCallbackState; public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapterOptions options) : this(next, options, loggerFactory: NullLoggerFactory.Instance) @@ -88,14 +92,14 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter internal HttpsConnectionMiddleware( ConnectionDelegate next, - ServerOptionsSelectionCallback serverOptionsSelectionCallback, - object serverOptionsSelectionCallbackState, + HttpsOptionsCallback httpsOptionsCallback, + object httpsOptionsCallbackState, ILoggerFactory loggerFactory) { _next = next; _logger = loggerFactory.CreateLogger(); - _serverOptionsSelectionCallback = serverOptionsSelectionCallback; - _serverOptionsSelectionCallbackState = serverOptionsSelectionCallbackState; + _httpsOptionsCallback = httpsOptionsCallback; + _httpsOptionsCallbackState = httpsOptionsCallbackState; _sslStreamFactory = s => new SslStream(s); } @@ -119,7 +123,7 @@ public async Task OnConnectionAsync(ConnectionContext context) try { using var cancellationTokenSource = new CancellationTokenSource(_options?.HandshakeTimeout ?? DefaultHandshakeTimeout); - if (_serverOptionsSelectionCallback is null) + if (_httpsOptionsCallback is null) { await DoOptionsBasedHandshakeAsync(context, sslStream, feature, cancellationTokenSource.Token); } @@ -316,7 +320,7 @@ private static async ValueTask ServerOptionsCall feature.HostName = clientHelloInfo.ServerName; - var sslOptions = await middleware._serverOptionsSelectionCallback(stream, clientHelloInfo, middleware._serverOptionsSelectionCallbackState, cancellationToken); + var sslOptions = await middleware._httpsOptionsCallback(context, stream, clientHelloInfo, middleware._httpsOptionsCallbackState, cancellationToken); // REVIEW: Cache results? We don't do this for ServerCertificateSelectionCallback. if (sslOptions.ServerCertificate is X509Certificate2 cert) diff --git a/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json b/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json index beb07a0c5c86..16c03fe35531 100644 --- a/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json +++ b/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json @@ -9,6 +9,7 @@ "Protocols": "Http1" }, "*": { + "SslProtocols": [ "Tls12", "Tls13" ] } } } From 79d221b6f7cf929f7a4fa24dd929bf81aeee1b82 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 29 Jul 2020 18:12:53 -0700 Subject: [PATCH 07/24] Support default handshake timeout --- .../Core/src/HttpsConnectionAdapterOptions.cs | 3 +-- .../Core/src/Internal/SniOptionsSelector.cs | 3 +-- .../Core/src/KestrelConfigurationLoader.cs | 2 +- .../Core/src/ListenOptionsHttpsExtensions.cs | 5 ++-- .../Middleware/HttpsConnectionMiddleware.cs | 24 +++++++++++-------- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs index 30d06f0c0ed3..365008497842 100644 --- a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs +++ b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs @@ -25,8 +25,7 @@ public class HttpsConnectionAdapterOptions public HttpsConnectionAdapterOptions() { ClientCertificateMode = ClientCertificateMode.NoCertificate; - // Defaults to 10 seconds - HandshakeTimeout = HttpsConnectionMiddleware.DefaultHandshakeTimeout; + HandshakeTimeout = TimeSpan.FromSeconds(10); } /// diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 507d00d3f2cd..8e43d895f4de 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -144,8 +144,7 @@ public SniOptions GetOptions(ConnectionContext connection, string serverName) Debug.Assert(_fallbackServerCertificateSelector != null, "The cached SniOptions ServerCertificate can only be null if there's a fallback certificate selector."); - // If a ServerCertificateSelector passed into HttpsConnectionMiddleware via HttpsConnectionAdapterOptions doesn't return a cert, - // HttpsConnectionMiddleware doesn't fallback to the ServerCertificate, so we don't do that here either. + // If a ServerCertificateSelector doesn't return a cert, HttpsConnectionMiddleware doesn't fallback to the ServerCertificate. options = options.Clone(); options.SslOptions.ServerCertificate = _fallbackServerCertificateSelector(connection, serverName); } diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index b7c3dd0797d4..75e31e6a177f 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -359,7 +359,7 @@ public void Load() var logger = Options.ApplicationServices.GetRequiredService>(); var sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols, logger); - listenOptions.UseHttps(ServerOptionsSelectionCallback, sniOptionsSelector); + listenOptions.UseHttps(ServerOptionsSelectionCallback, sniOptionsSelector, httpsOptions.HandshakeTimeout); } } diff --git a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs index 41080eebcc96..2a2f737ab77f 100644 --- a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs +++ b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs @@ -235,15 +235,16 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsConn /// The to configure. /// Callback to configure HTTPS options. /// State for the . + /// Specifies the maximum amount of time allowed for the TLS/SSL handshake. This must be positive and finite. /// The . - internal static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsOptionsCallback httpsOptionsCallback, object state = null) + internal static ListenOptions UseHttps(this ListenOptions listenOptions, HttpsOptionsCallback httpsOptionsCallback, object state, TimeSpan handshakeTimeout) { var loggerFactory = listenOptions.KestrelServerOptions?.ApplicationServices.GetRequiredService() ?? NullLoggerFactory.Instance; listenOptions.IsTls = true; listenOptions.Use(next => { - var middleware = new HttpsConnectionMiddleware(next, httpsOptionsCallback, state, loggerFactory); + var middleware = new HttpsConnectionMiddleware(next, httpsOptionsCallback, state, handshakeTimeout, loggerFactory); return middleware.OnConnectionAsync; }); diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index ab2338ee2cf9..d3edd280e7c1 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -31,9 +31,10 @@ internal class HttpsConnectionMiddleware { private const string EnableWindows81Http2 = "Microsoft.AspNetCore.Server.Kestrel.EnableWindows81Http2"; - internal static readonly TimeSpan DefaultHandshakeTimeout = TimeSpan.FromSeconds(10); + private static readonly bool _isWindowsVersionIncompatible = IsWindowsVersionIncompatible(); private readonly ConnectionDelegate _next; + private readonly TimeSpan _handshakeTimeout; private readonly ILogger _logger; private readonly Func _sslStreamFactory; @@ -43,7 +44,6 @@ internal class HttpsConnectionMiddleware private readonly Func _serverCertificateSelector; // The following fields are only set by ServerOptionsSelectionCallback ctor. - // If we ever expose this via a public API, we should really create a delegate type. private readonly HttpsOptionsCallback _httpsOptionsCallback; private readonly object _httpsOptionsCallbackState; @@ -59,12 +59,13 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter throw new ArgumentNullException(nameof(options)); } - _options = options; + _next = next; + _handshakeTimeout = options.HandshakeTimeout; _logger = loggerFactory.CreateLogger(); + _options = options; _options.HttpProtocols = ValidateAndNormalizeHttpProtocols(_options.HttpProtocols, _logger); - _next = next; // capture the certificate now so it can't be switched after validation _serverCertificate = options.ServerCertificate; _serverCertificateSelector = options.ServerCertificateSelector; @@ -94,10 +95,13 @@ internal HttpsConnectionMiddleware( ConnectionDelegate next, HttpsOptionsCallback httpsOptionsCallback, object httpsOptionsCallbackState, + TimeSpan handshakeTimeout, ILoggerFactory loggerFactory) { _next = next; + _handshakeTimeout = handshakeTimeout; _logger = loggerFactory.CreateLogger(); + _httpsOptionsCallback = httpsOptionsCallback; _httpsOptionsCallbackState = httpsOptionsCallbackState; _sslStreamFactory = s => new SslStream(s); @@ -122,7 +126,7 @@ public async Task OnConnectionAsync(ConnectionContext context) try { - using var cancellationTokenSource = new CancellationTokenSource(_options?.HandshakeTimeout ?? DefaultHandshakeTimeout); + using var cancellationTokenSource = new CancellationTokenSource(_handshakeTimeout); if (_httpsOptionsCallback is null) { await DoOptionsBasedHandshakeAsync(context, sslStream, feature, cancellationTokenSource.Token); @@ -255,9 +259,6 @@ internal static void ConfigureAlpn(SslServerAuthenticationOptions serverOptions, } } - private bool RemoteCertificateValidationCallback(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors) => - RemoteCertificateValidationCallback(_options.ClientCertificateMode, _options.ClientCertificateValidation, certificate, chain, sslPolicyErrors); - internal static bool RemoteCertificateValidationCallback( ClientCertificateMode clientCertificateMode, Func clientCertificateValidation, @@ -295,6 +296,9 @@ internal static bool RemoteCertificateValidationCallback( return true; } + private bool RemoteCertificateValidationCallback(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors) => + RemoteCertificateValidationCallback(_options.ClientCertificateMode, _options.ClientCertificateValidation, certificate, chain, sslPolicyErrors); + private SslDuplexPipe CreateSslDuplexPipe(IDuplexPipe transport, MemoryPool memoryPool) { var inputPipeOptions = new StreamPipeReaderOptions @@ -366,12 +370,12 @@ internal static HttpProtocols ValidateAndNormalizeHttpProtocols(HttpProtocols ht { throw new NotSupportedException(CoreStrings.Http2NoTlsOsx); } - else if (IsWindowsVersionIncompatible()) + else if (_isWindowsVersionIncompatible) { throw new NotSupportedException(CoreStrings.Http2NoTlsWin81); } } - else if (httpProtocols == HttpProtocols.Http1AndHttp2 && IsWindowsVersionIncompatible()) + else if (httpProtocols == HttpProtocols.Http1AndHttp2 && _isWindowsVersionIncompatible) { logger.Http2DefaultCiphersInsufficient(); return HttpProtocols.Http1; From b01b5ced5646939162e16f01a67fbd051ed70174 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 29 Jul 2020 19:06:27 -0700 Subject: [PATCH 08/24] Flow SNI HttpProtocols to middleware --- .../Core/src/Internal/HttpProtocolsFeature.cs | 15 +++ .../Kestrel/Core/src/Internal/SniOptions.cs | 36 ------ .../Core/src/Internal/SniOptionsSelector.cs | 111 ++++++++++++------ .../Core/src/KestrelConfigurationLoader.cs | 6 +- .../Middleware/HttpConnectionMiddleware.cs | 6 +- .../Middleware/HttpsConnectionMiddleware.cs | 1 - 6 files changed, 93 insertions(+), 82 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/HttpProtocolsFeature.cs delete mode 100644 src/Servers/Kestrel/Core/src/Internal/SniOptions.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpProtocolsFeature.cs b/src/Servers/Kestrel/Core/src/Internal/HttpProtocolsFeature.cs new file mode 100644 index 000000000000..c29b2cc5013c --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/HttpProtocolsFeature.cs @@ -0,0 +1,15 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal +{ + internal class HttpProtocolsFeature + { + public HttpProtocolsFeature(HttpProtocols httpProtocols) + { + HttpProtocols = httpProtocols; + } + + public HttpProtocols HttpProtocols { get; } + } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptions.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptions.cs deleted file mode 100644 index b3f245ea388e..000000000000 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptions.cs +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Net.Security; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal -{ - internal class SniOptions - { - public SslServerAuthenticationOptions SslOptions { get; set; } - public HttpProtocols HttpProtocols { get; set; } - - // TODO: Reflection based test to ensure we clone everything! - public SniOptions Clone() - { - return new SniOptions - { - SslOptions = new SslServerAuthenticationOptions - { - AllowRenegotiation = SslOptions.AllowRenegotiation, - ApplicationProtocols = SslOptions.ApplicationProtocols, - CertificateRevocationCheckMode = SslOptions.CertificateRevocationCheckMode, - CipherSuitesPolicy = SslOptions.CipherSuitesPolicy, - ClientCertificateRequired = SslOptions.ClientCertificateRequired, - EnabledSslProtocols = SslOptions.EnabledSslProtocols, - EncryptionPolicy = SslOptions.EncryptionPolicy, - RemoteCertificateValidationCallback = SslOptions.RemoteCertificateValidationCallback, - ServerCertificate = SslOptions.ServerCertificate, - ServerCertificateContext = SslOptions.ServerCertificateContext, - ServerCertificateSelectionCallback = SslOptions.ServerCertificateSelectionCallback, - }, - HttpProtocols = HttpProtocols, - }; - } - } -} diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 8e43d895f4de..872b1d772abb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Net.Security; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; @@ -42,13 +43,13 @@ public SniOptionsSelector( foreach (var (name, sniConfig) in endpointConfig.SNI) { - var sslServerOptions = new SslServerAuthenticationOptions + var sslOptions = new SslServerAuthenticationOptions { ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, endpointConfig.Name), EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols, }; - if (sslServerOptions.ServerCertificate is null) + if (sslOptions.ServerCertificate is null) { if (fallbackOptions.ServerCertificate is null && fallbackOptions.ServerCertificateSelector is null) { @@ -58,7 +59,7 @@ public SniOptionsSelector( if (fallbackOptions.ServerCertificateSelector is null) { // Cache the fallback ServerCertificate since there's no fallback ServerCertificateSelector taking precedence. - sslServerOptions.ServerCertificate = fallbackOptions.ServerCertificate; + sslOptions.ServerCertificate = fallbackOptions.ServerCertificate; } } @@ -66,19 +67,19 @@ public SniOptionsSelector( if (clientCertificateMode != ClientCertificateMode.NoCertificate) { - sslServerOptions.ClientCertificateRequired = true; - sslServerOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => + sslOptions.ClientCertificateRequired = true; + sslOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => HttpsConnectionMiddleware.RemoteCertificateValidationCallback( clientCertificateMode, fallbackOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors); } var httpProtocols = sniConfig.Protocols ?? fallbackHttpProtocols; httpProtocols = HttpsConnectionMiddleware.ValidateAndNormalizeHttpProtocols(httpProtocols, logger); - HttpsConnectionMiddleware.ConfigureAlpn(sslServerOptions, httpProtocols); + HttpsConnectionMiddleware.ConfigureAlpn(sslOptions, httpProtocols); var sniOptions = new SniOptions { - SslOptions = sslServerOptions, + SslOptions = sslOptions, HttpProtocols = httpProtocols, }; @@ -97,37 +98,19 @@ public SniOptionsSelector( } } - public SniOptions GetOptions(ConnectionContext connection, string serverName) + public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, string serverName) { - SniOptions options = null; + SniOptions sniOptions = null; - if (!string.IsNullOrEmpty(serverName)) + if (!string.IsNullOrEmpty(serverName) && !_fullNameOptions.TryGetValue(serverName, out sniOptions)) { - if (_fullNameOptions.TryGetValue(serverName, out options)) - { - return options; - } - - var matchedNameLength = 0; - ReadOnlySpan serverNameSpan = serverName; - - foreach (var (nameCandidate, optionsCandidate) in _wildcardPrefixOptions) - { - ReadOnlySpan nameCandidateSpan = nameCandidate; - - // Note that we only slice off the `*`. We want to match the leading `.` also. - if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) && - nameCandidateSpan.Length > matchedNameLength) - { - matchedNameLength = nameCandidateSpan.Length; - options = optionsCandidate; - } - } + TryGetWildcardPrefixedOptions(serverName, out sniOptions); } - options ??= _wildcardHostOptions; + // Fully wildcarded ("*") options can be used even when given an empty server name. + sniOptions ??= _wildcardHostOptions; - if (options is null) + if (sniOptions is null) { if (serverName is null) { @@ -139,25 +122,75 @@ public SniOptions GetOptions(ConnectionContext connection, string serverName) } } - if (options.SslOptions.ServerCertificate is null) + connection.Features.Set(new HttpProtocolsFeature(sniOptions.HttpProtocols)); + + var sslOptions = sniOptions.SslOptions; + + if (sslOptions.ServerCertificate is null) { Debug.Assert(_fallbackServerCertificateSelector != null, "The cached SniOptions ServerCertificate can only be null if there's a fallback certificate selector."); // If a ServerCertificateSelector doesn't return a cert, HttpsConnectionMiddleware doesn't fallback to the ServerCertificate. - options = options.Clone(); - options.SslOptions.ServerCertificate = _fallbackServerCertificateSelector(connection, serverName); + sslOptions = CloneSslOptions(sslOptions); + sslOptions.ServerCertificate = _fallbackServerCertificateSelector(connection, serverName); } if (_onAuthenticateCallback != null) { - options = options.Clone(); + sslOptions = CloneSslOptions(sslOptions); // From doc comments: "This is called after all of the other settings have already been applied." - _onAuthenticateCallback(connection, options.SslOptions); - } + _onAuthenticateCallback(connection, sslOptions); + } + + return sslOptions; + } - return options; + private bool TryGetWildcardPrefixedOptions(string serverName, out SniOptions sniOptions) + { + sniOptions = null; + + var matchedNameLength = 0; + ReadOnlySpan serverNameSpan = serverName; + + foreach (var (nameCandidate, optionsCandidate) in _wildcardPrefixOptions) + { + ReadOnlySpan nameCandidateSpan = nameCandidate; + + // Note that we only slice off the `*`. We want to match the leading `.` also. + if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) && + nameCandidateSpan.Length > matchedNameLength) + { + matchedNameLength = nameCandidateSpan.Length; + sniOptions = optionsCandidate; + } + } + + return sniOptions != null; + } + + // TODO: Reflection based test to ensure we clone everything! + internal static SslServerAuthenticationOptions CloneSslOptions(SslServerAuthenticationOptions sslOptions) => + new SslServerAuthenticationOptions + { + AllowRenegotiation = sslOptions.AllowRenegotiation, + ApplicationProtocols = sslOptions.ApplicationProtocols?.ToList(), + CertificateRevocationCheckMode = sslOptions.CertificateRevocationCheckMode, + CipherSuitesPolicy = sslOptions.CipherSuitesPolicy, + ClientCertificateRequired = sslOptions.ClientCertificateRequired, + EnabledSslProtocols = sslOptions.EnabledSslProtocols, + EncryptionPolicy = sslOptions.EncryptionPolicy, + RemoteCertificateValidationCallback = sslOptions.RemoteCertificateValidationCallback, + ServerCertificate = sslOptions.ServerCertificate, + ServerCertificateContext = sslOptions.ServerCertificateContext, + ServerCertificateSelectionCallback = sslOptions.ServerCertificateSelectionCallback, + }; + + private class SniOptions + { + public SslServerAuthenticationOptions SslOptions { get; set; } + public HttpProtocols HttpProtocols { get; set; } } } } diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 75e31e6a177f..270ae390aeaf 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -359,7 +359,7 @@ public void Load() var logger = Options.ApplicationServices.GetRequiredService>(); var sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols, logger); - listenOptions.UseHttps(ServerOptionsSelectionCallback, sniOptionsSelector, httpsOptions.HandshakeTimeout); + listenOptions.UseHttps(SniCallback, sniOptionsSelector, httpsOptions.HandshakeTimeout); } } @@ -372,11 +372,11 @@ public void Load() return (endpointsToStop, endpointsToStart); } - private static ValueTask ServerOptionsSelectionCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) + private static ValueTask SniCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) { var sniOptionsSelector = (SniOptionsSelector)state; var options = sniOptionsSelector.GetOptions(connection, clientHelloInfo.ServerName); - return new ValueTask(options.SslOptions); + return new ValueTask(options); } private void LoadDefaultCert(ConfigurationReader configReader) diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs index d5263f5c3741..3743b3d2fa0d 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpConnectionMiddleware.cs @@ -13,13 +13,13 @@ internal class HttpConnectionMiddleware { private readonly ServiceContext _serviceContext; private readonly IHttpApplication _application; - private readonly HttpProtocols _protocols; + private readonly HttpProtocols _endpointDefaultProtocols; public HttpConnectionMiddleware(ServiceContext serviceContext, IHttpApplication application, HttpProtocols protocols) { _serviceContext = serviceContext; _application = application; - _protocols = protocols; + _endpointDefaultProtocols = protocols; } public Task OnConnectionAsync(ConnectionContext connectionContext) @@ -30,7 +30,7 @@ public Task OnConnectionAsync(ConnectionContext connectionContext) { ConnectionId = connectionContext.ConnectionId, ConnectionContext = connectionContext, - Protocols = _protocols, + Protocols = connectionContext.Features.Get()?.HttpProtocols ?? _endpointDefaultProtocols, ServiceContext = _serviceContext, ConnectionFeatures = connectionContext.Features, MemoryPool = memoryPoolFeature?.MemoryPool ?? System.Buffers.MemoryPool.Shared, diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index d3edd280e7c1..6bdb766fcef1 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -24,7 +24,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal { - internal delegate ValueTask HttpsOptionsCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken); internal class HttpsConnectionMiddleware From 2f0dd944bd20e15187cc1fbd2a06f09700358587 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 29 Jul 2020 19:20:28 -0700 Subject: [PATCH 09/24] Use default CertificateRevocationCheckMode for SNI endpoints --- .../Core/src/Internal/SniOptionsSelector.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 872b1d772abb..7e45c6532dee 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -45,18 +45,19 @@ public SniOptionsSelector( { var sslOptions = new SslServerAuthenticationOptions { - ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, endpointConfig.Name), + ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, $"{endpointConfig.Name}:SNI:{name}"), EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols, + CertificateRevocationCheckMode = fallbackOptions.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, }; if (sslOptions.ServerCertificate is null) { - if (fallbackOptions.ServerCertificate is null && fallbackOptions.ServerCertificateSelector is null) + if (fallbackOptions.ServerCertificate is null && _fallbackServerCertificateSelector is null) { throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); } - if (fallbackOptions.ServerCertificateSelector is null) + if (_fallbackServerCertificateSelector is null) { // Cache the fallback ServerCertificate since there's no fallback ServerCertificateSelector taking precedence. sslOptions.ServerCertificate = fallbackOptions.ServerCertificate; @@ -138,9 +139,8 @@ public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, s if (_onAuthenticateCallback != null) { - sslOptions = CloneSslOptions(sslOptions); - // From doc comments: "This is called after all of the other settings have already been applied." + sslOptions = CloneSslOptions(sslOptions); _onAuthenticateCallback(connection, sslOptions); } @@ -158,8 +158,8 @@ private bool TryGetWildcardPrefixedOptions(string serverName, out SniOptions sni { ReadOnlySpan nameCandidateSpan = nameCandidate; - // Note that we only slice off the `*`. We want to match the leading `.` also. - if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(wildcardHost.Length), StringComparison.OrdinalIgnoreCase) && + // Only slice off 1 character, the `*`. We want to match the leading `.` also. + if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(1), StringComparison.OrdinalIgnoreCase) && nameCandidateSpan.Length > matchedNameLength) { matchedNameLength = nameCandidateSpan.Length; @@ -171,6 +171,8 @@ private bool TryGetWildcardPrefixedOptions(string serverName, out SniOptions sni } // TODO: Reflection based test to ensure we clone everything! + // This won't catch issues related to mutable subproperties, but the existing subproperties look like they're mosly immutable. + // The exception are the ApplicationProtocols list which we clone and the ServerCertificate because of methods like Import() and Reset() :( internal static SslServerAuthenticationOptions CloneSslOptions(SslServerAuthenticationOptions sslOptions) => new SslServerAuthenticationOptions { From b66f0ed0f254bebc43c43ace171b98d28ee8b320 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 3 Aug 2020 13:58:26 -0700 Subject: [PATCH 10/24] SNI -> Sni --- .../Core/src/Internal/ConfigurationReader.cs | 22 +++++++++---------- .../Core/src/Internal/SniOptionsSelector.cs | 4 ++-- .../Core/src/KestrelConfigurationLoader.cs | 2 +- .../Kestrel/test/ConfigurationReaderTests.cs | 14 ++++++------ .../SampleApp/appsettings.Production.json | 4 ++-- 5 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index 2c13103c16c9..2c70280175d7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -20,7 +20,7 @@ internal class ConfigurationReader private const string EndpointsKey = "Endpoints"; private const string UrlKey = "Url"; private const string ClientCertificateModeKey = "ClientCertificateMode"; - private const string SNIKey = "SNI"; + private const string SniKey = "Sni"; private readonly IConfiguration _configuration; @@ -99,7 +99,7 @@ private IEnumerable ReadEndpoints() Certificate = new CertificateConfig(endpointConfig.GetSection(CertificateKey)), SslProtocols = ParseSslProcotols(endpointConfig.GetSection(SslProtocolsKey)), ClientCertificateMode = ParseClientCertificateMode(endpointConfig[ClientCertificateModeKey]), - SNI = ReadSni(endpointConfig.GetSection(SNIKey)), + Sni = ReadSni(endpointConfig.GetSection(SniKey)), }; endpoints.Add(endpoint); @@ -114,7 +114,7 @@ private Dictionary ReadSni(IConfigurationSection sniConfig) foreach (var sniChild in sniConfig.GetChildren()) { - // "SNI": { + // "Sni": { // "a.example.org": { // "Protocols": "Http1AndHttp2", // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], @@ -205,11 +205,7 @@ internal class EndpointDefaults // "Password": "testPassword" // }, // "ClientCertificateMode" : "NoCertificate" - // } - // *OR* - // "EndpointName": { - // "Url": "https://*", - // "SNI": { + // "Sni": { // "a.example.org": { // "Protocols": "Http1AndHttp2", // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], @@ -224,7 +220,9 @@ internal class EndpointDefaults // "Path": "testCert2.pfx", // "Password": "testPassword" // } - // } + // }, + // // The following should work once https://github.com/dotnet/runtime/issues/40218 is resolved + // "*": {} // } // } internal class EndpointConfig @@ -238,7 +236,7 @@ internal class EndpointConfig public SslProtocols? SslProtocols { get; set; } public CertificateConfig Certificate { get; set; } public ClientCertificateMode? ClientCertificateMode { get; set; } - public Dictionary SNI { get; set; } + public Dictionary Sni { get; set; } // Compare config sections because it's accessible to app developers via an Action callback. // We cannot rely entirely on comparing config sections for equality, because KestrelConfigurationLoader.Reload() sets @@ -263,12 +261,12 @@ obj is EndpointConfig other && (SslProtocols ?? System.Security.Authentication.SslProtocols.None) == (other.SslProtocols ?? System.Security.Authentication.SslProtocols.None) && Certificate == other.Certificate && (ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate) == (other.ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate) && - CompareSniDictionaries(SNI, other.SNI) && + CompareSniDictionaries(Sni, other.Sni) && _configSectionClone == other._configSectionClone; public override int GetHashCode() => HashCode.Combine(Name, Url, Protocols ?? ListenOptions.DefaultHttpProtocols, SslProtocols ?? System.Security.Authentication.SslProtocols.None, - Certificate, ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate, SNI.Count, _configSectionClone); + Certificate, ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate, Sni.Count, _configSectionClone); public static bool operator ==(EndpointConfig lhs, EndpointConfig rhs) => lhs is null ? rhs is null : lhs.Equals(rhs); public static bool operator !=(EndpointConfig lhs, EndpointConfig rhs) => !(lhs == rhs); diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 7e45c6532dee..97e6ddafff8b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -41,11 +41,11 @@ public SniOptionsSelector( _fallbackServerCertificateSelector = fallbackOptions.ServerCertificateSelector; _onAuthenticateCallback = fallbackOptions.OnAuthenticate; - foreach (var (name, sniConfig) in endpointConfig.SNI) + foreach (var (name, sniConfig) in endpointConfig.Sni) { var sslOptions = new SslServerAuthenticationOptions { - ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, $"{endpointConfig.Name}:SNI:{name}"), + ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, $"{endpointConfig.Name}:Sni:{name}"), EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols, CertificateRevocationCheckMode = fallbackOptions.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, }; diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 270ae390aeaf..2d577f0d88ea 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -345,7 +345,7 @@ public void Load() // EndpointDefaults or configureEndpoint may have added an https adapter. if (https && !listenOptions.IsTls) { - if (endpoint.SNI.Count == 0) + if (endpoint.Sni.Count == 0) { if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) { diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index 0709ca149b6d..9ff4063dc8bb 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -284,8 +284,8 @@ public void ReadEndpointWithoutSniConfigured_ReturnsEmptyCollection() var reader = new ConfigurationReader(config); var endpoint = reader.Endpoints.First(); - Assert.NotNull(endpoint.SNI); - Assert.False(endpoint.SNI.Any()); + Assert.NotNull(endpoint.Sni); + Assert.False(endpoint.Sni.Any()); } [Fact] @@ -294,16 +294,16 @@ public void ReadEndpointWithSniConfigured_ReturnsCorrectValue() var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), - new KeyValuePair("Endpoints:End1:SNI:*.example.org:Protocols", "Http1"), - new KeyValuePair("Endpoints:End1:SNI:*.example.org:SslProtocols:0", "Tls12"), - new KeyValuePair("Endpoints:End1:SNI:*.example.org:Certificate:Path", "/path/cert.pfx"), - new KeyValuePair("Endpoints:End1:SNI:*.example.org:Certificate:Password", "certpassword"), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:Protocols", "Http1"), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:SslProtocols:0", "Tls12"), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:Path", "/path/cert.pfx"), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:Password", "certpassword"), new KeyValuePair("Endpoints:End1:SNI:*.example.org:ClientCertificateMode", "AllowCertificate"), }).Build(); var reader = new ConfigurationReader(config); var endpoint = reader.Endpoints.First(); - var sni = endpoint.SNI["*.EXAMPLE.org"]; + var sni = endpoint.Sni["*.EXAMPLE.org"]; Assert.NotNull(sni); diff --git a/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json b/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json index 16c03fe35531..91283390ad19 100644 --- a/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json +++ b/src/Servers/Kestrel/samples/SampleApp/appsettings.Production.json @@ -4,9 +4,9 @@ "NamedEndpoint": { "Url": "http://*:6000" }, "NamedHttpsEndpoint": { "Url": "https://*:6443", - "SNI": { + "Sni": { "localhost": { - "Protocols": "Http1" + "Protocols": "Http1AndHttp2" }, "*": { "SslProtocols": [ "Tls12", "Tls13" ] From f8b0f5689fe38c47a26e542661a1cb908d45694e Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 4 Aug 2020 14:22:08 -0700 Subject: [PATCH 11/24] Add CertificateConfigLoader for easier testing --- .../Certificates/CertificateConfigLoader.cs | 176 +++++++++++++++++ .../Certificates/ICertificateConfigLoader.cs | 12 ++ .../Core/src/Internal/LoggerExtensions.cs | 17 +- .../Core/src/Internal/SniOptionsSelector.cs | 7 +- .../Core/src/KestrelConfigurationLoader.cs | 187 +++--------------- .../Kestrel/Core/src/KestrelServerOptions.cs | 12 +- .../Middleware/HttpsConnectionMiddleware.cs | 12 +- .../Core/test/KestrelServerOptionsTests.cs | 20 +- .../test/KestrelConfigurationLoaderTests.cs | 1 - .../HttpsConnectionMiddlewareTests.cs | 10 +- 10 files changed, 268 insertions(+), 186 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs create mode 100644 src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs new file mode 100644 index 000000000000..e59de79ca5d2 --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs @@ -0,0 +1,176 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.IO; +using System.Runtime.InteropServices; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Server.Kestrel.Https; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates +{ + internal class CertificateConfigLoader : ICertificateConfigLoader + { + public CertificateConfigLoader(IHostEnvironment hostEnvironment, ILogger logger) + { + HostEnvironment = hostEnvironment; + Logger = logger; + } + + public IHostEnvironment HostEnvironment { get; } + public ILogger Logger { get; } + + public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) + { + if (certInfo is null) + { + return null; + } + + if (certInfo.IsFileCert && certInfo.IsStoreCert) + { + throw new InvalidOperationException(CoreStrings.FormatMultipleCertificateSources(endpointName)); + } + else if (certInfo.IsFileCert) + { + var certificatePath = Path.Combine(HostEnvironment.ContentRootPath, certInfo.Path); + if (certInfo.KeyPath != null) + { + var certificateKeyPath = Path.Combine(HostEnvironment.ContentRootPath, certInfo.KeyPath); + var certificate = GetCertificate(certificatePath); + + if (certificate != null) + { + certificate = LoadCertificateKey(certificate, certificateKeyPath, certInfo.Password); + } + else + { + Logger.FailedToLoadCertificate(certificateKeyPath); + } + + if (certificate != null) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return PersistKey(certificate); + } + + return certificate; + } + else + { + Logger.FailedToLoadCertificateKey(certificateKeyPath); + } + + throw new InvalidOperationException(CoreStrings.InvalidPemKey); + } + + return new X509Certificate2(Path.Combine(HostEnvironment.ContentRootPath, certInfo.Path), certInfo.Password); + } + else if (certInfo.IsStoreCert) + { + return LoadFromStoreCert(certInfo); + } + + return null; + } + + private static X509Certificate2 PersistKey(X509Certificate2 fullCertificate) + { + // We need to force the key to be persisted. + // See https://github.com/dotnet/runtime/issues/23749 + var certificateBytes = fullCertificate.Export(X509ContentType.Pkcs12, ""); + return new X509Certificate2(certificateBytes, "", X509KeyStorageFlags.DefaultKeySet); + } + + private static X509Certificate2 LoadCertificateKey(X509Certificate2 certificate, string keyPath, string password) + { + // OIDs for the certificate key types. + const string RSAOid = "1.2.840.113549.1.1.1"; + const string DSAOid = "1.2.840.10040.4.1"; + const string ECDsaOid = "1.2.840.10045.2.1"; + + var keyText = File.ReadAllText(keyPath); + return certificate.PublicKey.Oid.Value switch + { + RSAOid => AttachPemRSAKey(certificate, keyText, password), + ECDsaOid => AttachPemECDSAKey(certificate, keyText, password), + DSAOid => AttachPemDSAKey(certificate, keyText, password), + _ => throw new InvalidOperationException(string.Format(CoreStrings.UnrecognizedCertificateKeyOid, certificate.PublicKey.Oid.Value)) + }; + } + + private static X509Certificate2 GetCertificate(string certificatePath) + { + if (X509Certificate2.GetCertContentType(certificatePath) == X509ContentType.Cert) + { + return new X509Certificate2(certificatePath); + } + + return null; + } + + private static X509Certificate2 AttachPemRSAKey(X509Certificate2 certificate, string keyText, string password) + { + using var rsa = RSA.Create(); + if (password == null) + { + rsa.ImportFromPem(keyText); + } + else + { + rsa.ImportFromEncryptedPem(keyText, password); + } + + return certificate.CopyWithPrivateKey(rsa); + } + + private static X509Certificate2 AttachPemDSAKey(X509Certificate2 certificate, string keyText, string password) + { + using var dsa = DSA.Create(); + if (password == null) + { + dsa.ImportFromPem(keyText); + } + else + { + dsa.ImportFromEncryptedPem(keyText, password); + } + + return certificate.CopyWithPrivateKey(dsa); + } + + private static X509Certificate2 AttachPemECDSAKey(X509Certificate2 certificate, string keyText, string password) + { + using var ecdsa = ECDsa.Create(); + if (password == null) + { + ecdsa.ImportFromPem(keyText); + } + else + { + ecdsa.ImportFromEncryptedPem(keyText, password); + } + + return certificate.CopyWithPrivateKey(ecdsa); + } + + private static X509Certificate2 LoadFromStoreCert(CertificateConfig certInfo) + { + var subject = certInfo.Subject; + var storeName = string.IsNullOrEmpty(certInfo.Store) ? StoreName.My.ToString() : certInfo.Store; + var location = certInfo.Location; + var storeLocation = StoreLocation.CurrentUser; + if (!string.IsNullOrEmpty(location)) + { + storeLocation = (StoreLocation)Enum.Parse(typeof(StoreLocation), location, ignoreCase: true); + } + var allowInvalid = certInfo.AllowInvalid ?? false; + + return CertificateLoader.LoadFromStoreCert(subject, storeName, storeLocation, allowInvalid); + } + } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs new file mode 100644 index 000000000000..1928cb64e74b --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs @@ -0,0 +1,12 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Security.Cryptography.X509Certificates; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates +{ + internal interface ICertificateConfigLoader + { + X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName); + } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/LoggerExtensions.cs b/src/Servers/Kestrel/Core/src/Internal/LoggerExtensions.cs index a28c74ae872e..636dec7454f5 100644 --- a/src/Servers/Kestrel/Core/src/Internal/LoggerExtensions.cs +++ b/src/Servers/Kestrel/Core/src/Internal/LoggerExtensions.cs @@ -58,19 +58,20 @@ internal static class LoggerExtensions new EventId(7, "MissingOrInvalidCertificateKeyFile"), "The certificate key file at '{CertificateKeyFilePath}' can not be found, contains malformed data or does not contain a PEM encoded key in PKCS8 format."); - public static void LocatedDevelopmentCertificate(this ILogger logger, X509Certificate2 certificate) => _locatedDevelopmentCertificate(logger, certificate.Subject, certificate.Thumbprint, null); + public static void LocatedDevelopmentCertificate(this ILogger logger, X509Certificate2 certificate) => _locatedDevelopmentCertificate(logger, certificate.Subject, certificate.Thumbprint, null); - public static void UnableToLocateDevelopmentCertificate(this ILogger logger) => _unableToLocateDevelopmentCertificate(logger, null); + public static void UnableToLocateDevelopmentCertificate(this ILogger logger) => _unableToLocateDevelopmentCertificate(logger, null); - public static void FailedToLocateDevelopmentCertificateFile(this ILogger logger, string certificatePath) => _failedToLocateDevelopmentCertificateFile(logger, certificatePath, null); + public static void FailedToLocateDevelopmentCertificateFile(this ILogger logger, string certificatePath) => _failedToLocateDevelopmentCertificateFile(logger, certificatePath, null); - public static void FailedToLoadDevelopmentCertificate(this ILogger logger, string certificatePath) => _failedToLoadDevelopmentCertificate(logger, certificatePath, null); + public static void FailedToLoadDevelopmentCertificate(this ILogger logger, string certificatePath) => _failedToLoadDevelopmentCertificate(logger, certificatePath, null); - public static void BadDeveloperCertificateState(this ILogger logger) => _badDeveloperCertificateState(logger, null); + public static void BadDeveloperCertificateState(this ILogger logger) => _badDeveloperCertificateState(logger, null); - public static void DeveloperCertificateFirstRun(this ILogger logger, string message) => _developerCertificateFirstRun(logger, message, null); + public static void DeveloperCertificateFirstRun(this ILogger logger, string message) => _developerCertificateFirstRun(logger, message, null); - public static void FailedToLoadCertificate(this ILogger logger, string certificatePath) => _failedToLoadCertificate(logger, certificatePath, null); - public static void FailedToLoadCertificateKey(this ILogger logger, string certificateKeyPath) => _failedToLoadCertificateKey(logger, certificateKeyPath, null); + public static void FailedToLoadCertificate(this ILogger logger, string certificatePath) => _failedToLoadCertificate(logger, certificatePath, null); + + public static void FailedToLoadCertificateKey(this ILogger logger, string certificateKeyPath) => _failedToLoadCertificateKey(logger, certificateKeyPath, null); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 97e6ddafff8b..a037d32cf2b6 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -9,6 +9,7 @@ using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.Logging; @@ -30,11 +31,11 @@ internal class SniOptionsSelector private readonly SniOptions _wildcardHostOptions = null; public SniOptionsSelector( - KestrelConfigurationLoader configLoader, + ICertificateConfigLoader certifcateConfigLoader, EndpointConfig endpointConfig, HttpsConnectionAdapterOptions fallbackOptions, HttpProtocols fallbackHttpProtocols, - ILogger logger) + ILogger logger) { _endpointName = endpointConfig.Name; @@ -45,7 +46,7 @@ public SniOptionsSelector( { var sslOptions = new SslServerAuthenticationOptions { - ServerCertificate = configLoader.LoadCertificate(sniConfig.Certificate, $"{endpointConfig.Name}:Sni:{name}"), + ServerCertificate = certifcateConfigLoader.LoadCertificate(sniConfig.Certificate, $"{endpointConfig.Name}:Sni:{name}"), EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols, CertificateRevocationCheckMode = fallbackOptions.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, }; diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 2d577f0d88ea..6fcbc93a9816 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -7,7 +7,6 @@ using System.Linq; using System.Net; using System.Net.Security; -using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; @@ -18,6 +17,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.Configuration; @@ -31,13 +31,22 @@ public class KestrelConfigurationLoader { private bool _loaded = false; - internal KestrelConfigurationLoader(KestrelServerOptions options, IConfiguration configuration, bool reloadOnChange) + internal KestrelConfigurationLoader( + KestrelServerOptions options, + IConfiguration configuration, + IHostEnvironment hostEnvironment, + bool reloadOnChange, + ILogger logger) { Options = options ?? throw new ArgumentNullException(nameof(options)); Configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); + HostEnvironment = hostEnvironment ?? throw new ArgumentNullException(nameof(hostEnvironment)); + Logger = logger ?? throw new ArgumentNullException(nameof(logger)); + ReloadOnChange = reloadOnChange; ConfigurationReader = new ConfigurationReader(configuration); + CertificateConfigLoader = new CertificateConfigLoader(hostEnvironment, logger); } public KestrelServerOptions Options { get; } @@ -49,8 +58,12 @@ internal KestrelConfigurationLoader(KestrelServerOptions options, IConfiguration /// internal bool ReloadOnChange { get; } + private IHostEnvironment HostEnvironment { get; } + private ILogger Logger { get; } private ConfigurationReader ConfigurationReader { get; set; } + private ICertificateConfigLoader CertificateConfigLoader { get; set; } + private IDictionary> EndpointConfigurations { get; } = new Dictionary>(0, StringComparer.OrdinalIgnoreCase); @@ -312,7 +325,7 @@ public void Load() } // A cert specified directly on the endpoint overrides any defaults. - httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) + httpsOptions.ServerCertificate = CertificateConfigLoader.LoadCertificate(endpoint.Certificate, endpoint.Name) ?? httpsOptions.ServerCertificate; if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) @@ -356,8 +369,9 @@ public void Load() } else { - var logger = Options.ApplicationServices.GetRequiredService>(); - var sniOptionsSelector = new SniOptionsSelector(this, endpoint, httpsOptions, listenOptions.Protocols, logger); + // Use HttpsConnectionMiddleware logger instead of KestrelServer logger to preserve existing logging category. + var logger = Options.ApplicationServices.GetRequiredService>(); + var sniOptionsSelector = new SniOptionsSelector(CertificateConfigLoader, endpoint, httpsOptions, listenOptions.Protocols, logger); listenOptions.UseHttps(SniCallback, sniOptionsSelector, httpsOptions.HandshakeTimeout); } @@ -383,7 +397,7 @@ private void LoadDefaultCert(ConfigurationReader configReader) { if (configReader.Certificates.TryGetValue("Default", out var defaultCertConfig)) { - var defaultCert = LoadCertificate(defaultCertConfig, "Default"); + var defaultCert = CertificateConfigLoader.LoadCertificate(defaultCertConfig, "Default"); if (defaultCert != null) { DefaultCertificateConfig = defaultCertConfig; @@ -392,11 +406,10 @@ private void LoadDefaultCert(ConfigurationReader configReader) } else { - var logger = Options.ApplicationServices.GetRequiredService>(); - var (certificate, certificateConfig) = FindDeveloperCertificateFile(configReader, logger); + var (certificate, certificateConfig) = FindDeveloperCertificateFile(configReader, Logger); if (certificate != null) { - logger.LocatedDevelopmentCertificate(certificate); + Logger.LocatedDevelopmentCertificate(certificate); DefaultCertificateConfig = certificateConfig; Options.DefaultCertificate = certificate; } @@ -454,168 +467,14 @@ private static bool IsDevelopmentCertificate(X509Certificate2 certificate) private bool TryGetCertificatePath(out string path) { - var hostingEnvironment = Options.ApplicationServices.GetRequiredService(); - var appName = hostingEnvironment.ApplicationName; - // This will go away when we implement // https://github.com/aspnet/Hosting/issues/1294 var appData = Environment.GetEnvironmentVariable("APPDATA"); var home = Environment.GetEnvironmentVariable("HOME"); var basePath = appData != null ? Path.Combine(appData, "ASP.NET", "https") : null; basePath = basePath ?? (home != null ? Path.Combine(home, ".aspnet", "https") : null); - path = basePath != null ? Path.Combine(basePath, $"{appName}.pfx") : null; + path = basePath != null ? Path.Combine(basePath, $"{HostEnvironment.ApplicationName}.pfx") : null; return path != null; } - - internal X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) - { - if (certInfo is null) - { - return null; - } - - var logger = Options.ApplicationServices.GetRequiredService>(); - if (certInfo.IsFileCert && certInfo.IsStoreCert) - { - throw new InvalidOperationException(CoreStrings.FormatMultipleCertificateSources(endpointName)); - } - else if (certInfo.IsFileCert) - { - var environment = Options.ApplicationServices.GetRequiredService(); - var certificatePath = Path.Combine(environment.ContentRootPath, certInfo.Path); - if (certInfo.KeyPath != null) - { - var certificateKeyPath = Path.Combine(environment.ContentRootPath, certInfo.KeyPath); - var certificate = GetCertificate(certificatePath); - - if (certificate != null) - { - certificate = LoadCertificateKey(certificate, certificateKeyPath, certInfo.Password); - } - else - { - logger.FailedToLoadCertificate(certificateKeyPath); - } - - if (certificate != null) - { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - return PersistKey(certificate); - } - - return certificate; - } - else - { - logger.FailedToLoadCertificateKey(certificateKeyPath); - } - - throw new InvalidOperationException(CoreStrings.InvalidPemKey); - } - - return new X509Certificate2(Path.Combine(environment.ContentRootPath, certInfo.Path), certInfo.Password); - } - else if (certInfo.IsStoreCert) - { - return LoadFromStoreCert(certInfo); - } - return null; - - static X509Certificate2 PersistKey(X509Certificate2 fullCertificate) - { - // We need to force the key to be persisted. - // See https://github.com/dotnet/runtime/issues/23749 - var certificateBytes = fullCertificate.Export(X509ContentType.Pkcs12, ""); - return new X509Certificate2(certificateBytes, "", X509KeyStorageFlags.DefaultKeySet); - } - - static X509Certificate2 LoadCertificateKey(X509Certificate2 certificate, string keyPath, string password) - { - // OIDs for the certificate key types. - const string RSAOid = "1.2.840.113549.1.1.1"; - const string DSAOid = "1.2.840.10040.4.1"; - const string ECDsaOid = "1.2.840.10045.2.1"; - - var keyText = File.ReadAllText(keyPath); - return certificate.PublicKey.Oid.Value switch - { - RSAOid => AttachPemRSAKey(certificate, keyText, password), - ECDsaOid => AttachPemECDSAKey(certificate, keyText, password), - DSAOid => AttachPemDSAKey(certificate, keyText, password), - _ => throw new InvalidOperationException(string.Format(CoreStrings.UnrecognizedCertificateKeyOid, certificate.PublicKey.Oid.Value)) - }; - } - - static X509Certificate2 GetCertificate(string certificatePath) - { - if (X509Certificate2.GetCertContentType(certificatePath) == X509ContentType.Cert) - { - return new X509Certificate2(certificatePath); - } - - return null; - } - } - - private static X509Certificate2 AttachPemRSAKey(X509Certificate2 certificate, string keyText, string password) - { - using var rsa = RSA.Create(); - if (password == null) - { - rsa.ImportFromPem(keyText); - } - else - { - rsa.ImportFromEncryptedPem(keyText, password); - } - - return certificate.CopyWithPrivateKey(rsa); - } - - private static X509Certificate2 AttachPemDSAKey(X509Certificate2 certificate, string keyText, string password) - { - using var dsa = DSA.Create(); - if (password == null) - { - dsa.ImportFromPem(keyText); - } - else - { - dsa.ImportFromEncryptedPem(keyText, password); - } - - return certificate.CopyWithPrivateKey(dsa); - } - - private static X509Certificate2 AttachPemECDSAKey(X509Certificate2 certificate, string keyText, string password) - { - using var ecdsa = ECDsa.Create(); - if (password == null) - { - ecdsa.ImportFromPem(keyText); - } - else - { - ecdsa.ImportFromEncryptedPem(keyText, password); - } - - return certificate.CopyWithPrivateKey(ecdsa); - } - - private static X509Certificate2 LoadFromStoreCert(CertificateConfig certInfo) - { - var subject = certInfo.Subject; - var storeName = string.IsNullOrEmpty(certInfo.Store) ? StoreName.My.ToString() : certInfo.Store; - var location = certInfo.Location; - var storeLocation = StoreLocation.CurrentUser; - if (!string.IsNullOrEmpty(location)) - { - storeLocation = (StoreLocation)Enum.Parse(typeof(StoreLocation), location, ignoreCase: true); - } - var allowInvalid = certInfo.AllowInvalid ?? false; - - return CertificateLoader.LoadFromStoreCert(subject, storeName, storeLocation, allowInvalid); - } } } diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index 917c1c8a75f8..83041dea7784 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -14,7 +14,9 @@ using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Server.Kestrel.Core { @@ -240,7 +242,15 @@ private void EnsureDefaultCert() /// A for further endpoint configuration. public KestrelConfigurationLoader Configure(IConfiguration config, bool reloadOnChange) { - var loader = new KestrelConfigurationLoader(this, config, reloadOnChange); + if (ApplicationServices is null) + { + throw new InvalidOperationException($"{nameof(ApplicationServices)} must not be null. This is normally set automatically via {nameof(IConfigureOptions)}."); + } + + var logger = ApplicationServices!.GetRequiredService>(); + var hostEnvironment = ApplicationServices!.GetRequiredService(); + + var loader = new KestrelConfigurationLoader(this, config, hostEnvironment, reloadOnChange, logger); ConfigurationLoader = loader; return loader; } diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index 6bdb766fcef1..e9cba5098e66 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -34,7 +34,7 @@ internal class HttpsConnectionMiddleware private readonly ConnectionDelegate _next; private readonly TimeSpan _handshakeTimeout; - private readonly ILogger _logger; + private readonly ILogger _logger; private readonly Func _sslStreamFactory; // The following fields are only set by HttpsConnectionAdapterOptions ctor. @@ -360,7 +360,7 @@ private static X509Certificate2 ConvertToX509Certificate2(X509Certificate certif return new X509Certificate2(certificate); } - internal static HttpProtocols ValidateAndNormalizeHttpProtocols(HttpProtocols httpProtocols, ILogger logger) + internal static HttpProtocols ValidateAndNormalizeHttpProtocols(HttpProtocols httpProtocols, ILogger logger) { // This configuration will always fail per-request, preemptively fail it here. See HttpConnection.SelectProtocol(). if (httpProtocols == HttpProtocols.Http2) @@ -427,12 +427,12 @@ internal static class HttpsConnectionMiddlewareLoggerExtensions eventId: new EventId(4, "Http2DefaultCiphersInsufficient"), formatString: CoreStrings.Http2DefaultCiphersInsufficient); - public static void AuthenticationFailed(this ILogger logger, Exception exception) => _authenticationFailed(logger, exception); + public static void AuthenticationFailed(this ILogger logger, Exception exception) => _authenticationFailed(logger, exception); - public static void AuthenticationTimedOut(this ILogger logger) => _authenticationTimedOut(logger, null); + public static void AuthenticationTimedOut(this ILogger logger) => _authenticationTimedOut(logger, null); - public static void HttpsConnectionEstablished(this ILogger logger, string connectionId, SslProtocols sslProtocol) => _httpsConnectionEstablished(logger, connectionId, sslProtocol, null); + public static void HttpsConnectionEstablished(this ILogger logger, string connectionId, SslProtocols sslProtocol) => _httpsConnectionEstablished(logger, connectionId, sslProtocol, null); - public static void Http2DefaultCiphersInsufficient(this ILogger logger) => _http2DefaultCiphersInsufficient(logger, null); + public static void Http2DefaultCiphersInsufficient(this ILogger logger) => _http2DefaultCiphersInsufficient(logger, null); } } diff --git a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs index 956859b27962..61e8d8688ad2 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs @@ -3,6 +3,10 @@ using System; using System.Net; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests @@ -55,6 +59,13 @@ public void ConfigureEndpointDefaultsAppliesToNewEndpoints() public void CanCallListenAfterConfigure() { var options = new KestrelServerOptions(); + + // Ensure configure doesn't throw because of missing services. + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(Mock.Of>()); + serviceCollection.AddSingleton(Mock.Of()); + options.ApplicationServices = serviceCollection.BuildServiceProvider(); + options.Configure(); // This is a regression test to verify the Listen* methods don't throw a NullReferenceException if called after Configure(). @@ -63,12 +74,19 @@ public void CanCallListenAfterConfigure() } [Fact] - public void SettingRequestHeaderEncodingSelecterThrowsArgumentNullException() + public void SettingRequestHeaderEncodingSelecterToNullThrowsArgumentNullException() { var options = new KestrelServerOptions(); var ex = Assert.Throws(() => options.RequestHeaderEncodingSelector = null); Assert.Equal("value", ex.ParamName); } + + [Fact] + public void ConfigureThrowsInvalidOperationExceptionIfApplicationServicesIsNotSet() + { + var options = new KestrelServerOptions(); + Assert.Throws(() => options.Configure()); + } } } diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index 2e71cdb2239c..ac4404038489 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -8,7 +8,6 @@ using System.Security.Authentication; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; -using System.Text; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Https; diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs index 59c7222df767..6682cff2700e 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs @@ -12,6 +12,7 @@ using System.Security.Cryptography.X509Certificates; using System.Text; using System.Threading.Tasks; +using Castle.Core.Logging; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -24,6 +25,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using Moq; using Xunit; @@ -69,9 +71,13 @@ public async Task CanReadAndWriteWithHttpsConnectionMiddlewareWithPemCertificate var env = new Mock(); env.SetupGet(e => e.ContentRootPath).Returns(Directory.GetCurrentDirectory()); - options.ApplicationServices = new ServiceCollection().AddSingleton(env.Object).AddLogging().BuildServiceProvider(); - var loader = new KestrelConfigurationLoader(options, configuration, reloadOnChange: false); + var serviceProvider = new ServiceCollection().AddLogging().BuildServiceProvider(); + options.ApplicationServices = serviceProvider; + + var logger = serviceProvider.GetRequiredService>(); + var loader = new KestrelConfigurationLoader(options, configuration, env.Object, reloadOnChange: false, logger); loader.Load(); + void ConfigureListenOptions(ListenOptions listenOptions) { listenOptions.KestrelServerOptions = options; From 40a33b8ba8890847a9c7e6457ba73163be8d5fa2 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 4 Aug 2020 15:41:30 -0700 Subject: [PATCH 12/24] Add SniOptionsSelectorTests --- .../Core/src/Internal/ConfigurationReader.cs | 13 +- .../Core/src/Internal/SniOptionsSelector.cs | 28 +++- .../Core/src/KestrelConfigurationLoader.cs | 16 +-- .../Core/test/SniOptionsSelectorTests.cs | 120 ++++++++++++++++++ .../Kestrel/test/ConfigurationReaderTests.cs | 1 - 5 files changed, 157 insertions(+), 21 deletions(-) create mode 100644 src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index 2c70280175d7..a405fcada817 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -134,7 +134,6 @@ private Dictionary ReadSni(IConfigurationSection sniConfig) var sni = new SniConfig { - Name = sniChild.Key, Protocols = ParseProtocols(sniChild[ProtocolsKey]), Certificate = new CertificateConfig(sniChild.GetSection(CertificateKey)), SslProtocols = ParseSslProcotols(sniChild.GetSection(SslProtocolsKey)), @@ -292,7 +291,6 @@ private static bool CompareSniDictionaries(Dictionary lhs, Di internal class SniConfig { - public string Name { get; set; } public HttpProtocols? Protocols { get; set; } public SslProtocols? SslProtocols { get; set; } public CertificateConfig Certificate { get; set; } @@ -300,13 +298,12 @@ internal class SniConfig public override bool Equals(object obj) => obj is SniConfig other && - Name == other.Name && (Protocols ?? ListenOptions.DefaultHttpProtocols) == (other.Protocols ?? ListenOptions.DefaultHttpProtocols) && Certificate == other.Certificate && (SslProtocols ?? System.Security.Authentication.SslProtocols.None) == (other.SslProtocols ?? System.Security.Authentication.SslProtocols.None) && (ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate) == (other.ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate); - public override int GetHashCode() => HashCode.Combine(Name, + public override int GetHashCode() => HashCode.Combine( Protocols ?? ListenOptions.DefaultHttpProtocols, SslProtocols ?? System.Security.Authentication.SslProtocols.None, Certificate, ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate); @@ -326,6 +323,11 @@ public CertificateConfig(IConfigurationSection configSection) ConfigSection.Bind(this); } + // For testing + internal CertificateConfig() + { + } + public IConfigurationSection ConfigSection { get; } // File @@ -352,13 +354,14 @@ public CertificateConfig(IConfigurationSection configSection) public override bool Equals(object obj) => obj is CertificateConfig other && Path == other.Path && + KeyPath == other.KeyPath && Password == other.Password && Subject == other.Subject && Store == other.Store && Location == other.Location && (AllowInvalid ?? false) == (other.AllowInvalid ?? false); - public override int GetHashCode() => HashCode.Combine(Path, Password, Subject, Store, Location, AllowInvalid ?? false); + public override int GetHashCode() => HashCode.Combine(Path, KeyPath, Password, Subject, Store, Location, AllowInvalid ?? false); public static bool operator ==(CertificateConfig lhs, CertificateConfig rhs) => lhs is null ? rhs is null : lhs.Equals(rhs); public static bool operator !=(CertificateConfig lhs, CertificateConfig rhs) => !(lhs == rhs); diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index a037d32cf2b6..1be50cb68f64 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Net.Security; using System.Security.Authentication; @@ -27,7 +28,7 @@ internal class SniOptionsSelector private readonly Action _onAuthenticateCallback; private readonly Dictionary _fullNameOptions = new Dictionary(StringComparer.OrdinalIgnoreCase); - private readonly List<(string, SniOptions)> _wildcardPrefixOptions = new List<(string, SniOptions)>(); + private readonly SortedList _wildcardPrefixOptions = new SortedList(LongestStringFirstComparer.Instance); private readonly SniOptions _wildcardHostOptions = null; public SniOptionsSelector( @@ -91,7 +92,7 @@ public SniOptionsSelector( } else if (name.StartsWith(wildcardPrefix, StringComparison.Ordinal)) { - _wildcardPrefixOptions.Add((name, sniOptions)); + _wildcardPrefixOptions.Add(name, sniOptions); } else { @@ -152,7 +153,6 @@ private bool TryGetWildcardPrefixedOptions(string serverName, out SniOptions sni { sniOptions = null; - var matchedNameLength = 0; ReadOnlySpan serverNameSpan = serverName; foreach (var (nameCandidate, optionsCandidate) in _wildcardPrefixOptions) @@ -160,15 +160,14 @@ private bool TryGetWildcardPrefixedOptions(string serverName, out SniOptions sni ReadOnlySpan nameCandidateSpan = nameCandidate; // Only slice off 1 character, the `*`. We want to match the leading `.` also. - if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(1), StringComparison.OrdinalIgnoreCase) && - nameCandidateSpan.Length > matchedNameLength) + if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(1), StringComparison.OrdinalIgnoreCase)) { - matchedNameLength = nameCandidateSpan.Length; sniOptions = optionsCandidate; + return true; } } - return sniOptions != null; + return false; } // TODO: Reflection based test to ensure we clone everything! @@ -195,5 +194,20 @@ private class SniOptions public SslServerAuthenticationOptions SslOptions { get; set; } public HttpProtocols HttpProtocols { get; set; } } + + private class LongestStringFirstComparer : IComparer + { + public static LongestStringFirstComparer Instance { get; } = new LongestStringFirstComparer(); + + private LongestStringFirstComparer() + { + } + + public int Compare(string x, string y) + { + // Flip x and y to put the longest instead of the shortest string first in the SortedList. + return y.Length.CompareTo(x.Length); + } + } } } diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 6fcbc93a9816..675e3a557dc3 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -274,7 +274,7 @@ public void Load() ConfigurationReader = new ConfigurationReader(Configuration); - LoadDefaultCert(ConfigurationReader); + LoadDefaultCert(); foreach (var endpoint in ConfigurationReader.Endpoints) { @@ -393,9 +393,9 @@ private static ValueTask SniCallback(ConnectionC return new ValueTask(options); } - private void LoadDefaultCert(ConfigurationReader configReader) + private void LoadDefaultCert() { - if (configReader.Certificates.TryGetValue("Default", out var defaultCertConfig)) + if (ConfigurationReader.Certificates.TryGetValue("Default", out var defaultCertConfig)) { var defaultCert = CertificateConfigLoader.LoadCertificate(defaultCertConfig, "Default"); if (defaultCert != null) @@ -406,7 +406,7 @@ private void LoadDefaultCert(ConfigurationReader configReader) } else { - var (certificate, certificateConfig) = FindDeveloperCertificateFile(configReader, Logger); + var (certificate, certificateConfig) = FindDeveloperCertificateFile(); if (certificate != null) { Logger.LocatedDevelopmentCertificate(certificate); @@ -416,12 +416,12 @@ private void LoadDefaultCert(ConfigurationReader configReader) } } - private (X509Certificate2, CertificateConfig) FindDeveloperCertificateFile(ConfigurationReader configReader, ILogger logger) + private (X509Certificate2, CertificateConfig) FindDeveloperCertificateFile() { string certificatePath = null; try { - if (configReader.Certificates.TryGetValue("Development", out var certificateConfig) && + if (ConfigurationReader.Certificates.TryGetValue("Development", out var certificateConfig) && certificateConfig.Path == null && certificateConfig.Password != null && TryGetCertificatePath(out certificatePath) && @@ -436,12 +436,12 @@ private void LoadDefaultCert(ConfigurationReader configReader) } else if (!string.IsNullOrEmpty(certificatePath)) { - logger.FailedToLocateDevelopmentCertificateFile(certificatePath); + Logger.FailedToLocateDevelopmentCertificateFile(certificatePath); } } catch (CryptographicException) { - logger.FailedToLoadDevelopmentCertificate(certificatePath); + Logger.FailedToLoadDevelopmentCertificate(certificatePath); } return (null, null); diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs new file mode 100644 index 000000000000..c7847dff4c98 --- /dev/null +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -0,0 +1,120 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Generic; +using System.IO.Pipelines; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates; +using Microsoft.AspNetCore.Server.Kestrel.Https; +using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class SniOptionsSelectorTests + { + [Fact] + public void PrefersExactNameOverWildcardPrefixOverFullWildcard() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { + "www.example.org", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "ExactWWW" + } + } + }, + { + "*.a.example.org", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "WildcardPrefixLong" + } + } + }, + { + "*.example.org", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "WildcardPrefixShort" + } + } + }, + { + "*", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "WildcardOnly" + } + } + } + } + }; + + var mockCertificateConfigLoader = new MockCertificateConfigLoader(); + var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; + + var sniOptionsSelector = new SniOptionsSelector( + mockCertificateConfigLoader, + endpointConfig, + fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var wwwSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Equal("ExactWWW", pathDictionary[wwwSubdomainOptions.ServerCertificate]); + + var baSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "b.a.example.org"); + Assert.Equal("WildcardPrefixLong", pathDictionary[baSubdomainOptions.ServerCertificate]); + + var aSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "a.example.org"); + Assert.Equal("WildcardPrefixShort", pathDictionary[aSubdomainOptions.ServerCertificate]); + + // REVIEW: Are we OK with "example.org" matching "*" before "*.example.org"? Feels annoying to me. + // The alternative would have "a.example.org" match "*.a.example.org" before "*.example.org", but that also feels bad. + var noSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "example.org"); + Assert.Equal("WildcardOnly", pathDictionary[noSubdomainOptions.ServerCertificate]); + + var anotherTldOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "dot.net"); + Assert.Equal("WildcardOnly", pathDictionary[anotherTldOptions.ServerCertificate]); + } + + + private class MockCertificateConfigLoader : ICertificateConfigLoader + { + public Dictionary CertToPathDictionary { get; } = new Dictionary(ReferenceEqualityComparer.Instance); + + public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) + { + var cert = new X509Certificate2(); + CertToPathDictionary.Add(cert, certInfo.Path); + return cert; + } + } + + private class MockConnectionContext : ConnectionContext + { + public override IDuplexPipe Transport { get => throw new System.NotImplementedException(); set => throw new System.NotImplementedException(); } + public override string ConnectionId { get; set; } = "MockConnectionId"; + public override IFeatureCollection Features { get; } = new FeatureCollection(); + public override IDictionary Items { get; set; } = new Dictionary(); + } + } +} diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index 9ff4063dc8bb..461cc404cc85 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -307,7 +307,6 @@ public void ReadEndpointWithSniConfigured_ReturnsCorrectValue() Assert.NotNull(sni); - Assert.Equal("*.example.org", sni.Name); Assert.Equal(HttpProtocols.Http1, sni.Protocols); Assert.Equal(SslProtocols.Tls12, sni.SslProtocols); Assert.Equal("/path/cert.pfx", sni.Certificate.Path); From 55a2ff2abb13e53c3c56abc0b2cdaae7cc5ea24c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 4 Aug 2020 18:30:02 -0700 Subject: [PATCH 13/24] Add more SniOptionsSelectorTests - TODO: Add even more SniOptionsSelectorTests - Verify we clone all SslServerAuthenticationOptions properties - Verify every property from fallbackOptions are used correctly - TODO: Integration tests --- src/Servers/Kestrel/Core/src/CoreStrings.resx | 4 +- .../Core/src/Internal/SniOptionsSelector.cs | 4 +- .../Core/test/SniOptionsSelectorTests.cs | 394 +++++++++++++++++- 3 files changed, 382 insertions(+), 20 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index 84b9592fdc1a..755acff739c5 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -624,6 +624,6 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l Connection refused because no SNI configuration was found for '{serverName}' in '{endpointName}'. To allow all connections, add a wildcard ('*') SNI section. - Connection refulsed because the client did not specify a server name, and no wildcard ('*') SNI configuration was found in '{endpointName}'. + Connection refused because the client did not specify a server name, and no wildcard ('*') SNI configuration was found in '{endpointName}'. - \ No newline at end of file + diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 1be50cb68f64..2d89a75dab53 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Net.Security; using System.Security.Authentication; @@ -117,6 +116,7 @@ public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, s { if (serverName is null) { + // There was no ALPN throw new AuthenticationException(CoreStrings.FormatSniNotConfiguredToAllowNoServerName(_endpointName)); } else @@ -171,7 +171,7 @@ private bool TryGetWildcardPrefixedOptions(string serverName, out SniOptions sni } // TODO: Reflection based test to ensure we clone everything! - // This won't catch issues related to mutable subproperties, but the existing subproperties look like they're mosly immutable. + // This won't catch issues related to mutable subproperties, but the existing subproperties look like they're mostly immutable. // The exception are the ApplicationProtocols list which we clone and the ServerCertificate because of methods like Import() and Reset() :( internal static SslServerAuthenticationOptions CloneSslOptions(SslServerAuthenticationOptions sslOptions) => new SslServerAuthenticationOptions diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs index c7847dff4c98..bfad694bbb55 100644 --- a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -1,8 +1,11 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Collections.Generic; using System.IO.Pipelines; +using System.Net.Security; +using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http.Features; @@ -19,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public class SniOptionsSelectorTests { [Fact] - public void PrefersExactNameOverWildcardPrefixOverFullWildcard() + public void PrefersExactMatchOverWildcardPrefixOverWildcardOnly() { var endpointConfig = new EndpointConfig { @@ -31,17 +34,76 @@ public void PrefersExactNameOverWildcardPrefixOverFullWildcard() { Certificate = new CertificateConfig { - Path = "ExactWWW" + Path = "Exact" } } }, + { + "*.example.org", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "WildcardPrefix" + } + } + }, + { + "*", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "WildcardOnly" + } + } + } + } + }; + + var mockCertificateConfigLoader = new MockCertificateConfigLoader(); + var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; + + var sniOptionsSelector = new SniOptionsSelector( + mockCertificateConfigLoader, + endpointConfig, + fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var wwwSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Equal("Exact", pathDictionary[wwwSubdomainOptions.ServerCertificate]); + + var baSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "b.a.example.org"); + Assert.Equal("WildcardPrefix", pathDictionary[baSubdomainOptions.ServerCertificate]); + + var aSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "a.example.org"); + Assert.Equal("WildcardPrefix", pathDictionary[aSubdomainOptions.ServerCertificate]); + + // "*.example.org" is preferred over "*", but "*.example.org" doesn't match "example.org". + // REVIEW: Are we OK with "example.org" matching "*" instead of "*.example.org"? It feels annoying to me to have to configure example.org twice. + // Unfortunately, the alternative would have "a.example.org" match "*.a.example.org" before "*.example.org", and that just seems wrong. + var noSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "example.org"); + Assert.Equal("WildcardOnly", pathDictionary[noSubdomainOptions.ServerCertificate]); + + var anotherTldOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "dot.net"); + Assert.Equal("WildcardOnly", pathDictionary[anotherTldOptions.ServerCertificate]); + } + + [Fact] + public void PerfersLongerWildcardPrefixOverShorterWildcardPrefix() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { { "*.a.example.org", new SniConfig { Certificate = new CertificateConfig { - Path = "WildcardPrefixLong" + Path = "Long" } } }, @@ -51,10 +113,114 @@ public void PrefersExactNameOverWildcardPrefixOverFullWildcard() { Certificate = new CertificateConfig { - Path = "WildcardPrefixShort" + Path = "Short" + } + } + } + } + }; + + var mockCertificateConfigLoader = new MockCertificateConfigLoader(); + var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; + + var sniOptionsSelector = new SniOptionsSelector( + mockCertificateConfigLoader, + endpointConfig, + fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var baSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "b.a.example.org"); + Assert.Equal("Long", pathDictionary[baSubdomainOptions.ServerCertificate]); + + // "*.a.example.org" is preferred over "*.example.org", but "a.example.org" doesn't match "*.a.example.org". + var aSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "a.example.org"); + Assert.Equal("Short", pathDictionary[aSubdomainOptions.ServerCertificate]); + } + + [Fact] + public void ServerNameMatchingIsCaseInsensitive() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { + "Www.Example.Org", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "Exact" } } }, + { + "*.Example.Org", + new SniConfig + { + Certificate = new CertificateConfig + { + Path = "WildcardPrefix" + } + } + } + } + }; + + var mockCertificateConfigLoader = new MockCertificateConfigLoader(); + var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; + + var sniOptionsSelector = new SniOptionsSelector( + mockCertificateConfigLoader, + endpointConfig, + fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var wwwSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "wWw.eXample.oRg"); + Assert.Equal("Exact", pathDictionary[wwwSubdomainOptions.ServerCertificate]); + + var baSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "B.a.eXample.oRg"); + Assert.Equal("WildcardPrefix", pathDictionary[baSubdomainOptions.ServerCertificate]); + + var aSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "A.eXample.oRg"); + Assert.Equal("WildcardPrefix", pathDictionary[aSubdomainOptions.ServerCertificate]); + } + + [Fact] + public void GetOptionsThrowsAnAuthenticationExceptionIfThereIsNoMatchingSniSection() + { + var endpointConfig = new EndpointConfig + { + Name = "TestEndpointName", + Sni = new Dictionary() + }; + + var mockCertificateConfigLoader = new MockCertificateConfigLoader(); + var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; + + var sniOptionsSelector = new SniOptionsSelector( + mockCertificateConfigLoader, + endpointConfig, + fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var authExWithServerName = Assert.Throws(() => sniOptionsSelector.GetOptions(new MockConnectionContext(), "example.org")); + Assert.Equal(CoreStrings.FormatSniNotConfiguredForServerName("example.org", endpointConfig.Name), authExWithServerName.Message); + + var authExWithoutServerName = Assert.Throws(() => sniOptionsSelector.GetOptions(new MockConnectionContext(), null)); + Assert.Equal(CoreStrings.FormatSniNotConfiguredToAllowNoServerName(endpointConfig.Name), authExWithoutServerName.Message); + } + + [Fact] + public void WildcardOnlyMatchesNullServerNameDueToNoAlpn() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { { "*", new SniConfig @@ -78,24 +244,215 @@ public void PrefersExactNameOverWildcardPrefixOverFullWildcard() fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); - var wwwSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); - Assert.Equal("ExactWWW", pathDictionary[wwwSubdomainOptions.ServerCertificate]); + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), null); + Assert.Equal("WildcardOnly", pathDictionary[options.ServerCertificate]); + } - var baSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "b.a.example.org"); - Assert.Equal("WildcardPrefixLong", pathDictionary[baSubdomainOptions.ServerCertificate]); + [Fact] + public void CachesSslServerAuthenticationOptions() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { + "www.example.org", + new SniConfig + { + Certificate = new CertificateConfig() + } + } + } + }; - var aSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "a.example.org"); - Assert.Equal("WildcardPrefixShort", pathDictionary[aSubdomainOptions.ServerCertificate]); + var sniOptionsSelector = new SniOptionsSelector( + new MockCertificateConfigLoader(), + endpointConfig, + fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); - // REVIEW: Are we OK with "example.org" matching "*" before "*.example.org"? Feels annoying to me. - // The alternative would have "a.example.org" match "*.a.example.org" before "*.example.org", but that also feels bad. - var noSubdomainOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "example.org"); - Assert.Equal("WildcardOnly", pathDictionary[noSubdomainOptions.ServerCertificate]); + var options1 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + var options2 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Same(options1, options2); + } - var anotherTldOptions = sniOptionsSelector.GetOptions(new MockConnectionContext(), "dot.net"); - Assert.Equal("WildcardOnly", pathDictionary[anotherTldOptions.ServerCertificate]); + [Fact] + public void ClonesSslServerAuthenticationOptionsIfAnOnAuthenticateCallbackIsDefined() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { + "www.example.org", + new SniConfig + { + Certificate = new CertificateConfig() + } + } + } + }; + + SslServerAuthenticationOptions lastSeenSslOptions = null; + + var fallbackOptions = new HttpsConnectionAdapterOptions + { + OnAuthenticate = (context, sslOptions) => + { + lastSeenSslOptions = sslOptions; + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + new MockCertificateConfigLoader(), + endpointConfig, + fallbackOptions, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var options1 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Same(lastSeenSslOptions, options1); + + var options2 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Same(lastSeenSslOptions, options2); + + Assert.NotSame(options1, options2); } + [Fact] + public void ClonesSslServerAuthenticationOptionsIfTheFallbackServerCertificateSelectorIsUsed() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { + "selector.example.org", + new SniConfig() + }, + { + "config.example.org", + new SniConfig + { + Certificate = new CertificateConfig() + } + } + } + }; + + var selectorCertificate = new X509Certificate2(); + + var fallbackOptions = new HttpsConnectionAdapterOptions + { + ServerCertificate = new X509Certificate2(), + ServerCertificateSelector = (context, serverName) => selectorCertificate + }; + + var sniOptionsSelector = new SniOptionsSelector( + new MockCertificateConfigLoader(), + endpointConfig, + fallbackOptions, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var selectorOptions1 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "selector.example.org"); + Assert.Same(selectorCertificate, selectorOptions1.ServerCertificate); + + var selectorOptions2 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "selector.example.org"); + Assert.Same(selectorCertificate, selectorOptions2.ServerCertificate); + + // The SslServerAuthenticationOptions were cloned because the cert came from the ServerCertificateSelector fallback. + Assert.NotSame(selectorOptions1, selectorOptions2); + + var configOptions1 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "config.example.org"); + Assert.NotSame(selectorCertificate, configOptions1.ServerCertificate); + + var configOptions2 = sniOptionsSelector.GetOptions(new MockConnectionContext(), "config.example.org"); + Assert.NotSame(selectorCertificate, configOptions2.ServerCertificate); + + // The SslServerAuthenticationOptions don't need to be cloned if a static cert is defined in config for the given server name. + Assert.Same(configOptions1, configOptions2); + } + + [Fact] + public void ConstructorThrowsInvalidOperationExceptionIfNoCertificateDefiniedInConfigOrFallback() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { "www.example.org", new SniConfig() } + } + }; + + var ex = Assert.Throws( + () => new SniOptionsSelector( + new MockCertificateConfigLoader(), + endpointConfig, + fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>())); + + Assert.Equal(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound, ex.Message); + } + + [Fact] + public void FallsBackToHttpsConnectionAdapterCertificate() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { "www.example.org", new SniConfig() } + } + }; + + var fallbackOptions = new HttpsConnectionAdapterOptions + { + ServerCertificate = new X509Certificate2() + }; + + var sniOptionsSelector = new SniOptionsSelector( + new MockCertificateConfigLoader(), + endpointConfig, + fallbackOptions, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Same(fallbackOptions.ServerCertificate, options.ServerCertificate); + } + + [Fact] + public void FallsBackToHttpsConnectionAdapterServerCertificateSelectorOverServerCertificate() + { + var endpointConfig = new EndpointConfig + { + Sni = new Dictionary + { + { "www.example.org", new SniConfig() } + } + }; + + var selectorCertificate = new X509Certificate2(); + + var fallbackOptions = new HttpsConnectionAdapterOptions + { + ServerCertificate = new X509Certificate2(), + ServerCertificateSelector = (context, serverName) => selectorCertificate + }; + + var sniOptionsSelector = new SniOptionsSelector( + new MockCertificateConfigLoader(), + endpointConfig, + fallbackOptions, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Same(selectorCertificate, options.ServerCertificate); + } private class MockCertificateConfigLoader : ICertificateConfigLoader { @@ -103,6 +460,11 @@ private class MockCertificateConfigLoader : ICertificateConfigLoader public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) { + if (certInfo is null) + { + return null; + } + var cert = new X509Certificate2(); CertToPathDictionary.Add(cert, certInfo.Path); return cert; From 68489f9b5b20eeea10feda2d3165803c4482a113 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Aug 2020 01:19:43 -0700 Subject: [PATCH 14/24] Address PR feedback --- src/Servers/Kestrel/Core/src/CoreStrings.resx | 9 ++-- .../Core/src/Internal/ConfigurationReader.cs | 11 ++-- .../Core/src/Internal/SniOptionsSelector.cs | 53 +++++++------------ .../Middleware/HttpsConnectionMiddleware.cs | 8 +-- .../Kestrel/Core/test/KestrelServerTests.cs | 3 ++ .../Kestrel/test/ConfigurationReaderTests.cs | 15 ++++++ 6 files changed, 56 insertions(+), 43 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index 755acff739c5..980638e349e8 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -621,9 +621,12 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l Unknown algorithm for certificate with public key type '{0}'. - Connection refused because no SNI configuration was found for '{serverName}' in '{endpointName}'. To allow all connections, add a wildcard ('*') SNI section. + Connection refused because no SNI configuration section was found for '{serverName}' in '{endpointName}'. To allow all connections, add a wildcard ('*') SNI section. - Connection refused because the client did not specify a server name, and no wildcard ('*') SNI configuration was found in '{endpointName}'. + Connection refused because the client did not specify a server name, and no wildcard ('*') SNI configuration section was found in '{endpointName}'. - + + The endpoint {endpointName} is invalid because an SNI configuration section has an empty string as its key. Use a wildcard ('*') SNI section to match all server names. + + \ No newline at end of file diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index a405fcada817..0752b83ba6b9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -99,7 +99,7 @@ private IEnumerable ReadEndpoints() Certificate = new CertificateConfig(endpointConfig.GetSection(CertificateKey)), SslProtocols = ParseSslProcotols(endpointConfig.GetSection(SslProtocolsKey)), ClientCertificateMode = ParseClientCertificateMode(endpointConfig[ClientCertificateModeKey]), - Sni = ReadSni(endpointConfig.GetSection(SniKey)), + Sni = ReadSni(endpointConfig.GetSection(SniKey), endpointConfig.Key), }; endpoints.Add(endpoint); @@ -108,7 +108,7 @@ private IEnumerable ReadEndpoints() return endpoints; } - private Dictionary ReadSni(IConfigurationSection sniConfig) + private Dictionary ReadSni(IConfigurationSection sniConfig, string endpointName) { var sniDictionary = new Dictionary(0, StringComparer.OrdinalIgnoreCase); @@ -132,6 +132,11 @@ private Dictionary ReadSni(IConfigurationSection sniConfig) // } // } + if (string.IsNullOrEmpty(sniChild.Key)) + { + throw new InvalidOperationException(CoreStrings.FormatSniNameCannotBeEmpty(endpointName)); + } + var sni = new SniConfig { Protocols = ParseProtocols(sniChild[ProtocolsKey]), @@ -140,7 +145,7 @@ private Dictionary ReadSni(IConfigurationSection sniConfig) ClientCertificateMode = ParseClientCertificateMode(sniChild[ClientCertificateModeKey]) }; - sniDictionary[sniChild.Key] = sni; + sniDictionary.Add(sniChild.Key, sni); } return sniDictionary; diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 2d89a75dab53..47474c32d592 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -18,17 +18,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { internal class SniOptionsSelector { - private const string wildcardHost = "*"; - private const string wildcardPrefix = "*."; + private const string WildcardHost = "*"; + private const string WildcardPrefix = "*."; private readonly string _endpointName; private readonly Func _fallbackServerCertificateSelector; private readonly Action _onAuthenticateCallback; - private readonly Dictionary _fullNameOptions = new Dictionary(StringComparer.OrdinalIgnoreCase); + private readonly Dictionary _exactNameOptions = new Dictionary(StringComparer.OrdinalIgnoreCase); private readonly SortedList _wildcardPrefixOptions = new SortedList(LongestStringFirstComparer.Instance); - private readonly SniOptions _wildcardHostOptions = null; + private readonly SniOptions _wildcardOptions = null; public SniOptionsSelector( ICertificateConfigLoader certifcateConfigLoader, @@ -85,17 +85,18 @@ public SniOptionsSelector( HttpProtocols = httpProtocols, }; - if (name.Equals(wildcardHost, StringComparison.Ordinal)) + if (name.Equals(WildcardHost, StringComparison.Ordinal)) { - _wildcardHostOptions = sniOptions; + _wildcardOptions = sniOptions; } - else if (name.StartsWith(wildcardPrefix, StringComparison.Ordinal)) + else if (name.StartsWith(WildcardPrefix, StringComparison.Ordinal)) { - _wildcardPrefixOptions.Add(name, sniOptions); + // Only slice off 1 character, the `*`. We want to match the leading `.` also. + _wildcardPrefixOptions.Add(name.Substring(1), sniOptions); } else { - _fullNameOptions[name] = sniOptions; + _exactNameOptions.Add(name, sniOptions); } } } @@ -104,13 +105,20 @@ public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, s { SniOptions sniOptions = null; - if (!string.IsNullOrEmpty(serverName) && !_fullNameOptions.TryGetValue(serverName, out sniOptions)) + if (!string.IsNullOrEmpty(serverName) && !_exactNameOptions.TryGetValue(serverName, out sniOptions)) { - TryGetWildcardPrefixedOptions(serverName, out sniOptions); + foreach (var (suffix, options) in _wildcardPrefixOptions) + { + if (serverName.EndsWith(suffix, StringComparison.OrdinalIgnoreCase)) + { + sniOptions = options; + break; + } + } } // Fully wildcarded ("*") options can be used even when given an empty server name. - sniOptions ??= _wildcardHostOptions; + sniOptions ??= _wildcardOptions; if (sniOptions is null) { @@ -149,27 +157,6 @@ public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, s return sslOptions; } - private bool TryGetWildcardPrefixedOptions(string serverName, out SniOptions sniOptions) - { - sniOptions = null; - - ReadOnlySpan serverNameSpan = serverName; - - foreach (var (nameCandidate, optionsCandidate) in _wildcardPrefixOptions) - { - ReadOnlySpan nameCandidateSpan = nameCandidate; - - // Only slice off 1 character, the `*`. We want to match the leading `.` also. - if (serverNameSpan.EndsWith(nameCandidateSpan.Slice(1), StringComparison.OrdinalIgnoreCase)) - { - sniOptions = optionsCandidate; - return true; - } - } - - return false; - } - // TODO: Reflection based test to ensure we clone everything! // This won't catch issues related to mutable subproperties, but the existing subproperties look like they're mostly immutable. // The exception are the ApplicationProtocols list which we clone and the ServerCertificate because of methods like Import() and Reset() :( diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index e9cba5098e66..fdc637409e92 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -30,7 +30,7 @@ internal class HttpsConnectionMiddleware { private const string EnableWindows81Http2 = "Microsoft.AspNetCore.Server.Kestrel.EnableWindows81Http2"; - private static readonly bool _isWindowsVersionIncompatible = IsWindowsVersionIncompatible(); + private static readonly bool _isWindowsVersionIncompatibleWithHttp2 = IsWindowsVersionIncompatibleWithHttp2(); private readonly ConnectionDelegate _next; private readonly TimeSpan _handshakeTimeout; @@ -369,12 +369,12 @@ internal static HttpProtocols ValidateAndNormalizeHttpProtocols(HttpProtocols ht { throw new NotSupportedException(CoreStrings.Http2NoTlsOsx); } - else if (_isWindowsVersionIncompatible) + else if (_isWindowsVersionIncompatibleWithHttp2) { throw new NotSupportedException(CoreStrings.Http2NoTlsWin81); } } - else if (httpProtocols == HttpProtocols.Http1AndHttp2 && _isWindowsVersionIncompatible) + else if (httpProtocols == HttpProtocols.Http1AndHttp2 && _isWindowsVersionIncompatibleWithHttp2) { logger.Http2DefaultCiphersInsufficient(); return HttpProtocols.Http1; @@ -383,7 +383,7 @@ internal static HttpProtocols ValidateAndNormalizeHttpProtocols(HttpProtocols ht return httpProtocols; } - private static bool IsWindowsVersionIncompatible() + private static bool IsWindowsVersionIncompatibleWithHttp2() { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs index 4f41ddbe1900..a26e0c1b8ac8 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -495,6 +496,7 @@ public async Task ReloadsOnConfigurationChangeWhenOptedIn() var serviceCollection = new ServiceCollection(); serviceCollection.AddSingleton(mockLoggerFactory.Object); serviceCollection.AddSingleton(Mock.Of>()); + serviceCollection.AddSingleton(Mock.Of()); var options = new KestrelServerOptions { @@ -631,6 +633,7 @@ public async Task DoesNotReloadOnConfigurationChangeByDefault() var serviceCollection = new ServiceCollection(); serviceCollection.AddSingleton(mockLoggerFactory.Object); serviceCollection.AddSingleton(Mock.Of>()); + serviceCollection.AddSingleton(Mock.Of()); var options = new KestrelServerOptions { diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index 461cc404cc85..dddb30838482 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -288,6 +288,21 @@ public void ReadEndpointWithoutSniConfigured_ReturnsEmptyCollection() Assert.False(endpoint.Sni.Any()); } + [Fact] + public void ReadEndpointWithEmptySniKey_Throws() + { + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:Sni::Protocols", "Http1"), + }).Build(); + + var reader = new ConfigurationReader(config); + var ex = Assert.Throws(() => reader.Endpoints); + + Assert.Equal(CoreStrings.FormatSniNameCannotBeEmpty("End1"), ex.Message); + } + [Fact] public void ReadEndpointWithSniConfigured_ReturnsCorrectValue() { From 0c6522609c38b8caf034f556aebef18b5cfecbd9 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Aug 2020 02:22:44 -0700 Subject: [PATCH 15/24] Support SNI config in EndpointDefaults --- .../Core/src/HttpsConnectionAdapterOptions.cs | 1 - .../Certificates/CertificateConfigLoader.cs | 2 + .../Certificates/ICertificateConfigLoader.cs | 3 + .../Core/src/Internal/ConfigurationReader.cs | 107 ++++---- .../Core/src/Internal/SniOptionsSelector.cs | 53 ++-- .../Core/src/KestrelConfigurationLoader.cs | 77 ++++-- .../Kestrel/Core/src/KestrelServerOptions.cs | 11 +- .../Core/src/ListenOptionsHttpsExtensions.cs | 34 ++- .../Middleware/HttpsConnectionMiddleware.cs | 14 +- .../Core/test/KestrelServerOptionsTests.cs | 18 +- .../Kestrel/Core/test/KestrelServerTests.cs | 9 +- .../Core/test/SniOptionsSelectorTests.cs | 241 ++++++++---------- .../Kestrel/test/ConfigurationReaderTests.cs | 43 +++- .../test/KestrelConfigurationLoaderTests.cs | 47 +++- .../HttpsConnectionMiddlewareTests.cs | 5 +- 15 files changed, 396 insertions(+), 269 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs index 365008497842..4f861c30f634 100644 --- a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs +++ b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs @@ -8,7 +8,6 @@ using System.Threading; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core; -using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Https { diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs index e59de79ca5d2..d61d3052465f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs @@ -23,6 +23,8 @@ public CertificateConfigLoader(IHostEnvironment hostEnvironment, ILogger Logger { get; } + public bool SkipValidation => false; + public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) { if (certInfo is null) diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs index 1928cb64e74b..9e43d3dffdfb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs @@ -7,6 +7,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates { internal interface ICertificateConfigLoader { + // SkipValidation is only true in tests. + bool SkipValidation { get; } + X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index 0752b83ba6b9..df5c05cda7fe 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -16,12 +16,13 @@ internal class ConfigurationReader private const string CertificatesKey = "Certificates"; private const string CertificateKey = "Certificate"; private const string SslProtocolsKey = "SslProtocols"; - private const string EndpointDefaultsKey = "EndpointDefaults"; private const string EndpointsKey = "Endpoints"; private const string UrlKey = "Url"; private const string ClientCertificateModeKey = "ClientCertificateMode"; private const string SniKey = "Sni"; + internal const string EndpointDefaultsKey = "EndpointDefaults"; + private readonly IConfiguration _configuration; private IDictionary _certificates; @@ -51,9 +52,20 @@ private IDictionary ReadCertificates() } // "EndpointDefaults": { - // "Protocols": "Http1AndHttp2", - // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], - // "ClientCertificateMode" : "NoCertificate" + // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], + // "ClientCertificateMode" : "NoCertificate", + // "Sni": { + // "a.example.org": { + // "Certificate": { + // "Path": "testCertA.pfx", + // "Password": "testPassword" + // } + // }, + // "*.example.org": { + // "Protocols": "Http1", + // } + // } // } private EndpointDefaults ReadEndpointDefaults() { @@ -62,7 +74,8 @@ private EndpointDefaults ReadEndpointDefaults() { Protocols = ParseProtocols(configSection[ProtocolsKey]), SslProtocols = ParseSslProcotols(configSection.GetSection(SslProtocolsKey)), - ClientCertificateMode = ParseClientCertificateMode(configSection[ClientCertificateModeKey]) + ClientCertificateMode = ParseClientCertificateMode(configSection[ClientCertificateModeKey]), + Sni = ReadSni(configSection.GetSection(SniKey), EndpointDefaultsKey) }; } @@ -74,14 +87,25 @@ private IEnumerable ReadEndpoints() foreach (var endpointConfig in endpointsConfig) { // "EndpointName": { - // "Url": "https://*:5463", - // "Protocols": "Http1AndHttp2", - // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], - // "Certificate": { - // "Path": "testCert.pfx", - // "Password": "testPassword" - // }, - // "ClientCertificateMode" : "NoCertificate" + // "Url": "https://*:5463", + // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], + // "Certificate": { + // "Path": "testCert.pfx", + // "Password": "testPassword" + // }, + // "ClientCertificateMode" : "NoCertificate", + // "Sni": { + // "a.example.org": { + // "Certificate": { + // "Path": "testCertA.pfx", + // "Password": "testPassword" + // } + // }, + // "*.example.org": { + // "Protocols": "Http1", + // } + // } // } var url = endpointConfig[UrlKey]; @@ -116,20 +140,22 @@ private Dictionary ReadSni(IConfigurationSection sniConfig, s { // "Sni": { // "a.example.org": { - // "Protocols": "Http1AndHttp2", + // "Protocols": "Http1", // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], // "Certificate": { - // "Path": "testCert.pfx", + // "Path": "testCertA.pfx", // "Password": "testPassword" // }, // "ClientCertificateMode" : "NoCertificate" // }, // "*.example.org": { // "Certificate": { - // "Path": "testCert2.pfx", + // "Path": "testCertWildcard.pfx", // "Password": "testPassword" // } // } + // // The following should work once https://github.com/dotnet/runtime/issues/40218 is resolved + // "*": {} // } if (string.IsNullOrEmpty(sniChild.Key)) @@ -139,8 +165,8 @@ private Dictionary ReadSni(IConfigurationSection sniConfig, s var sni = new SniConfig { - Protocols = ParseProtocols(sniChild[ProtocolsKey]), Certificate = new CertificateConfig(sniChild.GetSection(CertificateKey)), + Protocols = ParseProtocols(sniChild[ProtocolsKey]), SslProtocols = ParseSslProcotols(sniChild.GetSection(SslProtocolsKey)), ClientCertificateMode = ParseClientCertificateMode(sniChild[ClientCertificateModeKey]) }; @@ -189,44 +215,37 @@ private Dictionary ReadSni(IConfigurationSection sniConfig, s } // "EndpointDefaults": { - // "Protocols": "Http1AndHttp2", - // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], - // "ClientCertificateMode" : "NoCertificate" + // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], + // "ClientCertificateMode" : "NoCertificate" // } internal class EndpointDefaults { public HttpProtocols? Protocols { get; set; } public SslProtocols? SslProtocols { get; set; } public ClientCertificateMode? ClientCertificateMode { get; set; } + public Dictionary Sni { get; set; } } // "EndpointName": { - // "Url": "https://*:5463", - // "Protocols": "Http1AndHttp2", - // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], - // "Certificate": { - // "Path": "testCert.pfx", - // "Password": "testPassword" - // }, - // "ClientCertificateMode" : "NoCertificate" + // "Url": "https://*:5463", + // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], + // "Certificate": { + // "Path": "testCert.pfx", + // "Password": "testPassword" + // }, + // "ClientCertificateMode" : "NoCertificate", // "Sni": { // "a.example.org": { - // "Protocols": "Http1AndHttp2", - // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], - // "Certificate": { - // "Path": "testCert.pfx", - // "Password": "testPassword" - // }, - // "ClientCertificateMode" : "NoCertificate" - // }, - // "*.example.org": { // "Certificate": { - // "Path": "testCert2.pfx", - // "Password": "testPassword" + // "Path": "testCertA.pfx", + // "Password": "testPasswordA" // } // }, - // // The following should work once https://github.com/dotnet/runtime/issues/40218 is resolved - // "*": {} + // "*.example.org": { + // "Protocols": "Http1", + // } // } // } internal class EndpointConfig @@ -304,8 +323,8 @@ internal class SniConfig public override bool Equals(object obj) => obj is SniConfig other && (Protocols ?? ListenOptions.DefaultHttpProtocols) == (other.Protocols ?? ListenOptions.DefaultHttpProtocols) && - Certificate == other.Certificate && (SslProtocols ?? System.Security.Authentication.SslProtocols.None) == (other.SslProtocols ?? System.Security.Authentication.SslProtocols.None) && + Certificate == other.Certificate && (ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate) == (other.ClientCertificateMode ?? Https.ClientCertificateMode.NoCertificate); public override int GetHashCode() => HashCode.Combine( @@ -317,8 +336,8 @@ public override int GetHashCode() => HashCode.Combine( } // "CertificateName": { - // "Path": "testCert.pfx", - // "Password": "testPassword" + // "Path": "testCert.pfx", + // "Password": "testPassword" // } internal class CertificateConfig { diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index 47474c32d592..ae558bfc80f2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -8,6 +8,8 @@ using System.Net.Security; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; +using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates; using Microsoft.AspNetCore.Server.Kestrel.Https; @@ -28,32 +30,33 @@ internal class SniOptionsSelector private readonly Dictionary _exactNameOptions = new Dictionary(StringComparer.OrdinalIgnoreCase); private readonly SortedList _wildcardPrefixOptions = new SortedList(LongestStringFirstComparer.Instance); - private readonly SniOptions _wildcardOptions = null; + private readonly SniOptions _wildcardOptions; public SniOptionsSelector( + string endpointName, + Dictionary sniDictionary, ICertificateConfigLoader certifcateConfigLoader, - EndpointConfig endpointConfig, - HttpsConnectionAdapterOptions fallbackOptions, + HttpsConnectionAdapterOptions fallbackHttpsOptions, HttpProtocols fallbackHttpProtocols, ILogger logger) { - _endpointName = endpointConfig.Name; + _endpointName = endpointName; - _fallbackServerCertificateSelector = fallbackOptions.ServerCertificateSelector; - _onAuthenticateCallback = fallbackOptions.OnAuthenticate; + _fallbackServerCertificateSelector = fallbackHttpsOptions.ServerCertificateSelector; + _onAuthenticateCallback = fallbackHttpsOptions.OnAuthenticate; - foreach (var (name, sniConfig) in endpointConfig.Sni) + foreach (var (name, sniConfig) in sniDictionary) { var sslOptions = new SslServerAuthenticationOptions { - ServerCertificate = certifcateConfigLoader.LoadCertificate(sniConfig.Certificate, $"{endpointConfig.Name}:Sni:{name}"), - EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackOptions.SslProtocols, - CertificateRevocationCheckMode = fallbackOptions.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, + ServerCertificate = certifcateConfigLoader.LoadCertificate(sniConfig.Certificate, $"{endpointName}:Sni:{name}"), + EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackHttpsOptions.SslProtocols, + CertificateRevocationCheckMode = fallbackHttpsOptions.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, }; if (sslOptions.ServerCertificate is null) { - if (fallbackOptions.ServerCertificate is null && _fallbackServerCertificateSelector is null) + if (fallbackHttpsOptions.ServerCertificate is null && _fallbackServerCertificateSelector is null) { throw new InvalidOperationException(CoreStrings.NoCertSpecifiedNoDevelopmentCertificateFound); } @@ -61,18 +64,24 @@ public SniOptionsSelector( if (_fallbackServerCertificateSelector is null) { // Cache the fallback ServerCertificate since there's no fallback ServerCertificateSelector taking precedence. - sslOptions.ServerCertificate = fallbackOptions.ServerCertificate; + sslOptions.ServerCertificate = fallbackHttpsOptions.ServerCertificate; } } - var clientCertificateMode = sniConfig.ClientCertificateMode ?? fallbackOptions.ClientCertificateMode; + // SkipValidation is only true in tests. + if (!certifcateConfigLoader.SkipValidation && sslOptions.ServerCertificate is X509Certificate2 cert2) + { + HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(cert2); + } + + var clientCertificateMode = sniConfig.ClientCertificateMode ?? fallbackHttpsOptions.ClientCertificateMode; if (clientCertificateMode != ClientCertificateMode.NoCertificate) { sslOptions.ClientCertificateRequired = true; sslOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => HttpsConnectionMiddleware.RemoteCertificateValidationCallback( - clientCertificateMode, fallbackOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors); + clientCertificateMode, fallbackHttpsOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors); } var httpProtocols = sniConfig.Protocols ?? fallbackHttpProtocols; @@ -144,7 +153,14 @@ public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, s // If a ServerCertificateSelector doesn't return a cert, HttpsConnectionMiddleware doesn't fallback to the ServerCertificate. sslOptions = CloneSslOptions(sslOptions); - sslOptions.ServerCertificate = _fallbackServerCertificateSelector(connection, serverName); + var fallbackCertificate = _fallbackServerCertificateSelector(connection, serverName); + + if (fallbackCertificate != null) + { + HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(fallbackCertificate); + } + + sslOptions.ServerCertificate = fallbackCertificate; } if (_onAuthenticateCallback != null) @@ -157,6 +173,13 @@ public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, s return sslOptions; } + public static ValueTask OptionsCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) + { + var sniOptionsSelector = (SniOptionsSelector)state; + var options = sniOptionsSelector.GetOptions(connection, clientHelloInfo.ServerName); + return new ValueTask(options); + } + // TODO: Reflection based test to ensure we clone everything! // This won't catch issues related to mutable subproperties, but the existing subproperties look like they're mostly immutable. // The exception are the ApplicationProtocols list which we clone and the ServerCertificate because of methods like Import() and Reset() :( diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 675e3a557dc3..a58eafe6ccb9 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -6,14 +6,9 @@ using System.IO; using System.Linq; using System.Net; -using System.Net.Security; -using System.Security.Authentication; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; -using System.Threading; -using System.Threading.Tasks; using Microsoft.AspNetCore.Certificates.Generation; -using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; @@ -21,7 +16,6 @@ using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -36,12 +30,14 @@ internal KestrelConfigurationLoader( IConfiguration configuration, IHostEnvironment hostEnvironment, bool reloadOnChange, - ILogger logger) + ILogger logger, + ILogger httpsLogger) { Options = options ?? throw new ArgumentNullException(nameof(options)); Configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); HostEnvironment = hostEnvironment ?? throw new ArgumentNullException(nameof(hostEnvironment)); Logger = logger ?? throw new ArgumentNullException(nameof(logger)); + HttpsLogger = httpsLogger ?? throw new ArgumentNullException(nameof(logger)); ReloadOnChange = reloadOnChange; @@ -60,9 +56,11 @@ internal KestrelConfigurationLoader( private IHostEnvironment HostEnvironment { get; } private ILogger Logger { get; } + private ILogger HttpsLogger { get; } + private ConfigurationReader ConfigurationReader { get; set; } - private ICertificateConfigLoader CertificateConfigLoader { get; set; } + private ICertificateConfigLoader CertificateConfigLoader { get; } private IDictionary> EndpointConfigurations { get; } = new Dictionary>(0, StringComparer.OrdinalIgnoreCase); @@ -233,9 +231,9 @@ public KestrelConfigurationLoader HandleEndpoint(ulong handle, Action>(); - var sniOptionsSelector = new SniOptionsSelector(CertificateConfigLoader, endpoint, httpsOptions, listenOptions.Protocols, logger); - - listenOptions.UseHttps(SniCallback, sniOptionsSelector, httpsOptions.HandshakeTimeout); + var sniOptionsSelector = new SniOptionsSelector(endpoint.Name, endpoint.Sni, CertificateConfigLoader, httpsOptions, listenOptions.Protocols, HttpsLogger); + listenOptions.UseHttps(SniOptionsSelector.OptionsCallback, sniOptionsSelector, httpsOptions.HandshakeTimeout); } } @@ -386,13 +418,6 @@ public void Load() return (endpointsToStop, endpointsToStart); } - private static ValueTask SniCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) - { - var sniOptionsSelector = (SniOptionsSelector)state; - var options = sniOptionsSelector.GetOptions(connection, clientHelloInfo.ServerName); - return new ValueTask(options); - } - private void LoadDefaultCert() { if (ConfigurationReader.Certificates.TryGetValue("Default", out var defaultCertConfig)) diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index 83041dea7784..3fa65dd14653 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Https; +using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -140,7 +141,7 @@ public void ConfigureEndpointDefaults(Action configureOptions) internal void ApplyEndpointDefaults(ListenOptions listenOptions) { listenOptions.KestrelServerOptions = this; - ConfigurationLoader?.ApplyConfigurationDefaults(listenOptions); + ConfigurationLoader?.ApplyEndpointDefaults(listenOptions); EndpointDefaults(listenOptions); } @@ -155,6 +156,7 @@ public void ConfigureHttpsDefaults(Action configu internal void ApplyHttpsDefaults(HttpsConnectionAdapterOptions httpsOptions) { + ConfigurationLoader?.ApplyHttpsDefaults(httpsOptions); HttpsDefaults(httpsOptions); } @@ -247,10 +249,11 @@ public KestrelConfigurationLoader Configure(IConfiguration config, bool reloadOn throw new InvalidOperationException($"{nameof(ApplicationServices)} must not be null. This is normally set automatically via {nameof(IConfigureOptions)}."); } - var logger = ApplicationServices!.GetRequiredService>(); - var hostEnvironment = ApplicationServices!.GetRequiredService(); + var hostEnvironment = ApplicationServices.GetRequiredService(); + var logger = ApplicationServices.GetRequiredService>(); + var httpsLogger = ApplicationServices.GetRequiredService>(); - var loader = new KestrelConfigurationLoader(this, config, hostEnvironment, reloadOnChange, logger); + var loader = new KestrelConfigurationLoader(this, config, hostEnvironment, reloadOnChange, logger, httpsLogger); ConfigurationLoader = loader; return loader; } diff --git a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs index 2a2f737ab77f..61ba0e1f3b31 100644 --- a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs +++ b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs @@ -6,6 +6,7 @@ using System.Net.Security; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.DependencyInjection; @@ -37,7 +38,7 @@ public static class ListenOptionsHttpsExtensions /// The . public static ListenOptions UseHttps(this ListenOptions listenOptions, string fileName) { - var env = listenOptions.KestrelServerOptions.ApplicationServices.GetRequiredService(); + var env = listenOptions.ApplicationServices.GetRequiredService(); return listenOptions.UseHttps(new X509Certificate2(Path.Combine(env.ContentRootPath, fileName))); } @@ -51,7 +52,7 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, string fi /// The . public static ListenOptions UseHttps(this ListenOptions listenOptions, string fileName, string password) { - var env = listenOptions.KestrelServerOptions.ApplicationServices.GetRequiredService(); + var env = listenOptions.ApplicationServices.GetRequiredService(); return listenOptions.UseHttps(new X509Certificate2(Path.Combine(env.ContentRootPath, fileName), password)); } @@ -66,7 +67,7 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, string fi public static ListenOptions UseHttps(this ListenOptions listenOptions, string fileName, string password, Action configureOptions) { - var env = listenOptions.KestrelServerOptions.ApplicationServices.GetRequiredService(); + var env = listenOptions.ApplicationServices.GetRequiredService(); return listenOptions.UseHttps(new X509Certificate2(Path.Combine(env.ContentRootPath, fileName), password), configureOptions); } @@ -183,12 +184,21 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, Action ServerOptionsCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) + private static async ValueTask ServerOptionsCallback(SslStream sslStream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) { var (middleware, context, feature) = (ValueTuple)state; feature.HostName = clientHelloInfo.ServerName; + context.Features.Set(sslStream); - var sslOptions = await middleware._httpsOptionsCallback(context, stream, clientHelloInfo, middleware._httpsOptionsCallbackState, cancellationToken); + var sslOptions = await middleware._httpsOptionsCallback(context, sslStream, clientHelloInfo, middleware._httpsOptionsCallbackState, cancellationToken); - // REVIEW: Cache results? We don't do this for ServerCertificateSelectionCallback. - if (sslOptions.ServerCertificate is X509Certificate2 cert) - { - EnsureCertificateIsAllowedForServerAuth(cert); - } - - // REVIEW: Should we write any event before this? KestrelEventSource.Log.TlsHandshakeStart(context, sslOptions); return sslOptions; } - private static void EnsureCertificateIsAllowedForServerAuth(X509Certificate2 certificate) + internal static void EnsureCertificateIsAllowedForServerAuth(X509Certificate2 certificate) { if (!CertificateLoader.IsCertificateAllowedForServerAuth(certificate)) { diff --git a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs index 61e8d8688ad2..3339f094534e 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs @@ -3,6 +3,7 @@ using System; using System.Net; +using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -55,6 +56,13 @@ public void ConfigureEndpointDefaultsAppliesToNewEndpoints() Assert.Equal(HttpProtocols.Http2, options.CodeBackedListenOptions[3].Protocols); } + [Fact] + public void ConfigureThrowsInvalidOperationExceptionIfApplicationServicesIsNotSet() + { + var options = new KestrelServerOptions(); + Assert.Throws(() => options.Configure()); + } + [Fact] public void CanCallListenAfterConfigure() { @@ -62,8 +70,9 @@ public void CanCallListenAfterConfigure() // Ensure configure doesn't throw because of missing services. var serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(Mock.Of>()); serviceCollection.AddSingleton(Mock.Of()); + serviceCollection.AddSingleton(Mock.Of>()); + serviceCollection.AddSingleton(Mock.Of>()); options.ApplicationServices = serviceCollection.BuildServiceProvider(); options.Configure(); @@ -81,12 +90,5 @@ public void SettingRequestHeaderEncodingSelecterToNullThrowsArgumentNullExceptio var ex = Assert.Throws(() => options.RequestHeaderEncodingSelector = null); Assert.Equal("value", ex.ParamName); } - - [Fact] - public void ConfigureThrowsInvalidOperationExceptionIfApplicationServicesIsNotSet() - { - var options = new KestrelServerOptions(); - Assert.Throws(() => options.Configure()); - } } } diff --git a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs index a26e0c1b8ac8..dac5a74c5324 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Hosting.Server.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -494,9 +495,9 @@ public async Task ReloadsOnConfigurationChangeWhenOptedIn() mockLoggerFactory.Setup(m => m.CreateLogger(It.IsAny())).Returns(Mock.Of()); var serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(mockLoggerFactory.Object); - serviceCollection.AddSingleton(Mock.Of>()); serviceCollection.AddSingleton(Mock.Of()); + serviceCollection.AddSingleton(Mock.Of>()); + serviceCollection.AddSingleton(Mock.Of>()); var options = new KestrelServerOptions { @@ -631,9 +632,9 @@ public async Task DoesNotReloadOnConfigurationChangeByDefault() mockLoggerFactory.Setup(m => m.CreateLogger(It.IsAny())).Returns(Mock.Of()); var serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(mockLoggerFactory.Object); - serviceCollection.AddSingleton(Mock.Of>()); serviceCollection.AddSingleton(Mock.Of()); + serviceCollection.AddSingleton(Mock.Of>()); + serviceCollection.AddSingleton(Mock.Of>()); var options = new KestrelServerOptions { diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs index bfad694bbb55..f745bf675464 100644 --- a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; +using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging; using Moq; using Xunit; @@ -21,41 +22,40 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { public class SniOptionsSelectorTests { + private static X509Certificate2 _x509Certificate2 = TestResources.GetTestCertificate(); + [Fact] public void PrefersExactMatchOverWildcardPrefixOverWildcardOnly() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary { + "www.example.org", + new SniConfig { - "www.example.org", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "Exact" - } + Path = "Exact" } - }, + } + }, + { + "*.example.org", + new SniConfig { - "*.example.org", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "WildcardPrefix" - } + Path = "WildcardPrefix" } - }, + } + }, + { + "*", + new SniConfig { - "*", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "WildcardOnly" - } + Path = "WildcardOnly" } } } @@ -65,9 +65,10 @@ public void PrefersExactMatchOverWildcardPrefixOverWildcardOnly() var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, mockCertificateConfigLoader, - endpointConfig, - fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpsOptions: new HttpsConnectionAdapterOptions(), fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -93,28 +94,25 @@ public void PrefersExactMatchOverWildcardPrefixOverWildcardOnly() [Fact] public void PerfersLongerWildcardPrefixOverShorterWildcardPrefix() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary { + "*.a.example.org", + new SniConfig { - "*.a.example.org", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "Long" - } + Path = "Long" } - }, + } + }, + { + "*.example.org", + new SniConfig { - "*.example.org", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "Short" - } + Path = "Short" } } } @@ -124,9 +122,10 @@ public void PerfersLongerWildcardPrefixOverShorterWildcardPrefix() var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, mockCertificateConfigLoader, - endpointConfig, - fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpsOptions: new HttpsConnectionAdapterOptions(), fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -141,28 +140,25 @@ public void PerfersLongerWildcardPrefixOverShorterWildcardPrefix() [Fact] public void ServerNameMatchingIsCaseInsensitive() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary { + "Www.Example.Org", + new SniConfig { - "Www.Example.Org", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "Exact" - } + Path = "Exact" } - }, + } + }, + { + "*.Example.Org", + new SniConfig { - "*.Example.Org", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "WildcardPrefix" - } + Path = "WildcardPrefix" } } } @@ -172,9 +168,10 @@ public void ServerNameMatchingIsCaseInsensitive() var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, mockCertificateConfigLoader, - endpointConfig, - fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpsOptions: new HttpsConnectionAdapterOptions(), fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -191,44 +188,33 @@ public void ServerNameMatchingIsCaseInsensitive() [Fact] public void GetOptionsThrowsAnAuthenticationExceptionIfThereIsNoMatchingSniSection() { - var endpointConfig = new EndpointConfig - { - Name = "TestEndpointName", - Sni = new Dictionary() - }; - - var mockCertificateConfigLoader = new MockCertificateConfigLoader(); - var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; - var sniOptionsSelector = new SniOptionsSelector( - mockCertificateConfigLoader, - endpointConfig, - fallbackOptions: new HttpsConnectionAdapterOptions(), + "TestEndpointName", + new Dictionary(), + new MockCertificateConfigLoader(), + fallbackHttpsOptions: new HttpsConnectionAdapterOptions(), fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); var authExWithServerName = Assert.Throws(() => sniOptionsSelector.GetOptions(new MockConnectionContext(), "example.org")); - Assert.Equal(CoreStrings.FormatSniNotConfiguredForServerName("example.org", endpointConfig.Name), authExWithServerName.Message); + Assert.Equal(CoreStrings.FormatSniNotConfiguredForServerName("example.org", "TestEndpointName"), authExWithServerName.Message); var authExWithoutServerName = Assert.Throws(() => sniOptionsSelector.GetOptions(new MockConnectionContext(), null)); - Assert.Equal(CoreStrings.FormatSniNotConfiguredToAllowNoServerName(endpointConfig.Name), authExWithoutServerName.Message); + Assert.Equal(CoreStrings.FormatSniNotConfiguredToAllowNoServerName("TestEndpointName"), authExWithoutServerName.Message); } [Fact] public void WildcardOnlyMatchesNullServerNameDueToNoAlpn() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary { + "*", + new SniConfig { - "*", - new SniConfig + Certificate = new CertificateConfig { - Certificate = new CertificateConfig - { - Path = "WildcardOnly" - } + Path = "WildcardOnly" } } } @@ -238,9 +224,10 @@ public void WildcardOnlyMatchesNullServerNameDueToNoAlpn() var pathDictionary = mockCertificateConfigLoader.CertToPathDictionary; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, mockCertificateConfigLoader, - endpointConfig, - fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpsOptions: new HttpsConnectionAdapterOptions(), fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -251,24 +238,22 @@ public void WildcardOnlyMatchesNullServerNameDueToNoAlpn() [Fact] public void CachesSslServerAuthenticationOptions() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary { + "www.example.org", + new SniConfig { - "www.example.org", - new SniConfig - { - Certificate = new CertificateConfig() - } + Certificate = new CertificateConfig() } } }; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, new MockCertificateConfigLoader(), - endpointConfig, - fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpsOptions: new HttpsConnectionAdapterOptions(), fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -280,16 +265,13 @@ public void CachesSslServerAuthenticationOptions() [Fact] public void ClonesSslServerAuthenticationOptionsIfAnOnAuthenticateCallbackIsDefined() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary { + "www.example.org", + new SniConfig { - "www.example.org", - new SniConfig - { - Certificate = new CertificateConfig() - } + Certificate = new CertificateConfig() } } }; @@ -305,8 +287,9 @@ public void ClonesSslServerAuthenticationOptionsIfAnOnAuthenticateCallbackIsDefi }; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, new MockCertificateConfigLoader(), - endpointConfig, fallbackOptions, fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -323,25 +306,22 @@ public void ClonesSslServerAuthenticationOptionsIfAnOnAuthenticateCallbackIsDefi [Fact] public void ClonesSslServerAuthenticationOptionsIfTheFallbackServerCertificateSelectorIsUsed() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary { + "selector.example.org", + new SniConfig() + }, + { + "config.example.org", + new SniConfig { - "selector.example.org", - new SniConfig() - }, - { - "config.example.org", - new SniConfig - { - Certificate = new CertificateConfig() - } + Certificate = new CertificateConfig() } } }; - var selectorCertificate = new X509Certificate2(); + var selectorCertificate = _x509Certificate2; var fallbackOptions = new HttpsConnectionAdapterOptions { @@ -350,8 +330,9 @@ public void ClonesSslServerAuthenticationOptionsIfTheFallbackServerCertificateSe }; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, new MockCertificateConfigLoader(), - endpointConfig, fallbackOptions, fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -378,19 +359,17 @@ public void ClonesSslServerAuthenticationOptionsIfTheFallbackServerCertificateSe [Fact] public void ConstructorThrowsInvalidOperationExceptionIfNoCertificateDefiniedInConfigOrFallback() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary - { - { "www.example.org", new SniConfig() } - } + { "www.example.org", new SniConfig() } }; var ex = Assert.Throws( () => new SniOptionsSelector( + "TestEndpointName", + sniDictionary, new MockCertificateConfigLoader(), - endpointConfig, - fallbackOptions: new HttpsConnectionAdapterOptions(), + fallbackHttpsOptions: new HttpsConnectionAdapterOptions(), fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>())); @@ -400,12 +379,9 @@ public void ConstructorThrowsInvalidOperationExceptionIfNoCertificateDefiniedInC [Fact] public void FallsBackToHttpsConnectionAdapterCertificate() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary - { - { "www.example.org", new SniConfig() } - } + { "www.example.org", new SniConfig() } }; var fallbackOptions = new HttpsConnectionAdapterOptions @@ -414,8 +390,9 @@ public void FallsBackToHttpsConnectionAdapterCertificate() }; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, new MockCertificateConfigLoader(), - endpointConfig, fallbackOptions, fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -427,15 +404,12 @@ public void FallsBackToHttpsConnectionAdapterCertificate() [Fact] public void FallsBackToHttpsConnectionAdapterServerCertificateSelectorOverServerCertificate() { - var endpointConfig = new EndpointConfig + var sniDictionary = new Dictionary { - Sni = new Dictionary - { - { "www.example.org", new SniConfig() } - } + { "www.example.org", new SniConfig() } }; - var selectorCertificate = new X509Certificate2(); + var selectorCertificate = _x509Certificate2; var fallbackOptions = new HttpsConnectionAdapterOptions { @@ -444,8 +418,9 @@ public void FallsBackToHttpsConnectionAdapterServerCertificateSelectorOverServer }; var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, new MockCertificateConfigLoader(), - endpointConfig, fallbackOptions, fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -458,6 +433,8 @@ private class MockCertificateConfigLoader : ICertificateConfigLoader { public Dictionary CertToPathDictionary { get; } = new Dictionary(ReferenceEqualityComparer.Instance); + public bool SkipValidation => true; + public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) { if (certInfo is null) diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index dddb30838482..8998b19aac6a 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -275,7 +275,7 @@ public void ReadEndpointWithNoSslProtocolSettings_ReturnsNull() } [Fact] - public void ReadEndpointWithoutSniConfigured_ReturnsEmptyCollection() + public void ReadEndpointsOrEndpointDefaultsWithEmptySniSection_ReturnsEmptyCollection() { var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { @@ -283,28 +283,36 @@ public void ReadEndpointWithoutSniConfigured_ReturnsEmptyCollection() }).Build(); var reader = new ConfigurationReader(config); + var endpoint = reader.Endpoints.First(); Assert.NotNull(endpoint.Sni); Assert.False(endpoint.Sni.Any()); + + var endpointDefaults = reader.EndpointDefaults; + Assert.NotNull(endpointDefaults.Sni); + Assert.False(endpointDefaults.Sni.Any()); } [Fact] - public void ReadEndpointWithEmptySniKey_Throws() + public void ReadEndpointsOrEndpointDefaultsWithEmptySniKey_Throws() { var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), new KeyValuePair("Endpoints:End1:Sni::Protocols", "Http1"), + new KeyValuePair("EndpointDefaults:Sni::Protocols", "Http1"), }).Build(); var reader = new ConfigurationReader(config); - var ex = Assert.Throws(() => reader.Endpoints); + var end1Ex = Assert.Throws(() => reader.Endpoints); + var defaultEx = Assert.Throws(() => reader.EndpointDefaults); - Assert.Equal(CoreStrings.FormatSniNameCannotBeEmpty("End1"), ex.Message); + Assert.Equal(CoreStrings.FormatSniNameCannotBeEmpty("End1"), end1Ex.Message); + Assert.Equal(CoreStrings.FormatSniNameCannotBeEmpty("EndpointDefaults"), defaultEx.Message); } [Fact] - public void ReadEndpointWithSniConfigured_ReturnsCorrectValue() + public void ReadEndpointsOrEndpointDefaultsWithSniConfigured_ReturnsCorrectValue() { var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { @@ -314,19 +322,28 @@ public void ReadEndpointWithSniConfigured_ReturnsCorrectValue() new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:Path", "/path/cert.pfx"), new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:Password", "certpassword"), new KeyValuePair("Endpoints:End1:SNI:*.example.org:ClientCertificateMode", "AllowCertificate"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "Http1"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:Certificate:Path", "/path/cert.pfx"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:Certificate:Password", "certpassword"), + new KeyValuePair("EndpointDefaults:SNI:*.example.org:ClientCertificateMode", "AllowCertificate"), }).Build(); var reader = new ConfigurationReader(config); - var endpoint = reader.Endpoints.First(); - var sni = endpoint.Sni["*.EXAMPLE.org"]; - Assert.NotNull(sni); + static void VerifySniConfig(SniConfig config) + { + Assert.NotNull(config); + + Assert.Equal(HttpProtocols.Http1, config.Protocols); + Assert.Equal(SslProtocols.Tls12, config.SslProtocols); + Assert.Equal("/path/cert.pfx", config.Certificate.Path); + Assert.Equal("certpassword", config.Certificate.Password); + Assert.Equal(ClientCertificateMode.AllowCertificate, config.ClientCertificateMode); + } - Assert.Equal(HttpProtocols.Http1, sni.Protocols); - Assert.Equal(SslProtocols.Tls12, sni.SslProtocols); - Assert.Equal("/path/cert.pfx", sni.Certificate.Path); - Assert.Equal("certpassword", sni.Certificate.Password); - Assert.Equal(ClientCertificateMode.AllowCertificate, sni.ClientCertificateMode); + VerifySniConfig(reader.Endpoints.First().Sni["*.Example.org"]); + VerifySniConfig(reader.EndpointDefaults.Sni["*.Example.org"]); } [Fact] diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index ac4404038489..055b23c6fa68 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -672,7 +672,6 @@ public void DefaultEndpointConfigureSection_CanSetSslProtocols() Assert.True(ran1); } - [Fact] public void DefaultEndpointConfigureSection_ConfigureHttpsDefaultsCanOverrideSslProtocols() { @@ -802,7 +801,6 @@ public void DefaultEndpointConfigureSection_CanSetClientCertificateMode() Assert.True(ran1); } - [Fact] public void DefaultEndpointConfigureSection_ConfigureHttpsDefaultsCanOverrideClientCertificateMode() { @@ -833,6 +831,51 @@ public void DefaultEndpointConfigureSection_ConfigureHttpsDefaultsCanOverrideCli Assert.True(ran1); } + [Fact] + public void DefaultConfigSection_CanConfigureSni() + { + var serverOptions = CreateServerOptions(); + + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "Http1"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:ClientCertificateMode", "AllowCertificate"), + }).Build(); + + var (_, endpointsToStart) = serverOptions.Configure(config).Reload(); + var end1 = Assert.Single(endpointsToStart); + var (name, sniConfig) = Assert.Single(end1?.EndpointConfig?.Sni); + + Assert.Equal("*.example.org", name); + Assert.Equal(HttpProtocols.Http1, sniConfig.Protocols); + Assert.Equal(SslProtocols.Tls12, sniConfig.SslProtocols); + Assert.Equal(ClientCertificateMode.AllowCertificate, sniConfig.ClientCertificateMode); + } + + [Fact] + public void DefaultConfigSection_SniConfigurationIsOverriddenByNotMergedWithEndpointSpecificConfigSection() + { + var serverOptions = CreateServerOptions(); + + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), + new KeyValuePair("Endpoints:End1:Sni:*:Protocols", "Http1AndHttp2"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "Http1"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:ClientCertificateMode", "AllowCertificate"), + }).Build(); + + var (_, endpointsToStart) = serverOptions.Configure(config).Reload(); + var end1 = Assert.Single(endpointsToStart); + var (name, sniConfig) = Assert.Single(end1?.EndpointConfig?.Sni); + + Assert.Equal("*", name); + Assert.Equal(HttpProtocols.Http1AndHttp2, sniConfig.Protocols); + } + [Fact] public void Reload_IdentifiesEndpointsToStartAndStop() { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs index 6682cff2700e..471fa47af44c 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs @@ -12,7 +12,6 @@ using System.Security.Cryptography.X509Certificates; using System.Text; using System.Threading.Tasks; -using Castle.Core.Logging; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -26,7 +25,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Testing; using Moq; using Xunit; @@ -75,7 +73,8 @@ public async Task CanReadAndWriteWithHttpsConnectionMiddlewareWithPemCertificate options.ApplicationServices = serviceProvider; var logger = serviceProvider.GetRequiredService>(); - var loader = new KestrelConfigurationLoader(options, configuration, env.Object, reloadOnChange: false, logger); + var httpsLogger = serviceProvider.GetRequiredService>(); + var loader = new KestrelConfigurationLoader(options, configuration, env.Object, reloadOnChange: false, logger, httpsLogger); loader.Load(); void ConfigureListenOptions(ListenOptions listenOptions) From a2f708cff3c770d676473d86891e305fa2f2ef4f Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Aug 2020 06:59:47 -0700 Subject: [PATCH 16/24] Test new internal UseHttps() overload --- .../Core/src/HttpsConnectionAdapterOptions.cs | 4 ++- .../Middleware/HttpsConnectionMiddleware.cs | 15 ++++++--- .../HttpsConnectionMiddlewareTests.cs | 31 +++++++++++++++++++ 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs index 4f861c30f634..9961101484ca 100644 --- a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs +++ b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs @@ -16,6 +16,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Https /// public class HttpsConnectionAdapterOptions { + internal static TimeSpan DefaultHandshakeTimeout = TimeSpan.FromSeconds(10); + private TimeSpan _handshakeTimeout; /// @@ -24,7 +26,7 @@ public class HttpsConnectionAdapterOptions public HttpsConnectionAdapterOptions() { ClientCertificateMode = ClientCertificateMode.NoCertificate; - HandshakeTimeout = TimeSpan.FromSeconds(10); + HandshakeTimeout = DefaultHandshakeTimeout; } /// diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index de1a9ea8ce39..eb8ab43591bf 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -58,20 +58,27 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter throw new ArgumentNullException(nameof(options)); } + if (options.ServerCertificate == null && options.ServerCertificateSelector == null) + { + throw new ArgumentException(CoreStrings.ServerCertificateRequired, nameof(options)); + } + _next = next; _handshakeTimeout = options.HandshakeTimeout; _logger = loggerFactory.CreateLogger(); + // Something similar to the following could allow us to remove more duplicate logic, but we need https://github.com/dotnet/runtime/issues/40402 to be fixed first. + //var sniOptionsSelector = new SniOptionsSelector("", new Dictionary { { "*", new SniConfig() } }, new NoopCertificateConfigLoader(), options, options.HttpProtocols, _logger); + //_httpsOptionsCallback = SniOptionsSelector.OptionsCallback; + //_httpsOptionsCallbackState = sniOptionsSelector; + //_sslStreamFactory = s => new SslStream(s); + _options = options; _options.HttpProtocols = ValidateAndNormalizeHttpProtocols(_options.HttpProtocols, _logger); // capture the certificate now so it can't be switched after validation _serverCertificate = options.ServerCertificate; _serverCertificateSelector = options.ServerCertificateSelector; - if (_serverCertificate == null && _serverCertificateSelector == null) - { - throw new ArgumentException(CoreStrings.ServerCertificateRequired, nameof(options)); - } // If a selector is provided then ignore the cert, it may be a default cert. if (_serverCertificateSelector != null) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs index 471fa47af44c..273fbe9de53b 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs @@ -165,6 +165,37 @@ void ConfigureListenOptions(ListenOptions listenOptions) } } + [Fact(Skip = "https://github.com/dotnet/runtime/issues/40402")] + public async Task ClientCertificateRequiredConfiguredInCallbackContinuesWhenNoCertificate() + { + void ConfigureListenOptions(ListenOptions listenOptions) + { + listenOptions.UseHttps((connection, stream, clientHelloInfo, state, cancellationToken) => + new ValueTask(new SslServerAuthenticationOptions + { + ServerCertificate = _x509Certificate2, + // From the API Docs: "Note that this is only a request -- + // if no certificate is provided, the server still accepts the connection request." + // Not to mention this is equivalent to the test above. + ClientCertificateRequired = true, + RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, + CertificateRevocationCheckMode = X509RevocationMode.NoCheck + }), state: null, HttpsConnectionAdapterOptions.DefaultHandshakeTimeout); + } + + await using (var server = new TestServer(context => + { + var tlsFeature = context.Features.Get(); + Assert.NotNull(tlsFeature); + Assert.Null(tlsFeature.ClientCertificate); + return context.Response.WriteAsync("hello world"); + }, new TestServiceContext(LoggerFactory), ConfigureListenOptions)) + { + var result = await server.HttpClientSlim.GetStringAsync($"https://localhost:{server.Port}/", validateCertificate: false); + Assert.Equal("hello world", result); + } + } + [Fact] public void ThrowsWhenNoServerCertificateIsProvided() { From 6237f5a7178c3b4e35d15ed485b26af83dac9782 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Aug 2020 13:57:49 -0700 Subject: [PATCH 17/24] Add CloneSslOptionsClonesAllProperties test --- .../Core/src/Internal/SniOptionsSelector.cs | 3 - .../Core/test/SniOptionsSelectorTests.cs | 86 +++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index ae558bfc80f2..c8dfa61cf2b8 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -180,9 +180,6 @@ public static ValueTask OptionsCallback(Connecti return new ValueTask(options); } - // TODO: Reflection based test to ensure we clone everything! - // This won't catch issues related to mutable subproperties, but the existing subproperties look like they're mostly immutable. - // The exception are the ApplicationProtocols list which we clone and the ServerCertificate because of methods like Import() and Reset() :( internal static SslServerAuthenticationOptions CloneSslOptions(SslServerAuthenticationOptions sslOptions) => new SslServerAuthenticationOptions { diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs index f745bf675464..11a01d0b8a6f 100644 --- a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -4,7 +4,9 @@ using System; using System.Collections.Generic; using System.IO.Pipelines; +using System.Linq; using System.Net.Security; +using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Connections; @@ -429,6 +431,90 @@ public void FallsBackToHttpsConnectionAdapterServerCertificateSelectorOverServer Assert.Same(selectorCertificate, options.ServerCertificate); } + [Fact] + public void CloneSslOptionsClonesAllProperties() + { + var propertyNames = typeof(SslServerAuthenticationOptions).GetProperties().Select(property => property.Name).ToList(); + + CipherSuitesPolicy cipherSuitesPolicy = null; + + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // The CipherSuitesPolicy ctor throws a PlatformNotSupportedException on Windows. + cipherSuitesPolicy = new CipherSuitesPolicy(new[] { TlsCipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 }); + } + + // Set options properties to non-default values to verify they're copied. + var options = new SslServerAuthenticationOptions + { + // Defaults to true + AllowRenegotiation = false, + // Defaults to null + ApplicationProtocols = new List { SslApplicationProtocol.Http3 }, + // Defaults to X509RevocationMode.NoCheck + CertificateRevocationCheckMode = X509RevocationMode.Offline, + // Defaults to null + CipherSuitesPolicy = cipherSuitesPolicy, + // Defaults to false + ClientCertificateRequired = true, + // Defaults to SslProtocols.None + EnabledSslProtocols = SslProtocols.Tls13 | SslProtocols.Tls11, + // Defaults to EncryptionPolicy.RequireEncryption + EncryptionPolicy = EncryptionPolicy.NoEncryption, + // Defaults to null + RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, + // Defaults to null + ServerCertificate = new X509Certificate2(), + // Defaults to null + ServerCertificateContext = SslStreamCertificateContext.Create(_x509Certificate2, additionalCertificates: null, offline: true), + // Defaults to null + ServerCertificateSelectionCallback = (sender, serverName) => null, + }; + + var clonedOptions = SniOptionsSelector.CloneSslOptions(options); + + Assert.NotSame(options, clonedOptions); + + Assert.Equal(options.AllowRenegotiation, clonedOptions.AllowRenegotiation); + Assert.True(propertyNames.Remove(nameof(options.AllowRenegotiation))); + + // Ensure the List is also cloned since it could be modified by a user callback. + Assert.NotSame(options.ApplicationProtocols, clonedOptions.ApplicationProtocols); + Assert.Equal(Assert.Single(options.ApplicationProtocols), Assert.Single(clonedOptions.ApplicationProtocols)); + Assert.True(propertyNames.Remove(nameof(options.ApplicationProtocols))); + + Assert.Equal(options.CertificateRevocationCheckMode, clonedOptions.CertificateRevocationCheckMode); + Assert.True(propertyNames.Remove(nameof(options.CertificateRevocationCheckMode))); + + Assert.Same(options.CipherSuitesPolicy, clonedOptions.CipherSuitesPolicy); + Assert.True(propertyNames.Remove(nameof(options.CipherSuitesPolicy))); + + Assert.Equal(options.ClientCertificateRequired, clonedOptions.ClientCertificateRequired); + Assert.True(propertyNames.Remove(nameof(options.ClientCertificateRequired))); + + Assert.Equal(options.EnabledSslProtocols, clonedOptions.EnabledSslProtocols); + Assert.True(propertyNames.Remove(nameof(options.EnabledSslProtocols))); + + Assert.Equal(options.EncryptionPolicy, clonedOptions.EncryptionPolicy); + Assert.True(propertyNames.Remove(nameof(options.EncryptionPolicy))); + + Assert.Same(options.RemoteCertificateValidationCallback, clonedOptions.RemoteCertificateValidationCallback); + Assert.True(propertyNames.Remove(nameof(options.RemoteCertificateValidationCallback))); + + // Technically the ServerCertificate could be reset/reimported, but I'm hoping this is uncommon. Trying to clone the certificate and/or context seems risky. + Assert.Same(options.ServerCertificate, clonedOptions.ServerCertificate); + Assert.True(propertyNames.Remove(nameof(options.ServerCertificate))); + + Assert.Same(options.ServerCertificateContext, clonedOptions.ServerCertificateContext); + Assert.True(propertyNames.Remove(nameof(options.ServerCertificateContext))); + + Assert.Same(options.ServerCertificateSelectionCallback, clonedOptions.ServerCertificateSelectionCallback); + Assert.True(propertyNames.Remove(nameof(options.ServerCertificateSelectionCallback))); + + // Ensure we've checked every property. When new properties get added, we'll have to update this test along with the CloneSslOptions implementation. + Assert.Empty(propertyNames); + } + private class MockCertificateConfigLoader : ICertificateConfigLoader { public Dictionary CertToPathDictionary { get; } = new Dictionary(ReferenceEqualityComparer.Instance); From 4869022ff777a7a8c96863df798a1773386076ad Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Aug 2020 15:10:25 -0700 Subject: [PATCH 18/24] More tests --- .../Core/test/KestrelServerOptionsTests.cs | 11 + .../Core/test/SniOptionsSelectorTests.cs | 224 ++++++++++++++++++ .../test/KestrelConfigurationLoaderTests.cs | 8 +- 3 files changed, 239 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs index 3339f094534e..e96c2d1940ca 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs @@ -63,6 +63,17 @@ public void ConfigureThrowsInvalidOperationExceptionIfApplicationServicesIsNotSe Assert.Throws(() => options.Configure()); } + [Fact] + public void ConfigureThrowsInvalidOperationExceptionIfApplicationServicesDoesntHaveRequiredServices() + { + var options = new KestrelServerOptions + { + ApplicationServices = new ServiceCollection().BuildServiceProvider() + }; + + Assert.Throws(() => options.Configure()); + } + [Fact] public void CanCallListenAfterConfigure() { diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs index 11a01d0b8a6f..e4da8fee570f 100644 --- a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -431,6 +431,230 @@ public void FallsBackToHttpsConnectionAdapterServerCertificateSelectorOverServer Assert.Same(selectorCertificate, options.ServerCertificate); } + [Fact] + public void PrefersHttpProtocolsDefinedInSniConfig() + { + var sniDictionary = new Dictionary + { + { + "www.example.org", + new SniConfig + { + Protocols = HttpProtocols.None, + Certificate = new CertificateConfig() + } + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, + new MockCertificateConfigLoader(), + new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1, + logger: Mock.Of>()); + + var mockConnectionContext = new MockConnectionContext(); + sniOptionsSelector.GetOptions(mockConnectionContext, "www.example.org"); + + var httpProtocolsFeature = mockConnectionContext.Features.Get(); + Assert.NotNull(httpProtocolsFeature); + Assert.Equal(HttpProtocols.None, httpProtocolsFeature.HttpProtocols); + } + + [Fact] + public void ConfiguresAlpnBasedOnConfiguredHttpProtocols() + { + var sniDictionary = new Dictionary + { + { + "www.example.org", + new SniConfig + { + // I'm not using Http1AndHttp2 or Http2 because I don't want to account for + // validation and normalization. Other tests cover that. + Protocols = HttpProtocols.Http1, + Certificate = new CertificateConfig() + } + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, + new MockCertificateConfigLoader(), + new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.None, + logger: Mock.Of>()); + + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + var alpnList = options.ApplicationProtocols; + + Assert.NotNull(alpnList); + var protocol = Assert.Single(alpnList); + Assert.Equal(SslApplicationProtocol.Http11, protocol); + } + + [Fact] + public void FallsBackToFallbackHttpProtocols() + { + var sniDictionary = new Dictionary + { + { + "www.example.org", + new SniConfig + { + Certificate = new CertificateConfig() + } + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, + new MockCertificateConfigLoader(), + new HttpsConnectionAdapterOptions(), + fallbackHttpProtocols: HttpProtocols.Http1, + logger: Mock.Of>()); + + var mockConnectionContext = new MockConnectionContext(); + sniOptionsSelector.GetOptions(mockConnectionContext, "www.example.org"); + + var httpProtocolsFeature = mockConnectionContext.Features.Get(); + Assert.NotNull(httpProtocolsFeature); + Assert.Equal(HttpProtocols.Http1, httpProtocolsFeature.HttpProtocols); + } + + [Fact] + public void PrefersSslProtocolsDefinedInSniConfig() + { + var sniDictionary = new Dictionary + { + { + "www.example.org", + new SniConfig + { + SslProtocols = SslProtocols.Tls13 | SslProtocols.Tls11, + Certificate = new CertificateConfig() + } + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, + new MockCertificateConfigLoader(), + new HttpsConnectionAdapterOptions + { + SslProtocols = SslProtocols.Tls13 + }, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Equal(SslProtocols.Tls13 | SslProtocols.Tls11, options.EnabledSslProtocols); + } + + [Fact] + public void FallsBackToFallbackSslProtocols() + { + var sniDictionary = new Dictionary + { + { + "www.example.org", + new SniConfig + { + Certificate = new CertificateConfig() + } + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, + new MockCertificateConfigLoader(), + new HttpsConnectionAdapterOptions + { + SslProtocols = SslProtocols.Tls13 + }, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + Assert.Equal(SslProtocols.Tls13, options.EnabledSslProtocols); + } + + + [Fact] + public void PrefersClientCertificateModeDefinedInSniConfig() + { + var sniDictionary = new Dictionary + { + { + "www.example.org", + new SniConfig + { + ClientCertificateMode = ClientCertificateMode.RequireCertificate, + Certificate = new CertificateConfig() + } + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, + new MockCertificateConfigLoader(), + new HttpsConnectionAdapterOptions + { + ClientCertificateMode = ClientCertificateMode.AllowCertificate + }, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + + Assert.True(options.ClientCertificateRequired); + + Assert.NotNull(options.RemoteCertificateValidationCallback); + // The RemoteCertificateValidationCallback should first check if the certificate is null and return false since it's required. + Assert.False(options.RemoteCertificateValidationCallback(sender: null, certificate: null, chain: null, SslPolicyErrors.None)); + } + + [Fact] + public void FallsBackToFallbackClientCertificateMode() + { + var sniDictionary = new Dictionary + { + { + "www.example.org", + new SniConfig + { + Certificate = new CertificateConfig() + } + } + }; + + var sniOptionsSelector = new SniOptionsSelector( + "TestEndpointName", + sniDictionary, + new MockCertificateConfigLoader(), + new HttpsConnectionAdapterOptions + { + ClientCertificateMode = ClientCertificateMode.AllowCertificate + }, + fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, + logger: Mock.Of>()); + + var options = sniOptionsSelector.GetOptions(new MockConnectionContext(), "www.example.org"); + + // Despite the confusing name, ClientCertificateRequired being true simply requests a certificate from the client, but doesn't require it. + Assert.True(options.ClientCertificateRequired); + + Assert.NotNull(options.RemoteCertificateValidationCallback); + // The RemoteCertificateValidationCallback should see we're in the AllowCertificate mode and return true. + Assert.True(options.RemoteCertificateValidationCallback(sender: null, certificate: null, chain: null, SslPolicyErrors.None)); + } + [Fact] public void CloneSslOptionsClonesAllProperties() { diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index 055b23c6fa68..85eec123a008 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -839,7 +839,7 @@ public void DefaultConfigSection_CanConfigureSni() var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "Http1"), + new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "None"), new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), new KeyValuePair("EndpointDefaults:Sni:*.example.org:ClientCertificateMode", "AllowCertificate"), }).Build(); @@ -849,7 +849,7 @@ public void DefaultConfigSection_CanConfigureSni() var (name, sniConfig) = Assert.Single(end1?.EndpointConfig?.Sni); Assert.Equal("*.example.org", name); - Assert.Equal(HttpProtocols.Http1, sniConfig.Protocols); + Assert.Equal(HttpProtocols.None, sniConfig.Protocols); Assert.Equal(SslProtocols.Tls12, sniConfig.SslProtocols); Assert.Equal(ClientCertificateMode.AllowCertificate, sniConfig.ClientCertificateMode); } @@ -862,7 +862,7 @@ public void DefaultConfigSection_SniConfigurationIsOverriddenByNotMergedWithEndp var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), - new KeyValuePair("Endpoints:End1:Sni:*:Protocols", "Http1AndHttp2"), + new KeyValuePair("Endpoints:End1:Sni:*:Protocols", "None"), new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "Http1"), new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), new KeyValuePair("EndpointDefaults:Sni:*.example.org:ClientCertificateMode", "AllowCertificate"), @@ -873,7 +873,7 @@ public void DefaultConfigSection_SniConfigurationIsOverriddenByNotMergedWithEndp var (name, sniConfig) = Assert.Single(end1?.EndpointConfig?.Sni); Assert.Equal("*", name); - Assert.Equal(HttpProtocols.Http1AndHttp2, sniConfig.Protocols); + Assert.Equal(HttpProtocols.None, sniConfig.Protocols); } [Fact] From 14b4d3be5fc349c7440c20b171a9f968823b6a36 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Aug 2020 19:48:12 -0700 Subject: [PATCH 19/24] Add functional test --- .../HttpsConnectionMiddlewareTests.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs index 273fbe9de53b..12e43d9e4e61 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs @@ -95,6 +95,73 @@ void ConfigureListenOptions(ListenOptions listenOptions) } } + [ConditionalFact] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "Missing SslStream ALPN support: https://github.com/dotnet/corefx/issues/30492")] + [MinimumOSVersion(OperatingSystems.Windows, WindowsVersions.Win81)] + public async Task CanUseDefaultSniConfiguration() + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary + { + ["EndpointDefaults:Sni:*.example.org:Protocols"] = "Http1", + ["EndpointDefaults:Sni:dot.net:Protocols"] = "None", + }).Build(); + + var options = new KestrelServerOptions(); + var env = new Mock(); + env.SetupGet(e => e.ContentRootPath).Returns(Directory.GetCurrentDirectory()); + + options.ApplicationServices = new ServiceCollection() + .AddLogging() + .AddSingleton(env.Object) + .BuildServiceProvider(); + + options.Configure(configuration).Load(); + + void ConfigureListenOptions(ListenOptions listenOptions) + { + listenOptions.KestrelServerOptions = options; + listenOptions.UseHttps(_x509Certificate2); + // We don't need to set listenOptions.Protocols since it will be flowed through the HttpProtocolsFeature + }; + + await using var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), ConfigureListenOptions); + + using var exampleConnection = server.CreateConnection(); + var exampleSslOptions = new SslClientAuthenticationOptions + { + TargetHost = "a.example.org", + ApplicationProtocols = new List { SslApplicationProtocol.Http11, SslApplicationProtocol.Http2 }, + }; + + using var exampleStream = OpenSslStream(exampleConnection.Stream); + await exampleStream.AuthenticateAsClientAsync(exampleSslOptions); + + Assert.Equal(SslApplicationProtocol.Http11, exampleStream.NegotiatedApplicationProtocol); + + using var dotnetConnection = server.CreateConnection(); + var dotnetSslOptions = new SslClientAuthenticationOptions + { + TargetHost = "dot.net", + ApplicationProtocols = new List { SslApplicationProtocol.Http11, SslApplicationProtocol.Http2 }, + }; + + using var dotnetStream = OpenSslStream(dotnetConnection.Stream); + await dotnetStream.AuthenticateAsClientAsync(dotnetSslOptions); + + // HttpProtocols.None was configured, so there is no negotiated protocol + Assert.True(dotnetStream.NegotiatedApplicationProtocol.Protocol.IsEmpty); + + using var emptyNameConnection = server.CreateConnection(); + var emptyNameSslOptions = new SslClientAuthenticationOptions + { + TargetHost = "", + }; + + using var refusedStream = OpenSslStream(emptyNameConnection.Stream); + // We expect the handshake to throw here because Kestrel refuses the connection due to there being no TargetHost and now wildcard config. + await Assert.ThrowsAsync(async () => await refusedStream.AuthenticateAsClientAsync(emptyNameSslOptions)); + } + [Fact] public async Task HandshakeDetailsAreAvailable() { From 24a4023ac01964e0c79738a3ccd77c85e7cc22e3 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 6 Aug 2020 17:18:07 -0700 Subject: [PATCH 20/24] SkipValidation -> IsTestMock --- .../Core/src/Internal/Certificates/CertificateConfigLoader.cs | 2 +- .../Core/src/Internal/Certificates/ICertificateConfigLoader.cs | 3 +-- src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs | 3 +-- src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs index d61d3052465f..f2663b9fbcd1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs @@ -23,7 +23,7 @@ public CertificateConfigLoader(IHostEnvironment hostEnvironment, ILogger Logger { get; } - public bool SkipValidation => false; + public bool IsTestMock => false; public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs index 9e43d3dffdfb..53cf84f42b9c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs @@ -7,8 +7,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates { internal interface ICertificateConfigLoader { - // SkipValidation is only true in tests. - bool SkipValidation { get; } + bool IsTestMock { get; } X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName); } diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index c8dfa61cf2b8..279ca5c65c38 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -68,8 +68,7 @@ public SniOptionsSelector( } } - // SkipValidation is only true in tests. - if (!certifcateConfigLoader.SkipValidation && sslOptions.ServerCertificate is X509Certificate2 cert2) + if (!certifcateConfigLoader.IsTestMock && sslOptions.ServerCertificate is X509Certificate2 cert2) { HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(cert2); } diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs index e4da8fee570f..d6215bcffd87 100644 --- a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -743,7 +743,7 @@ private class MockCertificateConfigLoader : ICertificateConfigLoader { public Dictionary CertToPathDictionary { get; } = new Dictionary(ReferenceEqualityComparer.Instance); - public bool SkipValidation => true; + public bool IsTestMock => true; public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) { From da3aa140db3e91fff6156ff1508abd9353d144d3 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 7 Aug 2020 12:35:02 -0700 Subject: [PATCH 21/24] Remove EndpointDefaults.Sni --- .../Core/src/Internal/ConfigurationReader.cs | 1 - .../Core/src/KestrelConfigurationLoader.cs | 23 ------- .../Core/src/ListenOptionsHttpsExtensions.cs | 27 ++------ .../Core/test/SniOptionsSelectorTests.cs | 2 +- .../Kestrel/test/ConfigurationReaderTests.cs | 19 +----- .../test/KestrelConfigurationLoaderTests.cs | 45 ------------- .../HttpsConnectionMiddlewareTests.cs | 67 ------------------- 7 files changed, 8 insertions(+), 176 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index df5c05cda7fe..2697749d178a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -75,7 +75,6 @@ private EndpointDefaults ReadEndpointDefaults() Protocols = ParseProtocols(configSection[ProtocolsKey]), SslProtocols = ParseSslProcotols(configSection.GetSection(SslProtocolsKey)), ClientCertificateMode = ParseClientCertificateMode(configSection[ClientCertificateModeKey]), - Sni = ReadSni(configSection.GetSection(SniKey), EndpointDefaultsKey) }; } diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index a58eafe6ccb9..651767399240 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -260,22 +260,6 @@ internal void ApplyHttpsDefaults(HttpsConnectionAdapterOptions httpsOptions) } } - internal SniOptionsSelector GetDefaultSniOptionsSelector(HttpsConnectionAdapterOptions fallbackHttpsOptions, HttpProtocols fallbackHttpProtocols) - { - if (ConfigurationReader.EndpointDefaults.Sni.Count == 0) - { - return null; - } - - return new SniOptionsSelector( - ConfigurationReader.EndpointDefaultsKey, - ConfigurationReader.EndpointDefaults.Sni, - CertificateConfigLoader, - fallbackHttpsOptions, - fallbackHttpProtocols, - HttpsLogger); - } - public void Load() { if (_loaded) @@ -352,13 +336,6 @@ public void Load() endpoint.ClientCertificateMode = ConfigurationReader.EndpointDefaults.ClientCertificateMode; } - if (endpoint.Sni.Count == 0) - { - // Ensure endpoint is reloaded if it used the default SNI config and it changed. - // No need to configure httpsOptions for SNI since the SniOptionsSelector will now use the EndpointDefaults SNI config. - endpoint.Sni = ConfigurationReader.EndpointDefaults.Sni; - } - // A cert specified directly on the endpoint overrides any defaults. httpsOptions.ServerCertificate = CertificateConfigLoader.LoadCertificate(endpoint.Certificate, endpoint.Name) ?? httpsOptions.ServerCertificate; diff --git a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs index 61ba0e1f3b31..a65c0937ac0c 100644 --- a/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs +++ b/src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs @@ -184,21 +184,12 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, Action { SslApplicationProtocol.Http3 }, + ApplicationProtocols = new List { SslApplicationProtocol.Http2 }, // Defaults to X509RevocationMode.NoCheck CertificateRevocationCheckMode = X509RevocationMode.Offline, // Defaults to null diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index 8998b19aac6a..f71286354736 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -275,7 +275,7 @@ public void ReadEndpointWithNoSslProtocolSettings_ReturnsNull() } [Fact] - public void ReadEndpointsOrEndpointDefaultsWithEmptySniSection_ReturnsEmptyCollection() + public void ReadEndpointWithEmptySniSection_ReturnsEmptyCollection() { var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { @@ -287,32 +287,25 @@ public void ReadEndpointsOrEndpointDefaultsWithEmptySniSection_ReturnsEmptyColle var endpoint = reader.Endpoints.First(); Assert.NotNull(endpoint.Sni); Assert.False(endpoint.Sni.Any()); - - var endpointDefaults = reader.EndpointDefaults; - Assert.NotNull(endpointDefaults.Sni); - Assert.False(endpointDefaults.Sni.Any()); } [Fact] - public void ReadEndpointsOrEndpointDefaultsWithEmptySniKey_Throws() + public void ReadEndpointWithEmptySniKey_Throws() { var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), new KeyValuePair("Endpoints:End1:Sni::Protocols", "Http1"), - new KeyValuePair("EndpointDefaults:Sni::Protocols", "Http1"), }).Build(); var reader = new ConfigurationReader(config); var end1Ex = Assert.Throws(() => reader.Endpoints); - var defaultEx = Assert.Throws(() => reader.EndpointDefaults); Assert.Equal(CoreStrings.FormatSniNameCannotBeEmpty("End1"), end1Ex.Message); - Assert.Equal(CoreStrings.FormatSniNameCannotBeEmpty("EndpointDefaults"), defaultEx.Message); } [Fact] - public void ReadEndpointsOrEndpointDefaultsWithSniConfigured_ReturnsCorrectValue() + public void ReadEndpointWithSniConfigured_ReturnsCorrectValue() { var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { @@ -322,11 +315,6 @@ public void ReadEndpointsOrEndpointDefaultsWithSniConfigured_ReturnsCorrectValue new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:Path", "/path/cert.pfx"), new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:Password", "certpassword"), new KeyValuePair("Endpoints:End1:SNI:*.example.org:ClientCertificateMode", "AllowCertificate"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "Http1"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:Certificate:Path", "/path/cert.pfx"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:Certificate:Password", "certpassword"), - new KeyValuePair("EndpointDefaults:SNI:*.example.org:ClientCertificateMode", "AllowCertificate"), }).Build(); var reader = new ConfigurationReader(config); @@ -343,7 +331,6 @@ static void VerifySniConfig(SniConfig config) } VerifySniConfig(reader.Endpoints.First().Sni["*.Example.org"]); - VerifySniConfig(reader.EndpointDefaults.Sni["*.Example.org"]); } [Fact] diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index 85eec123a008..cd2ac796fc52 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -831,51 +831,6 @@ public void DefaultEndpointConfigureSection_ConfigureHttpsDefaultsCanOverrideCli Assert.True(ran1); } - [Fact] - public void DefaultConfigSection_CanConfigureSni() - { - var serverOptions = CreateServerOptions(); - - var config = new ConfigurationBuilder().AddInMemoryCollection(new[] - { - new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "None"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:ClientCertificateMode", "AllowCertificate"), - }).Build(); - - var (_, endpointsToStart) = serverOptions.Configure(config).Reload(); - var end1 = Assert.Single(endpointsToStart); - var (name, sniConfig) = Assert.Single(end1?.EndpointConfig?.Sni); - - Assert.Equal("*.example.org", name); - Assert.Equal(HttpProtocols.None, sniConfig.Protocols); - Assert.Equal(SslProtocols.Tls12, sniConfig.SslProtocols); - Assert.Equal(ClientCertificateMode.AllowCertificate, sniConfig.ClientCertificateMode); - } - - [Fact] - public void DefaultConfigSection_SniConfigurationIsOverriddenByNotMergedWithEndpointSpecificConfigSection() - { - var serverOptions = CreateServerOptions(); - - var config = new ConfigurationBuilder().AddInMemoryCollection(new[] - { - new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), - new KeyValuePair("Endpoints:End1:Sni:*:Protocols", "None"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:Protocols", "Http1"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:SslProtocols:0", "Tls12"), - new KeyValuePair("EndpointDefaults:Sni:*.example.org:ClientCertificateMode", "AllowCertificate"), - }).Build(); - - var (_, endpointsToStart) = serverOptions.Configure(config).Reload(); - var end1 = Assert.Single(endpointsToStart); - var (name, sniConfig) = Assert.Single(end1?.EndpointConfig?.Sni); - - Assert.Equal("*", name); - Assert.Equal(HttpProtocols.None, sniConfig.Protocols); - } - [Fact] public void Reload_IdentifiesEndpointsToStartAndStop() { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs index 12e43d9e4e61..273fbe9de53b 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs @@ -95,73 +95,6 @@ void ConfigureListenOptions(ListenOptions listenOptions) } } - [ConditionalFact] - [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "Missing SslStream ALPN support: https://github.com/dotnet/corefx/issues/30492")] - [MinimumOSVersion(OperatingSystems.Windows, WindowsVersions.Win81)] - public async Task CanUseDefaultSniConfiguration() - { - var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary - { - ["EndpointDefaults:Sni:*.example.org:Protocols"] = "Http1", - ["EndpointDefaults:Sni:dot.net:Protocols"] = "None", - }).Build(); - - var options = new KestrelServerOptions(); - var env = new Mock(); - env.SetupGet(e => e.ContentRootPath).Returns(Directory.GetCurrentDirectory()); - - options.ApplicationServices = new ServiceCollection() - .AddLogging() - .AddSingleton(env.Object) - .BuildServiceProvider(); - - options.Configure(configuration).Load(); - - void ConfigureListenOptions(ListenOptions listenOptions) - { - listenOptions.KestrelServerOptions = options; - listenOptions.UseHttps(_x509Certificate2); - // We don't need to set listenOptions.Protocols since it will be flowed through the HttpProtocolsFeature - }; - - await using var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory), ConfigureListenOptions); - - using var exampleConnection = server.CreateConnection(); - var exampleSslOptions = new SslClientAuthenticationOptions - { - TargetHost = "a.example.org", - ApplicationProtocols = new List { SslApplicationProtocol.Http11, SslApplicationProtocol.Http2 }, - }; - - using var exampleStream = OpenSslStream(exampleConnection.Stream); - await exampleStream.AuthenticateAsClientAsync(exampleSslOptions); - - Assert.Equal(SslApplicationProtocol.Http11, exampleStream.NegotiatedApplicationProtocol); - - using var dotnetConnection = server.CreateConnection(); - var dotnetSslOptions = new SslClientAuthenticationOptions - { - TargetHost = "dot.net", - ApplicationProtocols = new List { SslApplicationProtocol.Http11, SslApplicationProtocol.Http2 }, - }; - - using var dotnetStream = OpenSslStream(dotnetConnection.Stream); - await dotnetStream.AuthenticateAsClientAsync(dotnetSslOptions); - - // HttpProtocols.None was configured, so there is no negotiated protocol - Assert.True(dotnetStream.NegotiatedApplicationProtocol.Protocol.IsEmpty); - - using var emptyNameConnection = server.CreateConnection(); - var emptyNameSslOptions = new SslClientAuthenticationOptions - { - TargetHost = "", - }; - - using var refusedStream = OpenSslStream(emptyNameConnection.Stream); - // We expect the handshake to throw here because Kestrel refuses the connection due to there being no TargetHost and now wildcard config. - await Assert.ThrowsAsync(async () => await refusedStream.AuthenticateAsClientAsync(emptyNameSslOptions)); - } - [Fact] public async Task HandshakeDetailsAreAvailable() { From 25d354b716d5dfa5da64d8025aa9836bac1c20d4 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 10 Aug 2020 16:00:05 -0700 Subject: [PATCH 22/24] Validate that HTTPS config is not applied to non-HTTPS endpoints --- src/Servers/Kestrel/Core/src/CoreStrings.resx | 3 + .../Core/src/Internal/ConfigurationReader.cs | 31 ++++- .../Core/src/KestrelConfigurationLoader.cs | 5 + .../test/KestrelConfigurationLoaderTests.cs | 112 ++++++++++++++++++ 4 files changed, 146 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index 980638e349e8..1dbd5c043564 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -629,4 +629,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l The endpoint {endpointName} is invalid because an SNI configuration section has an empty string as its key. Use a wildcard ('*') SNI section to match all server names. + + The non-HTTPS endpoint {endpointName} includes HTTPS-only configuration for {keyName}. + \ No newline at end of file diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index 2697749d178a..d21d0562e8a1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -16,13 +16,12 @@ internal class ConfigurationReader private const string CertificatesKey = "Certificates"; private const string CertificateKey = "Certificate"; private const string SslProtocolsKey = "SslProtocols"; + private const string EndpointDefaultsKey = "EndpointDefaults"; private const string EndpointsKey = "Endpoints"; private const string UrlKey = "Url"; private const string ClientCertificateModeKey = "ClientCertificateMode"; private const string SniKey = "Sni"; - internal const string EndpointDefaultsKey = "EndpointDefaults"; - private readonly IConfiguration _configuration; private IDictionary _certificates; @@ -131,7 +130,7 @@ private IEnumerable ReadEndpoints() return endpoints; } - private Dictionary ReadSni(IConfigurationSection sniConfig, string endpointName) + private static Dictionary ReadSni(IConfigurationSection sniConfig, string endpointName) { var sniDictionary = new Dictionary(0, StringComparer.OrdinalIgnoreCase); @@ -177,7 +176,7 @@ private Dictionary ReadSni(IConfigurationSection sniConfig, s } - private ClientCertificateMode? ParseClientCertificateMode(string clientCertificateMode) + private static ClientCertificateMode? ParseClientCertificateMode(string clientCertificateMode) { if (Enum.TryParse(clientCertificateMode, ignoreCase: true, out var result)) { @@ -211,6 +210,29 @@ private Dictionary ReadSni(IConfigurationSection sniConfig, s return acc; }); } + + internal static void ThrowIfContainsHttpsOnlyConfiguration(EndpointConfig endpoint) + { + if (endpoint.Certificate.IsFileCert || endpoint.Certificate.IsStoreCert) + { + throw new InvalidOperationException(CoreStrings.FormatEndpointHasUnusedHttpsConfig(endpoint.Name, CertificateKey)); + } + + if (endpoint.ClientCertificateMode.HasValue) + { + throw new InvalidOperationException(CoreStrings.FormatEndpointHasUnusedHttpsConfig(endpoint.Name, ClientCertificateModeKey)); + } + + if (endpoint.SslProtocols.HasValue) + { + throw new InvalidOperationException(CoreStrings.FormatEndpointHasUnusedHttpsConfig(endpoint.Name, SslProtocolsKey)); + } + + if (endpoint.Sni.Count > 0) + { + throw new InvalidOperationException(CoreStrings.FormatEndpointHasUnusedHttpsConfig(endpoint.Name, SniKey)); + } + } } // "EndpointDefaults": { @@ -223,7 +245,6 @@ internal class EndpointDefaults public HttpProtocols? Protocols { get; set; } public SslProtocols? SslProtocols { get; set; } public ClientCertificateMode? ClientCertificateMode { get; set; } - public Dictionary Sni { get; set; } } // "EndpointName": { diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 651767399240..cc353b292f9e 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -295,6 +295,11 @@ public void Load() { var listenOptions = AddressBinder.ParseAddress(endpoint.Url, out var https); + if (!https) + { + ConfigurationReader.ThrowIfContainsHttpsOnlyConfiguration(endpoint); + } + Options.ApplyEndpointDefaults(listenOptions); if (endpoint.Protocols.HasValue) diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index cd2ac796fc52..d78825eeb6dc 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -432,6 +432,88 @@ public void ConfigureEndpointDevelopmentCertificateGetsIgnoredIfPfxFileDoesNotEx } } + [Fact] + public void ConfigureEndpoint_ThrowsWhen_HttpsConfigIsDeclaredInNonHttpsEndpoints() + { + var serverOptions = CreateServerOptions(); + + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + // We shouldn't need to specify a real cert, because KestrelConfigurationLoader should check whether the endpoint requires a cert before trying to load it. + new KeyValuePair("Endpoints:End1:Certificate:Path", "fakecert.pfx"), + }).Build(); + + var ex = Assert.Throws(() => serverOptions.Configure(config).Load()); + Assert.Equal(CoreStrings.FormatEndpointHasUnusedHttpsConfig("End1", "Certificate"), ex.Message); + + config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:Certificate:Subject", "example.org"), + }).Build(); + + ex = Assert.Throws(() => serverOptions.Configure(config).Load()); + Assert.Equal(CoreStrings.FormatEndpointHasUnusedHttpsConfig("End1", "Certificate"), ex.Message); + + config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:ClientCertificateMode", ClientCertificateMode.RequireCertificate.ToString()), + }).Build(); + + ex = Assert.Throws(() => serverOptions.Configure(config).Load()); + Assert.Equal(CoreStrings.FormatEndpointHasUnusedHttpsConfig("End1", "ClientCertificateMode"), ex.Message); + + config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:SslProtocols:0", SslProtocols.Tls13.ToString()), + }).Build(); + + ex = Assert.Throws(() => serverOptions.Configure(config).Load()); + Assert.Equal(CoreStrings.FormatEndpointHasUnusedHttpsConfig("End1", "SslProtocols"), ex.Message); + + config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:Sni:Protocols", HttpProtocols.Http1.ToString()), + }).Build(); + + ex = Assert.Throws(() => serverOptions.Configure(config).Load()); + Assert.Equal(CoreStrings.FormatEndpointHasUnusedHttpsConfig("End1", "Sni"), ex.Message); + } + + [Fact] + public void ConfigureEndpoint_DoesNotThrowWhen_HttpsConfigIsDeclaredInEndpointDefaults() + { + var serverOptions = CreateServerOptions(); + + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("EndpointDefaults:ClientCertificateMode", ClientCertificateMode.RequireCertificate.ToString()), + }).Build(); + + var (_, endpointsToStart) = serverOptions.Configure(config).Reload(); + var end1 = Assert.Single(endpointsToStart); + Assert.NotNull(end1?.EndpointConfig); + Assert.Null(end1.EndpointConfig.ClientCertificateMode); + + serverOptions = CreateServerOptions(); + + config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("EndpointDefaults:SslProtocols:0", SslProtocols.Tls13.ToString()), + }).Build(); + + (_, endpointsToStart) = serverOptions.Configure(config).Reload(); + end1 = Assert.Single(endpointsToStart); + Assert.NotNull(end1?.EndpointConfig); + Assert.Null(end1.EndpointConfig.SslProtocols); + } + [ConditionalTheory] [InlineData("http1", HttpProtocols.Http1)] // [InlineData("http2", HttpProtocols.Http2)] // Not supported due to missing ALPN support. https://github.com/dotnet/corefx/issues/33016 @@ -746,6 +828,36 @@ public void EndpointConfigureSection_CanSetClientCertificateMode() Assert.True(ran2); } + + [Fact] + public void EndpointConfigureSection_CanConfigureSni() + { + var serverOptions = CreateServerOptions(); + var certPath = Path.Combine("shared", "TestCertificates", "https-ecdsa.pem"); + var keyPath = Path.Combine("shared", "TestCertificates", "https-ecdsa.key"); + + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:Protocols", HttpProtocols.None.ToString()), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:SslProtocols:0", SslProtocols.Tls13.ToString()), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:ClientCertificateMode", ClientCertificateMode.RequireCertificate.ToString()), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:Path", certPath), + new KeyValuePair("Endpoints:End1:Sni:*.example.org:Certificate:KeyPath", keyPath), + }).Build(); + + var (_, endpointsToStart) = serverOptions.Configure(config).Reload(); + var end1 = Assert.Single(endpointsToStart); + var (name, sniConfig) = Assert.Single(end1?.EndpointConfig?.Sni); + + Assert.Equal("*.example.org", name); + Assert.Equal(HttpProtocols.None, sniConfig.Protocols); + Assert.Equal(SslProtocols.Tls13, sniConfig.SslProtocols); + Assert.Equal(ClientCertificateMode.RequireCertificate, sniConfig.ClientCertificateMode); + Assert.Equal(certPath, sniConfig.Certificate.Path); + Assert.Equal(keyPath, sniConfig.Certificate.KeyPath); + } + [Fact] public void EndpointConfigureSection_CanOverrideClientCertificateModeFromConfigureHttpsDefaults() { From 052bd57c36d01d283f2560a5d6f4cf6148e0a783 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 11 Aug 2020 11:26:04 -0700 Subject: [PATCH 23/24] Fix CloneSslOptionsClonesAllProperties test on Ubuntu 16.04 --- .../Core/test/SniOptionsSelectorTests.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs index 8557a29682dc..a2c2be973b99 100644 --- a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -606,7 +606,7 @@ public void PrefersClientCertificateModeDefinedInSniConfig() new MockCertificateConfigLoader(), new HttpsConnectionAdapterOptions { - ClientCertificateMode = ClientCertificateMode.AllowCertificate + ClientCertificateMode = ClientCertificateMode.AllowCertificate }, fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -640,7 +640,7 @@ public void FallsBackToFallbackClientCertificateMode() new MockCertificateConfigLoader(), new HttpsConnectionAdapterOptions { - ClientCertificateMode = ClientCertificateMode.AllowCertificate + ClientCertificateMode = ClientCertificateMode.AllowCertificate }, fallbackHttpProtocols: HttpProtocols.Http1AndHttp2, logger: Mock.Of>()); @@ -664,8 +664,17 @@ public void CloneSslOptionsClonesAllProperties() if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - // The CipherSuitesPolicy ctor throws a PlatformNotSupportedException on Windows. - cipherSuitesPolicy = new CipherSuitesPolicy(new[] { TlsCipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 }); + try + { + // The CipherSuitesPolicy ctor throws a PlatformNotSupportedException on Windows. + cipherSuitesPolicy = new CipherSuitesPolicy(new[] { TlsCipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 }); + } + catch (PlatformNotSupportedException) + { + // The CipherSuitesPolicy ctor throws a PlatformNotSupportedException on Ubuntu 16.04. + // I don't know exactly which other distros/versions throw PNEs, but it isn't super relevant to this test, + // so let's just swallow this exception. + } } // Set options properties to non-default values to verify they're copied. @@ -760,7 +769,7 @@ public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpo private class MockConnectionContext : ConnectionContext { - public override IDuplexPipe Transport { get => throw new System.NotImplementedException(); set => throw new System.NotImplementedException(); } + public override IDuplexPipe Transport { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } public override string ConnectionId { get; set; } = "MockConnectionId"; public override IFeatureCollection Features { get; } = new FeatureCollection(); public override IDictionary Items { get; set; } = new Dictionary(); From a538c2f1b822ae2f4d763361efcf6ec2c64e72f8 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 11 Aug 2020 16:17:40 -0700 Subject: [PATCH 24/24] Address PR feedback - Hopefully this is it! --- .../Core/src/Internal/ConfigurationReader.cs | 13 +------------ .../HttpsConnectionMiddlewareTests.cs | 3 ++- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index d21d0562e8a1..06ca82a8a947 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -53,18 +53,7 @@ private IDictionary ReadCertificates() // "EndpointDefaults": { // "Protocols": "Http1AndHttp2", // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], - // "ClientCertificateMode" : "NoCertificate", - // "Sni": { - // "a.example.org": { - // "Certificate": { - // "Path": "testCertA.pfx", - // "Password": "testPassword" - // } - // }, - // "*.example.org": { - // "Protocols": "Http1", - // } - // } + // "ClientCertificateMode" : "NoCertificate" // } private EndpointDefaults ReadEndpointDefaults() { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs index 273fbe9de53b..0b65712c5efd 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsConnectionMiddlewareTests.cs @@ -165,7 +165,8 @@ void ConfigureListenOptions(ListenOptions listenOptions) } } - [Fact(Skip = "https://github.com/dotnet/runtime/issues/40402")] + [Fact] + [QuarantinedTest("https://github.com/dotnet/runtime/issues/40402")] public async Task ClientCertificateRequiredConfiguredInCallbackContinuesWhenNoCertificate() { void ConfigureListenOptions(ListenOptions listenOptions)