Fix If CACertificatePath Is Empty, Verification Fails

QuickFIXn/Transport/SslStreamFactory.cs

@@ -16,8 +16,8 @@ internal sealed class SslStreamFactory
 {
     private readonly SocketSettings _socketSettings;
     private readonly NonSessionLog _nonSessionLog;
-    private const string CLIENT_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.2";
-    private const string SERVER_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.1";
+    internal const string CLIENT_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.2";
+    internal const string SERVER_AUTHENTICATION_OID = "1.3.6.1.5.5.7.3.1";
 
     public SslStreamFactory(SocketSettings settings, NonSessionLog nonSessionLog)
     {
@@ -156,7 +156,7 @@ private bool ValidateClientCertificate(object sender, X509Certificate? certifica
     /// <param name="sslPolicyErrors">The SSL policy errors supplied by .Net.</param>
     /// <param name="enhancedKeyUsage">Enhanced key usage, which the remote computers certificate should contain.</param>
     /// <returns> <c>true</c> if the certificate should be treated as trusted; otherwise <c>false</c> </returns>
-    private bool VerifyRemoteCertificate(
+    internal bool VerifyRemoteCertificate(
         X509Certificate? certificate,
         SslPolicyErrors sslPolicyErrors,
         string enhancedKeyUsage)
@@ -170,43 +170,44 @@ private bool VerifyRemoteCertificate(
 
         // Validate enhanced key usage
         if (!ContainsEnhancedKeyUsage(certificate, enhancedKeyUsage)) {
-            var role = enhancedKeyUsage == CLIENT_AUTHENTICATION_OID ? "client" : "server";
+            var role = enhancedKeyUsage.Equals(CLIENT_AUTHENTICATION_OID, StringComparison.Ordinal) ? "client" : "server";
             _nonSessionLog.OnEvent(
                 $"Remote certificate is not intended for {role} authentication: It is missing enhanced key usage {enhancedKeyUsage}");
             return false;
         }
 
+        // If CA Certificate is specified then validate against the CA certificate, otherwise it is validated against the installed certificates
         if (string.IsNullOrEmpty(_socketSettings.CACertificatePath)) {
             _nonSessionLog.OnEvent("CACertificatePath is not specified");
-            return false;
-        }
-
-        string caCertPath = StringUtil.FixSlashes(_socketSettings.CACertificatePath);
-
-        // If CA Certificate is specified then validate against the CA certificate, otherwise it is validated against the installed certificates
-        X509Certificate2? cert = SslCertCache.LoadCertificate(caCertPath, null);
-        if (cert is null) {
-            _nonSessionLog.OnEvent(
-                $"Certificate '{caCertPath}' could not be loaded from store or path '{Directory.GetCurrentDirectory()}'");
-            return false;
         }
+        else
+        {
+            string caCertPath = StringUtil.FixSlashes(_socketSettings.CACertificatePath);
+
+            X509Certificate2? cert = SslCertCache.LoadCertificate(caCertPath, null);
+            if (cert is null) {
+                _nonSessionLog.OnEvent(
+                    $"Certificate '{caCertPath}' could not be loaded from store or path '{Directory.GetCurrentDirectory()}'");
+                return false;
+            }
 
-        X509Chain chain0 = new X509Chain();
-        chain0.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
-        // add all your extra certificate chain
+            X509Chain chain = new();
+            chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
+            // add all your extra certificate chain
 
-        chain0.ChainPolicy.ExtraStore.Add(cert);
-        chain0.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority;
-        bool isValid = chain0.Build((X509Certificate2)certificate);
+            chain.ChainPolicy.ExtraStore.Add(cert);
+            chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority;
 
-        if (isValid)
-        {
-            // resets the sslPolicyErrors.RemoteCertificateChainErrors status
-            sslPolicyErrors &= ~SslPolicyErrors.RemoteCertificateChainErrors;
-        }
-        else
-        {
-            sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
+            bool isValid = chain.Build((X509Certificate2)certificate);
+            if (isValid)
+            {
+                // resets the sslPolicyErrors.RemoteCertificateChainErrors status
+                sslPolicyErrors &= ~SslPolicyErrors.RemoteCertificateChainErrors;
+            }
+            else
+            {
+                sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
+            }
         }
 
         // Any basic authentication check failed, do after checking CA
@@ -236,7 +237,7 @@ private static bool ContainsEnhancedKeyUsage(X509Certificate certificate, string
             {
                 foreach (System.Security.Cryptography.Oid oid in keyUsage.EnhancedKeyUsages)
                 {
-                    if (oid.Value == enhancedKeyOid)
+                    if (enhancedKeyOid.Equals(oid.Value, StringComparison.Ordinal))
                         return true;
                 }
             }

UnitTests/SslStreamFactoryTest.cs

@@ -0,0 +1,227 @@
+using System.Security.Authentication;
+using System.Security.Cryptography.X509Certificates;
+using System.Security.Cryptography;
+using NUnit.Framework;
+using QuickFix;
+using QuickFix.Logger;
+using QuickFix.Transport;
+using System;
+using System.IO;
+using System.Net.Security;
+using System.Net.Sockets;
+
+namespace UnitTests
+{
+    [TestFixture]
+    public class SslStreamFactoryTest
+    {
+        const string CaCertificatePath = "CaCertificate.cer";
+        const string DifferentCaCertificatePath = "OtherCaCertificate.cer";
+        const string ServerCertificatePath = "serverCertificate.cer";
+        const string ClientCertificatePath = "clientCertificate.cer";
+
+        X509Certificate2 CaCertificate { get; set; } = null!;
+        X509Certificate2 ClientCertificate { get; set; } = null!;
+        X509Certificate2 ServerCertificate { get; set; } = null!;
+
+        [OneTimeSetUp]
+        public void BuildCerts()
+        {
+            var caCertificate = CreateCACertificate();
+            File.WriteAllBytes(CaCertificatePath, caCertificate.Export(X509ContentType.Cert));
+            CaCertificate = caCertificate;
+
+            var serverCertificate = CreateServerCertificate(caCertificate);
+            File.WriteAllBytes(ServerCertificatePath, serverCertificate.Export(X509ContentType.Cert));
+            ServerCertificate = serverCertificate;
+
+            var clientCertificate = CreateClientCertificate(caCertificate);
+            File.WriteAllBytes(ClientCertificatePath, clientCertificate.Export(X509ContentType.Cert));
+            ClientCertificate = clientCertificate;
+
+            var differentCaCertificate = CreateCACertificate();
+            File.WriteAllBytes(DifferentCaCertificatePath, differentCaCertificate.Export(X509ContentType.Cert));
+        }
+
+        [OneTimeTearDown]
+        public void ClearCerts()
+        {
+            File.Delete(CaCertificatePath);
+            File.Delete(DifferentCaCertificatePath);
+            File.Delete(ServerCertificatePath);
+            File.Delete(ClientCertificatePath);
+        }
+
+        static X509Certificate2 CreateCACertificate()
+        {
+            var rsa = RSA.Create();
+            var request = new CertificateRequest("CN=127.0.0.1 Test CA", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+            request.CertificateExtensions.Add(new X509BasicConstraintsExtension(true, false, 0, true));
+            request.CertificateExtensions.Add(new X509KeyUsageExtension(X509KeyUsageFlags.KeyCertSign | X509KeyUsageFlags.CrlSign, true));
+            request.CertificateExtensions.Add(new X509SubjectKeyIdentifierExtension(request.PublicKey, false));
+
+            X509Certificate2 certificate = request.CreateSelfSigned(DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddDays(5));
+            return certificate;
+        }
+
+        static X509Certificate2 CreateServerCertificate(X509Certificate2 caCertificate)
+        {
+            var rsa = RSA.Create();
+            var request = new CertificateRequest($"CN=127.0.0.1 Server Test", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+
+            var enhancedKeyUsages = new OidCollection
+            {
+                new Oid("1.3.6.1.5.5.7.3.1"), // OID for Server Authentication
+            };
+            request.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(enhancedKeyUsages, false));
+
+            X509Certificate2 certificate = request.Create(caCertificate, DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddDays(1), [0, 0, 0, 0, 0, 0, 0, 1]);
+            return certificate;
+        }
+
+        static X509Certificate2 CreateClientCertificate(X509Certificate2 caCertificate)
+        {
+            var rsa = RSA.Create();
+            var request = new CertificateRequest($"CN=127.0.0.1 Client Test", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+
+            var enhancedKeyUsages = new OidCollection
+            {
+                new Oid("1.3.6.1.5.5.7.3.2")  // OID for Client Authentication
+            };
+            request.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(enhancedKeyUsages, false));
+
+            X509Certificate2 certificate = request.Create(caCertificate, DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddDays(1), [0, 0, 0, 0, 0, 0, 0, 2]);
+            return certificate;
+        }
+
+        [Test]
+        public void VerifyServerCertificateChainEnhancedUsage()
+        {
+            SettingsDictionary dict = new();
+            dict.SetBool(SessionSettings.SSL_ENABLE, true);
+            dict.SetBool(SessionSettings.SSL_VALIDATE_CERTIFICATES, true);
+            dict.SetString(SessionSettings.SSL_CERTIFICATE, ServerCertificatePath);
+            dict.SetString(SessionSettings.SSL_CA_CERTIFICATE, CaCertificatePath);
+
+            var settings = new SocketSettings();
+            settings.Configure(dict);
+
+            var logger = new NonSessionLog(new ScreenLogFactory(true, true, true));
+            var factory = new SslStreamFactory(settings, logger);
+
+            var resultServer = factory.VerifyRemoteCertificate(ServerCertificate, SslPolicyErrors.None, SslStreamFactory.SERVER_AUTHENTICATION_OID);
+            var resultClient = factory.VerifyRemoteCertificate(ServerCertificate, SslPolicyErrors.None, SslStreamFactory.CLIENT_AUTHENTICATION_OID);
+
+            Assert.That(resultServer, Is.True);
+            Assert.That(resultClient, Is.False);
+        }
+
+        [Test]
+        public void VerifyServerCertificateChainFailsWithoutCA()
+        {
+            SettingsDictionary dict = new();
+            dict.SetBool(SessionSettings.SSL_ENABLE, true);
+            dict.SetBool(SessionSettings.SSL_VALIDATE_CERTIFICATES, true);
+            dict.SetString(SessionSettings.SSL_CERTIFICATE, ServerCertificatePath);
+
+            var settings = new SocketSettings();
+            settings.Configure(dict);
+
+            var logger = new NonSessionLog(new ScreenLogFactory(true, true, true));
+            var factory = new SslStreamFactory(settings, logger);
+
+            var resultServer = factory.VerifyRemoteCertificate(ServerCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.SERVER_AUTHENTICATION_OID);
+            var resultClient = factory.VerifyRemoteCertificate(ServerCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.CLIENT_AUTHENTICATION_OID);
+
+            Assert.That(resultServer, Is.False);
+            Assert.That(resultClient, Is.False);
+        }
+
+        [Test]
+        public void VerifyServerCertificateChainFailsWithWrongCA()
+        {
+            SettingsDictionary dict = new();
+            dict.SetBool(SessionSettings.SSL_ENABLE, true);
+            dict.SetBool(SessionSettings.SSL_VALIDATE_CERTIFICATES, true);
+            dict.SetString(SessionSettings.SSL_CERTIFICATE, ServerCertificatePath);
+            dict.SetString(SessionSettings.SSL_CA_CERTIFICATE, DifferentCaCertificatePath);
+
+            var settings = new SocketSettings();
+            settings.Configure(dict);
+
+            var logger = new NonSessionLog(new ScreenLogFactory(true, true, true));
+            var factory = new SslStreamFactory(settings, logger);
+
+            var resultServer = factory.VerifyRemoteCertificate(ServerCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.SERVER_AUTHENTICATION_OID);
+            var resultClient = factory.VerifyRemoteCertificate(ServerCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.CLIENT_AUTHENTICATION_OID);
+
+            Assert.That(resultServer, Is.False);
+            Assert.That(resultClient, Is.False);
+        }
+
+        [Test]
+        public void VerifyClientCertificateChainEnhancedUsage()
+        {
+            SettingsDictionary dict = new();
+            dict.SetBool(SessionSettings.SSL_ENABLE, true);
+            dict.SetBool(SessionSettings.SSL_VALIDATE_CERTIFICATES, true);
+            dict.SetString(SessionSettings.SSL_CERTIFICATE, ClientCertificatePath);
+            dict.SetString(SessionSettings.SSL_CA_CERTIFICATE, CaCertificatePath);
+
+            var settings = new SocketSettings();
+            settings.Configure(dict);
+
+            var logger = new NonSessionLog(new ScreenLogFactory(true, true, true));
+            var factory = new SslStreamFactory(settings, logger);
+
+            var resultServer = factory.VerifyRemoteCertificate(ClientCertificate, SslPolicyErrors.None, SslStreamFactory.SERVER_AUTHENTICATION_OID);
+            var resultClient = factory.VerifyRemoteCertificate(ClientCertificate, SslPolicyErrors.None, SslStreamFactory.CLIENT_AUTHENTICATION_OID);
+
+            Assert.That(resultServer, Is.False);
+            Assert.That(resultClient, Is.True);
+        }
+
+        [Test]
+        public void VerifyClientCertificateChainFailsWithoutCA()
+        {
+            SettingsDictionary dict = new();
+            dict.SetBool(SessionSettings.SSL_ENABLE, true);
+            dict.SetBool(SessionSettings.SSL_VALIDATE_CERTIFICATES, true);
+            dict.SetString(SessionSettings.SSL_CERTIFICATE, ClientCertificatePath);
+
+            var settings = new SocketSettings();
+            settings.Configure(dict);
+
+            var logger = new NonSessionLog(new ScreenLogFactory(true, true, true));
+            var factory = new SslStreamFactory(settings, logger);
+
+            var resultServer = factory.VerifyRemoteCertificate(ClientCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.SERVER_AUTHENTICATION_OID);
+            var resultClient = factory.VerifyRemoteCertificate(ClientCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.CLIENT_AUTHENTICATION_OID);
+
+            Assert.That(resultServer, Is.False);
+            Assert.That(resultClient, Is.False);
+        }
+
+        [Test]
+        public void VerifyClientCertificateChainFailsWithWrongCA()
+        {
+            SettingsDictionary dict = new();
+            dict.SetBool(SessionSettings.SSL_ENABLE, true);
+            dict.SetBool(SessionSettings.SSL_VALIDATE_CERTIFICATES, true);
+            dict.SetString(SessionSettings.SSL_CERTIFICATE, ClientCertificatePath);
+            dict.SetString(SessionSettings.SSL_CA_CERTIFICATE, DifferentCaCertificatePath);
+
+            var settings = new SocketSettings();
+            settings.Configure(dict);
+
+            var logger = new NonSessionLog(new ScreenLogFactory(true, true, true));
+            var factory = new SslStreamFactory(settings, logger);
+
+            var resultServer = factory.VerifyRemoteCertificate(ClientCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.SERVER_AUTHENTICATION_OID);
+            var resultClient = factory.VerifyRemoteCertificate(ClientCertificate, SslPolicyErrors.RemoteCertificateChainErrors, SslStreamFactory.CLIENT_AUTHENTICATION_OID);
+
+            Assert.That(resultServer, Is.False);
+            Assert.That(resultClient, Is.False);
+        }
+    }
+}