From 339bf3aea2d11cea2d3d151e2b226099cf0f6ab5 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sat, 15 Aug 2020 10:07:26 -0700 Subject: [PATCH 1/2] Add support for loading certificate chains from configuration. - Adds a ServerCertificateIntermediates property to HttpsConnectionAdapterOptions - Adds a Chain property to configuration. This only supports PEM certificates. - Import intermediates if chain path specified --- .../Core/src/HttpsConnectionAdapterOptions.cs | 5 ++++ .../Certificates/CertificateConfigLoader.cs | 28 +++++++++++++------ .../Certificates/ICertificateConfigLoader.cs | 4 +-- .../Core/src/Internal/ConfigurationReader.cs | 4 +++ .../Core/src/Internal/SniOptionsSelector.cs | 20 ++++++------- .../Core/src/KestrelConfigurationLoader.cs | 8 ++++-- .../Middleware/HttpsConnectionMiddleware.cs | 2 +- .../Core/test/SniOptionsSelectorTests.cs | 8 ++---- 8 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs index 9961101484ca..c51091c11db4 100644 --- a/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs +++ b/src/Servers/Kestrel/Core/src/HttpsConnectionAdapterOptions.cs @@ -39,6 +39,11 @@ public HttpsConnectionAdapterOptions() /// public X509Certificate2 ServerCertificate { get; set; } + /// + /// Specifies the intermediate certificates in the chain. + /// + public X509Certificate2Collection ServerCertificateIntermediates { get; set; } + /// /// /// A callback that will be invoked to dynamically select a server certificate. This is higher priority than ServerCertificate. diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs index ee3a1edcc054..a04ab17299d0 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/CertificateConfigLoader.cs @@ -24,13 +24,11 @@ public CertificateConfigLoader(IHostEnvironment hostEnvironment, ILogger Logger { get; } - public bool IsTestMock => false; - - public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) + public (X509Certificate2, X509Certificate2Collection) LoadCertificate(CertificateConfig certInfo, string endpointName) { if (certInfo is null) { - return null; + return (null, null); } if (certInfo.IsFileCert && certInfo.IsStoreCert) @@ -40,9 +38,21 @@ public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpo else if (certInfo.IsFileCert) { var certificatePath = Path.Combine(HostEnvironment.ContentRootPath, certInfo.Path); + + X509Certificate2Collection intermediates = null; + + if (certInfo.ChainPath != null) + { + var certificateChainPath = Path.Combine(HostEnvironment.ContentRootPath, certInfo.ChainPath); + + intermediates = new X509Certificate2Collection(); + intermediates.ImportFromPemFile(certificateChainPath); + } + if (certInfo.KeyPath != null) { var certificateKeyPath = Path.Combine(HostEnvironment.ContentRootPath, certInfo.KeyPath); + var certificate = GetCertificate(certificatePath); if (certificate != null) @@ -58,10 +68,10 @@ public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpo { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - return PersistKey(certificate); + return (PersistKey(certificate), intermediates); } - return certificate; + return (certificate, intermediates); } else { @@ -71,14 +81,14 @@ public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpo throw new InvalidOperationException(CoreStrings.InvalidPemKey); } - return new X509Certificate2(Path.Combine(HostEnvironment.ContentRootPath, certInfo.Path), certInfo.Password); + return (new X509Certificate2(Path.Combine(HostEnvironment.ContentRootPath, certInfo.Path), certInfo.Password), intermediates); } else if (certInfo.IsStoreCert) { - return LoadFromStoreCert(certInfo); + return (LoadFromStoreCert(certInfo), null); } - return null; + return (null, null); } private static X509Certificate2 PersistKey(X509Certificate2 fullCertificate) diff --git a/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs index 53cf84f42b9c..9b5e80fa7772 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Certificates/ICertificateConfigLoader.cs @@ -7,8 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Certificates { internal interface ICertificateConfigLoader { - bool IsTestMock { get; } - - X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName); + (X509Certificate2, X509Certificate2Collection) LoadCertificate(CertificateConfig certInfo, string endpointName); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index d749de79f8a9..f3bd571e7178 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -346,6 +346,8 @@ public override int GetHashCode() => HashCode.Combine( // "CertificateName": { // "Path": "testCert.pfx", + // "KeyPath": "", + // "ChainPath": "", // "Password": "testPassword" // } internal class CertificateConfig @@ -380,6 +382,8 @@ internal CertificateConfig() public string Path { get; set; } + public string ChainPath { get; set; } + public string KeyPath { get; set; } public string Password { get; set; } diff --git a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs index c5801c46be1f..a7389f120d2d 100644 --- a/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs +++ b/src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs @@ -49,12 +49,13 @@ public SniOptionsSelector( { var sslOptions = new SslServerAuthenticationOptions { - ServerCertificate = certifcateConfigLoader.LoadCertificate(sniConfig.Certificate, $"{endpointName}:Sni:{name}"), EnabledSslProtocols = sniConfig.SslProtocols ?? fallbackHttpsOptions.SslProtocols, CertificateRevocationCheckMode = fallbackHttpsOptions.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, }; - if (sslOptions.ServerCertificate is null) + var (serverCert, intermediates) = certifcateConfigLoader.LoadCertificate(sniConfig.Certificate, $"{endpointName}:Sni:{name}"); + + if (serverCert is null) { if (fallbackHttpsOptions.ServerCertificate is null && _fallbackServerCertificateSelector is null) { @@ -63,21 +64,18 @@ public SniOptionsSelector( if (_fallbackServerCertificateSelector is null) { - // Cache the fallback ServerCertificate since there's no fallback ServerCertificateSelector taking precedence. - sslOptions.ServerCertificate = fallbackHttpsOptions.ServerCertificate; + // Cache the fallback ServerCertificate since there's no fallback ServerCertificateSelector taking precedence. + serverCert = fallbackHttpsOptions.ServerCertificate; + intermediates = fallbackHttpsOptions.ServerCertificateIntermediates; } } - if (sslOptions.ServerCertificate != null) + if (serverCert != null) { // This might be do blocking IO but it'll resolve the certificate chain up front before any connections are // made to the server - sslOptions.ServerCertificateContext = SslStreamCertificateContext.Create((X509Certificate2)sslOptions.ServerCertificate, additionalCertificates: null); - } - - if (!certifcateConfigLoader.IsTestMock && sslOptions.ServerCertificate is X509Certificate2 cert2) - { - HttpsConnectionMiddleware.EnsureCertificateIsAllowedForServerAuth(cert2); + sslOptions.ServerCertificate = serverCert; + sslOptions.ServerCertificateContext = SslStreamCertificateContext.Create(serverCert, intermediates); } var clientCertificateMode = sniConfig.ClientCertificateMode ?? fallbackHttpsOptions.ClientCertificateMode; diff --git a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs index 9ce6406c4c57..7cc8e2df7470 100644 --- a/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs +++ b/src/Servers/Kestrel/Core/src/KestrelConfigurationLoader.cs @@ -355,8 +355,10 @@ public void Load() } // A cert specified directly on the endpoint overrides any defaults. - httpsOptions.ServerCertificate = CertificateConfigLoader.LoadCertificate(endpoint.Certificate, endpoint.Name) - ?? httpsOptions.ServerCertificate; + var (serverCert, intermediates) = CertificateConfigLoader.LoadCertificate(endpoint.Certificate, endpoint.Name); + + httpsOptions.ServerCertificate = serverCert ?? httpsOptions.ServerCertificate; + httpsOptions.ServerCertificateIntermediates = intermediates ?? httpsOptions.ServerCertificateIntermediates; if (httpsOptions.ServerCertificate == null && httpsOptions.ServerCertificateSelector == null) { @@ -417,7 +419,7 @@ private void LoadDefaultCert() { if (ConfigurationReader.Certificates.TryGetValue("Default", out var defaultCertConfig)) { - var defaultCert = CertificateConfigLoader.LoadCertificate(defaultCertConfig, "Default"); + var (defaultCert, intermediates) = CertificateConfigLoader.LoadCertificate(defaultCertConfig, "Default"); if (defaultCert != null) { DefaultCertificateConfig = defaultCertConfig; diff --git a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs index 6626fd04a04f..edb10efd0fec 100644 --- a/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs +++ b/src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs @@ -105,7 +105,7 @@ public HttpsConnectionMiddleware(ConnectionDelegate next, HttpsConnectionAdapter // This might be do blocking IO but it'll resolve the certificate chain up front before any connections are // made to the server - _serverCertificateContext = SslStreamCertificateContext.Create(certificate, additionalCertificates: null); + _serverCertificateContext = SslStreamCertificateContext.Create(certificate, additionalCertificates: options.ServerCertificateIntermediates); } var remoteCertificateValidationCallback = _options.ClientCertificateMode == ClientCertificateMode.NoCertificate ? diff --git a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs index adbe314802be..b8823eb40ac0 100644 --- a/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs +++ b/src/Servers/Kestrel/Core/test/SniOptionsSelectorTests.cs @@ -751,18 +751,16 @@ private class MockCertificateConfigLoader : ICertificateConfigLoader { public Dictionary CertToPathDictionary { get; } = new Dictionary(ReferenceEqualityComparer.Instance); - public bool IsTestMock => true; - - public X509Certificate2 LoadCertificate(CertificateConfig certInfo, string endpointName) + public (X509Certificate2, X509Certificate2Collection) LoadCertificate(CertificateConfig certInfo, string endpointName) { if (certInfo is null) { - return null; + return (null, null); } var cert = TestResources.GetTestCertificate(); CertToPathDictionary.Add(cert, certInfo.Path); - return cert; + return (cert, null); } } From 124b365f09d181bbb86b97837141c03d2048b767 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 14 Sep 2020 08:40:37 -0700 Subject: [PATCH 2/2] Actually read the chain path section --- .../Kestrel/Core/src/Internal/ConfigurationReader.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs index f3bd571e7178..a9f71f1366e7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs @@ -345,9 +345,9 @@ public override int GetHashCode() => HashCode.Combine( } // "CertificateName": { - // "Path": "testCert.pfx", - // "KeyPath": "", - // "ChainPath": "", + // "Path": "testCert.pem/pfx", + // "KeyPath": "key.pem", + // "ChainPath": "chain.pem", // "Password": "testPassword" // } internal class CertificateConfig @@ -359,6 +359,7 @@ public CertificateConfig(IConfigurationSection configSection) // Bind explictly to preserve linkability Path = configSection[nameof(Path)]; KeyPath = configSection[nameof(KeyPath)]; + ChainPath = configSection[nameof(ChainPath)]; Password = configSection[nameof(Password)]; Subject = configSection[nameof(Subject)]; Store = configSection[nameof(Store)];