avoid allocation of SelectClientCertificate delegate (#81096)

* avoid allocation of SelectClientCertificate delegate

* cleanup

diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs
index 839cb0f639e940721283920ad6f02538a4987a52..29140a3a06ca826c72aecd989db71f0bd6997722 100644 (file)
--- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs
+++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs
@@ -13,8 +13,6 @@ using System.Security.Cryptography.X509Certificates;
 
 namespace System.Net.Security
 {
-    internal delegate X509Certificate2? SelectClientCertificate(out bool sessionRestartAttempt);
-
     public partial class SslStream
     {
         private SafeFreeCredentials? _credentialsHandle;
@@ -256,10 +254,8 @@ namespace System.Net.Security
             return issuers;
         }
 
-        internal X509Certificate2? SelectClientCertificate(out bool sessionRestartAttempt)
+        internal X509Certificate2? SelectClientCertificate()
         {
-            sessionRestartAttempt = false;
-
             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.
@@ -275,7 +271,6 @@ namespace System.Net.Security
                 // private key, so we don't have to do any further processing.
                 //
 
-                sessionRestartAttempt = _credentialsHandle == null;
                 _selectedClientCertificate = _sslAuthenticationOptions.CertificateContext.Certificate;
                 if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"Selected cert = {_selectedClientCertificate}");
                 return _sslAuthenticationOptions.CertificateContext.Certificate;
@@ -300,11 +295,6 @@ namespace System.Net.Security
 
                 if (clientCertificate != null)
                 {
-                    if (_credentialsHandle == null)
-                    {
-                        sessionRestartAttempt = true;
-                    }
-
                     EnsureInitialized(ref filteredCerts).Add(clientCertificate);
                     if (NetEventSource.Log.IsEnabled())
                         NetEventSource.Log.CertificateFromDelegate(this);
@@ -315,8 +305,6 @@ namespace System.Net.Security
                     {
                         if (NetEventSource.Log.IsEnabled())
                             NetEventSource.Log.NoDelegateNoClientCert(this);
-
-                        sessionRestartAttempt = true;
                     }
                     else
                     {
@@ -330,7 +318,6 @@ namespace System.Net.Security
                 // This is where we attempt to restart a session by picking the FIRST cert from the collection.
                 // Otherwise it is either server sending a client cert request or the session is renegotiated.
                 clientCertificate = _sslAuthenticationOptions.ClientCertificates[0];
-                sessionRestartAttempt = true;
                 if (clientCertificate != null)
                 {
                     EnsureInitialized(ref filteredCerts).Add(clientCertificate);
@@ -526,11 +513,25 @@ namespace System.Net.Security
         private bool AcquireClientCredentials(ref byte[]? thumbPrint, bool newCredentialsRequested = false)
         {
             // 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);
+            X509Certificate2? selectedCert = SelectClientCertificate();
+
+            if (newCredentialsRequested)
+            {
+                if (selectedCert != null)
+                {
+                    // build the cert context only if it was not provided by the user
+                    _sslAuthenticationOptions.CertificateContext ??= SslStreamCertificateContext.Create(selectedCert);
+                }
+
+                if (SslStreamPal.TryUpdateClintCertificate(_credentialsHandle, _securityContext, _sslAuthenticationOptions))
+                {
+                    // If the certificate was updated we do not need to deal with the credential handle.
+                    return false;
+                }
+            }
+
             try
             {
                 // Try to locate cached creds first.
@@ -548,7 +549,7 @@ namespace System.Net.Security
                 // We can probably do some optimization here. If the selectedCert is returned by the delegate
                 // we can always go ahead and use the certificate to create our credential
                 // (instead of going anonymous as we do here).
-                if (sessionRestartAttempt &&
+                if (!newCredentialsRequested &&
                     cachedCredentialHandle == null &&
                     selectedCert != null &&
                     SslStreamPal.StartMutualAuthAsAnonymous)
@@ -853,9 +854,7 @@ namespace System.Net.Security
                                        _sslAuthenticationOptions.TargetHost,
                                        inputBuffer,
                                        ref result,
-                                       _sslAuthenticationOptions,
-                                       SelectClientCertificate
-                                       );
+                                       _sslAuthenticationOptions);
 
                         if (status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
                         {
@@ -869,10 +868,9 @@ namespace System.Net.Security
                                        ref _credentialsHandle!,
                                        ref _securityContext,
                                        _sslAuthenticationOptions.TargetHost,
-                                       inputBuffer,
+                                       ReadOnlySpan<byte>.Empty,
                                        ref result,
-                                       _sslAuthenticationOptions,
-                                       SelectClientCertificate);
+                                       _sslAuthenticationOptions);
                         }
                     }
                 } while (cachedCreds && _credentialsHandle == null);

diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs
index a7a35fb98c4feef71ee7cf99e836ca59659274da..882034acf4ecca7bf24e24a21a7e855800a7d9ba 100644 (file)
--- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs
+++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs
@@ -51,8 +51,7 @@ namespace System.Net.Security
             string? targetName,
             ReadOnlySpan<byte> inputBuffer,
             ref byte[]? outputBuffer,
-            SslAuthenticationOptions sslAuthenticationOptions,
-            SelectClientCertificate? clientCertificateSelectionCallback)
+            SslAuthenticationOptions sslAuthenticationOptions)
         {
             return HandshakeInternal(credential, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
@@ -178,6 +177,14 @@ namespace System.Net.Security
             connectionInfo.UpdateSslConnectionInfo(securityContext.SslContext);
         }
 
+        public static bool TryUpdateClintCertificate(
+            SafeFreeCredentials? _1,
+            SafeDeleteSslContext? _2,
+            SslAuthenticationOptions _3)
+        {
+            return false;
+        }
+
         private static SecurityStatusPal HandshakeInternal(
             SafeFreeCredentials credential,
             ref SafeDeleteSslContext? context,

diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs
index efa33eaef46b9527f1b840bb56467e1e362c3a3a..0e8796737f2faf7a5f45dadc24e9e9fd88ded8bc 100644 (file)
--- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs
+++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs
@@ -93,7 +93,7 @@ namespace System.Net.Security
             ref byte[]? outputBuffer,
             SslAuthenticationOptions sslAuthenticationOptions)
         {
-            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, null);
+            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
 
         public static SecurityStatusPal InitializeSecurityContext(
@@ -102,10 +102,9 @@ namespace System.Net.Security
             string? _ /*targetName*/,
             ReadOnlySpan<byte> inputBuffer,
             ref byte[]? outputBuffer,
-            SslAuthenticationOptions sslAuthenticationOptions,
-            SelectClientCertificate clientCertificateSelectionCallback)
+            SslAuthenticationOptions sslAuthenticationOptions)
         {
-            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, clientCertificateSelectionCallback);
+            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
 
         public static SecurityStatusPal Renegotiate(
@@ -273,12 +272,26 @@ namespace System.Net.Security
             connectionInfo.UpdateSslConnectionInfo(securityContext);
         }
 
+        public static bool TryUpdateClintCertificate(
+            SafeFreeCredentials? _,
+            SafeDeleteSslContext? context,
+            SslAuthenticationOptions sslAuthenticationOptions)
+        {
+            SafeDeleteSslContext? sslContext = ((SafeDeleteSslContext?)context);
+
+            if (sslAuthenticationOptions.CertificateContext != null)
+            {
+                SafeDeleteSslContext.SetCertificate(sslContext!.SslContext, sslAuthenticationOptions.CertificateContext);
+            }
+
+            return true;
+        }
+
         private static SecurityStatusPal HandshakeInternal(
             ref SafeDeleteSslContext? context,
             ReadOnlySpan<byte> inputBuffer,
             ref byte[]? outputBuffer,
-            SslAuthenticationOptions sslAuthenticationOptions,
-            SelectClientCertificate? clientCertificateSelectionCallback)
+            SslAuthenticationOptions sslAuthenticationOptions)
         {
             try
             {
@@ -301,24 +314,8 @@ namespace System.Net.Security
                 if (status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
                 {
                     // this should happen only for clients
-                    Debug.Assert(clientCertificateSelectionCallback != null);
-
-                    // The callback also saves the selected cert in SslStream.LocalCertificate
-                    X509Certificate2? clientCertificate = clientCertificateSelectionCallback(out bool _);
-
-                    if (clientCertificate != null)
-                    {
-                        // build the cert context only if it was not provided by the user
-                        sslAuthenticationOptions.CertificateContext ??= SslStreamCertificateContext.Create(clientCertificate);
-                    }
-
-                    if (sslAuthenticationOptions.CertificateContext != null)
-                    {
-                        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.
-                    status = PerformHandshake(sslHandle);
+                    Debug.Assert(sslAuthenticationOptions.IsClient);
+                    return status;
                 }
 
                 outputBuffer = sslContext.ReadPendingWrites();

diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs
index c894dad2abdf708dc7ebc0226b3892e2ece92c73..4812bdbb789f24804d57f2533fdb970eb4a7bf13 100644 (file)
--- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs
+++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs
@@ -41,7 +41,7 @@ namespace System.Net.Security
             ref byte[]? outputBuffer,
             SslAuthenticationOptions sslAuthenticationOptions)
         {
-            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, null);
+            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
 
         public static SecurityStatusPal InitializeSecurityContext(
@@ -50,10 +50,9 @@ namespace System.Net.Security
             string? _ /*targetName*/,
             ReadOnlySpan<byte> inputBuffer,
             ref byte[]? outputBuffer,
-            SslAuthenticationOptions sslAuthenticationOptions,
-            SelectClientCertificate? clientCertificateSelectionCallback)
+            SslAuthenticationOptions sslAuthenticationOptions)
         {
-            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, clientCertificateSelectionCallback);
+            return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions);
         }
 
         public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions _1, bool _2)
