From 4561eb9250bbcbc74f84de9a9b5a7b996da4f424 Mon Sep 17 00:00:00 2001 From: Filip Staffa Date: Fri, 12 Jun 2020 19:32:41 +0200 Subject: [PATCH 1/3] manually tested version --- .../Core/src/Internal/ConfigurationReader.cs | 29 +++++++- .../Core/src/KestrelConfigurationLoader.cs | 11 ++++ .../Kestrel/test/ConfigurationReaderTests.cs | 66 +++++++++++++++++++ ....cs => KestrelConfigurationLoaderTests.cs} | 2 +- 4 files changed, 106 insertions(+), 2 deletions(-) rename src/Servers/Kestrel/Kestrel/test/{KestrelConfigurationBuilderTests.cs => KestrelConfigurationLoaderTests.cs} (99%) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index cf37f5ec73f5..c126dbf7f2da 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -3,6 +3,8 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Security.Authentication; using Microsoft.Extensions.Configuration; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal @@ -12,6 +14,7 @@ internal class ConfigurationReader private const string ProtocolsKey = "Protocols"; 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"; @@ -49,13 +52,15 @@ private IDictionary ReadCertificates() // "EndpointDefaults": { // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], // } private EndpointDefaults ReadEndpointDefaults() { var configSection = _configuration.GetSection(EndpointDefaultsKey); return new EndpointDefaults { - Protocols = ParseProtocols(configSection[ProtocolsKey]) + Protocols = ParseProtocols(configSection[ProtocolsKey]), + SslProtocols = ParseSslProcotols(configSection.GetSection(SslProtocolsKey)) }; } @@ -69,6 +74,7 @@ private IEnumerable ReadEndpoints() // "EndpointName": { // "Url": "https://*:5463", // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], // "Certificate": { // "Path": "testCert.pfx", // "Password": "testPassword" @@ -88,6 +94,7 @@ private IEnumerable ReadEndpoints() Protocols = ParseProtocols(endpointConfig[ProtocolsKey]), ConfigSection = endpointConfig, Certificate = new CertificateConfig(endpointConfig.GetSection(CertificateKey)), + SslProtocols = ParseSslProcotols(endpointConfig.GetSection(SslProtocolsKey)) }; endpoints.Add(endpoint); @@ -105,19 +112,37 @@ private IEnumerable ReadEndpoints() return null; } + + private static SslProtocols? ParseSslProcotols(IConfigurationSection sslProtocols) + { + var stringProtocols = sslProtocols.Get(); + + return stringProtocols?.Aggregate(SslProtocols.None, (acc, current) => + { + if (Enum.TryParse(current, out SslProtocols parsed)) + { + return acc | parsed; + } + + return acc; + }); + } } // "EndpointDefaults": { // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], // } internal class EndpointDefaults { public HttpProtocols? Protocols { get; set; } + public SslProtocols? SslProtocols { get; set; } } // "EndpointName": { // "Url": "https://*:5463", // "Protocols": "Http1AndHttp2", + // "SslProtocols": [ "Tls11", "Tls12", "Tls13"], // "Certificate": { // "Path": "testCert.pfx", // "Password": "testPassword" @@ -131,6 +156,7 @@ internal class EndpointConfig public string Name { get; set; } public string Url { get; set; } public HttpProtocols? Protocols { get; set; } + public SslProtocols? SslProtocols { get; set; } public CertificateConfig Certificate { get; set; } // Compare config sections because it's accessible to app developers via an Action callback. @@ -154,6 +180,7 @@ obj is EndpointConfig other && Url == other.Url && (Protocols ?? ListenOptions.DefaultHttpProtocols) == (other.Protocols ?? ListenOptions.DefaultHttpProtocols) && Certificate == other.Certificate && + SslProtocols == other.SslProtocols && _configSectionClone == other._configSectionClone; public override int GetHashCode() => HashCode.Combine(Name, Url, Protocols ?? ListenOptions.DefaultHttpProtocols, Certificate, _configSectionClone); diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index f68b32405389..f3940a145ada 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -282,6 +282,12 @@ public void Load() // Defaults Options.ApplyHttpsDefaults(httpsOptions); + // Default SslProtocol from config + if (ConfigurationReader.EndpointDefaults.SslProtocols.HasValue) + { + httpsOptions.SslProtocols = ConfigurationReader.EndpointDefaults.SslProtocols.Value; + } + // Specified httpsOptions.ServerCertificate = LoadCertificate(endpoint.Certificate, endpoint.Name) ?? httpsOptions.ServerCertificate; @@ -313,6 +319,11 @@ public void Load() configureEndpoint(endpointConfig); } + if (https && endpoint.SslProtocols.HasValue) + { + httpsOptions.SslProtocols = endpoint.SslProtocols.Value; + } + // EndpointDefaults or configureEndpoint may have added an https adapter. if (https && !listenOptions.IsTls) { diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index 7097c165539c..63e471a117c7 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Authentication; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.Extensions.Configuration; using Xunit; @@ -173,5 +174,70 @@ public void ReadEndpointsSection_ReturnsCollection() Assert.Equal("cetlocation", cert4.Location); Assert.True(cert4.AllowInvalid); } + + [Fact] + public void ReadEndpointWithSingleSslProtocolSet_ReturnsCorrectValue() + { + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:SslProtocols:0", "Tls11"), + }).Build(); + var reader = new ConfigurationReader(config); + + var endpoint = reader.Endpoints.First(); + Assert.Equal(SslProtocols.Tls11, endpoint.SslProtocols); + } + + [Fact] + public void ReadEndpointWithMultipleSslProtocolsSet_ReturnsCorrectValue() + { + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:SslProtocols:0", "Tls11"), + new KeyValuePair("Endpoints:End1:SslProtocols:1", "Tls12"), + }).Build(); + var reader = new ConfigurationReader(config); + + var endpoint = reader.Endpoints.First(); + Assert.Equal(SslProtocols.Tls11|SslProtocols.Tls12, endpoint.SslProtocols); + } + + [Fact] + public void ReadEndpointWithNoSslProtocolSettings_ReturnsNull() + { + 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.Null(endpoint.SslProtocols); + } + + [Fact] + public void ReadEndpointDefaultsWithSingleSslProtocolSet_ReturnsCorrectValue() + { + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("EndpointDefaults:SslProtocols:0", "Tls11"), + }).Build(); + var reader = new ConfigurationReader(config); + + var endpoint = reader.EndpointDefaults; + Assert.Equal(SslProtocols.Tls11, endpoint.SslProtocols); + } + + [Fact] + public void ReadEndpointDefaultsWithNoSslProtocolSettings_ReturnsCorrectValue() + { + var config = new ConfigurationBuilder().Build(); + var reader = new ConfigurationReader(config); + + var endpoint = reader.EndpointDefaults; + Assert.Null(endpoint.SslProtocols); + } } } diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs similarity index 99% rename from src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs rename to src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index 2b38083cfd1b..2f58308b83b3 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationBuilderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Tests { - public class KestrelConfigurationBuilderTests + public class KestrelConfigurationLoaderTests { private KestrelServerOptions CreateServerOptions() { From 854555292407266e94396a90e9cfad336374f7c7 Mon Sep 17 00:00:00 2001 From: Filip Staffa Date: Tue, 16 Jun 2020 23:57:49 +0200 Subject: [PATCH 2/3] code review fixes --- .../Core/src/Internal/ConfigurationReader.cs | 4 +- .../Core/src/KestrelConfigurationLoader.cs | 13 +- .../Kestrel/test/ConfigurationReaderTests.cs | 14 ++ .../test/KestrelConfigurationLoaderTests.cs | 121 ++++++++++++++++++ 4 files changed, 142 insertions(+), 10 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index c126dbf7f2da..19ec02774a68 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -119,7 +119,7 @@ private IEnumerable ReadEndpoints() return stringProtocols?.Aggregate(SslProtocols.None, (acc, current) => { - if (Enum.TryParse(current, out SslProtocols parsed)) + if (Enum.TryParse(current, ignoreCase: true, out SslProtocols parsed)) { return acc | parsed; } @@ -180,7 +180,7 @@ obj is EndpointConfig other && Url == other.Url && (Protocols ?? ListenOptions.DefaultHttpProtocols) == (other.Protocols ?? ListenOptions.DefaultHttpProtocols) && Certificate == other.Certificate && - SslProtocols == other.SslProtocols && + (SslProtocols ?? System.Security.Authentication.SslProtocols.None) == (other.SslProtocols ?? System.Security.Authentication.SslProtocols.None) && _configSectionClone == other._configSectionClone; public override int GetHashCode() => HashCode.Combine(Name, Url, Protocols ?? ListenOptions.DefaultHttpProtocols, Certificate, _configSectionClone); diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index f3940a145ada..193d07ca3202 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -6,6 +6,7 @@ using System.IO; using System.Linq; using System.Net; +using System.Security.Authentication; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Certificates.Generation; @@ -279,13 +280,14 @@ public void Load() var httpsOptions = new HttpsConnectionAdapterOptions(); if (https) { + httpsOptions.SslProtocols = ConfigurationReader.EndpointDefaults.SslProtocols ?? SslProtocols.None; + // Defaults Options.ApplyHttpsDefaults(httpsOptions); - // Default SslProtocol from config - if (ConfigurationReader.EndpointDefaults.SslProtocols.HasValue) + if (endpoint.SslProtocols.HasValue) { - httpsOptions.SslProtocols = ConfigurationReader.EndpointDefaults.SslProtocols.Value; + httpsOptions.SslProtocols = endpoint.SslProtocols.Value; } // Specified @@ -319,11 +321,6 @@ public void Load() configureEndpoint(endpointConfig); } - if (https && endpoint.SslProtocols.HasValue) - { - httpsOptions.SslProtocols = endpoint.SslProtocols.Value; - } - // EndpointDefaults or configureEndpoint may have added an https adapter. if (https && !listenOptions.IsTls) { diff --git a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs index 63e471a117c7..a83e90818ded 100644 --- a/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/ConfigurationReaderTests.cs @@ -204,6 +204,20 @@ public void ReadEndpointWithMultipleSslProtocolsSet_ReturnsCorrectValue() Assert.Equal(SslProtocols.Tls11|SslProtocols.Tls12, endpoint.SslProtocols); } + [Fact] + public void ReadEndpointWithSslProtocolSet_ReadsCaseInsensitive() + { + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:Url", "http://*:5001"), + new KeyValuePair("Endpoints:End1:SslProtocols:0", "TLS11"), + }).Build(); + var reader = new ConfigurationReader(config); + + var endpoint = reader.Endpoints.First(); + Assert.Equal(SslProtocols.Tls11, endpoint.SslProtocols); + } + [Fact] public void ReadEndpointWithNoSslProtocolSettings_ReturnsNull() { diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index 2f58308b83b3..f72116c3fb9a 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Server.Kestrel.Core; @@ -456,6 +457,126 @@ private void EndpointConfigSectionCanSetProtocols(string input, HttpProtocols ex Assert.True(ran3); } + [Fact] + public void EndpointConfigureSection_CanSetSslProtocol() + { + var serverOptions = CreateServerOptions(); + var ranDefault = false; + + serverOptions.ConfigureHttpsDefaults(opt => + { + // Kestrel default + Assert.Equal(SslProtocols.None, opt.SslProtocols); + ranDefault = true; + }); + + var ran1 = false; + var ran2 = false; + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:SslProtocols:0", "Tls11"), + new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), + }).Build(); + serverOptions.Configure(config) + .Endpoint("End1", opt => + { + Assert.Equal(SslProtocols.Tls11, opt.HttpsOptions.SslProtocols); + ran1 = true; + }) + .Load(); + serverOptions.ListenAnyIP(0, opt => + { + opt.UseHttps(httpsOptions => + { + // Kestrel default. + Assert.Equal(SslProtocols.None, httpsOptions.SslProtocols); + ran2 = true; + }); + }); + + Assert.True(ranDefault); + Assert.True(ran1); + Assert.True(ran2); + } + + [Fact] + public void EndpointConfigureSection_CanOverrideSslProtocolsFromConfigureHttpsDefaults() + { + var serverOptions = CreateServerOptions(); + + serverOptions.ConfigureHttpsDefaults(opt => + { + opt.SslProtocols = SslProtocols.Tls12; + }); + + var ran1 = false; + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("Endpoints:End1:SslProtocols:0", "Tls11"), + new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), + }).Build(); + serverOptions.Configure(config) + .Endpoint("End1", opt => + { + Assert.Equal(SslProtocols.Tls11, opt.HttpsOptions.SslProtocols); + ran1 = true; + }) + .Load(); + + Assert.True(ran1); + } + + [Fact] + public void DefaultEndpointConfigureSection_CanSetSslProtocols() + { + var serverOptions = CreateServerOptions(); + + var ran1 = false; + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("EndpointDefaults:SslProtocols:0", "Tls11"), + new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), + }).Build(); + serverOptions.Configure(config) + .Endpoint("End1", opt => + { + Assert.Equal(SslProtocols.Tls11, opt.HttpsOptions.SslProtocols); + ran1 = true; + }) + .Load(); + + Assert.True(ran1); + } + + + [Fact] + public void DefaultEndpointConfigureSection_ConfigureHttpsDefaultsCanOverrideSslProtocols() + { + var serverOptions = CreateServerOptions(); + + serverOptions.ConfigureHttpsDefaults(opt => + { + Assert.Equal(SslProtocols.Tls11, opt.SslProtocols); + opt.SslProtocols = SslProtocols.Tls12; + }); + + var ran1 = false; + var config = new ConfigurationBuilder().AddInMemoryCollection(new[] + { + new KeyValuePair("EndpointDefaults:SslProtocols:0", "Tls11"), + new KeyValuePair("Endpoints:End1:Url", "https://*:5001"), + }).Build(); + serverOptions.Configure(config) + .Endpoint("End1", opt => + { + Assert.Equal(SslProtocols.Tls12, opt.HttpsOptions.SslProtocols); + ran1 = true; + }) + .Load(); + + Assert.True(ran1); + } + [Fact] public void Latin1RequestHeadersReadFromConfig() { From bffd464f5371078db466ad7a77018f2427e3bf2b Mon Sep 17 00:00:00 2001 From: Filip Staffa Date: Fri, 19 Jun 2020 10:02:38 +0200 Subject: [PATCH 3/3] add test certificates --- .../Kestrel/test/KestrelConfigurationLoaderTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index f72116c3fb9a..8d93023a467c 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -465,6 +465,8 @@ public void EndpointConfigureSection_CanSetSslProtocol() serverOptions.ConfigureHttpsDefaults(opt => { + opt.ServerCertificate = TestResources.GetTestCertificate(); + // Kestrel default Assert.Equal(SslProtocols.None, opt.SslProtocols); ranDefault = true; @@ -506,6 +508,7 @@ public void EndpointConfigureSection_CanOverrideSslProtocolsFromConfigureHttpsDe serverOptions.ConfigureHttpsDefaults(opt => { + opt.ServerCertificate = TestResources.GetTestCertificate(); opt.SslProtocols = SslProtocols.Tls12; }); @@ -531,6 +534,11 @@ public void DefaultEndpointConfigureSection_CanSetSslProtocols() { var serverOptions = CreateServerOptions(); + serverOptions.ConfigureHttpsDefaults(opt => + { + opt.ServerCertificate = TestResources.GetTestCertificate(); + }); + var ran1 = false; var config = new ConfigurationBuilder().AddInMemoryCollection(new[] { @@ -556,6 +564,8 @@ public void DefaultEndpointConfigureSection_ConfigureHttpsDefaultsCanOverrideSsl serverOptions.ConfigureHttpsDefaults(opt => { + opt.ServerCertificate = TestResources.GetTestCertificate(); + Assert.Equal(SslProtocols.Tls11, opt.SslProtocols); opt.SslProtocols = SslProtocols.Tls12; });