Procházet zdrojové kódy

Send 431 when HTTP/2&3 headers are too large or many #17861 #33622 (#44668)

Chris Ross před 3 roky
rodič
revize
90802f2bef

+ 5 - 1
src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

@@ -63,6 +63,8 @@ internal abstract partial class HttpProtocol : IHttpResponseControl
 
     private string? _requestId;
     private int _requestHeadersParsed;
+    // See MaxRequestHeaderCount, enforced during parsing and may be more relaxed to avoid connection faults.
+    protected int _eagerRequestHeadersParsedLimit;
 
     private long _responseBytesWritten;
 
@@ -107,6 +109,7 @@ internal abstract partial class HttpProtocol : IHttpResponseControl
     public long? MaxRequestBodySize { get; set; }
     public MinDataRate? MinRequestBodyDataRate { get; set; }
     public bool AllowSynchronousIO { get; set; }
+    protected int RequestHeadersParsed => _requestHeadersParsed;
 
     /// <summary>
     /// The request id. <seealso cref="HttpContext.TraceIdentifier"/>
@@ -416,6 +419,7 @@ internal abstract partial class HttpProtocol : IHttpResponseControl
         Output?.Reset();
 
         _requestHeadersParsed = 0;
+        _eagerRequestHeadersParsedLimit = ServerOptions.Limits.MaxRequestHeaderCount;
 
         _responseBytesWritten = 0;
 