@@ -148,7 +147,7 @@ namespace System.Net.Security
             {
                 return status;
             }
-            return HandshakeInternal(ref context!, null, ref outputBuffer, sslAuthenticationOptions, null);
+            return HandshakeInternal(ref context!, null, ref outputBuffer, sslAuthenticationOptions);
         }
 
         public static void QueryContextStreamSizes(SafeDeleteContext? _ /*securityContext*/, out StreamSizes streamSizes)
@@ -161,8 +160,18 @@ namespace System.Net.Security
             connectionInfo.UpdateSslConnectionInfo((SafeSslHandle)securityContext);
         }
 
-        private static SecurityStatusPal HandshakeInternal(ref SafeDeleteSslContext? context,
-            ReadOnlySpan<byte> inputBuffer, ref byte[]? outputBuffer, SslAuthenticationOptions sslAuthenticationOptions, SelectClientCertificate? clientCertificateSelectionCallback)
+        public static bool TryUpdateClintCertificate(
+            SafeFreeCredentials? _,
+            SafeDeleteSslContext? context,
+            SslAuthenticationOptions sslAuthenticationOptions)
+        {
+            Interop.OpenSsl.UpdateClientCertificate((SafeSslHandle)context!, sslAuthenticationOptions);
+
+            return true;
+        }
+
+         private static SecurityStatusPal HandshakeInternal(ref SafeDeleteSslContext? context,
+            ReadOnlySpan<byte> inputBuffer, ref byte[]? outputBuffer, SslAuthenticationOptions sslAuthenticationOptions)
         {
             byte[]? output = null;
             int outputSize = 0;
@@ -179,19 +188,8 @@ namespace System.Net.Security
                 if (errorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
                 {
                     // this should happen only for clients
-                    Debug.Assert(clientCertificateSelectionCallback != null);
-
-                    // The callback also saves the selected cert in SslStream.LocalCertificate
-                    X509Certificate2? clientCertificate = clientCertificateSelectionCallback(out bool _);
-
-                    if (clientCertificate != null)
-                    {
-                        // build the cert context only if it was not provided by the user
-                        sslAuthenticationOptions.CertificateContext ??= SslStreamCertificateContext.Create(clientCertificate);
-                    }
-
-                    Interop.OpenSsl.UpdateClientCertificate((SafeSslHandle)context, sslAuthenticationOptions);
-                    errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, null, out output, out outputSize);
+                    Debug.Assert(sslAuthenticationOptions.IsClient);
+                    return new SecurityStatusPal(errorCode);
                 }
 
                 // sometimes during renegotiation processing message does not yield new output.

diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs
index 2f82a669cd5472f305b47d20b0f55701a87c158e..6c4ab95fd54a2cd023cf972b91b9967f93cf9c65 100644 (file)
--- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs
+++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs
@@ -122,14 +122,22 @@ namespace System.Net.Security
             return SecurityStatusAdapterPal.GetSecurityStatusPalFromNativeInt(errorCode);
         }
 
+        public static bool TryUpdateClintCertificate(
+            SafeFreeCredentials? _1,
+            SafeDeleteSslContext? _2,
+            SslAuthenticationOptions _3)
+        {
+            // We will need to allocate new credential handle
+            return false;
+        }
+
         public static unsafe SecurityStatusPal InitializeSecurityContext(
             ref SafeFreeCredentials? credentialsHandle,
             ref SafeDeleteSslContext? context,
             string? targetName,
             ReadOnlySpan<byte> inputBuffer,
             ref byte[]? outputBuffer,
-            SslAuthenticationOptions sslAuthenticationOptions,
-            SelectClientCertificate? _ /*clientCertificateSelectionCallback*/)
+            SslAuthenticationOptions sslAuthenticationOptions)
         {
             Interop.SspiCli.ContextFlags unusedAttributes = default;