Added test case

Author: Tien Nguyen

src/AdoNetCore.AseClient/AseConnection.cs

@@ -627,6 +627,9 @@ public AseTransaction Transaction
             }
         }
 
+        /// <summary>
+        /// Allow consumer to override the default certificate validation
+        /// </summary>
         public RemoteCertificateValidationCallback UserCertificateValidationCallback { get; set; }
 
     }
@@ -674,25 +677,13 @@ public AseTransaction Transaction
     /// </remarks>
     public delegate void TraceExitEventHandler(AseConnection connection, object source, string method, object returnValue);
 
-    //
-    // Summary:
-    //     Verifies the remote Secure Sockets Layer (SSL) certificate used for authentication.
-    //
-    // Parameters:
-    //   sender:
-    //     An object that contains state information for this validation.
-    //
-    //   certificate:
-    //     The certificate used to authenticate the remote party.
-    //
-    //   chain:
-    //     The chain of certificate authorities associated with the remote certificate.
-    //
-    //   sslPolicyErrors:
-    //     One or more errors associated with the remote certificate.
-    //
-    // Returns:
-    //     A System.Boolean value that determines whether the specified certificate is accepted
-    //     for authentication.
+    /// <summary>
+    /// Verifies the remote Secure Sockets Layer (SSL) certificate used for authentication.
+    /// </summary>
+    /// <param name="sender">An object that contains state information for this validation.</param>
+    /// <param name="certificate">The certificate used to authenticate the remote party.</param>
+    /// <param name="chain">The chain of certificate authorities associated with the remote certificate.</param>
+    /// <param name="sslPolicyErrors">One or more errors associated with the remote certificate.</param>
+    /// <returns>A System.Boolean value that determines whether the specified certificate is accepted for authentication.</returns>
     public delegate bool RemoteCertificateValidationCallback(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors);
 }

src/AdoNetCore.AseClient/Internal/InternalConnectionFactory.cs

