[release/9.0-staging] Link peer's X509 stack handle to parent SSL safe handle (#115380)

* Link peer's X509 stack handle to parent SSL safe handle

* Update src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs

Co-authored-by: Kevin Jones <[email protected]>

* Add test for dispose parallel with handshake.

* Revert "Add test for dispose parallel with handshake."

This reverts commit abf96c8.

* Defer RemoteCertificate assignment after X509 Chain build (#114781)

* Defer RemoteCertificate assignment after X509 Chain build

* Add comment

* Fix SslStreamDisposeTest failures during parallel handshake (#113834)

* Fix SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE test failures

* Fix build

* Fix SslStreamDisposeTest for parallel handshake on Unix (#114100)

* [Test Failure] SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE on Unix
Fixes #113833

* fixup! [Test Failure] SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE on Unix Fixes #113833

* fixup! fixup! [Test Failure] SslStreamDisposeTest.Dispose_ParallelWithHandshake_ThrowsODE on Unix Fixes #113833

* Update src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamDisposeTest.cs

Co-authored-by: Copilot <[email protected]>

* Fix build

---------

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: Radek Zikmund <[email protected]>
Co-authored-by: Radek Zikmund <[email protected]>
Co-authored-by: Kevin Jones <[email protected]>
Co-authored-by: Copilot <[email protected]>

Author: 4 people

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

@@ -123,7 +123,14 @@ internal static unsafe ReadOnlySpan<byte> SslGetAlpnSelected(SafeSslHandle ssl)
         internal static partial IntPtr SslGetCertificate(IntPtr ssl);
 
         [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetPeerCertChain")]
-        internal static partial SafeSharedX509StackHandle SslGetPeerCertChain(SafeSslHandle ssl);
+        private static partial SafeSharedX509StackHandle SslGetPeerCertChain_private(SafeSslHandle ssl);
+
+        internal static SafeSharedX509StackHandle SslGetPeerCertChain(SafeSslHandle ssl)
+        {
+            return SafeInteriorHandle.OpenInteriorHandle(
+                SslGetPeerCertChain_private,
+                ssl);
+        }
 
         [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetPeerFinished")]
         internal static partial int SslGetPeerFinished(SafeSslHandle ssl, IntPtr buf, int count);

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

@@ -1057,8 +1057,9 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot
                     return true;
                 }
 
-                _remoteCertificate = certificate;
-                if (_remoteCertificate == null)
+                // don't assign to _remoteCertificate yet, this prevents weird exceptions if SslStream is disposed in parallel with X509Chain building
+
+                if (certificate == null)
                 {
                     if (NetEventSource.Log.IsEnabled() && RemoteCertRequired) NetEventSource.Error(this, $"Remote certificate required, but no remote certificate received");
                     sslPolicyErrors |= SslPolicyErrors.RemoteCertificateNotAvailable;
@@ -1100,15 +1101,17 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot
                     sslPolicyErrors |= CertificateValidationPal.VerifyCertificateProperties(
                         _securityContext!,
                         chain,
-                        _remoteCertificate,
+                        certificate,
                         _sslAuthenticationOptions.CheckCertName,
                         _sslAuthenticationOptions.IsServer,
                         TargetHostNameHelper.NormalizeHostName(_sslAuthenticationOptions.TargetHost));
                 }
 
+                _remoteCertificate = certificate;
+
                 if (remoteCertValidationCallback != null)
                 {
-                    success = remoteCertValidationCallback(this, _remoteCertificate, chain, sslPolicyErrors);
+                    success = remoteCertValidationCallback(this, certificate, chain, sslPolicyErrors);
                 }
                 else
                 {

src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamDisposeTest.cs

@@ -4,6 +4,7 @@
 using System.IO;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading;
+using System.Security.Authentication;
 using System.Threading.Tasks;
 
 using Xunit;
@@ -59,6 +60,7 @@ public async Task Dispose_PendingReadAsync_ThrowsODE(bool bufferedRead)
             using CancellationTokenSource cts = new CancellationTokenSource();
             cts.CancelAfter(TestConfiguration.PassingTestTimeout);
 
+
             (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(leaveInnerStreamOpen: true);
             using (client)
             using (server)
@@ -102,5 +104,65 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout(
                 await Assert.ThrowsAnyAsync<ObjectDisposedException>(() => client.ReadAsync(readBuffer, cts.Token).AsTask());
             }
         }
+
+        [Fact]
+        [OuterLoop("Computationally expensive")]
+        public async Task Dispose_ParallelWithHandshake_ThrowsODE()
+        {
+            using CancellationTokenSource cts = new CancellationTokenSource();
+            cts.CancelAfter(TestConfiguration.PassingTestTimeout);
+
+            await Parallel.ForEachAsync(System.Linq.Enumerable.Range(0, 10000), cts.Token, async (i, token) =>
+            {
+                (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams();
+
+                using SslStream client = new SslStream(clientStream);
+                using SslStream server = new SslStream(serverStream);
+                using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate();
+                using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate();
+
+                SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions()
+                {
+                    TargetHost = Guid.NewGuid().ToString("N"),
+                    RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true,
+                };
+
+                SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions()
+                {
+                    ServerCertificate = serverCertificate,
+                };
+
+                var clientTask = Task.Run(() => client.AuthenticateAsClientAsync(clientOptions, cts.Token));
+                var serverTask = Task.Run(() => server.AuthenticateAsServerAsync(serverOptions, cts.Token));
+
+                // Dispose the instances while the handshake is in progress.
+                client.Dispose();
+                server.Dispose();
+
+                await ValidateExceptionAsync(clientTask);
+                await ValidateExceptionAsync(serverTask);
+            });
+
+            static async Task ValidateExceptionAsync(Task task)
+            {
+                try
+                {
+                    await task;
+                }
+                catch (InvalidOperationException ex) when (ex.StackTrace?.Contains("System.IO.StreamBuffer.WriteAsync") ?? true)
+                {
+                    // Writing to a disposed ConnectedStream (test only, does not happen with NetworkStream)
+                    return;
+                }
+                catch (Exception ex) when (ex
+                    is ObjectDisposedException // disposed locally
+                    or IOException // disposed remotely (received unexpected EOF)
+                    or AuthenticationException) // disposed wrapped in AuthenticationException or error from platform library
+                {
+                    // expected
+                    return;
+                }
+            }
+        }
     }
 }