Detect HTTP/1.1 on HTTP/2 connection (#39358)

Co-authored-by: Chris Ross <chrross@microsoft.com>

Author: JamesNK

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

@@ -8,6 +8,7 @@
 using System.Net.Http;
 using System.Net.Http.HPack;
 using System.Security.Authentication;
+using System.Text;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Connections.Features;
 using Microsoft.AspNetCore.Hosting.Server;
@@ -23,6 +24,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
 internal partial class Http2Connection : IHttp2StreamLifetimeHandler, IHttpStreamHeadersHandler, IRequestProcessor
 {
     public static ReadOnlySpan<byte> ClientPreface => ClientPrefaceBytes;
+    public static byte[]? InvalidHttp1xErrorResponseBytes;
 
     private const PseudoHeaderFields _mandatoryRequestPseudoHeaderFields =
         PseudoHeaderFields.Method | PseudoHeaderFields.Path | PseudoHeaderFields.Scheme;
@@ -402,8 +404,40 @@ private void ValidateTlsRequirements()
         }
     }
 
+    [Flags]
+    private enum ReadPrefaceState
+    {
+        None = 0,
+        Preface = 1,
+        Http1x = 2,
+        All = Preface | Http1x
+    }
+
     private async Task<bool> TryReadPrefaceAsync()
     {
+        // HTTP/1.x and HTTP/2 support connections without TLS. That means ALPN hasn't been used to ensure both sides are
+        // using the same protocol. A common problem is someone using HTTP/1.x to talk to a HTTP/2 only endpoint.
+        //
+        // HTTP/2 starts a connection with a preface. This method reads and validates it. If the connection doesn't start
+        // with the preface, and it isn't using TLS, then we attempt to detect what the client is trying to do and send
+        // back a friendly error message.
+        //
+        // Outcomes from this method:
+        // 1. Successfully read HTTP/2 preface. Connection continues to be established.
+        // 2. Detect HTTP/1.x request. Send back HTTP/1.x 400 response.
+        // 3. Unknown content. Report HTTP/2 PROTOCOL_ERROR to client.
+        // 4. Timeout while waiting for content.
+        //
+        // Future improvement: Detect TLS frame. Useful for people starting TLS connection with a non-TLS endpoint.
+        var state = ReadPrefaceState.All;
+
+        // With TLS, ALPN should have already errored if the wrong HTTP version is used.
+        // Only perform additional validation if endpoint doesn't use TLS.
+        if (ConnectionFeatures.Get<ITlsHandshakeFeature>() != null)
+        {
+            state ^= ReadPrefaceState.Http1x;
+        }
+
         while (_isClosed == 0)
         {
             var result = await Input.ReadAsync();
@@ -415,9 +449,55 @@ private async Task<bool> TryReadPrefaceAsync()
             {
                 if (!readableBuffer.IsEmpty)
                 {
-                    if (ParsePreface(readableBuffer, out consumed, out examined))
+                    if (state.HasFlag(ReadPrefaceState.Preface))
+                    {
+                        if (readableBuffer.Length >= ClientPreface.Length)
+                        {
+                            if (IsPreface(readableBuffer, out consumed, out examined))
+                            {
+                                return true;
+                            }
+                            else
+                            {
+                                state ^= ReadPrefaceState.Preface;
+                            }
+                        }
+                    }
+
+                    if (state.HasFlag(ReadPrefaceState.Http1x))
+                    {
+                        if (ParseHttp1x(readableBuffer, out var detectedVersion))
+                        {
+                            if (detectedVersion == HttpVersion.Http10 || detectedVersion == HttpVersion.Http11)
+                            {
+                                Log.PossibleInvalidHttpVersionDetected(ConnectionId, HttpVersion.Http2, detectedVersion);
+
+                                var responseBytes = InvalidHttp1xErrorResponseBytes ??= Encoding.ASCII.GetBytes(
+                                    "HTTP/1.1 400 Bad Request\r\n" +
+                                    "Connection: close\r\n" +
+                                    "Content-Type: text/plain\r\n" +
+                                    "Content-Length: 56\r\n" +
+                                    "\r\n" +
+                                    "An HTTP/1.x request was sent to an HTTP/2 only endpoint.");
+
+                                await _context.Transport.Output.WriteAsync(responseBytes);
+
+                                // Close connection here so a GOAWAY frame isn't written.
+                                TryClose();
+
+                                return false;
+                            }
+                            else
+                            {
+                                state ^= ReadPrefaceState.Http1x;
+                            }
+                        }
+                    }
+
+                    // Tested all states. Return HTTP/2 protocol error.
+                    if (state == ReadPrefaceState.None)
                     {
-                        return true;
+                        throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR);
                     }
                 }
 
@@ -437,22 +517,44 @@ private async Task<bool> TryReadPrefaceAsync()
         return false;
     }
 
-    private static bool ParsePreface(in ReadOnlySequence<byte> buffer, out SequencePosition consumed, out SequencePosition examined)
+    private bool ParseHttp1x(ReadOnlySequence<byte> buffer, out HttpVersion httpVersion)
     {
-        consumed = buffer.Start;
-        examined = buffer.End;
+        httpVersion = HttpVersion.Unknown;
 
-        if (buffer.Length < ClientPreface.Length)
+        var reader = new SequenceReader<byte>(buffer.Length > Limits.MaxRequestLineSize ? buffer.Slice(0, Limits.MaxRequestLineSize) : buffer);
+        if (reader.TryReadTo(out ReadOnlySpan<byte> requestLine, (byte)'\n'))
         {
-            return false;
+            // Line should be long enough for HTTP/1.X and end with \r\n
+            if (requestLine.Length > 10 && requestLine[requestLine.Length - 1] == (byte)'\r')
+            {
+                httpVersion = HttpUtilities.GetKnownVersion(requestLine.Slice(requestLine.Length - 9, 8));
+            }
+
+            return true;
+        }
+
+        // Couldn't find newline within max request line size so this isn't valid HTTP/1.x.
+        if (buffer.Length > Limits.MaxRequestLineSize)
+        {
+            return true;
         }
 
+        return false;
+    }
+
+    private static bool IsPreface(in ReadOnlySequence<byte> buffer, out SequencePosition consumed, out SequencePosition examined)
+    {
+        consumed = buffer.Start;
+        examined = buffer.End;
+
+        Debug.Assert(buffer.Length >= ClientPreface.Length, "Not enough content to match preface.");
+
         var preface = buffer.Slice(0, ClientPreface.Length);
         var span = preface.ToSpan();
 
         if (!span.SequenceEqual(ClientPreface))
         {
-            throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR);
+            return false;
         }
 
         consumed = examined = preface.End;

src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs

@@ -3,6 +3,7 @@
 
 using System.Net.Http;
 using Microsoft.AspNetCore.Connections;
+using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3;
 using Microsoft.Extensions.Logging;
@@ -394,6 +395,17 @@ public void Http3GoAwayStreamId(string connectionId, long goAwayStreamId)
         Http3GoAwayStreamId(_http3Logger, connectionId, goAwayStreamId);
     }
 
