Fix IsMutuallyAuthenticated on Linux and OSX (#63945)

* WIP - prepared a failing test

* Fix IsMutuallyAuthenticated on Linux

* Fix failing unit tests

* Minor cleanup

* Port changes to OSX

* Fix comment

* Invoke cert selection inline, don't allocate new credentials on Linux/OSX

* Fix tests on OSX

* Code review feedback

* Move tests to separate file

* Fix build

* Fix Failing tests

Author: rzikm

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs

@@ -327,12 +327,9 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia
                         Crypto.ErrClearError();
                     }
 
-                    if (sslAuthenticationOptions.CertSelectionDelegate != null && sslAuthenticationOptions.CertificateContext == null)
-                    {
-                        // We don't have certificate but we have callback. We should wait for remote certificate and
-                        // possible trusted issuer list.
-                        Interop.Ssl.SslSetClientCertCallback(sslHandle, 1);
-                    }
+                    // Set client cert callback, this will interrupt the handshake with SecurityStatusPalErrorCode.CredentialsNeeded
+                    // if server actually requests a certificate.
+                    Ssl.SslSetClientCertCallback(sslHandle, 1);
                 }
 
                 if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired)

src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs

@@ -130,6 +130,7 @@ private static SafeSslHandle CreateSslContext(SafeFreeSslCredentials credential,
                     SetCertificate(sslContext, credential.CertificateContext);
                 }
 
+                Interop.AppleCrypto.SslBreakOnCertRequested(sslContext, true);
                 Interop.AppleCrypto.SslBreakOnServerAuth(sslContext, true);
                 Interop.AppleCrypto.SslBreakOnClientAuth(sslContext, true);
             }

src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs

@@ -295,62 +295,24 @@ private string[] GetRequestCertificateAuthorities()
             return issuers;
         }
 