@@ -546,7 +550,7 @@ internal abstract partial class HttpProtocol : IHttpResponseControl
     private void IncrementRequestHeadersCount()
     {
         _requestHeadersParsed++;
-        if (_requestHeadersParsed > ServerOptions.Limits.MaxRequestHeaderCount)
+        if (_requestHeadersParsed > _eagerRequestHeadersParsedLimit)
         {
             KestrelBadHttpRequestException.Throw(RequestRejectionReason.TooManyHeaders);
         }

+ 6 - 2
src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

@@ -1130,6 +1130,8 @@ internal sealed partial class Http2Connection : IHttp2StreamLifetimeHandler, IHt
 
         try
         {
+            _currentHeadersStream.TotalParsedHeaderSize = _totalParsedHeaderSize;
+
             // This must be initialized before we offload the request or else we may start processing request body frames without it.
             _currentHeadersStream.InputRemaining = _currentHeadersStream.RequestHeaders.ContentLength;
 
@@ -1412,8 +1414,10 @@ internal sealed partial class Http2Connection : IHttp2StreamLifetimeHandler, IHt
 
         // https://tools.ietf.org/html/rfc7540#section-6.5.2
         // "The value is based on the uncompressed size of header fields, including the length of the name and value in octets plus an overhead of 32 octets for each header field.";
-        _totalParsedHeaderSize += HeaderField.RfcOverhead + name.Length + value.Length;
-        if (_totalParsedHeaderSize > _context.ServiceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize)
+        // We don't include the 32 byte overhead hear so we can accept a little more than the advertised limit.
+        _totalParsedHeaderSize += name.Length + value.Length;
+        // Allow a 2x grace before aborting the connection. We'll check the size limit again later where we can send a 431.
+        if (_totalParsedHeaderSize > _context.ServiceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize * 2)
         {
             throw new Http2ConnectionErrorException(CoreStrings.BadRequest_HeadersExceedMaxTotalSize, Http2ErrorCode.PROTOCOL_ERROR);
         }

+ 17 - 0
src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs

@@ -25,6 +25,8 @@ internal abstract partial class Http2Stream : HttpProtocol, IThreadPoolWorkItem,
 
     private bool _decrementCalled;
 
+    public int TotalParsedHeaderSize { get; set; }
+
     public Pipe RequestBodyPipe { get; private set; } = default!;
 
     internal long DrainExpirationTicks { get; set; }
@@ -41,6 +43,9 @@ internal abstract partial class Http2Stream : HttpProtocol, IThreadPoolWorkItem,
         InputRemaining = null;
         RequestBodyStarted = false;
         DrainExpirationTicks = 0;
+        TotalParsedHeaderSize = 0;
+        // Allow up to 2x during parsing, enforce the hard limit after when we can preserve the connection.
+        _eagerRequestHeadersParsedLimit = ServerOptions.Limits.MaxRequestHeaderCount * 2;
 
         _context = context;
 
@@ -198,6 +203,18 @@ internal abstract partial class Http2Stream : HttpProtocol, IThreadPoolWorkItem,
         // do the reading from a pipeline, nor do we use endConnection to report connection-level errors.
         endConnection = !TryValidatePseudoHeaders();
 
+        // 431 if the headers are too large
+        if (TotalParsedHeaderSize > ServerOptions.Limits.MaxRequestHeadersTotalSize)
+        {
+            KestrelBadHttpRequestException.Throw(RequestRejectionReason.HeadersExceedMaxTotalSize);
+        }
+
+        // 431 if we received too many headers
+        if (RequestHeadersParsed > ServerOptions.Limits.MaxRequestHeaderCount)
+        {
+            KestrelBadHttpRequestException.Throw(RequestRejectionReason.TooManyHeaders);
+        }
+
         // Suppress pseudo headers from the public headers collection.
         HttpRequestHeaders.ClearPseudoRequestHeaders();
 

+ 19 - 3
src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs

@@ -96,6 +96,8 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS
         _requestHeaderParsingState = default;
         _parsedPseudoHeaderFields = default;
         _totalParsedHeaderSize = 0;
+        // Allow up to 2x during parsing, enforce the hard limit after when we can preserve the connection.
+        _eagerRequestHeadersParsedLimit = ServerOptions.Limits.MaxRequestHeaderCount * 2;
         _isMethodConnect = false;
         _completionState = default;
         StreamTimeoutTicks = 0;
@@ -275,10 +277,12 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS
 
     private void OnHeaderCore(HeaderType headerType, int? staticTableIndex, ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
     {
-        // https://tools.ietf.org/html/rfc7540#section-6.5.2
+        // https://httpwg.org/specs/rfc9114.html#rfc.section.4.2.2
         // "The value is based on the uncompressed size of header fields, including the length of the name and value in octets plus an overhead of 32 octets for each header field.";
-        _totalParsedHeaderSize += HeaderField.RfcOverhead + name.Length + value.Length;
-        if (_totalParsedHeaderSize > _context.ServiceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize)
+        // We don't include the 32 byte overhead hear so we can accept a little more than the advertised limit.
+        _totalParsedHeaderSize += name.Length + value.Length;
+        // Allow a 2x grace before aborting the stream. We'll check the size limit again later where we can send a 431.
+        if (_totalParsedHeaderSize > ServerOptions.Limits.MaxRequestHeadersTotalSize * 2)
         {
             throw new Http3StreamErrorException(CoreStrings.BadRequest_HeadersExceedMaxTotalSize, Http3ErrorCode.RequestRejected);
         }
@@ -939,6 +943,18 @@ internal abstract partial class Http3Stream : HttpProtocol, IHttp3Stream, IHttpS
     {
         endConnection = !TryValidatePseudoHeaders();
 
+        // 431 if the headers are too large
+        if (_totalParsedHeaderSize > ServerOptions.Limits.MaxRequestHeadersTotalSize)
+        {
+            KestrelBadHttpRequestException.Throw(RequestRejectionReason.HeadersExceedMaxTotalSize);
+        }
+
+        // 431 if we received too many headers
+        if (RequestHeadersParsed > ServerOptions.Limits.MaxRequestHeaderCount)
+        {
+            KestrelBadHttpRequestException.Throw(RequestRejectionReason.TooManyHeaders);
+        }
+
         // Suppress pseudo headers from the public headers collection.
         HttpRequestHeaders.ClearPseudoRequestHeaders();
 

+ 1 - 0
src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs

@@ -89,6 +89,7 @@ public class Http1ConnectionTests : Http1ConnectionTestsBase
     {
         const string headerLines = "Header-1: value1\r\nHeader-2: value2\r\n";
         _serviceContext.ServerOptions.Limits.MaxRequestHeaderCount = 1;
+        _http1Connection.Initialize(_http1ConnectionContext);
 
         await _application.Output.WriteAsync(Encoding.ASCII.GetBytes($"{headerLines}\r\n"));
         var readableBuffer = (await _transport.Input.ReadAsync()).Buffer;

+ 2 - 3
src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs

@@ -650,7 +650,7 @@ internal class Http3StreamBase
 
 internal class Http3RequestHeaderHandler
 {
-    public readonly byte[] HeaderEncodingBuffer = new byte[64 * 1024];
+    public readonly byte[] HeaderEncodingBuffer = new byte[96 * 1024];
     public readonly QPackDecoder QpackDecoder = new QPackDecoder(8192);
     public readonly Dictionary<string, string> DecodedHeaders = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
 }
@@ -699,9 +699,8 @@ internal class Http3RequestStream : Http3StreamBase, IHttpStreamHeadersHandler
         var done = QPackHeaderWriter.BeginEncodeHeaders(headers, buffer.Span, ref headersTotalSize, out var length);
         if (!done)
         {
-            throw new InvalidOperationException("Headers not sent.");
+            throw new InvalidOperationException("The headers are too large.");
         }
-
         await SendFrameAsync(Http3FrameType.Headers, buffer.Slice(0, length), endStream);
     }
 

+ 11 - 3
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs

@@ -2724,7 +2724,7 @@ public class Http2ConnectionTests : Http2TestBase
     [Fact]
     public Task HEADERS_Received_HeaderBlockOverLimit_ConnectionError()
     {
-        // > 32kb
+        // > 32kb * 2 to exceed graceful handling limit
         var headers = new[]
         {
             new KeyValuePair<string, string>(InternalHeaderNames.Method, "GET"),
@@ -2738,6 +2738,14 @@ public class Http2ConnectionTests : Http2TestBase
             new KeyValuePair<string, string>("f", _4kHeaderValue),
             new KeyValuePair<string, string>("g", _4kHeaderValue),
             new KeyValuePair<string, string>("h", _4kHeaderValue),
+            new KeyValuePair<string, string>("i", _4kHeaderValue),
+            new KeyValuePair<string, string>("j", _4kHeaderValue),
+            new KeyValuePair<string, string>("k", _4kHeaderValue),
+            new KeyValuePair<string, string>("l", _4kHeaderValue),
+            new KeyValuePair<string, string>("m", _4kHeaderValue),
+            new KeyValuePair<string, string>("n", _4kHeaderValue),
+            new KeyValuePair<string, string>("o", _4kHeaderValue),
+            new KeyValuePair<string, string>("p", _4kHeaderValue),
         };
 
         return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, CoreStrings.BadRequest_HeadersExceedMaxTotalSize);
@@ -2746,7 +2754,7 @@ public class Http2ConnectionTests : Http2TestBase
     [Fact]
     public Task HEADERS_Received_TooManyHeaders_ConnectionError()
     {
-        // > MaxRequestHeaderCount (100)
+        // > MaxRequestHeaderCount (100) * 2 to exceed graceful handling limit
         var headers = new List<KeyValuePair<string, string>>();
         headers.AddRange(new[]
         {
@@ -2754,7 +2762,7 @@ public class Http2ConnectionTests : Http2TestBase
             new KeyValuePair<string, string>(InternalHeaderNames.Path, "/"),
             new KeyValuePair<string, string>(InternalHeaderNames.Scheme, "http"),
         });
-        for (var i = 0; i < 100; i++)
+        for (var i = 0; i < 200; i++)
         {
             headers.Add(new KeyValuePair<string, string>(i.ToString(CultureInfo.InvariantCulture), i.ToString(CultureInfo.InvariantCulture)));
         }

+ 72 - 0
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Buffers;
 using System.Collections.Generic;
+using System.Globalization;
 using System.IO;
 using System.Linq;
 using System.Net.Http;
@@ -797,6 +798,77 @@ public class Http2StreamTests : Http2TestBase
         await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
     }
 
+    [Fact]
+    public async Task HEADERS_Received_MaxRequestHeadersTotalSize_431()
+    {
+        // > 32kb
+        var headers = new[]
+        {
+            new KeyValuePair<string, string>(InternalHeaderNames.Method, "GET"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Path, "/"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Scheme, "http"),
+            new KeyValuePair<string, string>("a", _4kHeaderValue),
+            new KeyValuePair<string, string>("b", _4kHeaderValue),
+            new KeyValuePair<string, string>("c", _4kHeaderValue),
+            new KeyValuePair<string, string>("d", _4kHeaderValue),
+            new KeyValuePair<string, string>("e", _4kHeaderValue),
+            new KeyValuePair<string, string>("f", _4kHeaderValue),
+            new KeyValuePair<string, string>("g", _4kHeaderValue),
+            new KeyValuePair<string, string>("h", _4kHeaderValue),
+        };
+        await InitializeConnectionAsync(_notImplementedApp);
+
+        await StartStreamAsync(1, headers, endStream: true);
+
+        var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
+            withLength: 40,
+            withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
+            withStreamId: 1);
+
+        await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
+
+        _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: true, handler: this);
+
+        Assert.Equal(3, _decodedHeaders.Count);
+        Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
+        Assert.Equal("431", _decodedHeaders[InternalHeaderNames.Status]);
+        Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]);
+    }
+
+    [Fact]
+    public async Task HEADERS_Received_MaxRequestHeaderCount_431()
+    {
+        // > 100 headers
+        var headers = new List<KeyValuePair<string, string>>()
+        {
+            new KeyValuePair<string, string>(InternalHeaderNames.Method, "GET"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Path, "/"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Scheme, "http"),
+        };
+        for (var i = 0; i < 101; i++)
+        {
+            var text = i.ToString(CultureInfo.InvariantCulture);
+            headers.Add(new KeyValuePair<string, string>(text, text));
+        }
+        await InitializeConnectionAsync(_notImplementedApp);
+
+        await StartStreamAsync(1, headers, endStream: true);
+
+        var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
+            withLength: 40,
+            withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
+            withStreamId: 1);
+
+        await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
+
+        _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: true, handler: this);
+
+        Assert.Equal(3, _decodedHeaders.Count);
+        Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
+        Assert.Equal("431", _decodedHeaders[InternalHeaderNames.Status]);
+        Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]);
+    }
+
     [Fact]
     public async Task ContentLength_Received_SingleDataFrame_Verified()
     {

+ 2 - 0
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs

@@ -134,6 +134,7 @@ public class Http2TestBase : TestApplicationErrorLoggerLoggedTest, IDisposable,
     protected readonly TaskCompletionSource _closedStateReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
 
     protected readonly RequestDelegate _noopApplication;
+    protected readonly RequestDelegate _notImplementedApp;
     protected readonly RequestDelegate _readHeadersApplication;
     protected readonly RequestDelegate _readTrailersApplication;
     protected readonly RequestDelegate _bufferingApplication;
@@ -176,6 +177,7 @@ public class Http2TestBase : TestApplicationErrorLoggerLoggedTest, IDisposable,
         });
 
         _noopApplication = context => Task.CompletedTask;
