[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 <Aliaksei.Minchankou@nreca.coop>
Co-authored-by: Aaron Stannard <aaron@petabridge.com>

* SSL

* (Parameter 'certificatePath')

* TestKit

* Fixed up assertion to no longer be whitespace sensitive

* [Bug] using FluentAssertions;

Co-authored-by: aminchenkov <alexei.minchenkov@gmail.com>
Co-authored-by: Aliaksei Minchankou <Aliaksei.Minchankou@nreca.coop>
Co-authored-by: Aaron Stannard <aaron@petabridge.com>

Author: 4 people

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>(), "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>(), "echo");
+            var echo = _sys2.ActorOf(Props.Create<Echo>(), "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>(), "echo");
+            var echo = _sys2.ActorOf(Props.Create<Echo>(), "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<RemoteTransportException>(() =>
             {
-                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<AggregateException>( () =>
+            {
+                Setup(true, null, Password);
+            });
+
+            var realException = GetInnerMostException<ArgumentNullException>(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<AggregateException>(() =>
+            {
+                Setup(true, ValidCertPath, null);
+            });
 
+            var realException = GetInnerMostException<CryptographicException>(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<T>(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()
             {

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),