-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix: Validate SSL certificate private key access at server startup #7847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Aaronontheweb
merged 4 commits into
akkadotnet:dev
from
Aaronontheweb:fix-tls-certificate-validation
Oct 2, 2025
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
062275c
Fix: Validate SSL certificate private key access at server startup
Aaronontheweb b4be13b
Update DotNettyTlsHandshakeFailureSpec to validate fail-fast behavior
Aaronontheweb 5d6979a
Merge branch 'dev' into fix-tls-certificate-validation
Aaronontheweb a59279e
Add ECDSA private key validation and improve disposal pattern
Aaronontheweb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
132 changes: 132 additions & 0 deletions
132
src/core/Akka.Remote.Tests/Transport/DotNettyCertificateValidationSpec.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| //----------------------------------------------------------------------- | ||
| // <copyright file="DotNettyCertificateValidationSpec.cs" company="Akka.NET Project"> | ||
| // Copyright (C) 2009-2022 Lightbend Inc. <http://www.lightbend.com> | ||
| // Copyright (C) 2013-2025 .NET Foundation <https://github.com/akkadotnet/akka.net> | ||
| // </copyright> | ||
| //----------------------------------------------------------------------- | ||
|
|
||
| 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 | ||
| { | ||
| /// <summary> | ||
| /// Tests that SSL certificate validation happens at startup, not during runtime. | ||
| /// This ensures fail-fast behavior when certificates are misconfigured. | ||
| /// </summary> | ||
| 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<AggregateException>(() => | ||
| { | ||
| 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<ConfigurationException>(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); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -180,6 +180,13 @@ protected async Task<IChannel> NewServer(EndPoint listenAddress) | |
|
|
||
| public override async Task<(Address, TaskCompletionSource<IAssociationEventListener>)> Listen() | ||
| { | ||
| // Validate SSL certificate before starting server | ||
| // This ensures fail-fast behavior if private key is inaccessible | ||
| if (Settings.EnableSsl) | ||
| { | ||
| Settings.Ssl.ValidateCertificate(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate the private key during server binding, which should trigger shutdown of the |
||
| } | ||
|
|
||
| EndPoint listenAddress; | ||
| if (IPAddress.TryParse(Settings.Hostname, out var ip)) | ||
| listenAddress = new IPEndPoint(ip, Settings.Port); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This spec is either somewhat redundant or unnecessary given the fail fast implementation - so it's been simplified.