diff --git a/cadente/Sisk.Cadente/HttpConnection.cs b/cadente/Sisk.Cadente/HttpConnection.cs index fe396fb..d2a81ea 100644 --- a/cadente/Sisk.Cadente/HttpConnection.cs +++ b/cadente/Sisk.Cadente/HttpConnection.cs @@ -62,7 +62,12 @@ public async Task HandleConnectionEventsAsync ( Cancellatio while (!disposedValue) { - (HttpRequestBase? nextRequest, bufferedRequestLength) = await HttpRequestReader.TryReadHttpRequestAsync ( requestPool.Memory, bufferedRequestLength, networkStream, shutdownToken, headerParsingTimeout, headerReadTimeoutSource ).ConfigureAwait ( false ); + (HttpRequestBase? nextRequest, bufferedRequestLength, bool headersTooLarge) = await HttpRequestReader.TryReadHttpRequestAsync ( requestPool.Memory, bufferedRequestLength, networkStream, shutdownToken, headerParsingTimeout, headerReadTimeoutSource ).ConfigureAwait ( false ); + + if (headersTooLarge) { + await networkStream.WriteAsync ( HttpResponseSerializer.GetRawMessage ( "Request header fields too large.", 431, "Request Header Fields Too Large" ), shutdownToken ).ConfigureAwait ( false ); + await networkStream.FlushAsync ( shutdownToken ).ConfigureAwait ( false ); + } if (nextRequest is null) { Logger.LogInformation ( $"TryReadHttpRequestAsync returned no request" ); diff --git a/cadente/Sisk.Cadente/HttpHost.cs b/cadente/Sisk.Cadente/HttpHost.cs index 8c87b21..8a7a4d2 100644 --- a/cadente/Sisk.Cadente/HttpHost.cs +++ b/cadente/Sisk.Cadente/HttpHost.cs @@ -466,7 +466,15 @@ private static async Task WritePlainHttpRedirectAsync ( Stream stream ) { using var memoryPin = MemoryPool.Shared.Rent ( HttpConnection.RESERVED_BUFFER_SIZE ); using var shutdownToken = new CancellationTokenSource ( TimeSpan.FromSeconds ( 15 ) ); - (HttpRequestBase? rawRequest, _) = await HttpRequestReader.TryReadHttpRequestAsync ( memoryPin.Memory, bufferedLength: 0, stream, shutdownToken.Token, headerReadTimeoutMs: 5_000 ).ConfigureAwait ( false ); + (HttpRequestBase? rawRequest, _, bool headersTooLarge) = await HttpRequestReader.TryReadHttpRequestAsync ( memoryPin.Memory, bufferedLength: 0, stream, shutdownToken.Token, headerReadTimeoutMs: 5_000 ).ConfigureAwait ( false ); + + if (headersTooLarge) { + try { + await stream.WriteAsync ( HttpResponseSerializer.GetRawMessage ( "Request header fields too large.", 431, "Request Header Fields Too Large" ), shutdownToken.Token ).ConfigureAwait ( false ); + } + catch { } + return; + } if (rawRequest is null) { diff --git a/cadente/Sisk.Cadente/HttpSerializer/HttpRequestReader.cs b/cadente/Sisk.Cadente/HttpSerializer/HttpRequestReader.cs index 022ff69..02fa8cc 100644 --- a/cadente/Sisk.Cadente/HttpSerializer/HttpRequestReader.cs +++ b/cadente/Sisk.Cadente/HttpSerializer/HttpRequestReader.cs @@ -35,9 +35,11 @@ static class HttpRequestReader { private static ReadOnlySpan TrimChars => " \t\r\n"u8; + private static readonly SearchValues MethodTokenChars = SearchValues.Create ( "!#$%&'*+-.^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"u8 ); + [SkipLocalsInit] [MethodImpl ( MethodImplOptions.AggressiveOptimization )] - public static async ValueTask<(HttpRequestBase? Request, int BufferedLength)> TryReadHttpRequestAsync ( Memory sharedBuffer, int bufferedLength, Stream stream, CancellationToken cancellationToken = default, int headerReadTimeoutMs = DefaultHeaderReadTimeoutMs, CancellationTokenSource? timeoutSource = null ) { + public static async ValueTask<(HttpRequestBase? Request, int BufferedLength, bool HeadersTooLarge)> TryReadHttpRequestAsync ( Memory sharedBuffer, int bufferedLength, Stream stream, CancellationToken cancellationToken = default, int headerReadTimeoutMs = DefaultHeaderReadTimeoutMs, CancellationTokenSource? timeoutSource = null ) { int bufferLength = sharedBuffer.Length; int totalRead = bufferedLength; @@ -50,18 +52,18 @@ static class HttpRequestReader { ReadOnlySpan searchRegion = sharedBuffer.Span.Slice ( effectiveSearchStart, totalRead - effectiveSearchStart ); if (searchRegion.IndexOf ( HeaderTerminator ) >= 0) { - return (ParseHttpRequest ( sharedBuffer.Slice ( 0, totalRead ) ), totalRead); + return (ParseHttpRequest ( sharedBuffer.Slice ( 0, totalRead ) ), totalRead, false); } if (totalRead >= bufferLength) { Logger.LogInformation ( $"failed to parse HTTP request: headers too large" ); - return (null, totalRead); + return (null, totalRead, true); } long currentTicks = Environment.TickCount64; if (currentTicks >= deadlineTicks) { Logger.LogInformation ( $"failed to parse HTTP request: header read timeout" ); - return (null, totalRead); + return (null, totalRead, false); } int remainingMs = (int) (deadlineTicks - currentTicks); @@ -78,7 +80,7 @@ static class HttpRequestReader { if (bytesRead == 0) { Logger.LogInformation ( $"failed to parse HTTP request: connection closed" ); - return (null, totalRead); + return (null, totalRead, false); } totalRead += bytesRead; @@ -86,19 +88,19 @@ static class HttpRequestReader { } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { Logger.LogInformation ( $"failed to parse HTTP request: header read timeout" ); - return (null, totalRead); + return (null, totalRead, false); } catch (OperationCanceledException) { Logger.LogInformation ( $"failed to parse HTTP request: operation cancelled" ); - return (null, totalRead); + return (null, totalRead, false); } catch (SocketException sex) { Logger.LogInformation ( $"failed to parse HTTP request: {sex.Message}" ); - return (null, totalRead); + return (null, totalRead, false); } catch (Exception ex) { Logger.LogInformation ( $"failed to parse HTTP request (exception): {ex.Message}" ); - return (null, totalRead); + return (null, totalRead, false); } } @@ -146,6 +148,10 @@ private static async ValueTask ReadWithTimeoutAsync ( Stream stream, Memory } ReadOnlyMemory method = buffer.Slice ( 0, methodEnd ); + if (method.Span.ContainsAnyExcept ( MethodTokenChars )) { + Logger.LogInformation ( $"failed to parse HTTP request: request method is not a valid token" ); + return null; + } int pathStart = methodEnd + 1; int pathEndRel = span.Slice ( pathStart ).IndexOf ( Space ); @@ -164,6 +170,10 @@ private static async ValueTask ReadWithTimeoutAsync ( Stream stream, Memory Logger.LogInformation ( $"failed to parse HTTP request: fragment is not allowed in request target" ); return null; } + if (path.Span.IndexOfAnyExceptInRange ( (byte) 0x21, (byte) 0x7E ) >= 0) { + Logger.LogInformation ( $"failed to parse HTTP request: request target contains invalid bytes" ); + return null; + } int protocolStart = pathEnd + 1; int protocolLineEndRel = span.Slice ( protocolStart ).IndexOf ( LineFeed ); diff --git a/tests/Sisk.Core/Tests/CadenteSecurityTests.cs b/tests/Sisk.Core/Tests/CadenteSecurityTests.cs index 6429dd5..375793e 100644 --- a/tests/Sisk.Core/Tests/CadenteSecurityTests.cs +++ b/tests/Sisk.Core/Tests/CadenteSecurityTests.cs @@ -131,6 +131,47 @@ public async Task HttpsListener_RedirectsPlainHttpRequest () { } } + [TestMethod] + public async Task HttpsListener_OversizedPlainHttpRequest_RespondsWithSingle431 () { + int port = GetFreePort (); + using var certificate = CertificateHelper.CreateDevelopmentCertificate ( "localhost" ); + using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) { + Handler = new EmptyHandler (), + HttpsOptions = new HttpsOptions ( certificate ) + }; + host.TimeoutManager.SslHandshakeTimeout = CloseWaitTimeout; + host.Start (); + + using var client = new TcpClient (); + using var ioCts = new CancellationTokenSource ( CloseWaitTimeout ); + await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token ); + + await using var stream = client.GetStream (); + byte [] requestBytes = Encoding.ASCII.GetBytes ( + "GET / HTTP/1.1\r\n" + + $"Host: localhost:{port}\r\n" + + $"X-Large: {new string ( 'a', 9000 )}\r\n" + + "\r\n" ); + await stream.WriteAsync ( requestBytes, ioCts.Token ); + await stream.FlushAsync ( ioCts.Token ); + + using var responseBuffer = new MemoryStream (); + byte [] buffer = new byte [ 1024 ]; + try { + int read; + while ((read = await stream.ReadAsync ( buffer, ioCts.Token )) > 0) { + responseBuffer.Write ( buffer, 0, read ); + } + } + catch (IOException) { + } + + string response = Encoding.ASCII.GetString ( responseBuffer.ToArray () ); + + StringAssert.StartsWith ( response, "HTTP/1.1 431" ); + Assert.AreEqual ( -1, response.IndexOf ( "HTTP/1.1 ", 1, StringComparison.Ordinal ), "Only one response must be written." ); + } + [TestMethod] public async Task FixedLengthResponseStream_WithoutWrite_FlushesHeadersBeforeKeepAliveRead () { int port = GetFreePort (); @@ -233,6 +274,101 @@ public async Task TransferEncoding_RejectsUnsupportedCodingsBeforeHandler ( stri $"Unsupported Transfer-Encoding values must not produce a successful response. Response: {response}" ); } + [DataTestMethod] + [DataRow ( "GET", "/admin\u0080x" )] + [DataRow ( "GET", "/admin\u001bx" )] + [DataRow ( "GET", "/admin\nx" )] + [DataRow ( "GET", "/admin\0x" )] + [DataRow ( "GET", "/admin\u007fx" )] + [DataRow ( "G\u0001T", "/" )] + [DataRow ( "G@T", "/" )] + [DataRow ( "GET\u0080", "/" )] + public async Task InvalidRequestLineBytes_AreRejectedBeforeHandler ( string method, string target ) { + int port = GetFreePort (); + var handler = new ContextInvocationHandler (); + using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) { + Handler = handler + }; + host.Start (); + + using var client = new TcpClient (); + using var ioCts = new CancellationTokenSource ( CloseWaitTimeout ); + await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token ); + + await using var stream = client.GetStream (); + byte [] requestBytes = Encoding.Latin1.GetBytes ( + $"{method} {target} HTTP/1.1\r\n" + + $"Host: localhost:{port}\r\n" + + "Connection: close\r\n" + + "\r\n" ); + + await stream.WriteAsync ( requestBytes, ioCts.Token ); + await stream.FlushAsync ( ioCts.Token ); + + string response = await ReadHeadersAsync ( stream, ioCts.Token ); + + Assert.IsFalse ( handler.ContextCreated.Task.IsCompleted, "Invalid request lines must be rejected before user handlers run." ); + Assert.IsFalse ( response.StartsWith ( "HTTP/1.1 2", StringComparison.Ordinal ), $"Invalid request lines must not produce a successful response. Response: {response}" ); + } + + [TestMethod] + public async Task OversizedHeaderBlock_RespondsWith431 () { + int port = GetFreePort (); + var handler = new ContextInvocationHandler (); + using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) { + Handler = handler + }; + host.Start (); + + using var client = new TcpClient (); + using var ioCts = new CancellationTokenSource ( CloseWaitTimeout ); + await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token ); + + await using var stream = client.GetStream (); + byte [] requestBytes = Encoding.ASCII.GetBytes ( + "GET / HTTP/1.1\r\n" + + $"Host: localhost:{port}\r\n" + + $"X-Large: {new string ( 'a', 9000 )}\r\n" + + "\r\n" ); + + await stream.WriteAsync ( requestBytes, ioCts.Token ); + await stream.FlushAsync ( ioCts.Token ); + + string response = await ReadHeadersAsync ( stream, ioCts.Token ); + + Assert.IsFalse ( handler.ContextCreated.Task.IsCompleted, "Oversized header blocks must be rejected before user handlers run." ); + StringAssert.StartsWith ( response, "HTTP/1.1 431" ); + } + + [DataTestMethod] + [DataRow ( "GET", "/a?b=c%20d" )] + [DataRow ( "get", "/" )] + [DataRow ( "M-SEARCH", "/" )] + public async Task ValidRequestLine_StillReachesHandler ( string method, string target ) { + int port = GetFreePort (); + var handler = new ContextInvocationHandler (); + using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) { + Handler = handler + }; + host.Start (); + + using var client = new TcpClient (); + using var ioCts = new CancellationTokenSource ( CloseWaitTimeout ); + await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token ); + + await using var stream = client.GetStream (); + byte [] requestBytes = Encoding.ASCII.GetBytes ( + $"{method} {target} HTTP/1.1\r\n" + + $"Host: localhost:{port}\r\n" + + "Connection: close\r\n" + + "\r\n" ); + + await stream.WriteAsync ( requestBytes, ioCts.Token ); + await stream.FlushAsync ( ioCts.Token ); + + await handler.ContextCreated.Task.WaitAsync ( CloseWaitTimeout ); + } + [TestMethod] public async Task ContentEncoding_DoesNotAffectRequestFraming () { int port = GetFreePort ();