From 1bff37bec1b185217857cbdffd147283bb22cd76 Mon Sep 17 00:00:00 2001 From: bashdx Date: Tue, 30 Jul 2019 19:26:54 +0200 Subject: [PATCH] Issue #11559: Show meaningful error message for TLS over HTTP endpoint. (#12697) --- .../Core/src/BadHttpRequestException.cs | 3 + src/Servers/Kestrel/Core/src/CoreStrings.resx | 59 ++++++++++--------- .../Core/src/Internal/Http/HttpParser.cs | 23 +++++++- .../Internal/Http/RequestRejectionReason.cs | 3 +- .../Kestrel/Core/test/HttpParserTests.cs | 19 +++++- 5 files changed, 76 insertions(+), 31 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs b/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs index 929a408778..16f7ab0fce 100644 --- a/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs +++ b/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs @@ -139,6 +139,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core BadHttpRequestException ex; switch (reason) { + case RequestRejectionReason.TlsOverHttpError: + ex = new BadHttpRequestException(CoreStrings.HttpParserTlsOverHttpError, StatusCodes.Status400BadRequest, reason); + break; case RequestRejectionReason.InvalidRequestLine: ex = new BadHttpRequestException(CoreStrings.FormatBadRequest_InvalidRequestLine_Detail(detail), StatusCodes.Status400BadRequest, reason); break; diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index 7e7b2dbb56..0c415400aa 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -1,17 +1,17 @@ - @@ -614,4 +614,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l The HTTP/2 stream was reset by the application with error code {errorCode}. - + + Detected a TLS handshake to an endpoint that does not have TLS enabled. + + \ No newline at end of file diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs index 2fe5dfdb36..ce63ec989f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs @@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private const byte ByteTab = (byte)'\t'; private const byte ByteQuestionMark = (byte)'?'; private const byte BytePercentage = (byte)'%'; + private const int MinTlsRequestSize = 1; // We need at least 1 byte to check for a proper TLS request line public unsafe bool ParseRequestLine(TRequestHandler handler, in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { @@ -415,9 +416,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return new Span(data, methodLength); } + private unsafe bool IsTlsHandshake(byte* data, int length) + { + const byte SslRecordTypeHandshake = (byte)0x16; + + // Make sure we can check at least for the existence of a TLS handshake - we check the first byte + // See https://serializethoughts.com/2014/07/27/dissecting-tls-client-hello-message/ + + return (length >= MinTlsRequestSize && data[0] == SslRecordTypeHandshake); + } + [StackTraceHidden] private unsafe void RejectRequestLine(byte* requestLine, int length) - => throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestLine, requestLine, length); + { + // Check for incoming TLS handshake over HTTP + if (IsTlsHandshake(requestLine, length)) + { + throw GetInvalidRequestException(RequestRejectionReason.TlsOverHttpError, requestLine, length); + } + else + { + throw GetInvalidRequestException(RequestRejectionReason.InvalidRequestLine, requestLine, length); + } + } [StackTraceHidden] private unsafe void RejectRequestHeader(byte* headerLine, int length) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs b/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs index fce21b6210..23dc6c67c6 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs @@ -1,10 +1,11 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { internal enum RequestRejectionReason { + TlsOverHttpError, UnrecognizedHTTPVersion, InvalidRequestLine, InvalidRequestHeader, diff --git a/src/Servers/Kestrel/Core/test/HttpParserTests.cs b/src/Servers/Kestrel/Core/test/HttpParserTests.cs index 7ce8587743..82d69d8b4d 100644 --- a/src/Servers/Kestrel/Core/test/HttpParserTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpParserTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -394,6 +394,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(buffer.End, examined); } + [Fact] + public void ParseRequestLineTlsOverHttp() + { + var parser = CreateParser(_nullTrace); + var buffer = ReadOnlySequenceFactory.CreateSegments(new byte[] { 0x16, 0x03, 0x01, 0x02, 0x00, 0x01, 0x00, 0xfc, 0x03, 0x03, 0x03, 0xca, 0xe0, 0xfd, 0x0a }); + + var requestHandler = new RequestHandler(); + + var badHttpRequestException = Assert.Throws(() => + { + parser.ParseRequestLine(requestHandler, buffer, out var consumed, out var examined); + }); + + Assert.Equal(badHttpRequestException.StatusCode, StatusCodes.Status400BadRequest); + Assert.Equal(RequestRejectionReason.TlsOverHttpError, badHttpRequestException.Reason); + } + [Fact] public void ParseHeadersWithGratuitouslySplitBuffers() {