From 81b6502efd98d38b9fe008f2bfe4467db286f480 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 5 Aug 2021 10:53:58 +0200 Subject: [PATCH] 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 --- .../src/System/Net/Security/SslStream.Implementation.cs | 15 ++++++++------- .../tests/FunctionalTests/SslStreamNetworkStreamTest.cs | 13 ++++++++++++- 2 files changed, 20 insertions(+), 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 index 0cd8884..add3a7d 100644 --- 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 @@ namespace System.Net.Security 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 index b3a5f5e..3428ba7 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -369,6 +369,7 @@ namespace System.Net.Security.Tests using (server) { using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate(); + using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate(); SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { @@ -376,8 +377,12 @@ namespace System.Net.Security.Tests 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 @@ namespace System.Net.Security.Tests await Assert.ThrowsAsync(()=> 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); } } -- 2.7.4