Detect HTTP/2 connection sent to HTTP/1.1 endpoint (#39402)

Co-authored-by: Stephen Halter <halter73@gmail.com>

Author: JamesNK

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<byte> 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<byte> 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<ITlsHandshakeFeature>() == 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) { }
 }

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

@@ -593,7 +593,10 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> 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();
         }

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<IHttpRequestFeature>().Scheme);
     }
 
-    [Fact]
-    public void ResetResetsTraceIdentifier()
-    {
-        _http1Connection.TraceIdentifier = "xyz";
-
-        _http1Connection.Reset();
-
-        var nextId = ((IFeatureCollection)_http1Connection).Get<IHttpRequestIdentifierFeature>().TraceIdentifier;
-        Assert.NotEqual("xyz", nextId);
-
-        _http1Connection.Reset();
-        var secondId = ((IFeatureCollection)_http1Connection).Get<IHttpRequestIdentifierFeature>().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<IHttpRequestIdentifierFeature>();
-        // 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<byte> reader = new SequenceReader<byte>(read.Buffer);
+            if (reader.TryReadTo(out ReadOnlySequence<byte> _, 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]

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<KeyValuePair<string, object>>, IDisposable
     {
         private IDisposable _subscription;