From a217206f1fe0d7a8abb67aaac54b3616e25d6f87 Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Thu, 7 Jun 2018 18:40:27 -0700 Subject: [PATCH] Enforce max frame size #2651 --- src/Kestrel.Core/CoreStrings.resx | 6 ++-- .../Internal/Http2/Http2Connection.cs | 2 +- .../Internal/Http2/Http2FrameReader.cs | 7 ++++- .../Properties/CoreStrings.Designer.cs | 28 +++++++++---------- .../Http2ConnectionTests.cs | 24 +++++++++++++++- 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index ee80a16312..0230dfca6b 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -497,9 +497,6 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l The endpoint {endpointName} specified multiple certificate sources. - - HTTP/2 support is experimental, see https://go.microsoft.com/fwlink/?linkid=866785 to enable it. - Cannot write to the response body, the response has completed. @@ -518,4 +515,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l The connection was timed out by the server. + + The received frame size of {size} exceeds the limit {limit}. + \ No newline at end of file diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index 94ff8f5c54..f458436662 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -157,7 +157,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { if (!readableBuffer.IsEmpty) { - if (Http2FrameReader.ReadFrame(readableBuffer, _incomingFrame, out consumed, out examined)) + if (Http2FrameReader.ReadFrame(readableBuffer, _incomingFrame, _serverSettings.MaxFrameSize, out consumed, out examined)) { Log.LogTrace($"Connection id {ConnectionId} received {_incomingFrame.Type} frame with flags 0x{_incomingFrame.Flags:x} and length {_incomingFrame.Length} for stream ID {_incomingFrame.StreamId}"); await ProcessFrameAsync(application); diff --git a/src/Kestrel.Core/Internal/Http2/Http2FrameReader.cs b/src/Kestrel.Core/Internal/Http2/Http2FrameReader.cs index 6b2ab584db..4ca9c09aed 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2FrameReader.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2FrameReader.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { public static class Http2FrameReader { - public static bool ReadFrame(ReadOnlySequence readableBuffer, Http2Frame frame, out SequencePosition consumed, out SequencePosition examined) + public static bool ReadFrame(ReadOnlySequence readableBuffer, Http2Frame frame, uint maxFrameSize, out SequencePosition consumed, out SequencePosition examined) { consumed = readableBuffer.Start; examined = readableBuffer.End; @@ -22,6 +22,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 var headerSlice = readableBuffer.Slice(0, Http2Frame.HeaderLength); headerSlice.CopyTo(frame.Raw); + if (frame.Length > maxFrameSize) + { + throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorFrameOverLimit(frame.Length, maxFrameSize), Http2ErrorCode.FRAME_SIZE_ERROR); + } + if (readableBuffer.Length < Http2Frame.HeaderLength + frame.Length) { return false; diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index c813873491..520d93cc2f 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -1778,20 +1778,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatMultipleCertificateSources(object endpointName) => string.Format(CultureInfo.CurrentCulture, GetString("MultipleCertificateSources", "endpointName"), endpointName); - /// - /// HTTP/2 support is experimental, see https://go.microsoft.com/fwlink/?linkid=866785 to enable it. - /// - internal static string Http2NotSupported - { - get => GetString("Http2NotSupported"); - } - - /// - /// HTTP/2 support is experimental, see https://go.microsoft.com/fwlink/?linkid=866785 to enable it. - /// - internal static string FormatHttp2NotSupported() - => GetString("Http2NotSupported"); - /// /// Cannot write to the response body, the response has completed. /// @@ -1876,6 +1862,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatConnectionTimedOutByServer() => GetString("ConnectionTimedOutByServer"); + /// + /// The received frame size of {size} exceeds the limit {limit}. + /// + internal static string Http2ErrorFrameOverLimit + { + get => GetString("Http2ErrorFrameOverLimit"); + } + + /// + /// The received frame size of {size} exceeds the limit {limit}. + /// + internal static string FormatHttp2ErrorFrameOverLimit(object size, object limit) + => string.Format(CultureInfo.CurrentCulture, GetString("Http2ErrorFrameOverLimit", "size", "limit"), size, limit); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs index c4836e60b7..7afba1e572 100644 --- a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs +++ b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs @@ -299,6 +299,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiStringNonNullCharacters(); } + [Fact] + public async Task Frame_Received_OverMaxSize_FrameError() + { + await InitializeConnectionAsync(_echoApplication); + + await StartStreamAsync(1, _browserRequestHeaders, endStream: false); + // Manually craft a frame where the size is too large. Our own frame class won't allow this. + // See Http2Frame.Length + var length = Http2Frame.MinAllowedMaxFrameSize + 1; // Too big + var frame = new byte[9 + length]; + frame[0] = (byte)((length & 0x00ff0000) >> 16); + frame[1] = (byte)((length & 0x0000ff00) >> 8); + frame[2] = (byte)(length & 0x000000ff); + await SendAsync(frame); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: true, + expectedLastStreamId: 1, + expectedErrorCode: Http2ErrorCode.FRAME_SIZE_ERROR, + expectedErrorMessage: CoreStrings.FormatHttp2ErrorFrameOverLimit(length, Http2Frame.MinAllowedMaxFrameSize)); + } + [Fact] public async Task DATA_Received_ReadByStream() { @@ -2794,7 +2816,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { Assert.True(buffer.Length > 0); - if (Http2FrameReader.ReadFrame(buffer, frame, out consumed, out examined)) + if (Http2FrameReader.ReadFrame(buffer, frame, 16_384, out consumed, out examined)) { return frame; }