From 2d4cbb5ac98ea575210fae915fb59039846893d5 Mon Sep 17 00:00:00 2001 From: Ebere Abanonu Date: Mon, 18 Jul 2022 13:09:48 +0100 Subject: [PATCH] [BACKPORT #6038] SSL Configuration fails even EnbleSsl property is set to false (#6043) * SSL Configuration Fails even EnbleSsl property is set to false (#6038) * Respect EnableSsl configuration propert * Update DotNettySslSupportSpec.cs * Update DotNettySslSupportSpec.cs * Update DotNettyTransportSettings.cs * Moved enableSsl variable initialization outside return statement Co-authored-by: Aliaksei Minchankou Co-authored-by: Aaron Stannard * SSL * (Parameter 'certificatePath') * TestKit * Fixed up assertion to no longer be whitespace sensitive * [Bug] using FluentAssertions; Co-authored-by: aminchenkov Co-authored-by: Aliaksei Minchankou Co-authored-by: Aaron Stannard --- .../Transport/DotNettySslSupportSpec.cs | 143 +++++++++++++++--- .../DotNetty/DotNettyTransportSettings.cs | 6 +- 2 files changed, 123 insertions(+), 26 deletions(-) diff --git a/src/core/Akka.Remote.Tests/Transport/DotNettySslSupportSpec.cs b/src/core/Akka.Remote.Tests/Transport/DotNettySslSupportSpec.cs index 8375caa452f..4cda9ab0470 100644 --- a/src/core/Akka.Remote.Tests/Transport/DotNettySslSupportSpec.cs +++ b/src/core/Akka.Remote.Tests/Transport/DotNettySslSupportSpec.cs @@ -6,12 +6,16 @@ //----------------------------------------------------------------------- using System; +using System.Linq; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; using Akka.Actor; using Akka.Configuration; using Akka.TestKit; using Xunit; using Xunit.Abstractions; +using FluentAssertions; using static Akka.Util.RuntimeDetector; namespace Akka.Remote.Tests.Transport @@ -21,7 +25,7 @@ public class DotNettySslSupportSpec : AkkaSpec #region Setup / Config // valid to 01/01/2037 - private static readonly string ValidCertPath = "Resources/akka-validcert.pfx"; + private const string ValidCertPath = "Resources/akka-validcert.pfx"; private const string Password = "password"; @@ -52,6 +56,32 @@ private static Config TestConfig(string certPath, string password) }"); } + private static Config TestConfig(bool enableSsl, string certPath, string password) + { + var config = 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.ToString().ToLowerInvariant() + @""" + log-transport = true + } + } + }"); + return !enableSsl + ? config + : config.WithFallback(@"akka.remote.dot-netty.tcp.ssl { + suppress-validation = """ + enableSsl.ToString().ToLowerInvariant() + @""" + certificate { + path = """ + certPath + @""" + password = """ + password + @""" + } + }"); + } + private static Config TestThumbprintConfig(string thumbPrint) { var config = ConfigurationFactory.ParseString(@" @@ -80,35 +110,47 @@ private static Config TestThumbprintConfig(string thumbPrint) }"); } - private ActorSystem sys2; - private Address address1; - private Address address2; + private ActorSystem _sys2; + private Address _address1; + private Address _address2; - private ActorPath echoPath; + private ActorPath _echoPath; private void Setup(string certPath, string password) { - sys2 = ActorSystem.Create("sys2", TestConfig(certPath, password)); - InitializeLogger(sys2); + _sys2 = ActorSystem.Create("sys2", TestConfig(certPath, password)); + InitializeLogger(_sys2); + + var echo = _sys2.ActorOf(Props.Create(), "echo"); + + _address1 = RARP.For(Sys).Provider.DefaultAddress; + _address2 = RARP.For(_sys2).Provider.DefaultAddress; + _echoPath = new RootActorPath(_address2) / "user" / "echo"; + } + + private void Setup(bool enableSsl, string certPath, string password) + { + _sys2 = ActorSystem.Create("sys2", TestConfig(enableSsl, certPath, password)); + InitializeLogger(_sys2); - var echo = sys2.ActorOf(Props.Create(), "echo"); + var echo = _sys2.ActorOf(Props.Create(), "echo"); - address1 = RARP.For(Sys).Provider.DefaultAddress; - address2 = RARP.For(sys2).Provider.DefaultAddress; - echoPath = new RootActorPath(address2) / "user" / "echo"; + _address1 = RARP.For(Sys).Provider.DefaultAddress; + _address2 = RARP.For(_sys2).Provider.DefaultAddress; + _echoPath = new RootActorPath(_address2) / "user" / "echo"; } private void SetupThumbprint(string certPath, string password) { InstallCert(); - sys2 = ActorSystem.Create("sys2", TestThumbprintConfig(Thumbprint)); - InitializeLogger(sys2); + _sys2 = ActorSystem.Create("sys2", TestThumbprintConfig(Thumbprint)); + InitializeLogger(_sys2); - var echo = sys2.ActorOf(Props.Create(), "echo"); + var echo = _sys2.ActorOf(Props.Create(), "echo"); - address1 = RARP.For(Sys).Provider.DefaultAddress; - address2 = RARP.For(sys2).Provider.DefaultAddress; - echoPath = new RootActorPath(address2) / "user" / "echo"; + _address1 = RARP.For(Sys).Provider.DefaultAddress; + _address2 = RARP.For(_sys2).Provider.DefaultAddress; + _echoPath = new RootActorPath(_address2) / "user" / "echo"; } #endregion @@ -134,12 +176,12 @@ public void Secure_transport_should_be_possible_between_systems_sharing_the_same AwaitAssert(() => { - Sys.ActorSelection(echoPath).Tell("hello", probe.Ref); + Sys.ActorSelection(_echoPath).Tell("hello", probe.Ref); probe.ExpectMsg("hello", TimeSpan.FromSeconds(3)); }, TimeSpan.FromSeconds(30), TimeSpan.FromMilliseconds(100)); } - [Fact] + [Fact(Skip = "Racy in Azure AzDo CI/CD")] public void Secure_transport_should_be_possible_between_systems_using_thumbprint() { // skip this test due to linux/mono certificate issues @@ -154,7 +196,7 @@ public void Secure_transport_should_be_possible_between_systems_using_thumbprint { AwaitAssert(() => { - Sys.ActorSelection(echoPath).Tell("hello", probe.Ref); + Sys.ActorSelection(_echoPath).Tell("hello", probe.Ref); probe.ExpectMsg("hello", TimeSpan.FromMilliseconds(100)); }, TimeSpan.FromSeconds(3), TimeSpan.FromMilliseconds(100)); }); @@ -173,20 +215,62 @@ public void Secure_transport_should_NOT_be_possible_between_systems_using_SSL_an var probe = CreateTestProbe(); Assert.Throws(() => { - Sys.ActorSelection(echoPath).Tell("hello", probe.Ref); + Sys.ActorSelection(_echoPath).Tell("hello", probe.Ref); probe.ExpectNoMsg(); }); } - #region helper classes / methods + [Fact] + public void If_EnableSsl_configuration_is_true_but_not_valid_certificate_is_provided_than_ArgumentNullException_should_be_thrown() + { + // skip this test due to linux/mono certificate issues + if (IsMono) return; + + var aggregateException = Assert.Throws( () => + { + Setup(true, null, Password); + }); + + var realException = GetInnerMostException(aggregateException); + Assert.NotNull(realException); + realException.Message.Should().Contain("Path to SSL certificate was not found (by default it can be found under `akka.remote.dot-netty.tcp.ssl.certificate.path`"); + } + + [Fact] + public void If_EnableSsl_configuration_is_true_but_not_valid_certificate_password_is_provided_than_WindowsCryptographicException_should_be_thrown() + { + // skip this test due to linux/mono certificate issues + if (IsMono) return; + + var aggregateException = Assert.Throws(() => + { + Setup(true, ValidCertPath, null); + }); + var realException = GetInnerMostException(aggregateException); + Assert.NotNull(realException); + // TODO: this error message is not correct, but wanted to keep this assertion here in case someone else + // wants to fix it in the future. + //Assert.Equal("The specified network password is not correct.", realException.Message); + } + + [Theory] + [InlineData(ValidCertPath, null)] + [InlineData(null, Password)] + [InlineData(null, null)] + [InlineData(ValidCertPath, Password)] + public void If_EnableSsl_configuration_is_false_than_no_exception_should_be_thrown_even_no_cert_detail_were_provided(string certPath, string password) + { + Setup(false, certPath, password); + } + #region helper classes / methods protected override void Dispose(bool disposing) { base.Dispose(disposing); if (disposing) { - Shutdown(sys2, TimeSpan.FromSeconds(3)); + Shutdown(_sys2, TimeSpan.FromSeconds(3)); } } @@ -217,7 +301,18 @@ private void RemoveCert() } } - public class Echo : ReceiveActor + private T GetInnerMostException(Exception ex) where T : Exception + { + Exception currentEx = ex; + while (currentEx.InnerException != null) + { + currentEx = currentEx.InnerException; + } + + return currentEx as T; + } + + private class Echo : ReceiveActor { public Echo() { diff --git a/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs b/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs index d680534a1b8..5c8e0753926 100644 --- a/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs +++ b/src/core/Akka.Remote/Transport/DotNetty/DotNettyTransportSettings.cs @@ -76,9 +76,11 @@ public static DotNettyTransportSettings Create(Config config) var batchWriterSettings = new BatchWriterSettings(config.GetConfig("batching")); + var enableSsl = config.GetBoolean("enable-ssl", false); + return new DotNettyTransportSettings( transportMode: transportMode == "tcp" ? TransportMode.Tcp : TransportMode.Udp, - enableSsl: config.GetBoolean("enable-ssl", false), + enableSsl: enableSsl, connectTimeout: config.GetTimeSpan("connection-timeout", TimeSpan.FromSeconds(15)), hostname: host, publicHostname: !string.IsNullOrEmpty(publicHost) ? publicHost : host, @@ -87,7 +89,7 @@ public static DotNettyTransportSettings Create(Config config) serverSocketWorkerPoolSize: ComputeWorkerPoolSize(config.GetConfig("server-socket-worker-pool")), clientSocketWorkerPoolSize: ComputeWorkerPoolSize(config.GetConfig("client-socket-worker-pool")), maxFrameSize: ToNullableInt(config.GetByteSize("maximum-frame-size", null)) ?? 128000, - ssl: config.HasPath("ssl") ? SslSettings.Create(config.GetConfig("ssl")) : SslSettings.Empty, + ssl: config.HasPath("ssl") && enableSsl ? SslSettings.Create(config.GetConfig("ssl")) : SslSettings.Empty, dnsUseIpv6: config.GetBoolean("dns-use-ipv6", false), tcpReuseAddr: ResolveTcpReuseAddrOption(config.GetString("tcp-reuse-addr", "off-for-windows")), tcpKeepAlive: config.GetBoolean("tcp-keepalive", true),