Fix lock during SslStream renegotiation request (#56470)

* Change lock and buffer test order * revert _nestedAuth clearing * Clear nested lock * Remove ability to renegotiate again when fail
dotnet · Aug 5, 2021 · 81b6502 · 81b6502
1 parent fda46d3
commit 81b6502
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 8 deletions.
diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs
@@ -268,16 +268,17 @@ private async Task RenegotiateAsync<TIOAdapter>(TIOAdapter adapter)
                 throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, nameof(WriteAsync), "write"));
             }
 
-            if (_decryptedBytesCount is not 0)
+            try
             {
-                throw new InvalidOperationException(SR.net_ssl_renegotiate_buffer);
-            }
+                if (_decryptedBytesCount is not 0)
+                {
+                    throw new InvalidOperationException(SR.net_ssl_renegotiate_buffer);
+                }
+
+                _sslAuthenticationOptions!.RemoteCertRequired = true;
+                _isRenego = true;
 
-            _sslAuthenticationOptions!.RemoteCertRequired = true;
-            _isRenego = true;
 
-            try
-            {
                 SecurityStatusPal status = _context!.Renegotiate(out byte[]? nextmsg);
 
                 if (nextmsg is {} && nextmsg.Length > 0)

diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs
@@ -369,15 +369,20 @@ public async Task SslStream_NegotiateClientCertificateAsync_ServerDontDrainClien
             using (server)
             {
                 using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate();
+                using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate();
 
                 SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions()
                 {
                     TargetHost = Guid.NewGuid().ToString("N"),
                     EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12,
                 };
                 clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
+                clientOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) =>
+                {
+                    return clientCertificate;
+                };
                 SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = serverCertificate };
-
+                serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
                 await TestConfiguration.WhenAllOrAnyFailedWithTimeout(
                                 client.AuthenticateAsClientAsync(clientOptions, cts.Token),
                                 server.AuthenticateAsServerAsync(serverOptions, cts.Token));
@@ -392,6 +397,12 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout(
                 await Assert.ThrowsAsync<InvalidOperationException>(()=>
                     server.NegotiateClientCertificateAsync(cts.Token)
                 );
+
+                // Drain client data.
+                await server.ReadAsync(new byte[499]);
+                // Verify that the session is usable even renego request failed.
+                await TestHelper.PingPong(client, server, cts.Token);
+                await TestHelper.PingPong(server, client, cts.Token);
             }
         }