+    [LoggerMessage(54, LogLevel.Debug, @"Connection id ""{ConnectionId}"": Invalid content received on connection. Possible incorrect HTTP version detected. Expected {ExpectedHttpVersion} but received {DetectedHttpVersion}.", EventName = "PossibleInvalidHttpVersionDetected", SkipEnabledCheck = true)]
+    private static partial void PossibleInvalidHttpVersionDetected(ILogger logger, string connectionId, string expectedHttpVersion, string detectedHttpVersion);
+
+    public void PossibleInvalidHttpVersionDetected(string connectionId, HttpVersion expectedHttpVersion, HttpVersion detectedHttpVersion)
+    {
+        if (_generalLogger.IsEnabled(LogLevel.Debug))
+        {
+            PossibleInvalidHttpVersionDetected(_badRequestsLogger, connectionId, HttpUtilities.VersionToString(expectedHttpVersion), HttpUtilities.VersionToString(detectedHttpVersion));
+        }
+    }
+
     public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
         => _generalLogger.Log(logLevel, eventId, state, exception, formatter);
 

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs

@@ -6,13 +6,14 @@
 using System.Globalization;
 using System.Net.Http;
 using System.Net.Http.HPack;
+using System.Net.Security;
+using System.Security.Authentication;
 using System.Text;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Connections.Features;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
-using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
 using Microsoft.AspNetCore.Testing;
@@ -5211,6 +5212,72 @@ await WaitForConnectionErrorAsync<Http2ConnectionErrorException>(
             });
     }
 