@@ -19,7 +19,8 @@ namespace AdoNetCore.AseClient.Internal
     internal class InternalConnectionFactory : IInternalConnectionFactory
     {
         private readonly IConnectionParameters _parameters;
-        private readonly RemoteCertificateValidationCallback _userCertificateValidationCallback;
+        private readonly System.Net.Security.RemoteCertificateValidationCallback _userCertificateValidationCallback;
+
 
 #if ENABLE_ARRAY_POOL
         private readonly System.Buffers.ArrayPool<byte> _arrayPool;
@@ -33,10 +34,7 @@ public InternalConnectionFactory(IConnectionParameters parameters, RemoteCertifi
 #endif
         {
             _parameters = parameters;
-            if (userCertificateValidationCallback == null)
-                userCertificateValidationCallback = GetDefaultUserCertificateValidationCallback();
-
-            _userCertificateValidationCallback = userCertificateValidationCallback;
+            _userCertificateValidationCallback = userCertificateValidationCallback == null ? UserCertificateValidationCallback : new System.Net.Security.RemoteCertificateValidationCallback(userCertificateValidationCallback);
 
 #if ENABLE_ARRAY_POOL
             _arrayPool = arrayPool;
@@ -116,7 +114,7 @@ private InternalConnection CreateConnection(Socket socket, CancellationToken tok
 
                 if (_parameters.Encryption)
                 {
-                    sslStream = new SslStream(networkStream, false, _userCertificateValidationCallback.Invoke);
+                    sslStream = new SslStream(networkStream, false, _userCertificateValidationCallback);
 
                     var authenticate = sslStream.AuthenticateAsClientAsync(_parameters.Server);
 
@@ -187,32 +185,30 @@ private InternalConnection CreateConnectionInternal(Stream networkStream)
         }
 
 
-        private RemoteCertificateValidationCallback GetDefaultUserCertificateValidationCallback()
+        private bool UserCertificateValidationCallback(object sender, X509Certificate serverCertificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
         {
-            //object sender, X509Certificate serverCertificate, X509Chain chain, SslPolicyErrors sslPolicyErrors
-            return (sender, serverCertificate, chain, sslPolicyErrors) => {
-                var certificateChainPolicyErrors = (sslPolicyErrors & SslPolicyErrors.RemoteCertificateChainErrors) == SslPolicyErrors.RemoteCertificateChainErrors;
-                var otherPolicyErrors = (sslPolicyErrors & ~SslPolicyErrors.RemoteCertificateChainErrors) != SslPolicyErrors.None;
+            var certificateChainPolicyErrors = (sslPolicyErrors & SslPolicyErrors.RemoteCertificateChainErrors) == SslPolicyErrors.RemoteCertificateChainErrors;
+            var otherPolicyErrors = (sslPolicyErrors & ~SslPolicyErrors.RemoteCertificateChainErrors) != SslPolicyErrors.None;
 
-                // We're not concerned with chain errors as we verify the chain below.
-                if (otherPolicyErrors)
-                {
-                    Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(GetDefaultUserCertificateValidationCallback)} secure connection failed due to policy errors: {sslPolicyErrors}");
-                    return false;
-                }
+            // We're not concerned with chain errors as we verify the chain below.
+            if (otherPolicyErrors)
+            {
+                Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(UserCertificateValidationCallback)} secure connection failed due to policy errors: {sslPolicyErrors}");
+                return false;
+            }
 
-                var mergedStatusFlags = X509ChainStatusFlags.NoError;
-                foreach (var status in chain.ChainStatus)
-                {
-                    mergedStatusFlags |= status.Status;
-                }
+            var mergedStatusFlags = X509ChainStatusFlags.NoError;
+            foreach (var status in chain.ChainStatus)
+            {
+                mergedStatusFlags |= status.Status;
+            }
 
-                var trustedCerts = LoadTrustedFile(_parameters.TrustedFile);
-                if (trustedCerts == null)
-                {
-                    Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(GetDefaultUserCertificateValidationCallback)} secure connection failed due to missing TrustedFile parameter.");
-                    return false;
-                }
+            var trustedCerts = LoadTrustedFile(_parameters.TrustedFile);
+            if (trustedCerts == null)
+            {
+                Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(UserCertificateValidationCallback)} secure connection failed due to missing TrustedFile parameter.");
+                return false;
+            }
 
 #if !(NETCOREAPP1_0 || NETCOREAPP1_1) // these frameworks do not have the following X509Certificate2 constructor...
             // sometimes the chain policy is only a partial chain because it doesn't include a self signed root?
@@ -233,41 +229,40 @@ private RemoteCertificateValidationCallback GetDefaultUserCertificateValidationC
             }
 #endif
 
-                var untrustedRootChainStatusFlags = (mergedStatusFlags & X509ChainStatusFlags.UntrustedRoot) == X509ChainStatusFlags.UntrustedRoot;
-                var otherChainStatusFlags = (mergedStatusFlags & ~X509ChainStatusFlags.UntrustedRoot) != X509ChainStatusFlags.NoError;
+            var untrustedRootChainStatusFlags = (mergedStatusFlags & X509ChainStatusFlags.UntrustedRoot) == X509ChainStatusFlags.UntrustedRoot;
+            var otherChainStatusFlags = (mergedStatusFlags & ~X509ChainStatusFlags.UntrustedRoot) != X509ChainStatusFlags.NoError;
 
-                if (otherChainStatusFlags)
-                {
-                    Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(GetDefaultUserCertificateValidationCallback)} secure connection failed due to chain status: {mergedStatusFlags}");
-                    return false;
-                }
+            if (otherChainStatusFlags)
+            {
+                Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(UserCertificateValidationCallback)} secure connection failed due to chain status: {mergedStatusFlags}");
+                return false;
+            }
 
