diff --git a/src/ServiceControl.Hosting/Https/HostApplicationBuilderExtensions.cs b/src/ServiceControl.Hosting/Https/HostApplicationBuilderExtensions.cs index 1f8df12831..f5152869ed 100644 --- a/src/ServiceControl.Hosting/Https/HostApplicationBuilderExtensions.cs +++ b/src/ServiceControl.Hosting/Https/HostApplicationBuilderExtensions.cs @@ -1,7 +1,6 @@ namespace ServiceControl.Hosting.Https; using System; -using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.HttpsPolicy; @@ -35,23 +34,18 @@ public static void AddServiceControlHttps(this WebApplicationBuilder hostBuilder // Kestrel HTTPS is disabled by default if (settings.Enabled) { + // The certificate was loaded and validated when HttpsSettings was constructed. Doing it + // here instead would defer the failure to endpoint binding, which happens after every + // hosted service has already started and has to be torn down again. + var certificate = settings.Certificate ?? throw new InvalidOperationException("HTTPS is enabled but no certificate was loaded."); + hostBuilder.WebHost.ConfigureKestrel(kestrel => { kestrel.ConfigureHttpsDefaults(httpsOptions => { - httpsOptions.ServerCertificate = LoadCertificate(settings); + httpsOptions.ServerCertificate = certificate; }); }); } } - - static X509Certificate2 LoadCertificate(HttpsSettings settings) - { - if (string.IsNullOrEmpty(settings.CertificatePassword)) - { - return X509CertificateLoader.LoadPkcs12FromFile(settings.CertificatePath, null); - } - - return X509CertificateLoader.LoadPkcs12FromFile(settings.CertificatePath, settings.CertificatePassword); - } } diff --git a/src/ServiceControl.Infrastructure/HttpsSettings.cs b/src/ServiceControl.Infrastructure/HttpsSettings.cs index 1db700c3c8..87c1001ca0 100644 --- a/src/ServiceControl.Infrastructure/HttpsSettings.cs +++ b/src/ServiceControl.Infrastructure/HttpsSettings.cs @@ -2,6 +2,8 @@ namespace ServiceControl.Infrastructure; using System; using System.IO; +using System.Linq; +using System.Security.Cryptography.X509Certificates; using System.Text.Json.Serialization; using Microsoft.Extensions.Logging; using ServiceControl.Configuration; @@ -24,7 +26,7 @@ public HttpsSettings(SettingsRootNamespace rootNamespace) CertificatePath = SettingsReader.Read(rootNamespace, "Https.CertificatePath"); CertificatePassword = SettingsReader.Read(rootNamespace, "Https.CertificatePassword"); - ValidateCertificateConfiguration(); + Certificate = LoadCertificate(); } // HTTPS redirection - disabled by default for backwards compatibility @@ -45,7 +47,7 @@ public HttpsSettings(SettingsRootNamespace rootNamespace) public bool Enabled { get; } /// - /// Path to the HTTPS certificate file (.pfx or .pem). + /// Path to the HTTPS certificate file (PKCS#12 / .pfx). /// Required when Https.Enabled is true. /// public string CertificatePath { get; } @@ -57,6 +59,12 @@ public HttpsSettings(SettingsRootNamespace rootNamespace) [JsonIgnore] public string CertificatePassword { get; } + /// + /// The certificate loaded from , or null when HTTPS is disabled. + /// + [JsonIgnore] + public X509Certificate2 Certificate { get; } + /// /// When true, HTTP requests will be redirected to HTTPS. /// Requires HTTPS to be properly configured. Default is false. @@ -88,11 +96,11 @@ public HttpsSettings(SettingsRootNamespace rootNamespace) /// public bool HstsIncludeSubDomains { get; } - void ValidateCertificateConfiguration() + X509Certificate2 LoadCertificate() { if (string.IsNullOrWhiteSpace(CertificatePath)) { - var message = "Https.CertificatePath is required when HTTPS is enabled. Please specify the path to a valid HTTPS certificate file (.pfx or .pem)"; + var message = "Https.CertificatePath is required when HTTPS is enabled. Please specify the path to a valid PKCS#12 (.pfx) certificate file"; logger.LogCritical(message); throw new InvalidOperationException(message); } @@ -103,6 +111,79 @@ void ValidateCertificateConfiguration() logger.LogCritical(message); throw new InvalidOperationException(message); } + + // Loaded here rather than when Kestrel binds its endpoints: an unusable certificate is a + // configuration error, and binding happens only after every hosted service has started. + X509Certificate2 certificate; + try + { + certificate = string.IsNullOrEmpty(CertificatePassword) + ? X509CertificateLoader.LoadPkcs12FromFile(CertificatePath, null) + : X509CertificateLoader.LoadPkcs12FromFile(CertificatePath, CertificatePassword); + } + catch (Exception ex) + { + // .NET reports several unrelated causes as "the password may be incorrect", so describe + // the file itself too. Never the password, only whether one was configured. + var cause = ex.GetBaseException(); + var file = new FileInfo(CertificatePath); + var message = $"The HTTPS certificate could not be loaded, so this instance cannot start. " + + $"Https.CertificatePath: '{CertificatePath}' ({file.Length} bytes, last modified {file.LastWriteTimeUtc:u}). " + + $"Https.CertificatePassword configured: {!string.IsNullOrEmpty(CertificatePassword)}. " + + $"{cause.GetType().Name}: {cause.Message} " + + $"Check that the file is a PKCS#12/PFX holding both the certificate and its private key, and that Https.CertificatePassword matches it. " + + $"To start without HTTPS while investigating, set Https.Enabled to false."; + logger.LogCritical(message); + throw new InvalidOperationException(message, ex); + } + + // Kestrel does not check this when binding. Without the private key every TLS handshake + // fails instead, which surfaces only as clients being unable to connect. + if (!certificate.HasPrivateKey) + { + var message = $"The HTTPS certificate does not contain a private key, so this instance cannot start. " + + $"Https.CertificatePath: '{CertificatePath}' (subject '{certificate.Subject}', thumbprint {certificate.Thumbprint}). " + + $"Export the certificate as PKCS#12/PFX including its private key. " + + $"To start without HTTPS while investigating, set Https.Enabled to false."; + logger.LogCritical(message); + throw new InvalidOperationException(message); + } + + // Kestrel applies this rule when the HTTPS endpoint is bound; checking it here reports it + // before any hosted service has started. A certificate without an EKU extension is accepted. + if (!IsAllowedForServerAuthentication(certificate)) + { + var message = $"The HTTPS certificate cannot be used for server authentication, so this instance cannot start. " + + $"Https.CertificatePath: '{CertificatePath}' (subject '{certificate.Subject}', thumbprint {certificate.Thumbprint}). " + + $"Its Extended Key Usage extension does not include Server Authentication (OID {ServerAuthenticationOid}). " + + $"To start without HTTPS while investigating, set Https.Enabled to false."; + logger.LogCritical(message); + throw new InvalidOperationException(message); + } + + return certificate; + } + + const string ServerAuthenticationOid = "1.3.6.1.5.5.7.3.1"; + + static bool IsAllowedForServerAuthentication(X509Certificate2 certificate) + { + var hasEkuExtension = false; + + foreach (var extension in certificate.Extensions.OfType()) + { + hasEkuExtension = true; + + foreach (var oid in extension.EnhancedKeyUsages) + { + if (string.Equals(oid.Value, ServerAuthenticationOid, StringComparison.Ordinal)) + { + return true; + } + } + } + + return !hasEkuExtension; } void LogConfiguration() diff --git a/src/ServiceControl.UnitTests/Infrastructure/Settings/HttpsSettingsTests.cs b/src/ServiceControl.UnitTests/Infrastructure/Settings/HttpsSettingsTests.cs index 8a0730e49b..ce9b94ed01 100644 --- a/src/ServiceControl.UnitTests/Infrastructure/Settings/HttpsSettingsTests.cs +++ b/src/ServiceControl.UnitTests/Infrastructure/Settings/HttpsSettingsTests.cs @@ -2,6 +2,8 @@ namespace ServiceControl.UnitTests.Infrastructure.Settings; using System; using System.IO; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; using NUnit.Framework; using ServiceControl.Configuration; using ServiceControl.Infrastructure; @@ -22,9 +24,27 @@ public class HttpsSettingsTests string tempCertPath; [SetUp] - public void SetUp() => - // Create a temporary file to simulate a certificate file - tempCertPath = Path.GetTempFileName(); + public void SetUp() + { + // The certificate is loaded as part of validation, so tests that get that far need a real PFX + tempCertPath = Path.Combine(Path.GetTempPath(), $"sc-test-{Guid.NewGuid():n}.pfx"); + WritePfx(tempCertPath); + } + + static void WritePfx(string path, string password = null, string enhancedKeyUsageOid = null) + { + using var key = RSA.Create(2048); + var request = new CertificateRequest("CN=ServiceControl.Tests", key, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + + if (enhancedKeyUsageOid != null) + { + request.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension([new Oid(enhancedKeyUsageOid)], critical: false)); + } + + using var certificate = request.CreateSelfSigned(DateTimeOffset.UtcNow.AddDays(-1), DateTimeOffset.UtcNow.AddYears(1)); + + File.WriteAllBytes(path, certificate.Export(X509ContentType.Pkcs12, password)); + } [TearDown] public void TearDown() @@ -89,6 +109,8 @@ public void Should_read_certificate_path() [Test] public void Should_read_certificate_password() { + WritePfx(tempCertPath, "my-secret-password"); + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_ENABLED", "true"); Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_CERTIFICATEPATH", tempCertPath); Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_CERTIFICATEPASSWORD", "my-secret-password"); @@ -119,6 +141,56 @@ public void Should_throw_when_certificate_path_does_not_exist() Assert.That(ex.Message, Does.Contain("does not exist")); } + [Test] + public void Should_load_certificate_when_https_enabled() + { + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_ENABLED", "true"); + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_CERTIFICATEPATH", tempCertPath); + + var settings = new HttpsSettings(TestNamespace); + + Assert.That(settings.Certificate, Is.Not.Null); + } + + [Test] + public void Should_throw_when_certificate_cannot_be_loaded() + { + WritePfx(tempCertPath, "correct-password"); + + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_ENABLED", "true"); + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_CERTIFICATEPATH", tempCertPath); + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_CERTIFICATEPASSWORD", "wrong-password"); + + var ex = Assert.Throws(() => new HttpsSettings(TestNamespace)); + + using (Assert.EnterMultipleScope()) + { + Assert.That(ex.Message, Does.Contain("could not be loaded")); + Assert.That(ex.Message, Does.Contain(tempCertPath)); + Assert.That(ex.Message, Does.Contain("Https.CertificatePassword configured: True")); + Assert.That(ex.Message, Does.Not.Contain("correct-password")); + Assert.That(ex.Message, Does.Not.Contain("wrong-password")); + } + } + + [Test] + public void Should_throw_when_certificate_is_not_valid_for_server_authentication() + { + const string clientAuthenticationOid = "1.3.6.1.5.5.7.3.2"; + WritePfx(tempCertPath, enhancedKeyUsageOid: clientAuthenticationOid); + + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_ENABLED", "true"); + Environment.SetEnvironmentVariable("SERVICECONTROL_HTTPS_CERTIFICATEPATH", tempCertPath); + + var ex = Assert.Throws(() => new HttpsSettings(TestNamespace)); + + using (Assert.EnterMultipleScope()) + { + Assert.That(ex.Message, Does.Contain("server authentication")); + Assert.That(ex.Message, Does.Contain(tempCertPath)); + } + } + [Test] public void Should_enable_redirect_when_configured() {