fix handling of Ssl2 and enable disabled tests (#36098)

* attempt to fix ssl2 * adjust length calculation * enable tests * fix ReadAsyncInternal * use PlatformDetection.SupportsSsl2 * feedback from review Co-authored-by: Tomas Weinfurt <furt@Shining.local>
dotnet · May 19, 2020 · ebc55e3 · ebc55e3
1 parent cf1390c
commit ebc55e3
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 28 deletions.
diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.SslProtocols.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.SslProtocols.cs
@@ -113,7 +113,6 @@ public static IEnumerable<object[]> GetAsync_AllowedSSLVersion_Succeeds_MemberDa
 
         [Theory]
         [MemberData(nameof(GetAsync_AllowedSSLVersion_Succeeds_MemberData))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/36100", TestPlatforms.Windows)]
         public async Task GetAsync_AllowedSSLVersion_Succeeds(SslProtocols acceptedProtocol, bool requestOnlyThisProtocol)
         {
             using (HttpClientHandler handler = CreateHttpClientHandler())
@@ -196,7 +195,7 @@ public async Task GetAsync_SupportedSSLVersion_Succeeds(SslProtocols sslProtocol
         public static IEnumerable<object[]> NotSupportedSSLVersionServers()
         {
 #pragma warning disable 0618
-            if (PlatformDetection.IsWindows10Version1607OrGreater)
+            if (PlatformDetection.SupportsSsl2)
             {
                 yield return new object[] { SslProtocols.Ssl2, Configuration.Http.SSLv2RemoteServer };
             }
@@ -249,7 +248,6 @@ await TestHelper.WhenAllCompletedOrAnyFailed(
         [InlineData(SslProtocols.Tls11 | SslProtocols.Tls12, SslProtocols.Tls)] // Skip this on WinHttpHandler.
         [InlineData(SslProtocols.Tls12, SslProtocols.Tls11)]
         [InlineData(SslProtocols.Tls, SslProtocols.Tls12)]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/36100", TestPlatforms.Windows)]
         public async Task GetAsync_AllowedClientSslVersionDiffersFromServer_ThrowsException(
             SslProtocols allowedClientProtocols, SslProtocols acceptedServerProtocols)
         {

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
@@ -24,11 +24,11 @@ public partial class SslStream
 
         private enum Framing
         {
-            Unknown = 0,
-            BeforeSSL3,
-            SinceSSL3,
-            Unified,
-            Invalid
+            Unknown = 0,    // Initial before any frame is processd.
+            BeforeSSL3,     // SSlv2
+            SinceSSL3,      // SSlv3 & TLS
+            Unified,        // Intermediate on first frame until response is processes.
+            Invalid         // Somthing is wrong.
         }
 
         // This is set on the first packet to figure out the framing style.
@@ -352,12 +352,12 @@ private async ValueTask<ProtocolToken> ReceiveBlobAsync<TIOAdapter>(TIOAdapter a
                 _framing = DetectFraming(_handshakeBuffer.ActiveReadOnlySpan);
             }
 
-            if (_framing == Framing.BeforeSSL3)
+            if (_framing != Framing.SinceSSL3)
             {
 #pragma warning disable 0618
                 _lastFrame.Header.Version = SslProtocols.Ssl2;
 #pragma warning restore 0618
-                _lastFrame.Header.Length = GetFrameSize(_handshakeBuffer.ActiveReadOnlySpan);
+                _lastFrame.Header.Length = GetFrameSize(_handshakeBuffer.ActiveReadOnlySpan) - TlsFrameHelper.HeaderSize;
             }
             else
             {
@@ -409,25 +409,28 @@ private ProtocolToken ProcessBlob(int frameSize)
             // ActiveSpan will exclude the "discarded" data.
             _handshakeBuffer.Discard(frameSize);
 
-            // Often more TLS messages fit into same packet. Get as many complete frames as we can.
-            while (_handshakeBuffer.ActiveLength > TlsFrameHelper.HeaderSize)
+            if (_framing == Framing.SinceSSL3)
             {
-                TlsFrameHeader nextHeader = default;
-
-                if (!TlsFrameHelper.TryGetFrameHeader(_handshakeBuffer.ActiveReadOnlySpan, ref nextHeader))
+                // Often more TLS messages fit into same packet. Get as many complete frames as we can.
+                while (_handshakeBuffer.ActiveLength > TlsFrameHelper.HeaderSize)
                 {
-                    break;
-                }
+                    TlsFrameHeader nextHeader = default;
 
-                frameSize = nextHeader.Length + TlsFrameHelper.HeaderSize;
-                if (nextHeader.Type == TlsContentType.AppData || frameSize > _handshakeBuffer.ActiveLength)
-                {
-                    // We don't have full frame left or we already have app data which needs to be processed by decrypt.
-                    break;
-                }
+                    if (!TlsFrameHelper.TryGetFrameHeader(_handshakeBuffer.ActiveReadOnlySpan, ref nextHeader))
+                    {
+                        break;
+                    }
+
+                    frameSize = nextHeader.Length + TlsFrameHelper.HeaderSize;
+                    if (nextHeader.Type == TlsContentType.AppData || frameSize > _handshakeBuffer.ActiveLength)
+                    {
+                        // We don't have full frame left or we already have app data which needs to be processed by decrypt.
+                        break;
+                    }
 
-                chunkSize += frameSize;
-                _handshakeBuffer.Discard(frameSize);
+                    chunkSize += frameSize;
+                    _handshakeBuffer.Discard(frameSize);
+                }
             }
 
             return _context!.NextMessage(availableData.Slice(0, chunkSize));
@@ -684,7 +687,7 @@ private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, M
                     Debug.Assert(_internalBufferCount >= SecureChannel.ReadHeaderSize);
 
                     // Parse the frame header to determine the payload size (which includes the header size).
-                    int payloadBytes = TlsFrameHelper.GetFrameSize(_internalBuffer.AsSpan(_internalOffset));
+                    int payloadBytes = GetFrameSize(_internalBuffer.AsSpan(_internalOffset));
                     if (payloadBytes < 0)
                     {
                         throw new IOException(SR.net_frame_read_size);

diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs
@@ -59,12 +59,11 @@ public async Task ClientAsyncAuthenticate_EachSupportedProtocol_Success(SslProto
 
         [Fact]
         [PlatformSpecific(TestPlatforms.Windows)]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/36100")]
         public async Task ClientAsyncAuthenticate_Ssl2WithSelf_Success()
         {
             // Test Ssl2 against itself.  This is a standalone test as even on versions where Windows supports Ssl2,
             // it appears to have rules around not using it when other protocols are mentioned.
-            if (!PlatformDetection.IsWindows10Version1607OrGreater)
+            if (PlatformDetection.SupportsSsl2)
             {
 #pragma warning disable 0618
                 await ClientAsyncSslHelper(SslProtocols.Ssl2, SslProtocols.Ssl2);