+        _notImplementedApp = _ => throw new NotImplementedException();
 
         _readHeadersApplication = context =>
         {

+ 69 - 2
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs

@@ -2300,7 +2300,7 @@ public class Http3StreamTests : Http3TestBase
     }
 
     [Fact]
-    public Task HEADERS_Received_HeaderBlockOverLimit_ConnectionError()
+    public async Task HEADERS_Received_HeaderBlockOverLimit_431()
     {
         // > 32kb
         var headers = new[]
@@ -2318,11 +2318,50 @@ public class Http3StreamTests : Http3TestBase
             new KeyValuePair<string, string>("h", _4kHeaderValue),
         };
 
+        var requestStream = await Http3Api.InitializeConnectionAndStreamsAsync(_notImplementedApp, headers, endStream: true);
+
+        var receivedHeaders = await requestStream.ExpectHeadersAsync();
+
+        await requestStream.ExpectReceiveEndOfStream();
+
+        Assert.Equal(3, receivedHeaders.Count);
+        Assert.Contains("date", receivedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
+        Assert.Equal("431", receivedHeaders[InternalHeaderNames.Status]);
+        Assert.Equal("0", receivedHeaders[HeaderNames.ContentLength]);
+    }
+
+    [Fact]
+    public Task HEADERS_Received_HeaderBlockOverLimitx2_ConnectionError()
+    {
+        // > 32kb * 2 to exceed graceful handling limit
+        var headers = new[]
+        {
+            new KeyValuePair<string, string>(InternalHeaderNames.Method, "GET"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Path, "/"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Scheme, "http"),
+            new KeyValuePair<string, string>("a", _4kHeaderValue),
+            new KeyValuePair<string, string>("b", _4kHeaderValue),
+            new KeyValuePair<string, string>("c", _4kHeaderValue),
+            new KeyValuePair<string, string>("d", _4kHeaderValue),
+            new KeyValuePair<string, string>("e", _4kHeaderValue),
+            new KeyValuePair<string, string>("f", _4kHeaderValue),
+            new KeyValuePair<string, string>("g", _4kHeaderValue),
+            new KeyValuePair<string, string>("h", _4kHeaderValue),
+            new KeyValuePair<string, string>("i", _4kHeaderValue),
+            new KeyValuePair<string, string>("j", _4kHeaderValue),
+            new KeyValuePair<string, string>("k", _4kHeaderValue),
+            new KeyValuePair<string, string>("l", _4kHeaderValue),
+            new KeyValuePair<string, string>("m", _4kHeaderValue),
+            new KeyValuePair<string, string>("n", _4kHeaderValue),
+            new KeyValuePair<string, string>("o", _4kHeaderValue),
+            new KeyValuePair<string, string>("p", _4kHeaderValue),
+        };
+
         return HEADERS_Received_InvalidHeaderFields_StreamError(headers, CoreStrings.BadRequest_HeadersExceedMaxTotalSize, Http3ErrorCode.RequestRejected);
     }
 
     [Fact]