-        /*++
-            AcquireCredentials - Attempts to find Client Credential
-            Information, that can be sent to the server.  In our case,
-            this is only Client Certificates, that we have Credential Info.
-
-            How it works:
-                case 0: Cert Selection delegate is present
-                        Always use its result as the client cert answer.
-                        Try to use cached credential handle whenever feasible.
-                        Do not use cached anonymous creds if the delegate has returned null
-                        and the collection is not empty (allow responding with the cert later).
-
-                case 1: Certs collection is empty
-                        Always use the same statically acquired anonymous SSL Credential
-
-                case 2: Before our Connection with the Server
-                        If we have a cached credential handle keyed by first X509Certificate
-                        **content** in the passed collection, then we use that cached
-                        credential and hoping to restart a session.
-
-                        Otherwise create a new anonymous (allow responding with the cert later).
-
-                case 3: After our Connection with the Server (i.e. during handshake or re-handshake)
-                        The server has requested that we send it a Certificate then
-                        we Enumerate a list of server sent Issuers trying to match against
-                        our list of Certificates, the first match is sent to the server.
-
-                        Once we got a cert we again try to match cached credential handle if possible.
-                        This will not restart a session but helps minimizing the number of handles we create.
-
-                In the case of an error getting a Certificate or checking its private Key we fall back
-                to the behavior of having no certs, case 1.
-
-            Returns: True if cached creds were used, false otherwise.
-
-        --*/
-
-        private bool AcquireClientCredentials(ref byte[]? thumbPrint)
+        internal X509Certificate2? SelectClientCertificate(out bool sessionRestartAttempt)
         {
-            // Acquire possible Client Certificate information and set it on the handle.
-            X509Certificate? clientCertificate = null;        // This is a candidate that can come from the user callback or be guessed when targeting a session restart.
-            List<X509Certificate>? filteredCerts = null;      // This is an intermediate client certs collection that try to use if no selectedCert is available yet.
-            string[] issuers;                                // This is a list of issuers sent by the server, only valid is we do know what the server cert is.
+            sessionRestartAttempt = false;
 
-            bool sessionRestartAttempt = false; // If true and no cached creds we will use anonymous creds.
+            X509Certificate? clientCertificate = null;        // candidate certificate that can come from the user callback or be guessed when targeting a session restart.
+            X509Certificate2? selectedCert = null;     // final selected cert (ensured that it does have private key with it).
+            List<X509Certificate>? filteredCerts = null;      // This is an intermediate client certs collection that try to use if no selectedCert is available yet.
+            string[] issuers;                                // This is a list of issuers sent by the server, only valid if we do know what the server cert is.
 
             if (_sslAuthenticationOptions.CertSelectionDelegate != null)
             {
-                issuers = GetRequestCertificateAuthorities();
-
                 if (NetEventSource.Log.IsEnabled())
                     NetEventSource.Info(this, "Calling CertificateSelectionCallback");
 
                 X509Certificate2? remoteCert = null;
                 try
                 {
+                    issuers = GetRequestCertificateAuthorities();
                     remoteCert = CertificateValidationPal.GetRemoteCertificate(_securityContext!);
                     if (_sslAuthenticationOptions.ClientCertificates == null)
                     {
@@ -363,7 +325,6 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
                     remoteCert?.Dispose();
                 }
 
-
                 if (clientCertificate != null)
                 {
                     if (_credentialsHandle == null)
@@ -505,9 +466,6 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
                 }
             }
 
-            bool cachedCred = false;                   // This is a return result from this method.
-            X509Certificate2? selectedCert = null;     // This is a final selected cert (ensured that it does have private key with it).
-
             clientCertificate = null;
 
             if (NetEventSource.Log.IsEnabled())
@@ -550,6 +508,56 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
 
             if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"Selected cert = {selectedCert}");
 
+            _selectedClientCertificate = clientCertificate;
+            return selectedCert;
+        }
+
+        /*++
+            AcquireCredentials - Attempts to find Client Credential
+            Information, that can be sent to the server.  In our case,
+            this is only Client Certificates, that we have Credential Info.
+
+            How it works:
+                case 0: Cert Selection delegate is present
+                        Always use its result as the client cert answer.
+                        Try to use cached credential handle whenever feasible.
+                        Do not use cached anonymous creds if the delegate has returned null
+                        and the collection is not empty (allow responding with the cert later).
+
+                case 1: Certs collection is empty
+                        Always use the same statically acquired anonymous SSL Credential
+
+                case 2: Before our Connection with the Server
+                        If we have a cached credential handle keyed by first X509Certificate
+                        **content** in the passed collection, then we use that cached
+                        credential and hoping to restart a session.
+
+                        Otherwise create a new anonymous (allow responding with the cert later).
+
+                case 3: After our Connection with the Server (i.e. during handshake or re-handshake)
+                        The server has requested that we send it a Certificate then
+                        we Enumerate a list of server sent Issuers trying to match against
+                        our list of Certificates, the first match is sent to the server.
+
+                        Once we got a cert we again try to match cached credential handle if possible.
+                        This will not restart a session but helps minimizing the number of handles we create.
+
+                In the case of an error getting a Certificate or checking its private Key we fall back
+                to the behavior of having no certs, case 1.
+
+            Returns: True if cached creds were used, false otherwise.
+
+        --*/
+
+        private bool AcquireClientCredentials(ref byte[]? thumbPrint)
+        {
+            // Acquire possible Client Certificate information and set it on the handle.
+
+            bool sessionRestartAttempt; // If true and no cached creds we will use anonymous creds.
+            bool cachedCred = false;                   // this is a return result from this method.
+
+            X509Certificate2? selectedCert = SelectClientCertificate(out sessionRestartAttempt);
+
             try
             {
                 // Try to locate cached creds first.
@@ -574,22 +582,21 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
                     // So we don't want to reuse **anonymous** cached credential for a new SSL connection if the client has passed some certificate.
                     // The following block happens if client did specify a certificate but no cached creds were found in the cache.
                     // Since we don't restart a session the server side can still challenge for a client cert.
-                    if ((object?)clientCertificate != (object?)selectedCert)
+                    if ((object?)_selectedClientCertificate != (object?)selectedCert)
                     {
                         selectedCert.Dispose();
                     }
 
                     guessedThumbPrint = null;
                     selectedCert = null;
-                    clientCertificate = null;
+                    _selectedClientCertificate = null;
                 }
 
                 if (cachedCredentialHandle != null)
                 {
                     if (NetEventSource.Log.IsEnabled())
                         NetEventSource.Log.UsingCachedCredential(this);
                     _credentialsHandle = cachedCredentialHandle;
-                    _selectedClientCertificate = clientCertificate;
                     cachedCred = true;
                     if (selectedCert != null)
                     {
@@ -607,7 +614,6 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint)
                             _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.EncryptionPolicy, _sslAuthenticationOptions.IsServer);
 
                     thumbPrint = guessedThumbPrint; // Delay until here in case something above threw.
-                    _selectedClientCertificate = clientCertificate;
                 }
             }
             finally