+    [Fact]
+    public async Task StartConnection_SendPreface_ReturnSettings()
+    {
+        await InitializeConnectionWithoutPrefaceAsync(_noopApplication);
+
+        await SendAsync(Http2Connection.ClientPreface);
+
+        await ExpectAsync(Http2FrameType.SETTINGS,
+            withLength: 3 * Http2FrameReader.SettingSize,
+            withFlags: 0,
+            withStreamId: 0);
+
+        await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: true);
+    }
+
+    [Fact]
+    public async Task StartConnection_SendHttp1xRequest_ReturnHttp11Status400()
+    {
+        await InitializeConnectionWithoutPrefaceAsync(_noopApplication);
+
+        await SendAsync(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n"));
+
+        var data = await ReadAllAsync();
+
+        Assert.NotNull(Http2Connection.InvalidHttp1xErrorResponseBytes);
+        Assert.Equal(Http2Connection.InvalidHttp1xErrorResponseBytes, data);
+    }
+
+    [Fact]
+    public async Task StartConnection_SendHttp1xRequest_ExceedsRequestLineLimit_ProtocolError()
+    {
+        await InitializeConnectionWithoutPrefaceAsync(_noopApplication);
+
+        await SendAsync(Encoding.ASCII.GetBytes($"GET /{new string('a', _connection.Limits.MaxRequestLineSize)} HTTP/1.1\r\n"));
+
+        await WaitForConnectionErrorAsync<Http2ConnectionErrorException>(
+            ignoreNonGoAwayFrames: false,
+            expectedLastStreamId: 0,
+            expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR,
+            expectedErrorMessage: CoreStrings.Http2ErrorInvalidPreface);
+    }
+
+    [Fact]
+    public async Task StartTlsConnection_SendHttp1xRequest_NoError()
+    {
+        CreateConnection();
+
+        var tlsHandshakeMock = new Mock<ITlsHandshakeFeature>();
+        tlsHandshakeMock.SetupGet(m => m.Protocol).Returns(SslProtocols.Tls12);
+        _connection.ConnectionFeatures.Set<ITlsHandshakeFeature>(tlsHandshakeMock.Object);
+
+        await InitializeConnectionWithoutPrefaceAsync(_noopApplication);
+
+        await SendAsync(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n"));
+
+        await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false);
+    }
+
+    [Fact]
+    public async Task StartConnection_SendNothing_NoError()
+    {
+        await InitializeConnectionWithoutPrefaceAsync(_noopApplication);
+
+        await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false);
+    }
+
     public static TheoryData<byte[]> UpperCaseHeaderNameData
     {
         get

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs

@@ -470,7 +470,7 @@ protected void CreateConnection()
         _timeoutControl.Initialize(_serviceContext.SystemClock.UtcNow.Ticks);
     }
 
-    protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 3, bool expectedWindowUpdate = true)
+    protected async Task InitializeConnectionWithoutPrefaceAsync(RequestDelegate application)
     {
         if (_connection == null)
         {
@@ -496,6 +496,11 @@ async Task CompletePipeOnTaskCompletion()
 
         // Lose xUnit's AsyncTestSyncContext so middleware always runs inline for better determinism.
         await ThreadPoolAwaitable.Instance;
+    }
+
+    protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 3, bool expectedWindowUpdate = true)
+    {
+        await InitializeConnectionWithoutPrefaceAsync(application);
         await SendPreambleAsync();
         await SendSettingsAsync();
 
@@ -1109,6 +1114,22 @@ protected Task SendUnknownFrameTypeAsync(int streamId, int frameType)
         return FlushAsync(outputWriter);
     }
 
+    internal async Task<byte[]> ReadAllAsync()
+    {
+        while (true)
+        {
+            var result = await _pair.Application.Input.ReadAsync().AsTask().DefaultTimeout();
+
+            if (result.IsCompleted)
+            {
+                return result.Buffer.ToArray();
+            }
+
+            // Consume nothing, just wait for everything
+            _pair.Application.Input.AdvanceTo(result.Buffer.Start, result.Buffer.End);
+        }
+    }
+
     internal async Task<Http2FrameWithPayload> ReceiveFrameAsync(uint maxFrameSize = Http2PeerSettings.DefaultMaxFrameSize)
     {
         var frame = new Http2FrameWithPayload();

src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs

@@ -0,0 +1,44 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Net;
+using System.Net.Http;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Server.Kestrel.Core;
+using Microsoft.AspNetCore.Testing;
+using Microsoft.Extensions.Hosting;
+
+namespace Interop.FunctionalTests.Http2;
+
+public class Http2RequestTests : LoggedTest
+{
+    [Fact]
+    public async Task GET_NoTLS_Http11RequestToHttp2Endpoint_400Result()
+    {
+        // Arrange
+        var builder = CreateHostBuilder(c => Task.CompletedTask, protocol: HttpProtocols.Http2, plaintext: true);
+
+        using (var host = builder.Build())
+        using (var client = HttpHelpers.CreateClient())
+        {
+            await host.StartAsync().DefaultTimeout();
+
+            var request = new HttpRequestMessage(HttpMethod.Get, $"http://127.0.0.1:{host.GetPort()}/");
+            request.Version = HttpVersion.Version11;
+            request.VersionPolicy = HttpVersionPolicy.RequestVersionExact;
+
+            // Act
+            var responseMessage = await client.SendAsync(request, CancellationToken.None).DefaultTimeout();
+
+            // Assert
+            Assert.Equal(HttpStatusCode.BadRequest, responseMessage.StatusCode);
+            Assert.Equal("An HTTP/1.x request was sent to an HTTP/2 only endpoint.", await responseMessage.Content.ReadAsStringAsync());
+        }
+    }
+
+    private IHostBuilder CreateHostBuilder(RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action<KestrelServerOptions> configureKestrel = null, bool? plaintext = null)
+    {
+        return HttpHelpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel, plaintext);
+    }
+}