Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
}
89 changes: 85 additions & 4 deletions src/ServiceControl.Infrastructure/HttpsSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,7 +26,7 @@ public HttpsSettings(SettingsRootNamespace rootNamespace)
CertificatePath = SettingsReader.Read<string>(rootNamespace, "Https.CertificatePath");
CertificatePassword = SettingsReader.Read<string>(rootNamespace, "Https.CertificatePassword");

ValidateCertificateConfiguration();
Certificate = LoadCertificate();
}

// HTTPS redirection - disabled by default for backwards compatibility
Expand All @@ -45,7 +47,7 @@ public HttpsSettings(SettingsRootNamespace rootNamespace)
public bool Enabled { get; }

/// <summary>
/// Path to the HTTPS certificate file (.pfx or .pem).
/// Path to the HTTPS certificate file (PKCS#12 / .pfx).
/// Required when Https.Enabled is true.
/// </summary>
public string CertificatePath { get; }
Expand All @@ -57,6 +59,12 @@ public HttpsSettings(SettingsRootNamespace rootNamespace)
[JsonIgnore]
public string CertificatePassword { get; }

/// <summary>
/// The certificate loaded from <see cref="CertificatePath"/>, or null when HTTPS is disabled.
/// </summary>
[JsonIgnore]
public X509Certificate2 Certificate { get; }

/// <summary>
/// When true, HTTP requests will be redirected to HTTPS.
/// Requires HTTPS to be properly configured. Default is false.
Expand Down Expand Up @@ -88,11 +96,11 @@ public HttpsSettings(SettingsRootNamespace rootNamespace)
/// </summary>
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);
}
Expand All @@ -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<X509EnhancedKeyUsageExtension>())
{
hasEkuExtension = true;

foreach (var oid in extension.EnhancedKeyUsages)
{
if (string.Equals(oid.Value, ServerAuthenticationOid, StringComparison.Ordinal))
{
return true;
}
}
}

return !hasEkuExtension;
}

void LogConfiguration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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<InvalidOperationException>(() => 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<InvalidOperationException>(() => 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()
{
Expand Down