-    public Task HEADERS_Received_TooManyHeaders_ConnectionError()
+    public async Task HEADERS_Received_TooManyHeaders_431()
     {
         // > MaxRequestHeaderCount (100)
         var headers = new List<KeyValuePair<string, string>>();
@@ -2337,6 +2376,34 @@ public class Http3StreamTests : Http3TestBase
             headers.Add(new KeyValuePair<string, string>(i.ToString(CultureInfo.InvariantCulture), i.ToString(CultureInfo.InvariantCulture)));
         }
 
+        var requestStream = await Http3Api.InitializeConnectionAndStreamsAsync(_notImplementedApp, headers, endStream: true);
+
+        var receivedHeaders = await requestStream.ExpectHeadersAsync();
+
+        await requestStream.ExpectReceiveEndOfStream();
+
+        Assert.Equal(3, receivedHeaders.Count);
+        Assert.Contains("date", receivedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
+        Assert.Equal("431", receivedHeaders[InternalHeaderNames.Status]);
+        Assert.Equal("0", receivedHeaders[HeaderNames.ContentLength]);
+    }
+
+    [Fact]
+    public Task HEADERS_Received_TooManyHeadersx2_ConnectionError()
+    {
+        // > MaxRequestHeaderCount (100) * 2 to exceed graceful handling limit
+        var headers = new List<KeyValuePair<string, string>>();
+        headers.AddRange(new[]
+        {
+            new KeyValuePair<string, string>(InternalHeaderNames.Method, "GET"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Path, "/"),
+            new KeyValuePair<string, string>(InternalHeaderNames.Scheme, "http"),
+        });
+        for (var i = 0; i < 200; i++)
+        {
+            headers.Add(new KeyValuePair<string, string>(i.ToString(CultureInfo.InvariantCulture), i.ToString(CultureInfo.InvariantCulture)));
+        }
+
         return HEADERS_Received_InvalidHeaderFields_StreamError(headers, CoreStrings.BadRequest_TooManyHeaders);
     }
 

