From 971674944c568ee12f07d8604bfdd2500c9eacfa Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 28 Dec 2021 20:42:45 -0800 Subject: [PATCH 1/2] avoid ArgumentOutOfRangeException while processing invalid or incomplete TLS frame --- .../Net/Security/SslStream.Implementation.cs | 7 +-- .../ServerAsyncAuthenticateTest.cs | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 6 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 f553f14a46c3d3..970af9be99fc5c 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 @@ -460,11 +460,6 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a where TIOAdapter : IReadWriteAdapter { int readBytes = await FillHandshakeBufferAsync(adapter, SecureChannel.ReadHeaderSize).ConfigureAwait(false); - if (readBytes == 0) - { - throw new IOException(SR.net_io_eof); - } - if (_framing == Framing.Unified || _framing == Framing.Unknown) { _framing = DetectFraming(_handshakeBuffer.ActiveReadOnlySpan); @@ -1083,7 +1078,7 @@ private ValueTask FillHandshakeBufferAsync(TIOAdapter adapter, int bytesRead = t.Result; if (bytesRead == 0) { - return new ValueTask(0); + throw new IOException(SR.net_io_eof); } _handshakeBuffer.Commit(bytesRead); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index 73d480a3048588..6edcace7d85a54 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -285,6 +285,49 @@ public async Task ServerAsyncAuthenticate_NoCertificate_Throws(bool useAsync) } } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ServerAsyncAuthenticate_InvalidHello_Throws(bool close) + { + (NetworkStream client, NetworkStream server) = TestHelper.GetConnectedTcpStreams(); + using (client) + using (SslStream ssl = new SslStream(server)) + { + byte[] buffer = new byte[182]; + buffer[0] = 178; + buffer[1] = 0; + buffer[2] = 0; + buffer[3] = 1; + buffer[4] = 133; + buffer[5] = 166; + + Task t1 = ssl.AuthenticateAsServerAsync(_serverCertificate, false, false); + Task t2 = client.WriteAsync(buffer).AsTask(); + if (close) + { + await t2.WaitAsync(TestConfiguration.PassingTestTimeout); + client.Socket.Shutdown(SocketShutdown.Send); + } + else + { + // Write enough data to full frame size + buffer = new byte[13000]; + t2 = client.WriteAsync(buffer).AsTask(); + await t2.WaitAsync(TestConfiguration.PassingTestTimeout); + } + + if (close) + { + await Assert.ThrowsAsync(() => t1); + } + else + { + await Assert.ThrowsAsync(() => t1); + } + } + } + public static IEnumerable ProtocolMismatchData() { if (PlatformDetection.SupportsSsl3) From 659200d6dac5acd97423d9fc0f12e05bc607a2b2 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 30 Dec 2021 16:55:33 -0800 Subject: [PATCH 2/2] feedback from review --- .../Net/Security/SslStream.Implementation.cs | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 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 970af9be99fc5c..1cc4fecd5ef50c 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 @@ -459,7 +459,7 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool private async ValueTask ReceiveBlobAsync(TIOAdapter adapter) where TIOAdapter : IReadWriteAdapter { - int readBytes = await FillHandshakeBufferAsync(adapter, SecureChannel.ReadHeaderSize).ConfigureAwait(false); + await FillHandshakeBufferAsync(adapter, SecureChannel.ReadHeaderSize).ConfigureAwait(false); if (_framing == Framing.Unified || _framing == Framing.Unknown) { _framing = DetectFraming(_handshakeBuffer.ActiveReadOnlySpan); @@ -1056,13 +1056,13 @@ private async ValueTask ReadAsyncInternal(TIOAdapter adapter, M // This function tries to make sure buffer has at least minSize bytes available. // If we have enough data, it returns synchronously. If not, it will try to read - // remaining bytes from given stream. - private ValueTask FillHandshakeBufferAsync(TIOAdapter adapter, int minSize) + // remaining bytes from given stream. It will throw if unable to fulfill minSize. + private ValueTask FillHandshakeBufferAsync(TIOAdapter adapter, int minSize) where TIOAdapter : IReadWriteAdapter { if (_handshakeBuffer.ActiveLength >= minSize) { - return new ValueTask(minSize); + return ValueTask.CompletedTask; } int bytesNeeded = minSize - _handshakeBuffer.ActiveLength; @@ -1084,9 +1084,9 @@ private ValueTask FillHandshakeBufferAsync(TIOAdapter adapter, _handshakeBuffer.Commit(bytesRead); } - return new ValueTask(minSize); + return ValueTask.CompletedTask; - async ValueTask InternalFillHandshakeBufferAsync(TIOAdapter adap, ValueTask task, int minSize) + async ValueTask InternalFillHandshakeBufferAsync(TIOAdapter adap, ValueTask task, int minSize) { while (true) { @@ -1099,7 +1099,7 @@ async ValueTask InternalFillHandshakeBufferAsync(TIOAdapter adap, ValueTas _handshakeBuffer.Commit(bytesRead); if (_handshakeBuffer.ActiveLength >= minSize) { - return minSize; + return; } task = adap.ReadAsync(_handshakeBuffer.AvailableMemory); @@ -1107,24 +1107,6 @@ async ValueTask InternalFillHandshakeBufferAsync(TIOAdapter adap, ValueTas } } - private async ValueTask FillBufferAsync(TIOAdapter adapter, int numBytesRequired) - where TIOAdapter : IReadWriteAdapter - { - Debug.Assert(_internalBufferCount > 0); - Debug.Assert(_internalBufferCount < numBytesRequired); - - while (_internalBufferCount < numBytesRequired) - { - int bytesRead = await adapter.ReadAsync(_internalBuffer.AsMemory(_internalBufferCount)).ConfigureAwait(false); - if (bytesRead == 0) - { - throw new IOException(SR.net_io_eof); - } - - _internalBufferCount += bytesRead; - } - } - private async ValueTask WriteAsyncInternal(TIOAdapter writeAdapter, ReadOnlyMemory buffer) where TIOAdapter : struct, IReadWriteAdapter {