From ceaa3c86fc95c7d43509ebdd1ff9dc669481d482 Mon Sep 17 00:00:00 2001 From: John Luo Date: Fri, 21 Sep 2018 15:59:29 -0700 Subject: [PATCH] Add configurability for max header field size in HPACK --- src/Kestrel.Core/Http2Limits.cs | 23 ++++++++++++++++- .../Internal/Http2/HPack/HPackDecoder.cs | 22 ++++++++-------- .../Internal/Http2/Http2Connection.cs | 13 ++++++---- test/Kestrel.Core.Tests/HPackDecoderTests.cs | 25 ++++++++++++++++--- .../KestrelServerLimitsTests.cs | 25 ++++++++++++++++--- .../Http2/Http2TestBase.cs | 5 ++-- 6 files changed, 86 insertions(+), 27 deletions(-) diff --git a/src/Kestrel.Core/Http2Limits.cs b/src/Kestrel.Core/Http2Limits.cs index d615df0e94..e2914ac150 100644 --- a/src/Kestrel.Core/Http2Limits.cs +++ b/src/Kestrel.Core/Http2Limits.cs @@ -14,11 +14,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core private int _maxStreamsPerConnection = 100; private int _headerTableSize = (int)Http2PeerSettings.DefaultHeaderTableSize; private int _maxFrameSize = (int)Http2PeerSettings.DefaultMaxFrameSize; + private int _maxRequestHeaderFieldSize = 8192; /// /// Limits the number of concurrent request streams per HTTP/2 connection. Excess streams will be refused. /// - /// Defaults to 100 + /// Value must be greater than 0, defaults to 100 /// /// public int MaxStreamsPerConnection @@ -74,5 +75,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core _maxFrameSize = value; } } + + /// + /// Indicates the size of the maximum allowed size of a request header field sequence. This limit applies to both name and value sequences in their compressed and uncompressed representations. + /// + /// Value must be greater than 0, defaults to 8192 + /// + /// + public int MaxRequestHeaderFieldSize + { + get => _maxRequestHeaderFieldSize; + set + { + if (value <= 0) + { + throw new ArgumentOutOfRangeException(nameof(value), value, CoreStrings.GreaterThanZeroRequired); + } + + _maxRequestHeaderFieldSize = value; + } + } } } diff --git a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs index 3b6cc0f00b..e88a4e077b 100644 --- a/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs +++ b/src/Kestrel.Core/Internal/Http2/HPack/HPackDecoder.cs @@ -23,9 +23,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack DynamicTableSizeUpdate } - // TODO: add new configurable limit - public const int MaxStringOctets = 4096; - // http://httpwg.org/specs/rfc7541.html#rfc.section.6.1 // 0 1 2 3 4 5 6 7 // +---+---+---+---+---+---+---+---+ @@ -83,9 +80,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack private readonly int _maxDynamicTableSize; private readonly DynamicTable _dynamicTable; private readonly IntegerDecoder _integerDecoder = new IntegerDecoder(); - private readonly byte[] _stringOctets = new byte[MaxStringOctets]; - private readonly byte[] _headerNameOctets = new byte[MaxStringOctets]; - private readonly byte[] _headerValueOctets = new byte[MaxStringOctets]; + private readonly byte[] _stringOctets; + private readonly byte[] _headerNameOctets; + private readonly byte[] _headerValueOctets; private State _state = State.Ready; private byte[] _headerName; @@ -97,17 +94,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack private bool _huffman; private bool _headersObserved; - public HPackDecoder(int maxDynamicTableSize) - : this(maxDynamicTableSize, new DynamicTable(maxDynamicTableSize)) - { - _maxDynamicTableSize = maxDynamicTableSize; - } + public HPackDecoder(int maxDynamicTableSize, int maxRequestHeaderFieldSize) + : this(maxDynamicTableSize, maxRequestHeaderFieldSize, new DynamicTable(maxDynamicTableSize)) { } // For testing. - internal HPackDecoder(int maxDynamicTableSize, DynamicTable dynamicTable) + internal HPackDecoder(int maxDynamicTableSize, int maxRequestHeaderFieldSize, DynamicTable dynamicTable) { _maxDynamicTableSize = maxDynamicTableSize; _dynamicTable = dynamicTable; + + _stringOctets = new byte[maxRequestHeaderFieldSize]; + _headerNameOctets = new byte[maxRequestHeaderFieldSize]; + _headerValueOctets = new byte[maxRequestHeaderFieldSize]; } public void Decode(ReadOnlySequence data, bool endHeaders, IHttpHeadersHandler handler) diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 25500bb28a..9bbf1a9236 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -86,13 +86,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public Http2Connection(Http2ConnectionContext context) { + var httpLimits = context.ServiceContext.ServerOptions.Limits; + var http2Limits = httpLimits.Http2; + _context = context; _frameWriter = new Http2FrameWriter(context.Transport.Output, context.ConnectionContext, _outputFlowControl, this, context.ConnectionId, context.ServiceContext.Log); - _serverSettings.MaxConcurrentStreams = (uint)context.ServiceContext.ServerOptions.Limits.Http2.MaxStreamsPerConnection; - _serverSettings.MaxFrameSize = (uint)context.ServiceContext.ServerOptions.Limits.Http2.MaxFrameSize; - _serverSettings.HeaderTableSize = (uint)context.ServiceContext.ServerOptions.Limits.Http2.HeaderTableSize; - _hpackDecoder = new HPackDecoder((int)_serverSettings.HeaderTableSize); - _serverSettings.MaxHeaderListSize = (uint)context.ServiceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize; + _serverSettings.MaxConcurrentStreams = (uint)http2Limits.MaxStreamsPerConnection; + _serverSettings.MaxFrameSize = (uint)http2Limits.MaxFrameSize; + _serverSettings.HeaderTableSize = (uint)http2Limits.HeaderTableSize; + _hpackDecoder = new HPackDecoder(http2Limits.HeaderTableSize, http2Limits.MaxRequestHeaderFieldSize); + _serverSettings.MaxHeaderListSize = (uint)httpLimits.MaxRequestHeadersTotalSize; } public string ConnectionId => _context.ConnectionId; diff --git a/test/Kestrel.Core.Tests/HPackDecoderTests.cs b/test/Kestrel.Core.Tests/HPackDecoderTests.cs index c45614348a..598a7a6ed5 100644 --- a/test/Kestrel.Core.Tests/HPackDecoderTests.cs +++ b/test/Kestrel.Core.Tests/HPackDecoderTests.cs @@ -17,6 +17,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public class HPackDecoderTests : IHttpHeadersHandler { private const int DynamicTableInitialMaxSize = 4096; + private const int MaxRequestHeaderFieldSize = 8192; // Indexed Header Field Representation - Static Table - Index 2 (:method: GET) private static readonly byte[] _indexedHeaderStatic = new byte[] { 0x82 }; @@ -94,7 +95,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public HPackDecoderTests() { _dynamicTable = new DynamicTable(DynamicTableInitialMaxSize); - _decoder = new HPackDecoder(DynamicTableInitialMaxSize, _dynamicTable); + _decoder = new HPackDecoder(DynamicTableInitialMaxSize, MaxRequestHeaderFieldSize, _dynamicTable); } void IHttpHeadersHandler.OnHeader(Span name, Span value) @@ -438,14 +439,32 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public void DecodesStringLength_GreaterThanLimit_Error() { var encoded = _literalHeaderFieldWithoutIndexingNewName - .Concat(new byte[] { 0xff, 0x82, 0x1f }) // 4097 encoded with 7-bit prefix + .Concat(new byte[] { 0xff, 0x82, 0x3f }) // 8193 encoded with 7-bit prefix .ToArray(); var exception = Assert.Throws(() => _decoder.Decode(new ReadOnlySequence(encoded), endHeaders: true, handler: this)); - Assert.Equal(CoreStrings.FormatHPackStringLengthTooLarge(4097, HPackDecoder.MaxStringOctets), exception.Message); + Assert.Equal(CoreStrings.FormatHPackStringLengthTooLarge(MaxRequestHeaderFieldSize + 1, MaxRequestHeaderFieldSize), exception.Message); Assert.Empty(_decodedHeaders); } + [Fact] + public void DecodesStringLength_LimitConfigurable() + { + var decoder = new HPackDecoder(DynamicTableInitialMaxSize, MaxRequestHeaderFieldSize + 1); + var string8193 = new string('a', MaxRequestHeaderFieldSize + 1); + + var encoded = _literalHeaderFieldWithoutIndexingNewName + .Concat(new byte[] { 0x7f, 0x82, 0x3f }) // 8193 encoded with 7-bit prefix, no Huffman encoding + .Concat(Encoding.ASCII.GetBytes(string8193)) + .Concat(new byte[] { 0x7f, 0x82, 0x3f }) // 8193 encoded with 7-bit prefix, no Huffman encoding + .Concat(Encoding.ASCII.GetBytes(string8193)) + .ToArray(); + + decoder.Decode(new ReadOnlySequence(encoded), endHeaders: true, handler: this); + + Assert.Equal(string8193, _decodedHeaders[string8193]); + } + public static readonly TheoryData _incompleteHeaderBlockData = new TheoryData { // Indexed Header Field Representation - incomplete index encoding diff --git a/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs b/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs index cf46717f85..17ca8d1967 100644 --- a/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerLimitsTests.cs @@ -315,7 +315,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Theory] - [InlineData(1 << 14 - 1)] + [InlineData((1 << 14) - 1)] [InlineData(1 << 24)] [InlineData(-1)] public void Http2MaxFrameSizeInvalid(int value) @@ -331,12 +331,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Theory] - [InlineData(4097)] + [InlineData(int.MinValue)] [InlineData(-1)] + [InlineData(0)] public void Http2HeaderTableSizeInvalid(int value) { - var ex = Assert.Throws(() => new KestrelServerLimits().Http2.MaxFrameSize = value); - Assert.Contains("A value between", ex.Message); + var ex = Assert.Throws(() => new KestrelServerLimits().Http2.HeaderTableSize = value); + Assert.StartsWith(CoreStrings.GreaterThanZeroRequired, ex.Message); + } + + [Fact] + public void Http2MaxRequestHeaderFieldSizeDefault() + { + Assert.Equal(8192, new KestrelServerLimits().Http2.MaxRequestHeaderFieldSize); + } + + [Theory] + [InlineData(int.MinValue)] + [InlineData(-1)] + [InlineData(0)] + public void Http2MaxRequestHeaderFieldSizeInvalid(int value) + { + var ex = Assert.Throws(() => new KestrelServerLimits().Http2.MaxRequestHeaderFieldSize = value); + Assert.StartsWith(CoreStrings.GreaterThanZeroRequired, ex.Message); } public static TheoryData TimeoutValidData => new TheoryData diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs index ed1fecc8d9..4023b5080f 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -31,7 +31,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { public class Http2TestBase : TestApplicationErrorLoggerLoggedTest, IDisposable, IHttpHeadersHandler { - protected static readonly string _4kHeaderValue = new string('a', HPackDecoder.MaxStringOctets); + protected static readonly int MaxRequestHeaderFieldSize = 8192; + protected static readonly string _4kHeaderValue = new string('a', 4096); protected static readonly IEnumerable> _browserRequestHeaders = new[] { @@ -99,7 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests ); _pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions); - _hpackDecoder = new HPackDecoder((int)_clientSettings.HeaderTableSize); + _hpackDecoder = new HPackDecoder((int)_clientSettings.HeaderTableSize, MaxRequestHeaderFieldSize); _noopApplication = context => Task.CompletedTask;