Avoid mutating SslClientAuthenticationOptions in SslState

We shouldn't be mutating the options bag supplied by the caller.  This a) will leak back to the caller, and b) will result in problematic race conditions if the options bag is used with multiple SslStreams, such as if a cached instance is used.

Author: stephentoub

src/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs

@@ -11,12 +11,12 @@ namespace System.Net.Security
 {
     internal class SslAuthenticationOptions
     {
-        internal SslAuthenticationOptions(SslClientAuthenticationOptions sslClientAuthenticationOptions)
+        internal SslAuthenticationOptions(SslClientAuthenticationOptions sslClientAuthenticationOptions, RemoteCertValidationCallback remoteCallback, LocalCertSelectionCallback localCallback)
         {
             // Common options.
             AllowRenegotiation = sslClientAuthenticationOptions.AllowRenegotiation;
             ApplicationProtocols = sslClientAuthenticationOptions.ApplicationProtocols;
-            CertValidationDelegate = sslClientAuthenticationOptions._certValidationDelegate;
+            CertValidationDelegate = remoteCallback;
             CheckCertName = true;
             EnabledSslProtocols = sslClientAuthenticationOptions.EnabledSslProtocols;
             EncryptionPolicy = sslClientAuthenticationOptions.EncryptionPolicy;
@@ -26,7 +26,7 @@ internal SslAuthenticationOptions(SslClientAuthenticationOptions sslClientAuthen
             TargetHost = sslClientAuthenticationOptions.TargetHost;
 
             // Client specific options.
-            CertSelectionDelegate = sslClientAuthenticationOptions._certSelectionDelegate;
+            CertSelectionDelegate = localCallback;
             CertificateRevocationCheckMode = sslClientAuthenticationOptions.CertificateRevocationCheckMode;
             ClientCertificates = sslClientAuthenticationOptions.ClientCertificates;
             LocalCertificateSelectionCallback = sslClientAuthenticationOptions.LocalCertificateSelectionCallback;

src/System.Net.Security/src/System/Net/Security/SslClientAuthenticationOptions.cs

@@ -16,9 +16,6 @@ public class SslClientAuthenticationOptions
         private SslProtocols _enabledSslProtocols = SecurityProtocol.SystemDefaultSecurityProtocols;
         private bool _allowRenegotiation = true;
 
-        internal RemoteCertValidationCallback _certValidationDelegate;
-        internal LocalCertSelectionCallback _certSelectionDelegate;
-
         public bool AllowRenegotiation
         {
             get => _allowRenegotiation;

src/System.Net.Security/src/System/Net/Security/SslState.cs

@@ -97,7 +97,7 @@ private void ThrowIfExceptional()
             }
         }
 
-        internal void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthenticationOptions)
+        internal void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthenticationOptions, RemoteCertValidationCallback remoteCallback, LocalCertSelectionCallback localCallback)
         {
             ThrowIfExceptional();
 
@@ -116,15 +116,14 @@ internal void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuth
                 throw new ArgumentNullException(nameof(sslClientAuthenticationOptions.TargetHost));
             }
 
-            if (sslClientAuthenticationOptions.TargetHost.Length == 0)
-            {
-                sslClientAuthenticationOptions.TargetHost = "?" + Interlocked.Increment(ref s_uniqueNameInteger).ToString(NumberFormatInfo.InvariantInfo);
-            }
-
             _exception = null;
             try
             {
-                _sslAuthenticationOptions = new SslAuthenticationOptions(sslClientAuthenticationOptions);
+                _sslAuthenticationOptions = new SslAuthenticationOptions(sslClientAuthenticationOptions, remoteCallback, localCallback);
+                if (_sslAuthenticationOptions.TargetHost.Length == 0)
+                {
+                    _sslAuthenticationOptions.TargetHost = "?" + Interlocked.Increment(ref s_uniqueNameInteger).ToString(NumberFormatInfo.InvariantInfo);
+                }
                 _context = new SecureChannel(_sslAuthenticationOptions);
             }
             catch (Win32Exception e)

src/System.Net.Security/src/System/Net/Security/SslStream.cs

@@ -178,11 +178,7 @@ internal virtual IAsyncResult BeginAuthenticateAsClient(SslClientAuthenticationO
             SetAndVerifyValidationCallback(sslClientAuthenticationOptions.RemoteCertificateValidationCallback);
             SetAndVerifySelectionCallback(sslClientAuthenticationOptions.LocalCertificateSelectionCallback);
 
-            // Set the delegates on the options.
-            sslClientAuthenticationOptions._certValidationDelegate = _certValidationDelegate;
-            sslClientAuthenticationOptions._certSelectionDelegate = _certSelectionDelegate;
-
-            _sslState.ValidateCreateContext(sslClientAuthenticationOptions);
+            _sslState.ValidateCreateContext(sslClientAuthenticationOptions, _certValidationDelegate, _certSelectionDelegate);
 
             LazyAsyncResult result = new LazyAsyncResult(_sslState, asyncState, asyncCallback);
             _sslState.ProcessAuthentication(result);
@@ -302,11 +298,7 @@ private void AuthenticateAsClient(SslClientAuthenticationOptions sslClientAuthen
             SetAndVerifyValidationCallback(sslClientAuthenticationOptions.RemoteCertificateValidationCallback);
             SetAndVerifySelectionCallback(sslClientAuthenticationOptions.LocalCertificateSelectionCallback);
 
-            // Set the delegates on the options.
-            sslClientAuthenticationOptions._certValidationDelegate = _certValidationDelegate;
-            sslClientAuthenticationOptions._certSelectionDelegate = _certSelectionDelegate;
-
-            _sslState.ValidateCreateContext(sslClientAuthenticationOptions);
+            _sslState.ValidateCreateContext(sslClientAuthenticationOptions, _certValidationDelegate, _certSelectionDelegate);
             _sslState.ProcessAuthentication(null);
         }
 

src/System.Net.Security/tests/UnitTests/Fakes/FakeSslState.cs

@@ -21,7 +21,7 @@ internal SslState(Stream innerStream)
         {
         }
 
-        internal void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthenticationOptions)
+        internal void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthenticationOptions, RemoteCertValidationCallback remoteCallback, LocalCertSelectionCallback localCallback)
         {
         }