+ 2 - 0
src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3TestBase.cs

@@ -49,6 +49,7 @@ public abstract class Http3TestBase : TestApplicationErrorLoggerLoggedTest, IDis
     internal readonly Mock<ITimeoutHandler> _mockTimeoutHandler = new Mock<ITimeoutHandler>();
 
     protected readonly RequestDelegate _noopApplication;
+    protected readonly RequestDelegate _notImplementedApp;
     protected readonly RequestDelegate _echoApplication;
     protected readonly RequestDelegate _readRateApplication;
     protected readonly RequestDelegate _echoMethod;
@@ -79,6 +80,7 @@ public abstract class Http3TestBase : TestApplicationErrorLoggerLoggedTest, IDis
     public Http3TestBase()
     {
         _noopApplication = context => Task.CompletedTask;
+        _notImplementedApp = _ => throw new NotImplementedException();
 
         _echoApplication = async context =>
         {

+ 3 - 3
src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs

@@ -1369,10 +1369,10 @@ public class HttpClientHttp2InteropTests : LoggedTest
         {
             request.Headers.Add("header" + i, oneKbString + i);
         }
-        // Kestrel closes the connection rather than sending the recommended 431 response. https://github.com/dotnet/aspnetcore/issues/17861
-        await Assert.ThrowsAsync<HttpRequestException>(() => client.SendAsync(request)).DefaultTimeout();
-
+        var response = await client.SendAsync(request).DefaultTimeout();
         await host.StopAsync().DefaultTimeout();
+
+        Assert.Equal(HttpStatusCode.RequestHeaderFieldsTooLarge, response.StatusCode);
     }
 
     [Theory]