diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index f088a9973a75..230cbc3f90f9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -6,13 +6,17 @@ using System.Globalization; using System.IO.Pipelines; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; internal partial class Http1Connection : HttpProtocol, IRequestProcessor, IHttpOutputAborter { + internal static ReadOnlySpan Http2GoAwayHttp11RequiredBytes => new byte[17] { 0, 0, 8, 7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 13 }; + private const byte ByteAsterisk = (byte)'*'; private const byte ByteForwardSlash = (byte)'/'; private const string Asterisk = "*"; @@ -39,6 +43,9 @@ internal partial class Http1Connection : HttpProtocol, IRequestProcessor, IHttpO private long _remainingRequestHeadersBytesAllowed; + // Tracks whether a HTTP/2 preface was detected during the first request. + private bool _http2PrefaceDetected; + public Http1Connection(HttpConnectionContext context) { Initialize(context); @@ -618,7 +625,6 @@ protected override void OnReset() _requestTargetForm = HttpRequestTarget.Unknown; _absoluteRequestTarget = null; _remainingRequestHeadersBytesAllowed = ServerOptions.Limits.MaxRequestHeadersTotalSize + 2; - _requestCount++; MinResponseDataRate = ServerOptions.Limits.MinResponseDataRate; @@ -642,6 +648,7 @@ protected override void BeginRequestProcessing() { // Reset the features and timeout. Reset(); + _requestCount++; TimeoutControl.SetTimeout(_keepAliveTicks, TimeoutReason.KeepAlive); } @@ -667,6 +674,14 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio } throw; } +#pragma warning disable CS0618 // Type or member is obsolete + catch (BadHttpRequestException ex) + { + DetectHttp2Preface(result.Buffer, ex); + + throw; + } +#pragma warning restore CS0618 // Type or member is obsolete finally { Input.AdvanceTo(reader.Position, isConsumed ? reader.Position : result.Buffer.End); @@ -713,5 +728,45 @@ protected override bool TryParseRequest(ReadResult result, out bool endConnectio } } +#pragma warning disable CS0618 // Type or member is obsolete + private void DetectHttp2Preface(ReadOnlySequence requestData, BadHttpRequestException ex) +#pragma warning restore CS0618 // Type or member is obsolete + { + const int PrefaceLineLength = 16; + + // Only check for HTTP/2 preface on non-TLS connection. + // When TLS is used then ALPN is used to negotiate correct version. + if (ConnectionFeatures.Get() == null) + { + // If there is an unrecognized HTTP version, it is the first request on the connection, and the request line + // bytes matches the HTTP/2 preface request line bytes then log and return a HTTP/2 GOAWAY frame. + if (ex.Reason == RequestRejectionReason.UnrecognizedHTTPVersion + && _requestCount == 1 + && requestData.Length >= PrefaceLineLength) + { + var clientPrefaceRequestLine = Http2.Http2Connection.ClientPreface.Slice(0, PrefaceLineLength); + var currentRequestLine = requestData.Slice(0, PrefaceLineLength).ToSpan(); + if (currentRequestLine.SequenceEqual(clientPrefaceRequestLine)) + { + Log.PossibleInvalidHttpVersionDetected(ConnectionId, Http.HttpVersion.Http11, Http.HttpVersion.Http2); + + // Can't write GOAWAY here. Set flag so TryProduceInvalidRequestResponse writes GOAWAY. + _http2PrefaceDetected = true; + } + } + } + } + + protected override Task TryProduceInvalidRequestResponse() + { + if (_http2PrefaceDetected) + { + _context.Transport.Output.Write(Http2GoAwayHttp11RequiredBytes); + return _context.Transport.Output.FlushAsync().GetAsTask(); + } + + return base.TryProduceInvalidRequestResponse(); + } + void IRequestProcessor.Tick(DateTimeOffset now) { } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index fc6508fe0a7c..298c16dd8a55 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -593,7 +593,10 @@ public async Task ProcessRequestsAsync(IHttpApplication appl { try { - await TryProduceInvalidRequestResponse(); + if (_requestRejectedException != null) + { + await TryProduceInvalidRequestResponse(); + } } catch (Exception ex) { @@ -985,10 +988,12 @@ private void VerifyInitializeState(int firstWriteByteCount) VerifyAndUpdateWrite(firstWriteByteCount); } - protected Task TryProduceInvalidRequestResponse() + protected virtual Task TryProduceInvalidRequestResponse() { - // If _requestAborted is set, the connection has already been closed. - if (_requestRejectedException != null && !_connectionAborted) + Debug.Assert(_requestRejectedException != null); + + // If _connectionAborted is set, the connection has already been closed. + if (!_connectionAborted) { return ProduceEnd(); } diff --git a/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs b/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs index 78d1cb5ac849..25d6961a0279 100644 --- a/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs @@ -3,6 +3,7 @@ using System.Buffers; using System.Collections; +using System.IO.Pipelines; using System.Text; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http; @@ -146,21 +147,6 @@ public void ResetResetsScheme() Assert.Equal("http", ((IFeatureCollection)_http1Connection).Get().Scheme); } - [Fact] - public void ResetResetsTraceIdentifier() - { - _http1Connection.TraceIdentifier = "xyz"; - - _http1Connection.Reset(); - - var nextId = ((IFeatureCollection)_http1Connection).Get().TraceIdentifier; - Assert.NotEqual("xyz", nextId); - - _http1Connection.Reset(); - var secondId = ((IFeatureCollection)_http1Connection).Get().TraceIdentifier; - Assert.NotEqual(nextId, secondId); - } - [Fact] public void ResetResetsMinRequestBodyDataRate() { @@ -182,31 +168,73 @@ public void ResetResetsMinResponseDataRate() } [Fact] - public void TraceIdentifierCountsRequestsPerHttp1Connection() + public async Task TraceIdentifierCountsRequestsPerHttp1Connection() { var connectionId = _http1ConnectionContext.ConnectionId; var feature = ((IFeatureCollection)_http1Connection).Get(); - // Reset() is called once in the test ctor + + var requestProcessingTask = _http1Connection.ProcessRequestsAsync(new DummyApplication()); + var count = 1; - void Reset() + async Task SendRequestAsync() { - _http1Connection.Reset(); + var data = Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\nHost:\r\n\r\n"); + await _application.Output.WriteAsync(data); + + while (true) + { + var read = await _application.Input.ReadAsync(); + SequencePosition consumed = read.Buffer.Start; + SequencePosition examined = read.Buffer.End; + try + { + if (TryReadResponse(read, out consumed, out examined)) + { + break; + } + } + finally + { + _application.Input.AdvanceTo(consumed, examined); + } + } + count++; } + static bool TryReadResponse(ReadResult read, out SequencePosition consumed, out SequencePosition examined) + { + consumed = read.Buffer.Start; + examined = read.Buffer.End; + + SequenceReader reader = new SequenceReader(read.Buffer); + if (reader.TryReadTo(out ReadOnlySequence _, new byte[] { (byte)'\r', (byte)'\n', (byte)'\r', (byte)'\n' }, advancePastDelimiter: true)) + { + consumed = reader.Position; + examined = reader.Position; + return true; + } + + return false; + } + var nextId = feature.TraceIdentifier; Assert.Equal($"{connectionId}:00000001", nextId); - Reset(); + await SendRequestAsync(); + var secondId = feature.TraceIdentifier; Assert.Equal($"{connectionId}:00000002", secondId); - var big = 1_000_000; + var big = 10_000; while (big-- > 0) { - Reset(); + await SendRequestAsync(); } Assert.Equal($"{connectionId}:{count:X8}", feature.TraceIdentifier); + + _http1Connection.StopProcessingNextRequest(); + await requestProcessingTask.DefaultTimeout(); } [Fact] @@ -216,9 +244,6 @@ public void TraceIdentifierGeneratesWhenNull() var id = _http1Connection.TraceIdentifier; Assert.NotNull(id); Assert.Equal(id, _http1Connection.TraceIdentifier); - - _http1Connection.Reset(); - Assert.NotEqual(id, _http1Connection.TraceIdentifier); } [Fact] diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs index b5ae949c60e5..4bc3117769af 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Text; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport; using Microsoft.AspNetCore.Testing; @@ -193,6 +195,23 @@ await client.SendAll( } } + [Fact] + public async Task BadRequestForHttp2() + { + await using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory))) + { + using (var client = server.CreateConnection()) + { + await client.Stream.WriteAsync(Core.Internal.Http2.Http2Connection.ClientPreface.ToArray()).DefaultTimeout(); + + var data = await client.Stream.ReadAtLeastLengthAsync(17); + + Assert.Equal(Http1Connection.Http2GoAwayHttp11RequiredBytes.ToArray(), data); + Assert.Empty(await client.Stream.ReadUntilEndAsync().DefaultTimeout()); + } + } + } + private class BadRequestEventListener : IObserver>, IDisposable { private IDisposable _subscription;