@@ -792,6 +798,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
                     if (_sslAuthenticationOptions.IsServer)
                     {
                         status = SslStreamPal.AcceptSecurityContext(
+                                      this,
                                       ref _credentialsHandle!,
                                       ref _securityContext,
                                       inputBuffer,
@@ -801,6 +808,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
                     else
                     {
                         status = SslStreamPal.InitializeSecurityContext(
+                                       this,
                                        ref _credentialsHandle!,
                                        ref _securityContext,
                                        _sslAuthenticationOptions.TargetHost,
@@ -841,6 +849,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte
         internal SecurityStatusPal Renegotiate(out byte[]? output)
         {
             return SslStreamPal.Renegotiate(
+                                      this,
                                       ref _credentialsHandle!,
                                       ref _securityContext,
                                       _sslAuthenticationOptions,

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs

@@ -25,6 +25,7 @@ public static void VerifyPackageInfo()
         }
 
         public static SecurityStatusPal AcceptSecurityContext(
+            SecureChannel secureChannel,
             ref SafeFreeCredentials credential,
             ref SafeDeleteSslContext? context,
             ReadOnlySpan<byte> inputBuffer,
@@ -35,6 +36,7 @@ public static SecurityStatusPal AcceptSecurityContext(
         }
 
         public static SecurityStatusPal InitializeSecurityContext(
+            SecureChannel secureChannel,
             ref SafeFreeCredentials credential,
             ref SafeDeleteSslContext? context,
             string? targetName,
@@ -45,7 +47,12 @@ public static SecurityStatusPal InitializeSecurityContext(
             return HandshakeInternal(credential, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
 
-        public static SecurityStatusPal Renegotiate(ref SafeFreeCredentials? credentialsHandle, ref SafeDeleteSslContext? context, SslAuthenticationOptions sslAuthenticationOptions, out byte[]? outputBuffer)
+        public static SecurityStatusPal Renegotiate(
+            SecureChannel secureChannel,
+            ref SafeFreeCredentials? credentialsHandle,
+            ref SafeDeleteSslContext? context,
+            SslAuthenticationOptions sslAuthenticationOptions,
+            out byte[]? outputBuffer)
         {
             throw new PlatformNotSupportedException();
         }

src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs

@@ -20,7 +20,7 @@ public static Exception GetException(SecurityStatusPal status)
             return status.Exception ?? new Win32Exception((int)status.ErrorCode);
         }
 
-        internal const bool StartMutualAuthAsAnonymous = false;
+        internal const bool StartMutualAuthAsAnonymous = true;
 
         // SecureTransport is okay with a 0 byte input, but it produces a 0 byte output.
         // Since ST is not producing the framed empty message just call this false and avoid the
@@ -32,27 +32,34 @@ public static void VerifyPackageInfo()
         }
 
         public static SecurityStatusPal AcceptSecurityContext(
+            SecureChannel secureChannel,
             ref SafeFreeCredentials credential,
             ref SafeDeleteSslContext? context,
             ReadOnlySpan<byte> inputBuffer,
             ref byte[]? outputBuffer,
             SslAuthenticationOptions sslAuthenticationOptions)
         {
-            return HandshakeInternal(credential, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
+            return HandshakeInternal(secureChannel, credential, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
 
         public static SecurityStatusPal InitializeSecurityContext(
+            SecureChannel secureChannel,
             ref SafeFreeCredentials credential,
             ref SafeDeleteSslContext? context,
             string? targetName,
             ReadOnlySpan<byte> inputBuffer,
             ref byte[]? outputBuffer,
             SslAuthenticationOptions sslAuthenticationOptions)
         {
-            return HandshakeInternal(credential, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
+            return HandshakeInternal(secureChannel, credential, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
 
-        public static SecurityStatusPal Renegotiate(ref SafeFreeCredentials? credentialsHandle, ref SafeDeleteSslContext? context, SslAuthenticationOptions sslAuthenticationOptions, out byte[]? outputBuffer)
+        public static SecurityStatusPal Renegotiate(
+            SecureChannel secureChannel,
+            ref SafeFreeCredentials? credentialsHandle,
+            ref SafeDeleteSslContext? context,
+            SslAuthenticationOptions sslAuthenticationOptions,
+            out byte[]? outputBuffer)
         {
             throw new PlatformNotSupportedException();
         }
@@ -224,6 +231,7 @@ public static void QueryContextConnectionInfo(
         }
 
         private static SecurityStatusPal HandshakeInternal(
+            SecureChannel secureChannel,
             SafeFreeCredentials credential,
             ref SafeDeleteSslContext? context,
             ReadOnlySpan<byte> inputBuffer,
@@ -268,28 +276,11 @@ private static SecurityStatusPal HandshakeInternal(
                 SecurityStatusPal status = PerformHandshake(sslHandle);
                 if (status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
                 {
-                    // we should not be here if CertSelectionDelegate is null but better check before dereferencing..
-                    if (sslAuthenticationOptions.CertSelectionDelegate != null)
+                    X509Certificate2? clientCertificate = secureChannel.SelectClientCertificate(out _);
+                    if (clientCertificate != null)
                     {
-                        X509Certificate2? remoteCert = null;
-                        try
-                        {
-                            string[] issuers = CertificateValidationPal.GetRequestCertificateAuthorities(context);
-                            remoteCert = CertificateValidationPal.GetRemoteCertificate(context);
-                            if (sslAuthenticationOptions.ClientCertificates == null)
-                            {
-                                sslAuthenticationOptions.ClientCertificates = new X509CertificateCollection();
-                            }
-                            X509Certificate2 clientCertificate = (X509Certificate2)sslAuthenticationOptions.CertSelectionDelegate(sslAuthenticationOptions.TargetHost!, sslAuthenticationOptions.ClientCertificates, remoteCert, issuers);
-                            if (clientCertificate != null)
-                            {
-                                SafeDeleteSslContext.SetCertificate(sslContext.SslContext,  SslStreamCertificateContext.Create(clientCertificate));
-                            }
-                        }
-                        finally
-                        {
-                            remoteCert?.Dispose();
-                        }
+                        sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(clientCertificate);
+                        SafeDeleteSslContext.SetCertificate(sslContext.SslContext, sslAuthenticationOptions.CertificateContext);
                     }
 
                     // We either got certificate or we can proceed without it. It is up to the server to decide if either is OK.