From 9782377368cb3f0b1cea73a1f85d243af99c471a Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 15 Mar 2023 23:09:03 +0100 Subject: [PATCH 1/4] Ensure free buffer space when reading TLS messages --- .../src/System/Net/Security/SslStream.IO.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index 38b097ebd961bd..941a9b90d9b58e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -684,15 +684,10 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } - if (frameSize < int.MaxValue) - { - _buffer.EnsureAvailableSpace(frameSize - _buffer.EncryptedLength); - } - while (_buffer.EncryptedLength < frameSize) { - // there should be space left to read into - Debug.Assert(_buffer.AvailableLength > 0, "_buffer.AvailableBytes > 0"); + // make sure we have space to read into + _buffer.EnsureAvailableSpace(Math.Min(frameSize, _buffer.Capacity) - _buffer.EncryptedLength); // We either don't have full frame or we don't have enough data to even determine the size. int bytesRead = await TIOAdapter.ReadAsync(InnerStream, _buffer.AvailableMemory, cancellationToken).ConfigureAwait(false); From 9ac03e74ce7e3dd90690f0341cde1e43b83875d7 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 16 Mar 2023 08:52:40 +0100 Subject: [PATCH 2/4] Move buffer expansion outside of the loop to prevent unbounded grow --- .../src/System/Net/Security/SslStream.IO.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index 941a9b90d9b58e..6e9180793e841d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -684,10 +684,15 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } + // make sure we have space to read into, there are two cases which can happen + // - we know the exact frame size (frameSize != int.MaxValue) - we make sure we have space for the whole frame + // - we don't know the frame size (frameSize == int.MaxValue) - we move existing data to the beginning of the buffer (they will be couple of bytes only) + _buffer.EnsureAvailableSpace(Math.Min(frameSize, _buffer.Capacity) - _buffer.EncryptedLength); + while (_buffer.EncryptedLength < frameSize) { - // make sure we have space to read into - _buffer.EnsureAvailableSpace(Math.Min(frameSize, _buffer.Capacity) - _buffer.EncryptedLength); + // there should be space left to read into + Debug.Assert(_buffer.AvailableLength > 0, "_buffer.AvailableBytes > 0"); // We either don't have full frame or we don't have enough data to even determine the size. int bytesRead = await TIOAdapter.ReadAsync(InnerStream, _buffer.AvailableMemory, cancellationToken).ConfigureAwait(false); From 08ea2e43d36fcd41a0cf96f65f42cc5b02e6c12d Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 16 Mar 2023 12:48:39 +0100 Subject: [PATCH 3/4] Fix failing tests The initial size is not enough to cover later TLS frames --- .../src/System/Net/Security/SslStream.IO.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index 6e9180793e841d..b7fd97e46579d9 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -684,10 +684,18 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } - // make sure we have space to read into, there are two cases which can happen - // - we know the exact frame size (frameSize != int.MaxValue) - we make sure we have space for the whole frame - // - we don't know the frame size (frameSize == int.MaxValue) - we move existing data to the beginning of the buffer (they will be couple of bytes only) - _buffer.EnsureAvailableSpace(Math.Min(frameSize, _buffer.Capacity) - _buffer.EncryptedLength); + if (frameSize != int.MaxValue) + { + // make sure we have space for the whole frame + _buffer.EnsureAvailableSpace(frameSize - _buffer.EncryptedLength); + } + else + { + // move existing data to the beginning of the buffer (they will + // be couple of bytes only, otherwise we would have entire + // header and know exact size) + _buffer.EnsureAvailableSpace(_buffer.Capacity - _buffer.EncryptedLength); + } while (_buffer.EncryptedLength < frameSize) { From 1026dd253e08e26ced79af52628ac7a26a75c7b9 Mon Sep 17 00:00:00 2001 From: Radek Zikmund <32671551+rzikm@users.noreply.github.com> Date: Tue, 21 Mar 2023 14:47:21 +0100 Subject: [PATCH 4/4] Remove unwanted changes --- .../System.Net.Security/src/System/Net/Security/SslStream.IO.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index b7fd97e46579d9..174995454aec15 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -684,7 +684,7 @@ private async ValueTask EnsureFullTlsFrameAsync(CancellationTok return frameSize; } - if (frameSize != int.MaxValue) + if (frameSize < int.MaxValue) { // make sure we have space for the whole frame _buffer.EnsureAvailableSpace(frameSize - _buffer.EncryptedLength);