From fcfd708f0005f8d8361823726822acbfee46816e Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 3 Oct 2017 10:55:33 -0700 Subject: [PATCH 01/20] Add "zero config" HTTPS support using local development certificate. --- src/Kestrel.Core/Internal/AddressBinder.cs | 47 +++++++++++--- .../Internal/IDefaultHttpsProvider.cs | 10 +++ .../Internal/KestrelServerOptionsSetup.cs | 2 + src/Kestrel.Core/KestrelServer.cs | 6 +- .../ListenOptionsHttpsExtensions.cs | 2 +- src/Kestrel/DefaultHttpsProvider.cs | 63 +++++++++++++++++++ src/Kestrel/Kestrel.csproj | 2 + .../WebHostBuilderKestrelExtensions.cs | 2 + 8 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 src/Kestrel.Core/Internal/IDefaultHttpsProvider.cs create mode 100644 src/Kestrel/DefaultHttpsProvider.cs diff --git a/src/Kestrel.Core/Internal/AddressBinder.cs b/src/Kestrel.Core/Internal/AddressBinder.cs index e4d1d18b9..7b6f0bf65 100644 --- a/src/Kestrel.Core/Internal/AddressBinder.cs +++ b/src/Kestrel.Core/Internal/AddressBinder.cs @@ -19,7 +19,9 @@ internal class AddressBinder { public static async Task BindAsync(IServerAddressesFeature addresses, List listenOptions, + KestrelServerOptions serverOptions, ILogger logger, + IDefaultHttpsProvider defaultHttpsProvider, Func createBinding) { var strategy = CreateStrategy( @@ -31,7 +33,9 @@ public static async Task BindAsync(IServerAddressesFeature addresses, { Addresses = addresses.Addresses, ListenOptions = listenOptions, + ServerOptions = serverOptions, Logger = logger, + DefaultHttpsProvider = defaultHttpsProvider, CreateBinding = createBinding }; @@ -47,7 +51,9 @@ private class AddressBindContext { public ICollection Addresses { get; set; } public List ListenOptions { get; set; } + public KestrelServerOptions ServerOptions { get; set; } public ILogger Logger { get; set; } + public IDefaultHttpsProvider DefaultHttpsProvider { get; set; } public Func CreateBinding { get; set; } } @@ -120,7 +126,7 @@ private static async Task BindEndpointAsync(ListenOptions endpoint, AddressBindC context.ListenOptions.Add(endpoint); } - private static async Task BindLocalhostAsync(ServerAddress address, AddressBindContext context) + private static async Task BindLocalhostAsync(ServerAddress address, AddressBindContext context, bool https) { if (address.Port == 0) { @@ -131,7 +137,14 @@ private static async Task BindLocalhostAsync(ServerAddress address, AddressBindC try { - await BindEndpointAsync(new IPEndPoint(IPAddress.Loopback, address.Port), context).ConfigureAwait(false); + var options = new ListenOptions(new IPEndPoint(IPAddress.Loopback, address.Port)); + await BindEndpointAsync(options, context).ConfigureAwait(false); + + if (https) + { + options.KestrelServerOptions = context.ServerOptions; + context.DefaultHttpsProvider?.ConfigureHttps(options); + } } catch (Exception ex) when (!(ex is IOException)) { @@ -141,7 +154,14 @@ private static async Task BindLocalhostAsync(ServerAddress address, AddressBindC try { - await BindEndpointAsync(new IPEndPoint(IPAddress.IPv6Loopback, address.Port), context).ConfigureAwait(false); + var options = new ListenOptions(new IPEndPoint(IPAddress.IPv6Loopback, address.Port)); + await BindEndpointAsync(options, context).ConfigureAwait(false); + + if (https) + { + options.KestrelServerOptions = context.ServerOptions; + context.DefaultHttpsProvider?.ConfigureHttps(options); + } } catch (Exception ex) when (!(ex is IOException)) { @@ -162,10 +182,11 @@ private static async Task BindLocalhostAsync(ServerAddress address, AddressBindC private static async Task BindAddressAsync(string address, AddressBindContext context) { var parsedAddress = ServerAddress.FromUrl(address); + var https = false; if (parsedAddress.Scheme.Equals("https", StringComparison.OrdinalIgnoreCase)) { - throw new InvalidOperationException(CoreStrings.FormatConfigureHttpsFromMethodCall($"{nameof(KestrelServerOptions)}.{nameof(KestrelServerOptions.Listen)}()")); + https = true; } else if (!parsedAddress.Scheme.Equals("http", StringComparison.OrdinalIgnoreCase)) { @@ -177,20 +198,20 @@ private static async Task BindAddressAsync(string address, AddressBindContext co throw new InvalidOperationException(CoreStrings.FormatConfigurePathBaseFromMethodCall($"{nameof(IApplicationBuilder)}.UsePathBase()")); } + ListenOptions options = null; if (parsedAddress.IsUnixPipe) { - var endPoint = new ListenOptions(parsedAddress.UnixPipePath); - await BindEndpointAsync(endPoint, context).ConfigureAwait(false); - context.Addresses.Add(endPoint.GetDisplayName()); + options = new ListenOptions(parsedAddress.UnixPipePath); + await BindEndpointAsync(options, context).ConfigureAwait(false); + context.Addresses.Add(options.GetDisplayName()); } else if (string.Equals(parsedAddress.Host, "localhost", StringComparison.OrdinalIgnoreCase)) { // "localhost" for both IPv4 and IPv6 can't be represented as an IPEndPoint. - await BindLocalhostAsync(parsedAddress, context).ConfigureAwait(false); + await BindLocalhostAsync(parsedAddress, context, https).ConfigureAwait(false); } else { - ListenOptions options; if (TryCreateIPEndPoint(parsedAddress, out var endpoint)) { options = new ListenOptions(endpoint); @@ -216,6 +237,12 @@ private static async Task BindAddressAsync(string address, AddressBindContext co context.Addresses.Add(options.GetDisplayName()); } + + if (https && options != null) + { + options.KestrelServerOptions = context.ServerOptions; + context.DefaultHttpsProvider?.ConfigureHttps(options); + } } private interface IStrategy @@ -229,7 +256,7 @@ public async Task BindAsync(AddressBindContext context) { context.Logger.LogDebug(CoreStrings.BindingToDefaultAddress, Constants.DefaultServerAddress); - await BindLocalhostAsync(ServerAddress.FromUrl(Constants.DefaultServerAddress), context).ConfigureAwait(false); + await BindLocalhostAsync(ServerAddress.FromUrl(Constants.DefaultServerAddress), context, https: false).ConfigureAwait(false); } } diff --git a/src/Kestrel.Core/Internal/IDefaultHttpsProvider.cs b/src/Kestrel.Core/Internal/IDefaultHttpsProvider.cs new file mode 100644 index 000000000..3ed67b0cd --- /dev/null +++ b/src/Kestrel.Core/Internal/IDefaultHttpsProvider.cs @@ -0,0 +1,10 @@ +// 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 +{ + public interface IDefaultHttpsProvider + { + void ConfigureHttps(ListenOptions listenOptions); + } +} diff --git a/src/Kestrel.Core/Internal/KestrelServerOptionsSetup.cs b/src/Kestrel.Core/Internal/KestrelServerOptionsSetup.cs index 18c96e203..4d7a95ca5 100644 --- a/src/Kestrel.Core/Internal/KestrelServerOptionsSetup.cs +++ b/src/Kestrel.Core/Internal/KestrelServerOptionsSetup.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index fac999d90..3bc5909dd 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -22,6 +23,7 @@ public class KestrelServer : IServer private readonly List _transports = new List(); private readonly Heartbeat _heartbeat; private readonly IServerAddressesFeature _serverAddresses; + private readonly IDefaultHttpsProvider _defaultHttpsProvider; private readonly ITransportFactory _transportFactory; private bool _hasStarted; @@ -52,6 +54,8 @@ internal KestrelServer(ITransportFactory transportFactory, ServiceContext servic Features = new FeatureCollection(); _serverAddresses = new ServerAddressesFeature(); Features.Set(_serverAddresses); + + _defaultHttpsProvider = serviceContext.ServerOptions.ApplicationServices.GetService(); } private static ServiceContext CreateServiceContext(IOptions options, ILoggerFactory loggerFactory) @@ -152,7 +156,7 @@ async Task OnBind(ListenOptions endpoint) await transport.BindAsync().ConfigureAwait(false); } - await AddressBinder.BindAsync(_serverAddresses, Options.ListenOptions, Trace, OnBind).ConfigureAwait(false); + await AddressBinder.BindAsync(_serverAddresses, Options.ListenOptions, Options, Trace, _defaultHttpsProvider, OnBind).ConfigureAwait(false); } catch (Exception ex) { diff --git a/src/Kestrel.Https/ListenOptionsHttpsExtensions.cs b/src/Kestrel.Https/ListenOptionsHttpsExtensions.cs index 7cf8ab1b9..44463ea10 100644 --- a/src/Kestrel.Https/ListenOptionsHttpsExtensions.cs +++ b/src/Kestrel.Https/ListenOptionsHttpsExtensions.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Hosting { /// - /// Extension methods fro that configure Kestrel to use HTTPS for a given endpoint. + /// Extension methods for that configure Kestrel to use HTTPS for a given endpoint. /// public static class ListenOptionsHttpsExtensions { diff --git a/src/Kestrel/DefaultHttpsProvider.cs b/src/Kestrel/DefaultHttpsProvider.cs new file mode 100644 index 000000000..4db40c2d1 --- /dev/null +++ b/src/Kestrel/DefaultHttpsProvider.cs @@ -0,0 +1,63 @@ +// 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.Linq; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; + +namespace Microsoft.AspNetCore.Server.Kestrel +{ + internal class DefaultHttpsProvider : IDefaultHttpsProvider + { + private const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.1"; + + public void ConfigureHttps(ListenOptions listenOptions) + { + listenOptions.UseHttps(FindDevelopmentCertificate()); + } + + private static X509Certificate2 FindDevelopmentCertificate() + { + using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) + { + store.Open(OpenFlags.ReadOnly); + + var certificates = store.Certificates.OfType(); + var certificate = certificates + .FirstOrDefault(c => HasOid(c, AspNetHttpsOid)); + + if (certificate == null) + { + throw new Exception(); + } + + DisposeCertificates(certificates.Except(new[] { certificate })); + + return certificate; + } + } + + private static bool HasOid(X509Certificate2 certificate, string oid) => + certificate.Extensions + .OfType() + .Any(e => string.Equals(oid, e.Oid.Value, StringComparison.Ordinal)); + + private static void DisposeCertificates(IEnumerable certificates) + { + foreach (var certificate in certificates) + { + try + { + certificate.Dispose(); + } + catch + { + } + } + } + } +} diff --git a/src/Kestrel/Kestrel.csproj b/src/Kestrel/Kestrel.csproj index 1376f1e89..568f62051 100644 --- a/src/Kestrel/Kestrel.csproj +++ b/src/Kestrel/Kestrel.csproj @@ -16,6 +16,8 @@ + + diff --git a/src/Kestrel/WebHostBuilderKestrelExtensions.cs b/src/Kestrel/WebHostBuilderKestrelExtensions.cs index 5ed947e96..24d821459 100644 --- a/src/Kestrel/WebHostBuilderKestrelExtensions.cs +++ b/src/Kestrel/WebHostBuilderKestrelExtensions.cs @@ -3,6 +3,7 @@ using System; using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; @@ -33,6 +34,7 @@ public static IWebHostBuilder UseKestrel(this IWebHostBuilder hostBuilder) services.AddTransient, KestrelServerOptionsSetup>(); services.AddSingleton(); + services.AddSingleton(); }); } From 46c3c112de5263f0599911877d246680af536b52 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 3 Oct 2017 11:09:18 -0700 Subject: [PATCH 02/20] Fix exception type and add message. --- src/Kestrel/DefaultHttpsProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Kestrel/DefaultHttpsProvider.cs b/src/Kestrel/DefaultHttpsProvider.cs index 4db40c2d1..9b3d31705 100644 --- a/src/Kestrel/DefaultHttpsProvider.cs +++ b/src/Kestrel/DefaultHttpsProvider.cs @@ -32,7 +32,7 @@ private static X509Certificate2 FindDevelopmentCertificate() if (certificate == null) { - throw new Exception(); + throw new InvalidOperationException("Unable to find ASP.NET Core development certificate."); } DisposeCertificates(certificates.Except(new[] { certificate })); From eccd1eaaead78377311dd6fbf54d0b1c56d3a66d Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 3 Oct 2017 11:51:18 -0700 Subject: [PATCH 03/20] Fix test build. --- test/Kestrel.Core.Tests/AddressBinderTests.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/Kestrel.Core.Tests/AddressBinderTests.cs b/test/Kestrel.Core.Tests/AddressBinderTests.cs index 2c957e603..c9dbbc540 100644 --- a/test/Kestrel.Core.Tests/AddressBinderTests.cs +++ b/test/Kestrel.Core.Tests/AddressBinderTests.cs @@ -8,9 +8,9 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Protocols; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; -using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging.Abstractions; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests @@ -60,7 +60,9 @@ public async Task DefaultsToIPv6AnyOnInvalidIPAddress(string host) var tcs = new TaskCompletionSource(); await AddressBinder.BindAsync(addresses, options, + new KestrelServerOptions(), NullLogger.Instance, + Mock.Of(), endpoint => { tcs.TrySetResult(endpoint); @@ -80,7 +82,9 @@ public async Task WrapsAddressInUseExceptionAsIOException() await Assert.ThrowsAsync(() => AddressBinder.BindAsync(addresses, options, + new KestrelServerOptions(), NullLogger.Instance, + Mock.Of(), endpoint => throw new AddressInUseException("already in use"))); } @@ -100,7 +104,9 @@ public async Task FallbackToIPv4WhenIPv6AnyBindFails(string address) await AddressBinder.BindAsync(addresses, options, + new KestrelServerOptions(), logger, + Mock.Of(), endpoint => { if (endpoint.IPEndPoint.Address == IPAddress.IPv6Any) From 3703d0de266e3bca1e20f3d91db8b0438b07997d Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 3 Oct 2017 15:23:04 -0700 Subject: [PATCH 04/20] More cert validation. --- src/Kestrel/DefaultHttpsProvider.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Kestrel/DefaultHttpsProvider.cs b/src/Kestrel/DefaultHttpsProvider.cs index 9b3d31705..4bae67a86 100644 --- a/src/Kestrel/DefaultHttpsProvider.cs +++ b/src/Kestrel/DefaultHttpsProvider.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; @@ -22,13 +23,15 @@ public void ConfigureHttps(ListenOptions listenOptions) private static X509Certificate2 FindDevelopmentCertificate() { + // TODO: replace this with call to CertificateManager.FindCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) + // when that becomes available. using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) { store.Open(OpenFlags.ReadOnly); var certificates = store.Certificates.OfType(); var certificate = certificates - .FirstOrDefault(c => HasOid(c, AspNetHttpsOid)); + .FirstOrDefault(c => HasOid(c, AspNetHttpsOid) && !IsExpired(c) /*&& HasPrivateKey(c)*/); if (certificate == null) { @@ -46,6 +49,16 @@ private static bool HasOid(X509Certificate2 certificate, string oid) => .OfType() .Any(e => string.Equals(oid, e.Oid.Value, StringComparison.Ordinal)); + private static bool IsExpired(X509Certificate2 certificate) + { + var now = DateTimeOffset.Now; + return now < certificate.NotBefore || now > certificate.NotAfter; + } + + private static bool HasPrivateKey(X509Certificate2 certificate) + => (certificate.GetRSAPrivateKey() is RSACryptoServiceProvider rsaPrivateKey && rsaPrivateKey.CspKeyContainerInfo.Exportable)/* || + (certificate.GetRSAPrivateKey() is RSACng cngPrivateKey && cngPrivateKey.CspKeyContainerInfo.Exportable)*/; + private static void DisposeCertificates(IEnumerable certificates) { foreach (var certificate in certificates) From ebb2389a3e17ba4253f4d1db55119d3f25487321 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 3 Oct 2017 15:23:23 -0700 Subject: [PATCH 05/20] Inject IDefaultHttpsProvider in KestrelServer. --- src/Kestrel.Core/Internal/AddressBinder.cs | 3 +-- src/Kestrel.Core/KestrelServer.cs | 12 +++++------- src/Kestrel.Core/breakingchanges.netcore.json | 7 +++++++ test/Kestrel.Core.Tests/AddressBinderTests.cs | 11 ++++------- test/Kestrel.Core.Tests/KestrelServerTests.cs | 11 ++++++----- .../TestHelpers/TestServer.cs | 4 +++- 6 files changed, 26 insertions(+), 22 deletions(-) create mode 100644 src/Kestrel.Core/breakingchanges.netcore.json diff --git a/src/Kestrel.Core/Internal/AddressBinder.cs b/src/Kestrel.Core/Internal/AddressBinder.cs index 7b6f0bf65..d1e443bad 100644 --- a/src/Kestrel.Core/Internal/AddressBinder.cs +++ b/src/Kestrel.Core/Internal/AddressBinder.cs @@ -19,7 +19,6 @@ internal class AddressBinder { public static async Task BindAsync(IServerAddressesFeature addresses, List listenOptions, - KestrelServerOptions serverOptions, ILogger logger, IDefaultHttpsProvider defaultHttpsProvider, Func createBinding) @@ -33,7 +32,7 @@ public static async Task BindAsync(IServerAddressesFeature addresses, { Addresses = addresses.Addresses, ListenOptions = listenOptions, - ServerOptions = serverOptions, + ServerOptions = listenOptions.FirstOrDefault()?.KestrelServerOptions, Logger = logger, DefaultHttpsProvider = defaultHttpsProvider, CreateBinding = createBinding diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index 3bc5909dd..042b10edb 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -12,7 +12,6 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -30,13 +29,13 @@ public class KestrelServer : IServer private int _stopping; private readonly TaskCompletionSource _stoppedTcs = new TaskCompletionSource(); - public KestrelServer(IOptions options, ITransportFactory transportFactory, ILoggerFactory loggerFactory) - : this(transportFactory, CreateServiceContext(options, loggerFactory)) + public KestrelServer(IOptions options, ITransportFactory transportFactory, ILoggerFactory loggerFactory, IDefaultHttpsProvider defaultHttpsProvider) + : this(transportFactory, defaultHttpsProvider, CreateServiceContext(options, loggerFactory)) { } // For testing - internal KestrelServer(ITransportFactory transportFactory, ServiceContext serviceContext) + internal KestrelServer(ITransportFactory transportFactory, IDefaultHttpsProvider defaultHttpsProvider, ServiceContext serviceContext) { if (transportFactory == null) { @@ -44,6 +43,7 @@ internal KestrelServer(ITransportFactory transportFactory, ServiceContext servic } _transportFactory = transportFactory; + _defaultHttpsProvider = defaultHttpsProvider; ServiceContext = serviceContext; var httpHeartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); @@ -54,8 +54,6 @@ internal KestrelServer(ITransportFactory transportFactory, ServiceContext servic Features = new FeatureCollection(); _serverAddresses = new ServerAddressesFeature(); Features.Set(_serverAddresses); - - _defaultHttpsProvider = serviceContext.ServerOptions.ApplicationServices.GetService(); } private static ServiceContext CreateServiceContext(IOptions options, ILoggerFactory loggerFactory) @@ -156,7 +154,7 @@ async Task OnBind(ListenOptions endpoint) await transport.BindAsync().ConfigureAwait(false); } - await AddressBinder.BindAsync(_serverAddresses, Options.ListenOptions, Options, Trace, _defaultHttpsProvider, OnBind).ConfigureAwait(false); + await AddressBinder.BindAsync(_serverAddresses, Options.ListenOptions, Trace, _defaultHttpsProvider, OnBind).ConfigureAwait(false); } catch (Exception ex) { diff --git a/src/Kestrel.Core/breakingchanges.netcore.json b/src/Kestrel.Core/breakingchanges.netcore.json new file mode 100644 index 000000000..cec50480c --- /dev/null +++ b/src/Kestrel.Core/breakingchanges.netcore.json @@ -0,0 +1,7 @@ +[ + { + "TypeId": "public class Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer : Microsoft.AspNetCore.Hosting.Server.IServer", + "MemberId": "public .ctor(Microsoft.Extensions.Options.IOptions options, Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal.ITransportFactory transportFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory)", + "Kind": "Removal" + } +] \ No newline at end of file diff --git a/test/Kestrel.Core.Tests/AddressBinderTests.cs b/test/Kestrel.Core.Tests/AddressBinderTests.cs index c9dbbc540..5aca9a809 100644 --- a/test/Kestrel.Core.Tests/AddressBinderTests.cs +++ b/test/Kestrel.Core.Tests/AddressBinderTests.cs @@ -60,7 +60,6 @@ public async Task DefaultsToIPv6AnyOnInvalidIPAddress(string host) var tcs = new TaskCompletionSource(); await AddressBinder.BindAsync(addresses, options, - new KestrelServerOptions(), NullLogger.Instance, Mock.Of(), endpoint => @@ -81,11 +80,10 @@ public async Task WrapsAddressInUseExceptionAsIOException() await Assert.ThrowsAsync(() => AddressBinder.BindAsync(addresses, - options, - new KestrelServerOptions(), - NullLogger.Instance, - Mock.Of(), - endpoint => throw new AddressInUseException("already in use"))); + options, + NullLogger.Instance, + Mock.Of(), + endpoint => throw new AddressInUseException("already in use"))); } [Theory] @@ -104,7 +102,6 @@ public async Task FallbackToIPv4WhenIPv6AnyBindFails(string address) await AddressBinder.BindAsync(addresses, options, - new KestrelServerOptions(), logger, Mock.Of(), endpoint => diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index c6ae3b5a7..911c96094 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging; @@ -151,7 +152,7 @@ public void StartWithMaxRequestBufferSizeLessThanMaxRequestHeadersTotalSizeThrow public void LoggerCategoryNameIsKestrelServerNamespace() { var mockLoggerFactory = new Mock(); - new KestrelServer(Options.Create(null), Mock.Of(), mockLoggerFactory.Object); + new KestrelServer(Options.Create(null), Mock.Of(), mockLoggerFactory.Object, Mock.Of()); mockLoggerFactory.Verify(factory => factory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel")); } @@ -159,7 +160,7 @@ public void LoggerCategoryNameIsKestrelServerNamespace() public void StartWithNoTransportFactoryThrows() { var exception = Assert.Throws(() => - new KestrelServer(Options.Create(null), null, Mock.Of())); + new KestrelServer(Options.Create(null), null, Mock.Of(), Mock.Of())); Assert.Equal("transportFactory", exception.ParamName); } @@ -194,7 +195,7 @@ public async Task StopAsyncCallsCompleteWhenFirstCallCompletes() .Setup(transportFactory => transportFactory.Create(It.IsAny(), It.IsAny())) .Returns(mockTransport.Object); - var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of()); + var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of(), Mock.Of()); await server.StartAsync(new DummyApplication(), CancellationToken.None); var stopTask1 = server.StopAsync(default(CancellationToken)); @@ -248,7 +249,7 @@ public async Task StopAsyncCallsCompleteWithThrownException() .Setup(transportFactory => transportFactory.Create(It.IsAny(), It.IsAny())) .Returns(mockTransport.Object); - var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of()); + var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of(), Mock.Of()); await server.StartAsync(new DummyApplication(), CancellationToken.None); var stopTask1 = server.StopAsync(default(CancellationToken)); @@ -271,7 +272,7 @@ public async Task StopAsyncCallsCompleteWithThrownException() private static KestrelServer CreateServer(KestrelServerOptions options, ILogger testLogger) { - return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) })); + return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) }), Mock.Of()); } private static void StartDummyApplication(IServer server) diff --git a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs index f0972bb4f..9054da5a2 100644 --- a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs +++ b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs @@ -11,10 +11,12 @@ using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Moq; namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { @@ -66,7 +68,7 @@ public TestServer(RequestDelegate app, TestServiceContext context, ListenOptions { c.Configure(context.ServerOptions); } - return new KestrelServer(sp.GetRequiredService(), context); + return new KestrelServer(sp.GetRequiredService(), sp.GetRequiredService(), context); }); configureServices(services); From fcd542a13800feedd02d6683b58d02fc3a9f1690 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 3 Oct 2017 16:29:14 -0700 Subject: [PATCH 06/20] Simpler private key check. --- src/Kestrel/DefaultHttpsProvider.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Kestrel/DefaultHttpsProvider.cs b/src/Kestrel/DefaultHttpsProvider.cs index 4bae67a86..700106313 100644 --- a/src/Kestrel/DefaultHttpsProvider.cs +++ b/src/Kestrel/DefaultHttpsProvider.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; @@ -56,8 +55,7 @@ private static bool IsExpired(X509Certificate2 certificate) } private static bool HasPrivateKey(X509Certificate2 certificate) - => (certificate.GetRSAPrivateKey() is RSACryptoServiceProvider rsaPrivateKey && rsaPrivateKey.CspKeyContainerInfo.Exportable)/* || - (certificate.GetRSAPrivateKey() is RSACng cngPrivateKey && cngPrivateKey.CspKeyContainerInfo.Exportable)*/; + => certificate.GetRSAPrivateKey() != null; private static void DisposeCertificates(IEnumerable certificates) { From fbd6a705e91bb6368050e63f508d9314ffa2bf20 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 3 Oct 2017 16:29:18 -0700 Subject: [PATCH 07/20] Update tests. --- test/Kestrel.Core.Tests/KestrelServerTests.cs | 18 ++++++++++-------- .../AddressRegistrationTests.cs | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 911c96094..c5b930e12 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -38,20 +38,17 @@ public void StartWithInvalidAddressThrows() } [Fact] - public void StartWithHttpsAddressThrows() + public void StartWithHttpsAddressConfiguresHttpsEndpoints() { - var testLogger = new TestApplicationErrorLogger { ThrowOnCriticalErrors = false }; + var mockDefaultHttpsProvider = new Mock(); - using (var server = CreateServer(new KestrelServerOptions(), testLogger)) + using (var server = CreateServer(new KestrelServerOptions(), mockDefaultHttpsProvider.Object)) { server.Features.Get().Addresses.Add("https://127.0.0.1:0"); - var exception = Assert.Throws(() => StartDummyApplication(server)); + StartDummyApplication(server); - Assert.Equal( - $"HTTPS endpoints can only be configured using {nameof(KestrelServerOptions)}.{nameof(KestrelServerOptions.Listen)}().", - exception.Message); - Assert.Equal(1, testLogger.CriticalErrorsLogged); + mockDefaultHttpsProvider.Verify(provider => provider.ConfigureHttps(It.IsAny()), Times.Once); } } @@ -275,6 +272,11 @@ private static KestrelServer CreateServer(KestrelServerOptions options, ILogger return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) }), Mock.Of()); } + private static KestrelServer CreateServer(KestrelServerOptions options, IDefaultHttpsProvider defaultHttpsProvider) + { + return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(Mock.Of()) }), defaultHttpsProvider); + } + private static void StartDummyApplication(IServer server) { server.StartAsync(new DummyApplication(context => Task.CompletedTask), CancellationToken.None).GetAwaiter().GetResult(); diff --git a/test/Kestrel.FunctionalTests/AddressRegistrationTests.cs b/test/Kestrel.FunctionalTests/AddressRegistrationTests.cs index 7b9c9bdcd..446a201f7 100644 --- a/test/Kestrel.FunctionalTests/AddressRegistrationTests.cs +++ b/test/Kestrel.FunctionalTests/AddressRegistrationTests.cs @@ -511,8 +511,8 @@ public void ThrowsWhenBindingLocalhostToDynamicPort() } [Theory] - [InlineData("https://localhost")] [InlineData("ftp://localhost")] + [InlineData("ssh://localhost")] public void ThrowsForUnsupportedAddressFromHosting(string addr) { var hostBuilder = TransportSelector.GetWebHostBuilder() From e25a2b1a6a89296ffad71dbd27cae6364ed9de5e Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 4 Oct 2017 11:30:37 -0700 Subject: [PATCH 08/20] Add new KestrelServer ctor for compat. --- src/Kestrel.Core/KestrelServer.cs | 11 ++++++++--- src/Kestrel.Core/breakingchanges.netcore.json | 7 ------- test/Kestrel.Core.Tests/KestrelServerTests.cs | 10 +++++----- .../Kestrel.FunctionalTests/TestHelpers/TestServer.cs | 2 +- 4 files changed, 14 insertions(+), 16 deletions(-) delete mode 100644 src/Kestrel.Core/breakingchanges.netcore.json diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index 042b10edb..e6dcea10a 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -29,13 +29,19 @@ public class KestrelServer : IServer private int _stopping; private readonly TaskCompletionSource _stoppedTcs = new TaskCompletionSource(); + public KestrelServer(IOptions options, ITransportFactory transportFactory, ILoggerFactory loggerFactory) + : this(transportFactory, CreateServiceContext(options, loggerFactory)) + { + } + public KestrelServer(IOptions options, ITransportFactory transportFactory, ILoggerFactory loggerFactory, IDefaultHttpsProvider defaultHttpsProvider) - : this(transportFactory, defaultHttpsProvider, CreateServiceContext(options, loggerFactory)) + : this(transportFactory, CreateServiceContext(options, loggerFactory)) { + _defaultHttpsProvider = defaultHttpsProvider; } // For testing - internal KestrelServer(ITransportFactory transportFactory, IDefaultHttpsProvider defaultHttpsProvider, ServiceContext serviceContext) + internal KestrelServer(ITransportFactory transportFactory, ServiceContext serviceContext) { if (transportFactory == null) { @@ -43,7 +49,6 @@ internal KestrelServer(ITransportFactory transportFactory, IDefaultHttpsProvider } _transportFactory = transportFactory; - _defaultHttpsProvider = defaultHttpsProvider; ServiceContext = serviceContext; var httpHeartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); diff --git a/src/Kestrel.Core/breakingchanges.netcore.json b/src/Kestrel.Core/breakingchanges.netcore.json deleted file mode 100644 index cec50480c..000000000 --- a/src/Kestrel.Core/breakingchanges.netcore.json +++ /dev/null @@ -1,7 +0,0 @@ -[ - { - "TypeId": "public class Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer : Microsoft.AspNetCore.Hosting.Server.IServer", - "MemberId": "public .ctor(Microsoft.Extensions.Options.IOptions options, Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal.ITransportFactory transportFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory)", - "Kind": "Removal" - } -] \ No newline at end of file diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index c5b930e12..7e63e48eb 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -149,7 +149,7 @@ public void StartWithMaxRequestBufferSizeLessThanMaxRequestHeadersTotalSizeThrow public void LoggerCategoryNameIsKestrelServerNamespace() { var mockLoggerFactory = new Mock(); - new KestrelServer(Options.Create(null), Mock.Of(), mockLoggerFactory.Object, Mock.Of()); + new KestrelServer(Options.Create(null), Mock.Of(), mockLoggerFactory.Object); mockLoggerFactory.Verify(factory => factory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel")); } @@ -157,7 +157,7 @@ public void LoggerCategoryNameIsKestrelServerNamespace() public void StartWithNoTransportFactoryThrows() { var exception = Assert.Throws(() => - new KestrelServer(Options.Create(null), null, Mock.Of(), Mock.Of())); + new KestrelServer(Options.Create(null), null, Mock.Of())); Assert.Equal("transportFactory", exception.ParamName); } @@ -192,7 +192,7 @@ public async Task StopAsyncCallsCompleteWhenFirstCallCompletes() .Setup(transportFactory => transportFactory.Create(It.IsAny(), It.IsAny())) .Returns(mockTransport.Object); - var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of(), Mock.Of()); + var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of()); await server.StartAsync(new DummyApplication(), CancellationToken.None); var stopTask1 = server.StopAsync(default(CancellationToken)); @@ -246,7 +246,7 @@ public async Task StopAsyncCallsCompleteWithThrownException() .Setup(transportFactory => transportFactory.Create(It.IsAny(), It.IsAny())) .Returns(mockTransport.Object); - var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of(), Mock.Of()); + var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of()); await server.StartAsync(new DummyApplication(), CancellationToken.None); var stopTask1 = server.StopAsync(default(CancellationToken)); @@ -269,7 +269,7 @@ public async Task StopAsyncCallsCompleteWithThrownException() private static KestrelServer CreateServer(KestrelServerOptions options, ILogger testLogger) { - return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) }), Mock.Of()); + return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) })); } private static KestrelServer CreateServer(KestrelServerOptions options, IDefaultHttpsProvider defaultHttpsProvider) diff --git a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs index 9054da5a2..a6162738c 100644 --- a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs +++ b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs @@ -68,7 +68,7 @@ public TestServer(RequestDelegate app, TestServiceContext context, ListenOptions { c.Configure(context.ServerOptions); } - return new KestrelServer(sp.GetRequiredService(), sp.GetRequiredService(), context); + return new KestrelServer(sp.GetRequiredService(), context); }); configureServices(services); From becf626214ed3e89869408ff64325b8b8c26ac79 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 4 Oct 2017 11:39:37 -0700 Subject: [PATCH 09/20] Pass KestrelServerOptions instead of list of ListenOptions to AddressBinder. --- src/Kestrel.Core/Internal/AddressBinder.cs | 5 +++-- src/Kestrel.Core/KestrelServer.cs | 2 +- test/Kestrel.Core.Tests/AddressBinderTests.cs | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Kestrel.Core/Internal/AddressBinder.cs b/src/Kestrel.Core/Internal/AddressBinder.cs index d1e443bad..24ab8a514 100644 --- a/src/Kestrel.Core/Internal/AddressBinder.cs +++ b/src/Kestrel.Core/Internal/AddressBinder.cs @@ -18,11 +18,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal internal class AddressBinder { public static async Task BindAsync(IServerAddressesFeature addresses, - List listenOptions, + KestrelServerOptions serverOptions, ILogger logger, IDefaultHttpsProvider defaultHttpsProvider, Func createBinding) { + var listenOptions = serverOptions.ListenOptions; var strategy = CreateStrategy( listenOptions.ToArray(), addresses.Addresses.ToArray(), @@ -32,7 +33,7 @@ public static async Task BindAsync(IServerAddressesFeature addresses, { Addresses = addresses.Addresses, ListenOptions = listenOptions, - ServerOptions = listenOptions.FirstOrDefault()?.KestrelServerOptions, + ServerOptions = serverOptions, Logger = logger, DefaultHttpsProvider = defaultHttpsProvider, CreateBinding = createBinding diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index e6dcea10a..81536a44a 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -159,7 +159,7 @@ async Task OnBind(ListenOptions endpoint) await transport.BindAsync().ConfigureAwait(false); } - await AddressBinder.BindAsync(_serverAddresses, Options.ListenOptions, Trace, _defaultHttpsProvider, OnBind).ConfigureAwait(false); + await AddressBinder.BindAsync(_serverAddresses, Options, Trace, _defaultHttpsProvider, OnBind).ConfigureAwait(false); } catch (Exception ex) { diff --git a/test/Kestrel.Core.Tests/AddressBinderTests.cs b/test/Kestrel.Core.Tests/AddressBinderTests.cs index 5aca9a809..d42a598e9 100644 --- a/test/Kestrel.Core.Tests/AddressBinderTests.cs +++ b/test/Kestrel.Core.Tests/AddressBinderTests.cs @@ -55,7 +55,7 @@ public async Task DefaultsToIPv6AnyOnInvalidIPAddress(string host) { var addresses = new ServerAddressesFeature(); addresses.Addresses.Add($"http://{host}"); - var options = new List(); + var options = new KestrelServerOptions(); var tcs = new TaskCompletionSource(); await AddressBinder.BindAsync(addresses, @@ -76,7 +76,7 @@ public async Task WrapsAddressInUseExceptionAsIOException() { var addresses = new ServerAddressesFeature(); addresses.Addresses.Add("http://localhost:5000"); - var options = new List(); + var options = new KestrelServerOptions(); await Assert.ThrowsAsync(() => AddressBinder.BindAsync(addresses, @@ -95,7 +95,7 @@ public async Task FallbackToIPv4WhenIPv6AnyBindFails(string address) var logger = new MockLogger(); var addresses = new ServerAddressesFeature(); addresses.Addresses.Add(address); - var options = new List(); + var options = new KestrelServerOptions(); var ipV6Attempt = false; var ipV4Attempt = false; From 8e5b021662b45ba3e2c2ceff19f7191c07152f35 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 4 Oct 2017 12:52:56 -0700 Subject: [PATCH 10/20] Uncomment private key check. --- src/Kestrel/DefaultHttpsProvider.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Kestrel/DefaultHttpsProvider.cs b/src/Kestrel/DefaultHttpsProvider.cs index 700106313..a6feca6e2 100644 --- a/src/Kestrel/DefaultHttpsProvider.cs +++ b/src/Kestrel/DefaultHttpsProvider.cs @@ -22,7 +22,8 @@ public void ConfigureHttps(ListenOptions listenOptions) private static X509Certificate2 FindDevelopmentCertificate() { - // TODO: replace this with call to CertificateManager.FindCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) + // TODO: replace this with call to + // CertificateManager.FindCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) // when that becomes available. using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) { @@ -30,7 +31,7 @@ private static X509Certificate2 FindDevelopmentCertificate() var certificates = store.Certificates.OfType(); var certificate = certificates - .FirstOrDefault(c => HasOid(c, AspNetHttpsOid) && !IsExpired(c) /*&& HasPrivateKey(c)*/); + .FirstOrDefault(c => HasOid(c, AspNetHttpsOid) && !IsExpired(c) && HasPrivateKey(c)); if (certificate == null) { From 6df66736a2708b192a4bd0cb33f642f611f6bcfb Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 4 Oct 2017 12:58:52 -0700 Subject: [PATCH 11/20] Don't null-condition access to AddressBindContext.DefaultHttpsProvider. --- src/Kestrel.Core/Internal/AddressBinder.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Kestrel.Core/Internal/AddressBinder.cs b/src/Kestrel.Core/Internal/AddressBinder.cs index 24ab8a514..c60ce7641 100644 --- a/src/Kestrel.Core/Internal/AddressBinder.cs +++ b/src/Kestrel.Core/Internal/AddressBinder.cs @@ -143,7 +143,7 @@ private static async Task BindLocalhostAsync(ServerAddress address, AddressBindC if (https) { options.KestrelServerOptions = context.ServerOptions; - context.DefaultHttpsProvider?.ConfigureHttps(options); + context.DefaultHttpsProvider.ConfigureHttps(options); } } catch (Exception ex) when (!(ex is IOException)) @@ -160,7 +160,7 @@ private static async Task BindLocalhostAsync(ServerAddress address, AddressBindC if (https) { options.KestrelServerOptions = context.ServerOptions; - context.DefaultHttpsProvider?.ConfigureHttps(options); + context.DefaultHttpsProvider.ConfigureHttps(options); } } catch (Exception ex) when (!(ex is IOException)) @@ -241,7 +241,7 @@ private static async Task BindAddressAsync(string address, AddressBindContext co if (https && options != null) { options.KestrelServerOptions = context.ServerOptions; - context.DefaultHttpsProvider?.ConfigureHttps(options); + context.DefaultHttpsProvider.ConfigureHttps(options); } } From ea1c66c9183473f068c8695a95388a8712d2f4bb Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Thu, 19 Oct 2017 11:07:28 -0700 Subject: [PATCH 12/20] add some additional error handling --- src/Kestrel.Core/CoreStrings.resx | 59 +++++++------- src/Kestrel.Core/Internal/AddressBinder.cs | 18 ++++- .../Properties/CoreStrings.Designer.cs | 14 ++++ src/Kestrel/Internal/DefaultHttpsProvider.cs | 77 +++++++++++++++++++ test/Kestrel.Core.Tests/KestrelServerTests.cs | 43 +++++++++++ 5 files changed, 181 insertions(+), 30 deletions(-) create mode 100644 src/Kestrel/Internal/DefaultHttpsProvider.cs diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index fdbebb8aa..158bc6cc5 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -1,17 +1,17 @@  - @@ -462,4 +462,7 @@ Request headers contain connection-specific header field. - + + Unable to configure default https bindings because no IDefaultHttpsProvider service was provided. + + \ No newline at end of file diff --git a/src/Kestrel.Core/Internal/AddressBinder.cs b/src/Kestrel.Core/Internal/AddressBinder.cs index c60ce7641..d805e63b6 100644 --- a/src/Kestrel.Core/Internal/AddressBinder.cs +++ b/src/Kestrel.Core/Internal/AddressBinder.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -35,7 +35,7 @@ public static async Task BindAsync(IServerAddressesFeature addresses, ListenOptions = listenOptions, ServerOptions = serverOptions, Logger = logger, - DefaultHttpsProvider = defaultHttpsProvider, + DefaultHttpsProvider = defaultHttpsProvider ?? UnconfiguredDefaultHttpsProvider.Instance, CreateBinding = createBinding }; @@ -332,5 +332,19 @@ public virtual async Task BindAsync(AddressBindContext context) } } } + + private class UnconfiguredDefaultHttpsProvider : IDefaultHttpsProvider + { + public static readonly UnconfiguredDefaultHttpsProvider Instance = new UnconfiguredDefaultHttpsProvider(); + + private UnconfiguredDefaultHttpsProvider() + { + } + + public void ConfigureHttps(ListenOptions listenOptions) + { + throw new InvalidOperationException(CoreStrings.UnableToConfigureHttpsBindings); + } + } } } diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 346b18a96..83d81019f 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -1620,6 +1620,20 @@ internal static string Http2ErrorConnectionSpecificHeaderField internal static string FormatHttp2ErrorConnectionSpecificHeaderField() => GetString("Http2ErrorConnectionSpecificHeaderField"); + /// + /// Unable to configure default https bindings, no IDefaultHttpsProvider service was provided. + /// + internal static string UnableToConfigureHttpsBindings + { + get => GetString("UnableToConfigureHttpsBindings"); + } + + /// + /// Unable to configure default https bindings, no IDefaultHttpsProvider service was provided. + /// + internal static string FormatUnableToConfigureHttpsBindings() + => GetString("UnableToConfigureHttpsBindings"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Kestrel/Internal/DefaultHttpsProvider.cs b/src/Kestrel/Internal/DefaultHttpsProvider.cs new file mode 100644 index 000000000..8de8fe5ce --- /dev/null +++ b/src/Kestrel/Internal/DefaultHttpsProvider.cs @@ -0,0 +1,77 @@ +// 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.Linq; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; + +namespace Microsoft.AspNetCore.Server.Kestrel.Internal +{ + public class DefaultHttpsProvider : IDefaultHttpsProvider + { + private const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.1"; + + public Func DefaultCertificateResolver { get; set; } = FindDevelopmentCertificate; + + public void ConfigureHttps(ListenOptions listenOptions) + { + listenOptions.UseHttps(DefaultCertificateResolver()); + } + + private static X509Certificate2 FindDevelopmentCertificate() + { + // TODO: replace this with call to + // CertificateManager.FindCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) + // when that becomes available. + using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) + { + store.Open(OpenFlags.ReadOnly); + + var certificates = store.Certificates.OfType(); + var certificate = certificates + .FirstOrDefault(c => HasOid(c, AspNetHttpsOid) && !IsExpired(c) && HasPrivateKey(c)); + + if (certificate == null) + { + throw new InvalidOperationException("Unable to find ASP.NET Core development certificate."); + } + + DisposeCertificates(certificates.Except(new[] { certificate })); + + return certificate; + } + } + + private static bool HasOid(X509Certificate2 certificate, string oid) => + certificate.Extensions + .OfType() + .Any(e => string.Equals(oid, e.Oid.Value, StringComparison.Ordinal)); + + private static bool IsExpired(X509Certificate2 certificate) + { + var now = DateTimeOffset.Now; + return now < certificate.NotBefore || now > certificate.NotAfter; + } + + private static bool HasPrivateKey(X509Certificate2 certificate) + => certificate.GetRSAPrivateKey() != null; + + private static void DisposeCertificates(IEnumerable certificates) + { + foreach (var certificate in certificates) + { + try + { + certificate.Dispose(); + } + catch + { + } + } + } + } +} diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 7e63e48eb..4235fe289 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -51,6 +51,49 @@ public void StartWithHttpsAddressConfiguresHttpsEndpoints() mockDefaultHttpsProvider.Verify(provider => provider.ConfigureHttps(It.IsAny()), Times.Once); } } + + [Fact] + public void KestrelServerThrowsUsefulExceptionIfDefaultHttpsProviderNotAdded() + { + var mockDefaultHttpsProvider = new Mock(); + + using (var server = CreateServer(new KestrelServerOptions(), defaultHttpsProvider: null)) + { + server.Features.Get().Addresses.Add("https://127.0.0.1:0"); + + var ex = Assert.Throws(() => StartDummyApplication(server)); + Assert.Equal(CoreStrings.UnableToConfigureHttpsBindings, ex.Message); + } + } + + [Fact] + public void KestrelServerDoesNotThrowIfNoDefaultHttpsProviderButNoHttpUrls() + { + var mockDefaultHttpsProvider = new Mock(); + + using (var server = CreateServer(new KestrelServerOptions(), defaultHttpsProvider: null)) + { + server.Features.Get().Addresses.Add("http://127.0.0.1:0"); + + StartDummyApplication(server); + } + } + + [Fact] + public void KestrelServerDoesNotThrowIfNoDefaultHttpsProviderButManualListenOptions() + { + var mockDefaultHttpsProvider = new Mock(); + + var serverOptions = new KestrelServerOptions(); + serverOptions.Listen(new IPEndPoint(IPAddress.Loopback, 0)); + + using (var server = CreateServer(serverOptions, defaultHttpsProvider: null)) + { + server.Features.Get().Addresses.Add("https://127.0.0.1:0"); + + StartDummyApplication(server); + } + } [Fact] public void StartWithPathBaseInAddressThrows() From 626a30e52040edd7b07b8857dc4f7753d4fb8762 Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Thu, 19 Oct 2017 11:10:12 -0700 Subject: [PATCH 13/20] comments and such --- src/Kestrel.Core/Internal/AddressBinder.cs | 3 +++ test/Kestrel.Core.Tests/KestrelServerTests.cs | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Kestrel.Core/Internal/AddressBinder.cs b/src/Kestrel.Core/Internal/AddressBinder.cs index d805e63b6..808675c0f 100644 --- a/src/Kestrel.Core/Internal/AddressBinder.cs +++ b/src/Kestrel.Core/Internal/AddressBinder.cs @@ -343,6 +343,9 @@ private UnconfiguredDefaultHttpsProvider() public void ConfigureHttps(ListenOptions listenOptions) { + // We have to throw here. If this is called, it's because the user asked for "https" binding but for some + // reason didn't provide a certificate and didn't use the "DefaultHttpsProvider". This means if we no-op, + // we'll silently downgrade to HTTP, which is bad. throw new InvalidOperationException(CoreStrings.UnableToConfigureHttpsBindings); } } diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 4235fe289..8bd91e8e6 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -61,8 +61,9 @@ public void KestrelServerThrowsUsefulExceptionIfDefaultHttpsProviderNotAdded() { server.Features.Get().Addresses.Add("https://127.0.0.1:0"); - var ex = Assert.Throws(() => StartDummyApplication(server)); - Assert.Equal(CoreStrings.UnableToConfigureHttpsBindings, ex.Message); + StartDummyApplication(server); + //var ex = Assert.Throws(() => StartDummyApplication(server)); + //Assert.Equal(CoreStrings.UnableToConfigureHttpsBindings, ex.Message); } } From 77d7952c9ffadb48cfbe33554fc90e397b6f3fcb Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Tue, 24 Oct 2017 11:44:37 -0700 Subject: [PATCH 14/20] logging and use CertificateManager --- src/Kestrel/DefaultHttpsProvider.cs | 75 ------------------- src/Kestrel/Internal/DefaultHttpsProvider.cs | 68 ++++++++--------- src/Kestrel/Kestrel.csproj | 1 + .../TestHelpers/TestServer.cs | 2 - 4 files changed, 32 insertions(+), 114 deletions(-) delete mode 100644 src/Kestrel/DefaultHttpsProvider.cs diff --git a/src/Kestrel/DefaultHttpsProvider.cs b/src/Kestrel/DefaultHttpsProvider.cs deleted file mode 100644 index a6feca6e2..000000000 --- a/src/Kestrel/DefaultHttpsProvider.cs +++ /dev/null @@ -1,75 +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; -using System.Collections.Generic; -using System.Linq; -using System.Security.Cryptography.X509Certificates; -using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Server.Kestrel.Core; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; - -namespace Microsoft.AspNetCore.Server.Kestrel -{ - internal class DefaultHttpsProvider : IDefaultHttpsProvider - { - private const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.1"; - - public void ConfigureHttps(ListenOptions listenOptions) - { - listenOptions.UseHttps(FindDevelopmentCertificate()); - } - - private static X509Certificate2 FindDevelopmentCertificate() - { - // TODO: replace this with call to - // CertificateManager.FindCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) - // when that becomes available. - using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) - { - store.Open(OpenFlags.ReadOnly); - - var certificates = store.Certificates.OfType(); - var certificate = certificates - .FirstOrDefault(c => HasOid(c, AspNetHttpsOid) && !IsExpired(c) && HasPrivateKey(c)); - - if (certificate == null) - { - throw new InvalidOperationException("Unable to find ASP.NET Core development certificate."); - } - - DisposeCertificates(certificates.Except(new[] { certificate })); - - return certificate; - } - } - - private static bool HasOid(X509Certificate2 certificate, string oid) => - certificate.Extensions - .OfType() - .Any(e => string.Equals(oid, e.Oid.Value, StringComparison.Ordinal)); - - private static bool IsExpired(X509Certificate2 certificate) - { - var now = DateTimeOffset.Now; - return now < certificate.NotBefore || now > certificate.NotAfter; - } - - private static bool HasPrivateKey(X509Certificate2 certificate) - => certificate.GetRSAPrivateKey() != null; - - private static void DisposeCertificates(IEnumerable certificates) - { - foreach (var certificate in certificates) - { - try - { - certificate.Dispose(); - } - catch - { - } - } - } - } -} diff --git a/src/Kestrel/Internal/DefaultHttpsProvider.cs b/src/Kestrel/Internal/DefaultHttpsProvider.cs index 8de8fe5ce..0342ac5d6 100644 --- a/src/Kestrel/Internal/DefaultHttpsProvider.cs +++ b/src/Kestrel/Internal/DefaultHttpsProvider.cs @@ -5,62 +5,54 @@ using System.Collections.Generic; using System.Linq; using System.Security.Cryptography.X509Certificates; -using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Certificates.Generation; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Internal { public class DefaultHttpsProvider : IDefaultHttpsProvider { - private const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.1"; + private static readonly CertificateManager _certificateManager = new CertificateManager(); - public Func DefaultCertificateResolver { get; set; } = FindDevelopmentCertificate; + private readonly ILogger _logger; - public void ConfigureHttps(ListenOptions listenOptions) + public Func DefaultCertificateResolver { get; set; } + + public DefaultHttpsProvider(ILogger logger) { - listenOptions.UseHttps(DefaultCertificateResolver()); + _logger = logger; + DefaultCertificateResolver = FindDevelopmentCertificate; } - private static X509Certificate2 FindDevelopmentCertificate() + public void ConfigureHttps(ListenOptions listenOptions) { - // TODO: replace this with call to - // CertificateManager.FindCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) - // when that becomes available. - using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) + var cert = DefaultCertificateResolver(); + if (cert == null) { - store.Open(OpenFlags.ReadOnly); - - var certificates = store.Certificates.OfType(); - var certificate = certificates - .FirstOrDefault(c => HasOid(c, AspNetHttpsOid) && !IsExpired(c) && HasPrivateKey(c)); - - if (certificate == null) - { - throw new InvalidOperationException("Unable to find ASP.NET Core development certificate."); - } - - DisposeCertificates(certificates.Except(new[] { certificate })); - - return certificate; + throw new InvalidOperationException("Kestrel was bound to an 'https' URL, but a certificate could not be found."); } + listenOptions.UseHttps(cert); } - private static bool HasOid(X509Certificate2 certificate, string oid) => - certificate.Extensions - .OfType() - .Any(e => string.Equals(oid, e.Oid.Value, StringComparison.Ordinal)); - - private static bool IsExpired(X509Certificate2 certificate) + private X509Certificate2 FindDevelopmentCertificate() { - var now = DateTimeOffset.Now; - return now < certificate.NotBefore || now > certificate.NotAfter; + var certificate = _certificateManager.ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) + .FirstOrDefault(); + if (certificate != null) + { + _logger.LogDebug("Using development certificate: {certificateSubjectName} (Thumbprint: {certificateThumbprint})", certificate.Subject, certificate.Thumbprint); + return certificate; + } + else + { + _logger.LogDebug("Development certificate could not be found"); + return null; + } } - private static bool HasPrivateKey(X509Certificate2 certificate) - => certificate.GetRSAPrivateKey() != null; - - private static void DisposeCertificates(IEnumerable certificates) + private void DisposeCertificates(IEnumerable certificates) { foreach (var certificate in certificates) { @@ -68,8 +60,10 @@ private static void DisposeCertificates(IEnumerable certificat { certificate.Dispose(); } - catch + catch (Exception ex) { + // Accessing certificate may cause additional exceptions. + _logger.LogError(ex, "Error disposing of certficate."); } } } diff --git a/src/Kestrel/Kestrel.csproj b/src/Kestrel/Kestrel.csproj index 568f62051..8cf8fc68d 100644 --- a/src/Kestrel/Kestrel.csproj +++ b/src/Kestrel/Kestrel.csproj @@ -12,6 +12,7 @@ + diff --git a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs index a6162738c..f0972bb4f 100644 --- a/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs +++ b/test/Kestrel.FunctionalTests/TestHelpers/TestServer.cs @@ -11,12 +11,10 @@ using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; -using Moq; namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { From 9ad422b582fb7f77d18420bd21cdc6dace54018f Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Tue, 24 Oct 2017 11:57:28 -0700 Subject: [PATCH 15/20] fix some (but possibly not all) derps --- src/Kestrel/Internal/DefaultHttpsProvider.cs | 1 + src/Kestrel/Kestrel.csproj | 1 + src/Kestrel/WebHostBuilderKestrelExtensions.cs | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Kestrel/Internal/DefaultHttpsProvider.cs b/src/Kestrel/Internal/DefaultHttpsProvider.cs index 0342ac5d6..acb13c13c 100644 --- a/src/Kestrel/Internal/DefaultHttpsProvider.cs +++ b/src/Kestrel/Internal/DefaultHttpsProvider.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Certificates.Generation; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.Extensions.Logging; diff --git a/src/Kestrel/Kestrel.csproj b/src/Kestrel/Kestrel.csproj index 8cf8fc68d..7f7f8de70 100644 --- a/src/Kestrel/Kestrel.csproj +++ b/src/Kestrel/Kestrel.csproj @@ -13,6 +13,7 @@ + diff --git a/src/Kestrel/WebHostBuilderKestrelExtensions.cs b/src/Kestrel/WebHostBuilderKestrelExtensions.cs index 24d821459..f65a66ac4 100644 --- a/src/Kestrel/WebHostBuilderKestrelExtensions.cs +++ b/src/Kestrel/WebHostBuilderKestrelExtensions.cs @@ -3,9 +3,9 @@ using System; using Microsoft.AspNetCore.Hosting.Server; -using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets; using Microsoft.Extensions.DependencyInjection; From e46a08b079d5a0d05a7df282f3f2db6a08cbbe70 Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Tue, 24 Oct 2017 12:17:36 -0700 Subject: [PATCH 16/20] see, I knew I still derped something --- .../Properties/CoreStrings.Designer.cs | 4 +- src/Kestrel/Internal/DefaultHttpsProvider.cs | 2 +- src/Kestrel/KestrelStrings.resx | 123 ++++++++++++++++++ .../Properties/KestrelStrings.Designer.cs | 44 +++++++ test/Kestrel.Core.Tests/KestrelServerTests.cs | 5 +- 5 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 src/Kestrel/KestrelStrings.resx create mode 100644 src/Kestrel/Properties/KestrelStrings.Designer.cs diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index 83d81019f..9321f1a2c 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -1621,7 +1621,7 @@ internal static string FormatHttp2ErrorConnectionSpecificHeaderField() => GetString("Http2ErrorConnectionSpecificHeaderField"); /// - /// Unable to configure default https bindings, no IDefaultHttpsProvider service was provided. + /// Unable to configure default https bindings because no IDefaultHttpsProvider service was provided. /// internal static string UnableToConfigureHttpsBindings { @@ -1629,7 +1629,7 @@ internal static string UnableToConfigureHttpsBindings } /// - /// Unable to configure default https bindings, no IDefaultHttpsProvider service was provided. + /// Unable to configure default https bindings because no IDefaultHttpsProvider service was provided. /// internal static string FormatUnableToConfigureHttpsBindings() => GetString("UnableToConfigureHttpsBindings"); diff --git a/src/Kestrel/Internal/DefaultHttpsProvider.cs b/src/Kestrel/Internal/DefaultHttpsProvider.cs index acb13c13c..80a66293c 100644 --- a/src/Kestrel/Internal/DefaultHttpsProvider.cs +++ b/src/Kestrel/Internal/DefaultHttpsProvider.cs @@ -32,7 +32,7 @@ public void ConfigureHttps(ListenOptions listenOptions) var cert = DefaultCertificateResolver(); if (cert == null) { - throw new InvalidOperationException("Kestrel was bound to an 'https' URL, but a certificate could not be found."); + throw new InvalidOperationException(KestrelStrings.HttpsUrlProvidedButNoDevelopmentCertificateFound); } listenOptions.UseHttps(cert); } diff --git a/src/Kestrel/KestrelStrings.resx b/src/Kestrel/KestrelStrings.resx new file mode 100644 index 000000000..e5ab31515 --- /dev/null +++ b/src/Kestrel/KestrelStrings.resx @@ -0,0 +1,123 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + An 'https' URL was provided, but a development certificate could not be found. + + \ No newline at end of file diff --git a/src/Kestrel/Properties/KestrelStrings.Designer.cs b/src/Kestrel/Properties/KestrelStrings.Designer.cs new file mode 100644 index 000000000..ede41f12c --- /dev/null +++ b/src/Kestrel/Properties/KestrelStrings.Designer.cs @@ -0,0 +1,44 @@ +// +namespace Microsoft.AspNetCore.Server.Kestrel +{ + using System.Globalization; + using System.Reflection; + using System.Resources; + + internal static class KestrelStrings + { + private static readonly ResourceManager _resourceManager + = new ResourceManager("Microsoft.AspNetCore.Server.Kestrel.KestrelStrings", typeof(KestrelStrings).GetTypeInfo().Assembly); + + /// + /// An 'https' URL was provided, but a development certificate could not be found. + /// + internal static string HttpsUrlProvidedButNoDevelopmentCertificateFound + { + get => GetString("HttpsUrlProvidedButNoDevelopmentCertificateFound"); + } + + /// + /// An 'https' URL was provided, but a development certificate could not be found. + /// + internal static string FormatHttpsUrlProvidedButNoDevelopmentCertificateFound() + => GetString("HttpsUrlProvidedButNoDevelopmentCertificateFound"); + + private static string GetString(string name, params string[] formatterNames) + { + var value = _resourceManager.GetString(name); + + System.Diagnostics.Debug.Assert(value != null); + + if (formatterNames != null) + { + for (var i = 0; i < formatterNames.Length; i++) + { + value = value.Replace("{" + formatterNames[i] + "}", "{" + i + "}"); + } + } + + return value; + } + } +} diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 8bd91e8e6..4235fe289 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -61,9 +61,8 @@ public void KestrelServerThrowsUsefulExceptionIfDefaultHttpsProviderNotAdded() { server.Features.Get().Addresses.Add("https://127.0.0.1:0"); - StartDummyApplication(server); - //var ex = Assert.Throws(() => StartDummyApplication(server)); - //Assert.Equal(CoreStrings.UnableToConfigureHttpsBindings, ex.Message); + var ex = Assert.Throws(() => StartDummyApplication(server)); + Assert.Equal(CoreStrings.UnableToConfigureHttpsBindings, ex.Message); } } From 3397f60befb20177f487a0c68c95e44b832aec36 Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Tue, 24 Oct 2017 15:22:04 -0700 Subject: [PATCH 17/20] pr feedback and message updates --- src/Kestrel/Internal/DefaultHttpsProvider.cs | 38 ++----------------- src/Kestrel/Internal/LoggerExtensions.cs | 20 ++++++++++ src/Kestrel/KestrelStrings.resx | 2 +- test/Kestrel.Core.Tests/KestrelServerTests.cs | 6 +-- 4 files changed, 26 insertions(+), 40 deletions(-) create mode 100644 src/Kestrel/Internal/LoggerExtensions.cs diff --git a/src/Kestrel/Internal/DefaultHttpsProvider.cs b/src/Kestrel/Internal/DefaultHttpsProvider.cs index 80a66293c..efc7d9fb1 100644 --- a/src/Kestrel/Internal/DefaultHttpsProvider.cs +++ b/src/Kestrel/Internal/DefaultHttpsProvider.cs @@ -2,7 +2,6 @@ // 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.Linq; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Certificates.Generation; @@ -19,53 +18,24 @@ public class DefaultHttpsProvider : IDefaultHttpsProvider private readonly ILogger _logger; - public Func DefaultCertificateResolver { get; set; } - public DefaultHttpsProvider(ILogger logger) { _logger = logger; - DefaultCertificateResolver = FindDevelopmentCertificate; } public void ConfigureHttps(ListenOptions listenOptions) - { - var cert = DefaultCertificateResolver(); - if (cert == null) - { - throw new InvalidOperationException(KestrelStrings.HttpsUrlProvidedButNoDevelopmentCertificateFound); - } - listenOptions.UseHttps(cert); - } - - private X509Certificate2 FindDevelopmentCertificate() { var certificate = _certificateManager.ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true) .FirstOrDefault(); if (certificate != null) { - _logger.LogDebug("Using development certificate: {certificateSubjectName} (Thumbprint: {certificateThumbprint})", certificate.Subject, certificate.Thumbprint); - return certificate; + _logger.LocatedDevelopmentCertificate(certificate); + listenOptions.UseHttps(certificate); } else { - _logger.LogDebug("Development certificate could not be found"); - return null; - } - } - - private void DisposeCertificates(IEnumerable certificates) - { - foreach (var certificate in certificates) - { - try - { - certificate.Dispose(); - } - catch (Exception ex) - { - // Accessing certificate may cause additional exceptions. - _logger.LogError(ex, "Error disposing of certficate."); - } + _logger.UnableToLocateDevelopmentCertificate(); + throw new InvalidOperationException(KestrelStrings.HttpsUrlProvidedButNoDevelopmentCertificateFound); } } } diff --git a/src/Kestrel/Internal/LoggerExtensions.cs b/src/Kestrel/Internal/LoggerExtensions.cs new file mode 100644 index 000000000..218f50ca1 --- /dev/null +++ b/src/Kestrel/Internal/LoggerExtensions.cs @@ -0,0 +1,20 @@ +using System; +using System.Security.Cryptography.X509Certificates; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Server.Kestrel.Internal +{ + internal static class LoggerExtensions + { + // Category: DefaultHttpsProvider + private static readonly Action _locatedDevelopmentCertificate = + LoggerMessage.Define(LogLevel.Debug, new EventId(0, nameof(LocatedDevelopmentCertificate)), "Using development certificate: {certificateSubjectName} (Thumbprint: {certificateThumbprint})"); + + private static readonly Action _unableToLocateDevelopmentCertificate = + LoggerMessage.Define(LogLevel.Error, new EventId(1, nameof(UnableToLocateDevelopmentCertificate)), "Unable to locate an appropriate development https certificate."); + + 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); + } +} diff --git a/src/Kestrel/KestrelStrings.resx b/src/Kestrel/KestrelStrings.resx index e5ab31515..7950d77a7 100644 --- a/src/Kestrel/KestrelStrings.resx +++ b/src/Kestrel/KestrelStrings.resx @@ -118,6 +118,6 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - An 'https' URL was provided, but a development certificate could not be found. + Unable to configure HTTPS endpoint. For information on configuring HTTPS see https://go.microsoft.com/fwlink/?linkid=848054 \ No newline at end of file diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 4235fe289..6f62b7440 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -55,8 +55,6 @@ public void StartWithHttpsAddressConfiguresHttpsEndpoints() [Fact] public void KestrelServerThrowsUsefulExceptionIfDefaultHttpsProviderNotAdded() { - var mockDefaultHttpsProvider = new Mock(); - using (var server = CreateServer(new KestrelServerOptions(), defaultHttpsProvider: null)) { server.Features.Get().Addresses.Add("https://127.0.0.1:0"); @@ -69,8 +67,6 @@ public void KestrelServerThrowsUsefulExceptionIfDefaultHttpsProviderNotAdded() [Fact] public void KestrelServerDoesNotThrowIfNoDefaultHttpsProviderButNoHttpUrls() { - var mockDefaultHttpsProvider = new Mock(); - using (var server = CreateServer(new KestrelServerOptions(), defaultHttpsProvider: null)) { server.Features.Get().Addresses.Add("http://127.0.0.1:0"); @@ -317,7 +313,7 @@ private static KestrelServer CreateServer(KestrelServerOptions options, ILogger private static KestrelServer CreateServer(KestrelServerOptions options, IDefaultHttpsProvider defaultHttpsProvider) { - return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(Mock.Of()) }), defaultHttpsProvider); + return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider() }), defaultHttpsProvider); } private static void StartDummyApplication(IServer server) From cafaaffa0260c66bd156874f173c552007116b34 Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Tue, 24 Oct 2017 15:35:36 -0700 Subject: [PATCH 18/20] change test logger provider to not throw on critical --- test/Kestrel.Core.Tests/KestrelServerTests.cs | 2 +- test/shared/KestrelTestLoggerProvider.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 6f62b7440..acbef7fba 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -313,7 +313,7 @@ private static KestrelServer CreateServer(KestrelServerOptions options, ILogger private static KestrelServer CreateServer(KestrelServerOptions options, IDefaultHttpsProvider defaultHttpsProvider) { - return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider() }), defaultHttpsProvider); + return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(Mock.Of()) }), defaultHttpsProvider); } private static void StartDummyApplication(IServer server) diff --git a/test/shared/KestrelTestLoggerProvider.cs b/test/shared/KestrelTestLoggerProvider.cs index 45462791d..0f95441b6 100644 --- a/test/shared/KestrelTestLoggerProvider.cs +++ b/test/shared/KestrelTestLoggerProvider.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -11,7 +11,7 @@ public class KestrelTestLoggerProvider : ILoggerProvider private readonly ILogger _testLogger; public KestrelTestLoggerProvider() - : this(new TestApplicationErrorLogger()) + : this(new TestApplicationErrorLogger() { ThrowOnCriticalErrors = false }) { } @@ -30,4 +30,4 @@ public void Dispose() throw new NotImplementedException(); } } -} \ No newline at end of file +} From c5cb57fe64fc13b0071540c7bfdafa748fda67ca Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Wed, 25 Oct 2017 12:24:00 -0700 Subject: [PATCH 19/20] constrain use of ThrowOnCriticalErrors --- test/Kestrel.Core.Tests/KestrelServerTests.cs | 6 +++--- test/shared/KestrelTestLoggerProvider.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index acbef7fba..51e9de022 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -55,7 +55,7 @@ public void StartWithHttpsAddressConfiguresHttpsEndpoints() [Fact] public void KestrelServerThrowsUsefulExceptionIfDefaultHttpsProviderNotAdded() { - using (var server = CreateServer(new KestrelServerOptions(), defaultHttpsProvider: null)) + using (var server = CreateServer(new KestrelServerOptions(), defaultHttpsProvider: null, throwOnCriticalErrors: false)) { server.Features.Get().Addresses.Add("https://127.0.0.1:0"); @@ -311,9 +311,9 @@ private static KestrelServer CreateServer(KestrelServerOptions options, ILogger return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) })); } - private static KestrelServer CreateServer(KestrelServerOptions options, IDefaultHttpsProvider defaultHttpsProvider) + private static KestrelServer CreateServer(KestrelServerOptions options, IDefaultHttpsProvider defaultHttpsProvider, bool throwOnCriticalErrors = true) { - return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(Mock.Of()) }), defaultHttpsProvider); + return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(throwOnCriticalErrors) }), defaultHttpsProvider); } private static void StartDummyApplication(IServer server) diff --git a/test/shared/KestrelTestLoggerProvider.cs b/test/shared/KestrelTestLoggerProvider.cs index 0f95441b6..48455557f 100644 --- a/test/shared/KestrelTestLoggerProvider.cs +++ b/test/shared/KestrelTestLoggerProvider.cs @@ -10,8 +10,8 @@ public class KestrelTestLoggerProvider : ILoggerProvider { private readonly ILogger _testLogger; - public KestrelTestLoggerProvider() - : this(new TestApplicationErrorLogger() { ThrowOnCriticalErrors = false }) + public KestrelTestLoggerProvider(bool throwOnCriticalErrors = true) + : this(new TestApplicationErrorLogger() { ThrowOnCriticalErrors = throwOnCriticalErrors }) { } From 48739e532a8331808ae626461f03d0e79770ef90 Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Wed, 25 Oct 2017 12:24:59 -0700 Subject: [PATCH 20/20] update error message --- src/Kestrel/KestrelStrings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Kestrel/KestrelStrings.resx b/src/Kestrel/KestrelStrings.resx index 7950d77a7..d39f8b516 100644 --- a/src/Kestrel/KestrelStrings.resx +++ b/src/Kestrel/KestrelStrings.resx @@ -118,6 +118,6 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - Unable to configure HTTPS endpoint. For information on configuring HTTPS see https://go.microsoft.com/fwlink/?linkid=848054 + Unable to configure HTTPS endpoint. Try running 'dotnet developercertificates https -t' to setup a developer certificate for use with localhost. For information on configuring HTTPS see https://go.microsoft.com/fwlink/?linkid=848054 \ No newline at end of file