From 9c7cb3548dc6064cf07d988c32314ceacd4a0300 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 2 Oct 2025 14:57:05 -0500 Subject: [PATCH] Fix: Validate SSL certificate private key access at server startup (#7847) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: Validate SSL certificate private key access at server startup **Problem**: Akka.Remote server starts successfully even when the application lacks permissions to access the SSL certificate's private key. The server appears healthy but fails when clients attempt to connect, making issues hard to diagnose. **Root Cause**: Certificate loading in DotNettyTransportSettings only validates that the certificate EXISTS in the Windows certificate store, not whether the application can ACCESS the private key. Private key access is checked separately by Windows ACL, which can fail even when Certificate.HasPrivateKey returns true. **Solution**: 1. Add ValidateCertificate() method to SslSettings class that: - Checks Certificate.HasPrivateKey - Actually tests private key access with GetRSAPrivateKey() (not just presence) - Throws ConfigurationException with clear error message on failure 2. Call validation in Listen() method before server socket binds: - Ensures fail-fast behavior at startup - Prevents server from running in broken state - Provides clear error message for administrators 3. Add comprehensive tests: - Server should fail at startup with inaccessible private key - Server should start successfully with valid certificate - Server should start successfully without SSL **Impact**: - Existing misconfigured deployments will now fail at startup (correct behavior) - Clear error messages guide administrators to fix permissions - No breaking changes for correctly configured systems - Related to Freshdesk #538 (BNSF Railway) Fixes #538 * Update DotNettyTlsHandshakeFailureSpec to validate fail-fast behavior **Changes**: 1. Renamed first test to `Server_should_fail_at_startup_with_certificate_without_private_key` - Now validates that server FAILS AT STARTUP with bad certificate - Tests fail-fast behavior instead of runtime TLS handshake failure 2. Removed redundant `Server_side_tls_handshake_failure_should_shutdown_server` test - This test validated the OLD (incorrect) behavior where server starts successfully - Now impossible with fail-fast validation in place - Scenario already covered by the updated first test 3. Kept `Client_side_tls_handshake_failure_should_shutdown_client` unchanged - Still valid - tests client-side validation failure - Not affected by server startup validation **Result**: Tests now validate correct fail-fast behavior at server startup * Add ECDSA private key validation and improve disposal pattern Addresses review feedback from @Arkatufus: **Changes**: 1. Check both RSA and ECDSA private keys - SslStream supports both RSA and ECDSA certificates - GetRSAPrivateKey() returns null for ECDSA certs (and vice versa) - Validation now checks both key types to match TLS handler behavior 2. Use `using` statements for proper disposal - Prevents resource leaks if exception is thrown - Both rsaKey and ecdsaKey are properly disposed - Exception-safe resource management **TLS Handler Relationship**: The TLS handler uses `TlsHandler.Server(Settings.Ssl.Certificate)` which internally extracts either RSA or ECDSA private keys via SslStream. Our validation now matches this behavior by checking both key types. **Behavior**: - RSA certificate: GetRSAPrivateKey() succeeds, GetECDsaPrivateKey() returns null ✅ - ECDSA certificate: GetECDsaPrivateKey() succeeds, GetRSAPrivateKey() returns null ✅ - Neither accessible: Both return null, validation fails with clear error ✅ - Permission denied: CryptographicException caught, clear error message ✅ --- .../DotNettyCertificateValidationSpec.cs | 132 ++++++++++++++++++ .../DotNettyTlsHandshakeFailureSpec.cs | 117 ++-------------- .../Transport/DotNetty/DotNettyTransport.cs | 7 + .../DotNetty/DotNettyTransportSettings.cs | 46 ++++++ 4 files changed, 199 insertions(+), 103 deletions(-) create mode 100644 src/core/Akka.Remote.Tests/Transport/DotNettyCertificateValidationSpec.cs diff --git a/src/core/Akka.Remote.Tests/Transport/DotNettyCertificateValidationSpec.cs b/src/core/Akka.Remote.Tests/Transport/DotNettyCertificateValidationSpec.cs new file mode 100644 index 00000000000..b4dcf64c630 --- /dev/null +++ b/src/core/Akka.Remote.Tests/Transport/DotNettyCertificateValidationSpec.cs @@ -0,0 +1,132 @@ +//----------------------------------------------------------------------- +// +// Copyright (C) 2009-2022 Lightbend Inc. +// Copyright (C) 2013-2025 .NET Foundation +// +//----------------------------------------------------------------------- + +using System; +using System.IO; +using System.Security.Cryptography.X509Certificates; +using Akka.Actor; +using Akka.Configuration; +using Akka.TestKit; +using Xunit; +using Xunit.Abstractions; + +namespace Akka.Remote.Tests.Transport +{ + /// + /// Tests that SSL certificate validation happens at startup, not during runtime. + /// This ensures fail-fast behavior when certificates are misconfigured. + /// + public class DotNettyCertificateValidationSpec : AkkaSpec + { + private const string ValidCertPath = "Resources/akka-validcert.pfx"; + private const string Password = "password"; + private static readonly string NoKeyCertPath = Path.Combine("Resources", "validation-no-key.cer"); + + public DotNettyCertificateValidationSpec(ITestOutputHelper output) : base(ConfigurationFactory.Empty, output) + { + } + + private static Config CreateConfig(bool enableSsl, string certPath, string certPassword) + { + var baseConfig = ConfigurationFactory.ParseString(@"akka { + loglevel = DEBUG + actor.provider = ""Akka.Remote.RemoteActorRefProvider,Akka.Remote"" + remote.dot-netty.tcp { + port = 0 + hostname = ""127.0.0.1"" + enable-ssl = " + (enableSsl ? "on" : "off") + @" + log-transport = off + } + }"); + + if (!enableSsl || string.IsNullOrEmpty(certPath)) + return baseConfig; + + var escapedPath = certPath.Replace("\\", "\\\\"); + var ssl = $@"akka.remote.dot-netty.tcp.ssl {{ + suppress-validation = on + certificate {{ + path = ""{escapedPath}"" + password = ""{certPassword ?? string.Empty}"" + }} + }}"; + return baseConfig.WithFallback(ssl); + } + + private static void CreateCertificateWithoutPrivateKey() + { + var fullCert = new X509Certificate2(ValidCertPath, Password, X509KeyStorageFlags.Exportable); + var publicKeyBytes = fullCert.Export(X509ContentType.Cert); + var dir = Path.GetDirectoryName(NoKeyCertPath); + if (!string.IsNullOrEmpty(dir) && !Directory.Exists(dir)) + Directory.CreateDirectory(dir); + File.WriteAllBytes(NoKeyCertPath, publicKeyBytes); + } + + [Fact] + public void Server_should_fail_at_startup_with_certificate_without_private_key() + { + CreateCertificateWithoutPrivateKey(); + + try + { + // Server with cert that has no private key should FAIL TO START + var serverConfig = CreateConfig(true, NoKeyCertPath, null); + + // This should throw an exception during ActorSystem.Create (wrapped in AggregateException) + var aggregateEx = Assert.Throws(() => + { + using var server = ActorSystem.Create("ServerSystem", serverConfig); + }); + + // Unwrap the inner exception + var innerEx = aggregateEx.InnerException ?? aggregateEx; + while (innerEx is AggregateException agg && agg.InnerException != null) + innerEx = agg.InnerException; + + // Should be ConfigurationException about private key + Assert.IsType(innerEx); + Assert.Contains("private key", innerEx.Message, StringComparison.OrdinalIgnoreCase); + } + finally + { + try + { + if (File.Exists(NoKeyCertPath)) + File.Delete(NoKeyCertPath); + } + catch { /* ignore */ } + } + } + + [Fact] + public void Server_should_start_successfully_with_valid_certificate() + { + // Server with valid cert should start normally + var serverConfig = CreateConfig(true, ValidCertPath, Password); + + using var server = ActorSystem.Create("ServerSystem", serverConfig); + InitializeLogger(server); + + // Server should be running + Assert.False(server.WhenTerminated.IsCompleted); + } + + [Fact] + public void Server_should_start_successfully_without_ssl() + { + // Server without SSL should start normally + var serverConfig = CreateConfig(false, null, null); + + using var server = ActorSystem.Create("ServerSystem", serverConfig); + InitializeLogger(server); + + // Server should be running + Assert.False(server.WhenTerminated.IsCompleted); + } + } +} diff --git a/src/core/Akka.Remote.Tests/Transport/DotNettyTlsHandshakeFailureSpec.cs b/src/core/Akka.Remote.Tests/Transport/DotNettyTlsHandshakeFailureSpec.cs index 9332445cf63..f88647f79d1 100644 --- a/src/core/Akka.Remote.Tests/Transport/DotNettyTlsHandshakeFailureSpec.cs +++ b/src/core/Akka.Remote.Tests/Transport/DotNettyTlsHandshakeFailureSpec.cs @@ -68,122 +68,32 @@ private static void CreateCertificateWithoutPrivateKey() [Fact] - public async Task Tls_handshake_failure_should_be_logged_and_shutdown_server() + public async Task Server_should_fail_at_startup_with_certificate_without_private_key() { CreateCertificateWithoutPrivateKey(); - ActorSystem server = null; - ActorSystem client = null; - try { - // Start TLS server with a cert that has no private key + // Server with cert that has no private key should FAIL TO START var serverConfig = CreateConfig(true, NoKeyCertPath, null, suppressValidation: true); - server = ActorSystem.Create("ServerSystem", serverConfig); - InitializeLogger(server, "[SERVER] "); - - // Server started - add an echo actor and subscribe to errors - server.ActorOf(Props.Create(() => new EchoActor()), "echo"); - - var errorProbe = CreateTestProbe(server); - server.EventStream.Subscribe(errorProbe.Ref, typeof(Event.Error)); - - // Start client with valid TLS cert - var clientConfig = CreateConfig(true, ValidCertPath, Password, suppressValidation: true); - client = ActorSystem.Create("ClientSystem", clientConfig); - InitializeLogger(client, "[CLIENT] "); - - var serverAddress = RARP.For(server).Provider.DefaultAddress; - var echoPath = new RootActorPath(serverAddress) / "user" / "echo"; - var echoSel = client.ActorSelection(echoPath); - - // Trigger association attempt - var probe = CreateTestProbe(client); - echoSel.Tell("ping", probe.Ref); - - // Expect server to log TLS handshake failure promptly - var err = errorProbe.ExpectMsg(TimeSpan.FromSeconds(10)); - var msg = err.ToString(); - Assert.Contains("TLS handshake failed", msg, StringComparison.OrdinalIgnoreCase); - - // Server should shutdown due to TLS failure - await AwaitAssertAsync(async () => + // ActorSystem.Create should throw during startup due to certificate validation + var aggregateEx = Assert.Throws(() => { - Assert.True(server.WhenTerminated.IsCompleted); - await Task.CompletedTask; - }, TimeSpan.FromSeconds(10), TimeSpan.FromMilliseconds(100)); - } - finally - { - if (client != null) - Shutdown(client, TimeSpan.FromSeconds(10)); - if (server != null) - Shutdown(server, TimeSpan.FromSeconds(10)); - try - { - if (File.Exists(NoKeyCertPath)) - File.Delete(NoKeyCertPath); - } catch { /* ignore */ } - } - await Task.CompletedTask; - } + using var server = ActorSystem.Create("ServerSystem", serverConfig); + }); - [Fact] - public async Task Server_side_tls_handshake_failure_should_shutdown_server() - { - CreateCertificateWithoutPrivateKey(); + // Unwrap to find the ConfigurationException + var innerEx = aggregateEx.InnerException ?? aggregateEx; + while (innerEx is AggregateException agg && agg.InnerException != null) + innerEx = agg.InnerException; - ActorSystem server = null; - ActorSystem client = null; - - try - { - // Server with invalid server cert (no private key) -> server TLS handshake fails - var serverConfig = CreateConfig(true, NoKeyCertPath, null, suppressValidation: true); - server = ActorSystem.Create("ServerSystem", serverConfig); - InitializeLogger(server, "[SERVER] "); - - // Client with valid cert - var clientConfig = CreateConfig(true, ValidCertPath, Password, suppressValidation: true); - client = ActorSystem.Create("ClientSystem", clientConfig); - InitializeLogger(client, "[CLIENT] "); - - // Echo actor on server and client - var serverEcho = server.ActorOf(Props.Create(() => new EchoActor()), "echo"); - var clientEcho = client.ActorOf(Props.Create(() => new EchoActor()), "echo"); - - var serverAddr = RARP.For(server).Provider.DefaultAddress; - var clientAddr = RARP.For(client).Provider.DefaultAddress; - - var serverEchoPath = new RootActorPath(serverAddr) / "user" / "echo"; - var clientEchoPath = new RootActorPath(clientAddr) / "user" / "echo"; - - // Subscribe to server errors to ensure TLS handshake failure is observed - var serverErrorProbe = CreateTestProbe(server); - server.EventStream.Subscribe(serverErrorProbe.Ref, typeof(Event.Error)); - - // Trigger inbound handshake failure on server: client tries to talk to server - var clientProbe = CreateTestProbe(client); - client.ActorSelection(serverEchoPath).Tell("ping", clientProbe.Ref); - - // Expect server to log TLS handshake failure promptly - var err = await serverErrorProbe.ExpectMsgAsync(TimeSpan.FromSeconds(10)); - Assert.Contains("TLS handshake failed", err.ToString(), StringComparison.OrdinalIgnoreCase); - - // Server should shutdown due to TLS failure - await AwaitAssertAsync(async () => - { - Assert.True(server.WhenTerminated.IsCompleted); - await Task.CompletedTask; - }, TimeSpan.FromSeconds(10), TimeSpan.FromMilliseconds(100)); + // Should be ConfigurationException about private key + Assert.IsType(innerEx); + Assert.Contains("private key", innerEx.Message, StringComparison.OrdinalIgnoreCase); } finally { - if (client != null) - Shutdown(client, TimeSpan.FromSeconds(10)); - if (server != null) - Shutdown(server, TimeSpan.FromSeconds(10)); try { if (File.Exists(NoKeyCertPath)) @@ -191,6 +101,7 @@ await AwaitAssertAsync(async () => } catch { /* ignore */ } } + await Task.CompletedTask; } [Fact] diff --git a/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransport.cs b/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransport.cs index c62da27b042..566321fdcbc 100644 --- a/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransport.cs +++ b/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransport.cs @@ -180,6 +180,13 @@ protected async Task NewServer(EndPoint listenAddress) public override async Task<(Address, TaskCompletionSource)> Listen() { + // Validate SSL certificate before starting server + // This ensures fail-fast behavior if private key is inaccessible + if (Settings.EnableSsl) + { + Settings.Ssl.ValidateCertificate(); + } + EndPoint listenAddress; if (IPAddress.TryParse(Settings.Hostname, out var ip)) listenAddress = new IPEndPoint(ip, Settings.Port); diff --git a/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs b/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs index 26135f86c0d..c84bac48360 100644 --- a/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs +++ b/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs @@ -342,6 +342,52 @@ public SslSettings(X509Certificate2 certificate, bool suppressValidation) SuppressValidation = suppressValidation; } + /// + /// Validates that the SSL certificate has an accessible private key. + /// Should be called before starting the server to ensure proper TLS configuration. + /// + /// + /// Thrown when certificate lacks private key or application cannot access it. + /// + public void ValidateCertificate() + { + if (Certificate == null) + return; // No SSL configured + + if (!Certificate.HasPrivateKey) + { + throw new ConfigurationException( + "SSL certificate does not have a private key. " + + "Ensure certificate is installed with private key permissions."); + } + + // Actually test private key access (not just presence) + // SslStream supports both RSA and ECDSA keys - check both types + try + { + using (var rsaKey = Certificate.GetRSAPrivateKey()) + using (var ecdsaKey = Certificate.GetECDsaPrivateKey()) + { + // Certificate must have either RSA or ECDSA private key accessible + if (rsaKey == null && ecdsaKey == null) + { + throw new ConfigurationException( + "Cannot access private key for SSL certificate. " + + "Certificate has private key but application lacks permissions to access it. " + + "Verify application has permissions to the certificate's private key."); + } + // Successfully accessed private key - validation passed + } + } + catch (System.Security.Cryptography.CryptographicException ex) + { + throw new ConfigurationException( + "SSL certificate private key exists but cannot be accessed. " + + "Verify application user has permissions to the private key in certificate store. " + + $"Error: {ex.Message}", ex); + } + } + private SslSettings(string certificateThumbprint, string storeName, StoreLocation storeLocation, bool suppressValidation) { using var store = new X509Store(storeName, storeLocation);