-                if (!(certificateChainPolicyErrors || untrustedRootChainStatusFlags))
-                {
-                    //No chain Errors, we will trust the server certificate.
-                    return true;
-                }
+            if (!(certificateChainPolicyErrors || untrustedRootChainStatusFlags))
+            {
+                //No chain Errors, we will trust the server certificate.
+                return true;
+            }
 
-                // If any certificates in the chain are trusted, then we will trust the server certificate.
-                // To do this fairly quickly we can check if thumbprints exist in the set of trusted roots.
-                var set = new HashSet<string>(trustedCerts.Select(c => c.Thumbprint));
+            // If any certificates in the chain are trusted, then we will trust the server certificate.
+            // To do this fairly quickly we can check if thumbprints exist in the set of trusted roots.
+            var set = new HashSet<string>(trustedCerts.Select(c => c.Thumbprint));
 
-                // the chain is in an array from leaf at 0 to root at [count - 1]
-                // looping from end to start should find cases generated according to sybase documentation on the first attempt
-                // but it is possible that someone puts an intermediate or even the leaf cert in their trusted file
-                for (int i = chain.ChainElements.Count - 1; i >= 0; i--)
+            // the chain is in an array from leaf at 0 to root at [count - 1]
+            // looping from end to start should find cases generated according to sybase documentation on the first attempt
+            // but it is possible that someone puts an intermediate or even the leaf cert in their trusted file
+            for (int i = chain.ChainElements.Count - 1; i >= 0; i--)
+            {
+                var potentialTrusted = chain.ChainElements[i].Certificate.Thumbprint;
+                if (set.Contains(potentialTrusted))
                 {
-                    var potentialTrusted = chain.ChainElements[i].Certificate.Thumbprint;
-                    if (set.Contains(potentialTrusted))
-                    {
-                        return true;
-                    }
+                    return true;
                 }
+            }
 
-                Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(GetDefaultUserCertificateValidationCallback)} secure connection failed due to missing root or intermediate certificate in the certificate store, or the TrustedFile.");
+            Logger.Instance?.WriteLine($"{nameof(InternalConnectionFactory)}.{nameof(UserCertificateValidationCallback)} secure connection failed due to missing root or intermediate certificate in the certificate store, or the TrustedFile.");
 
-                return false;
-            };
+            return false;
         }
 
         private static X509Certificate2[] LoadTrustedFile(string trustedFile)

test/AdoNetCore.AseClient.Tests/Integration/ConnectionTests.cs

@@ -43,5 +43,21 @@ public void OpenConnection_ToNonListeningServer_ThrowsAseException()
                 Assert.AreEqual(30294, ex.Errors[0].MessageNumber);
             }
         }
+
+        [Test]
+        //Note: for this to work, we would need a sybase SSL setup with a separate SSL port
+        //for this test, no certificate on the consuming side
+        public void UserCertificateValidationCallback_ShouldWork()
+        {
+            var isDelegateCalled = false;
+
+            using (var connection = new AseConnection(ConnectionStrings.Tls))
+            {
+                connection.UserCertificateValidationCallback = (o, cert, chain, pol) => { isDelegateCalled = true; return true; };
+
+                connection.Open();
+                Assert.True(isDelegateCalled);
+            }
+        }
     }
 }

test/AdoNetCore.AseClient.Tests/Unit/AseConnectionTests.cs

@@ -173,14 +173,13 @@ public void ChangeDatabase_WhenOpen_Succeeds()
 
             var connection = new AseConnection("Data Source=myASEserver;Port=5000;Database=foo;Uid=myUsername;Pwd=myPassword;", mockConnectionPoolManager.Object);
 
-            //connection.UserCertificateValidationCallback = (sender, certificate, chain, errors) => true;
             // Act
             connection.Open();
             connection.ChangeDatabase("bar");
-
             // Assert
             // No error...
         }
+
         [Test]
         public void ChangeDatabase_WhenNotOpen_Succeeds()
         {