From fa410ebdd82059d66816e445256bf56d004d508d Mon Sep 17 00:00:00 2001 From: Cory Nelson Date: Mon, 29 Jul 2019 10:46:49 -0700 Subject: [PATCH] HTTP2: Validate dynamic table size updates (dotnet/corefx#39669) * Validate dynamic table size updates. * Set dynamic table size to 0, to reduce a per-connection ~2048 byte allocation to a ~0 byte allocation. Resolves dotnet/corefx#39666 Commit migrated from https://github.com/dotnet/corefx/commit/14b604c5fe52e0022881a0866bf39f344d247fd7 --- .../System/Net/Http/Http2LoopbackConnection.cs | 5 +++ .../System.Net.Http/src/Resources/Strings.resx | 5 ++- .../Http/SocketsHttpHandler/HPack/HPackDecoder.cs | 9 ++++-- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 2 +- .../FunctionalTests/HttpClientHandlerTest.Http2.cs | 36 ++++++++++++++++++++++ 5 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index bebf73e..f5245c6 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -546,6 +546,11 @@ namespace System.Net.Test.Common return bytesGenerated; } + public static int EncodeDynamicTableSizeUpdate(int newMaximumSize, Span headerBlock) + { + return EncodeInteger(newMaximumSize, 0b00100000, 0b11100000, headerBlock); + } + public async Task ReadBodyAsync() { byte[] body = null; diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index ada59bf..dafd9d1 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -522,4 +522,7 @@ The object was disposed while operations were in progress. - + + Dynamic table size update to {0} bytes exceeds limit of {1} bytes. + + \ No newline at end of file diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackDecoder.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackDecoder.cs index a1bc6ec..588a9c31 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackDecoder.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackDecoder.cs @@ -214,7 +214,12 @@ namespace System.Net.Http.HPack if (_integerDecoder.StartDecode((byte)(b & ~DynamicTableSizeUpdateMask), DynamicTableSizeUpdatePrefix)) { - // TODO: validate that it's less than what's defined via SETTINGS + if (_integerDecoder.Value > _maxDynamicTableSize) + { + // Dynamic table size update is too large. + throw new HPackDecodingException(SR.Format(SR.net_http_hpack_large_table_size_update, _integerDecoder.Value, _maxDynamicTableSize)); + } + _dynamicTable.Resize(_integerDecoder.Value); } else @@ -321,7 +326,7 @@ namespace System.Net.Http.HPack if (_integerDecoder.Value > _maxDynamicTableSize) { // Dynamic table size update is too large. - throw new HPackDecodingException(); + throw new HPackDecodingException(SR.Format(SR.net_http_hpack_large_table_size_update, _integerDecoder.Value, _maxDynamicTableSize)); } _dynamicTable.Resize(_integerDecoder.Value); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 429d29e..8779d55 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -105,7 +105,7 @@ namespace System.Net.Http _outgoingBuffer = new ArrayBuffer(InitialConnectionBufferSize); _headerBuffer = new ArrayBuffer(InitialConnectionBufferSize); - _hpackDecoder = new HPackDecoder(maxResponseHeadersLength: pool.Settings._maxResponseHeadersLength * 1024); + _hpackDecoder = new HPackDecoder(maxDynamicTableSize: 0, maxResponseHeadersLength: pool.Settings._maxResponseHeadersLength * 1024); _httpStreams = new Dictionary(); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs index 154efba..1b28fb1 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -3006,5 +3006,41 @@ namespace System.Net.Http.Functional.Tests await con.ShutdownIgnoringErrorsAsync(streamId); }); } + + [Fact] + public async Task DynamicTableSizeUpdate_Exceeds_Settings_Throws() + { + await Http2LoopbackServer.CreateClientAndServerAsync( + async uri => + { + using HttpClient client = CreateHttpClient(); + Exception e = await Assert.ThrowsAsync(() => client.GetAsync(uri)); + + Assert.NotNull(e.InnerException); + Assert.Contains("Dynamic table size update", e.InnerException.Message); + }, + async server => + { + (Http2LoopbackConnection con, SettingsFrame settings) = await server.EstablishConnectionGetSettingsAsync(); + int streamId = await con.ReadRequestHeaderAsync(); + + int headerTableSize = 4096; // Default per HTTP2 spec. + + foreach (SettingsEntry setting in settings.Entries) + { + if (setting.SettingId == SettingId.HeaderTableSize) + { + headerTableSize = (int)setting.Value; + } + } + + byte[] headerData = new byte[16]; + int headersLen = Http2LoopbackConnection.EncodeDynamicTableSizeUpdate(headerTableSize + 1, headerData); + HeadersFrame frame = new HeadersFrame(headerData.AsMemory(0, headersLen), FrameFlags.EndHeaders | FrameFlags.EndStream, 0, 0, 0, streamId); + + await con.WriteFrameAsync(frame); + await con.ShutdownIgnoringErrorsAsync(streamId); + }); + } } } -- 2.7.4