Close connection after processing CL+TE

src/Servers/HttpSys/src/RequestProcessing/Request.cs

@@ -21,6 +21,8 @@ namespace Microsoft.AspNetCore.Server.HttpSys;
 
 internal sealed partial class Request
 {
+    private static readonly bool AllowKeepAliveAfterCLTE = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.HttpSys.AllowKeepAliveAfterCLTE", out var value) && value;
+
     private X509Certificate2? _clientCert;
     // TODO: https://github.com/aspnet/HttpSysServer/issues/231
     // private byte[] _providedTokenBindingId;
@@ -203,6 +205,8 @@ internal Request(RequestContext requestContext)
 
     private RequestContext RequestContext { get; }
 
+    public bool KeepAlive { get; private set; } = true;
+
     // With the leading ?, if any
     public string QueryString { get; }
 
@@ -514,24 +518,36 @@ private void RemoveContentLengthIfTransferEncodingContainsChunked()
             return;
         }
 
-        // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2
+        // https://www.rfc-editor.org/rfc/rfc9112#section-6.2
         // A sender MUST NOT send a Content-Length header field in any message
         // that contains a Transfer-Encoding header field.
-        // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3
+        // https://www.rfc-editor.org/rfc/rfc9112#section-6.3-2.3
         // If a message is received with both a Transfer-Encoding and a
         // Content-Length header field, the Transfer-Encoding overrides the
-        // Content-Length.  Such a message might indicate an attempt to
-        // perform request smuggling (Section 9.5) or response splitting
-        // (Section 9.4) and ought to be handled as an error.  A sender MUST
-        // remove the received Content-Length field prior to forwarding such
-        // a message downstream.
+        // Content-Length. Such a message might indicate an attempt to
+        // perform request smuggling (Section 11.2) or response splitting
+        // (Section 11.1) and ought to be handled as an error. An intermediary
+        // that chooses to forward the message MUST first remove the received
+        // Content-Length field and process the Transfer-Encoding
+        // (as described below) prior to forwarding the message downstream.
         // We should remove the Content-Length request header in this case, for compatibility
-        // reasons, include X-Content-Length so that the original Content-Length is still available.
+        // reasons, include x-Content-Length so that the original Content-Length is still available.
         IHeaderDictionary headerDictionary = Headers;
 
         // dont overwrite if user explicitly set X-Content-Length
         _ = headerDictionary.TryAdd("X-Content-Length", headerDictionary[HeaderNames.ContentLength]);
         Headers.ContentLength = StringValues.Empty;
+
+        if (!AllowKeepAliveAfterCLTE)
+        {
+            // https://www.rfc-editor.org/rfc/rfc9112#section-6.1
+            // A server MAY reject a request that contains both Content-Length
+            // and Transfer-Encoding or process such a request in accordance
+            // with the Transfer-Encoding alone. Regardless, the server MUST
+            // close the connection after responding to such a request to
+            // avoid the potential attacks.
+            KeepAlive = false;
+        }
     }
 
     private static bool IsChunked(string? transferEncoding)

src/Servers/HttpSys/src/RequestProcessing/Response.cs

@@ -419,7 +419,7 @@ internal uint ComputeHeaders(long writeCount, bool endOfRequest = false)
         var statusCanHaveBody = CanSendResponseBody(RequestContext.Response.StatusCode);
 
         // Determine if the connection will be kept alive or closed.
-        var keepConnectionAlive = true;
+        var keepConnectionAlive = Request.KeepAlive;
 
         // An HTTP/1.1 server may also establish persistent connections with
         // HTTP/1.0 clients upon receipt of a Keep-Alive connection token.

src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs

@@ -475,6 +475,11 @@ private uint ComputeLeftToWrite(long writeCount, bool endOfRequest = false)
             }
         }
 
+        if (!_requestContext.Request.KeepAlive)
+        {
+            flags |= PInvoke.HTTP_SEND_RESPONSE_FLAG_DISCONNECT;
+        }
+
         if (endOfRequest && _requestContext.Response.BoundaryType == BoundaryType.Close)
         {
             flags |= PInvoke.HTTP_SEND_RESPONSE_FLAG_DISCONNECT;

src/Servers/HttpSys/test/FunctionalTests/RequestHeaderTests.cs

@@ -210,6 +210,79 @@ public async Task RequestHeaders_ClientSendTransferEncodingAndContentLengthAndXC
         }
     }
 
+    [ConditionalFact]
+    public async Task CloseConnectionAfterProcessingContentLengthPlusChunkedRequest()
+    {
+        string address;
+        using (Utilities.CreateHttpServer(out address, async httpContext =>
+        {
+            var requestHeaders = httpContext.Request.Headers;
+            var request = httpContext.Features.Get<RequestContext>().Request;
+            Assert.Single(requestHeaders["Transfer-Encoding"]);
+            Assert.Equal("chunked", requestHeaders.TransferEncoding);
+
+            Assert.Null(request.ContentLength);
+            Assert.True(request.HasEntityBody);
+
+            Assert.False(requestHeaders.ContainsKey("Content-Length"));
+            Assert.Null(requestHeaders.ContentLength);
+
+            Assert.Single(requestHeaders["X-Content-Length"]);
+            Assert.Equal("1", requestHeaders["X-Content-Length"]);
+
+            await httpContext.Response.WriteAsync("Hello World");
+        }, LoggerFactory))
+        {
+            var uri = new Uri(address);
+            using (Socket socket = new Socket(SocketType.Stream, ProtocolType.Tcp))
+            {
+                socket.Connect(uri.Host, uri.Port);
+                // Send 2 requests with both CL and TE header
+                // expect the second to not be processed and the connection to be closed
+                for (var i = 0; i < 2; i++)
+                {
+                    socket.Send(Encoding.ASCII.GetBytes(string.Join("\r\n",
+                    "POST / HTTP/1.1",
+                    $"Host: {uri.Authority}",
+                    "Transfer-Encoding: chunked",
+                    "Connection: keep-alive",
+                    "Content-Length: 1",
+                    "",
+                    "5", "Hello",
+                    "6", " World",
+                    "0",
+                    "",
+                    "")));
+                }
+                byte[] response = new byte[1024 * 5];
+                var sb = new StringBuilder();
+
+                int read = 0;
+                do
+                {
+                    read = await Task.Run(() => socket.Receive(response));
+                    var s = Encoding.ASCII.GetString(response, 0, read);
+                    sb.Append(s);
+                } while (read != 0);
+
+                var resp = sb.ToString();
+
+                Assert.Matches(@"HTTP/1\.1 200 OK
+Transfer-Encoding: chunked
+Server: Microsoft-HTTPAPI/2\.0
+Date: .+
+Connection: close
+
+B
+Hello World
+0
+
+$",
+                resp);
+            }
+        }
+    }
+
     [ConditionalFact]
     public async Task RequestHeaders_AllKnownHeadersKeys_Received()
     {

src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp

@@ -437,6 +437,16 @@ http_close_connection(
     return S_OK;
 }
 
+EXTERN_C __declspec(dllexport)
+HRESULT
+http_set_close(
+    _In_ IN_PROCESS_HANDLER* pInProcessHandler
+)
+{
+    pInProcessHandler->QueryHttpContext()->GetResponse()->SetNeedDisconnect();
+    return S_OK;
+}
+
 EXTERN_C __declspec(dllexport)
 HRESULT
 http_response_set_unknown_header(

src/Servers/IIS/IIS/src/Core/IISHttpContext.cs

@@ -30,6 +30,8 @@ namespace Microsoft.AspNetCore.Server.IIS.Core;
 
 internal abstract partial class IISHttpContext : NativeRequestContext, IThreadPoolWorkItem, IDisposable
 {
+    private static readonly bool AllowKeepAliveAfterCLTE = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.IIS.AllowKeepAliveAfterCLTE", out var value) && value;
+
     private const int MinAllocBufferSize = 2048;
 
     protected readonly NativeSafeHandle _requestNativeHandle;
@@ -42,6 +44,7 @@ internal abstract partial class IISHttpContext : NativeRequestContext, IThreadPo
 
     private int _statusCode;
     private string? _reasonPhrase;
+
     // Used to synchronize callback registration and native method calls
     internal readonly object _contextLock = new object();
 
@@ -387,24 +390,36 @@ private bool CheckRequestCanHaveBody()
         string transferEncoding = RequestHeaders.TransferEncoding.ToString();
         if (IsChunked(transferEncoding))
         {
-            // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2
+            // https://www.rfc-editor.org/rfc/rfc9112#section-6.2
             // A sender MUST NOT send a Content-Length header field in any message
             // that contains a Transfer-Encoding header field.
-            // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3
+            // https://www.rfc-editor.org/rfc/rfc9112#section-6.3-2.3
             // If a message is received with both a Transfer-Encoding and a
             // Content-Length header field, the Transfer-Encoding overrides the
-            // Content-Length.  Such a message might indicate an attempt to
-            // perform request smuggling (Section 9.5) or response splitting
-            // (Section 9.4) and ought to be handled as an error.  A sender MUST
-            // remove the received Content-Length field prior to forwarding such
-            // a message downstream.
+            // Content-Length. Such a message might indicate an attempt to
+            // perform request smuggling (Section 11.2) or response splitting
+            // (Section 11.1) and ought to be handled as an error. An intermediary
+            // that chooses to forward the message MUST first remove the received
+            // Content-Length field and process the Transfer-Encoding
+            // (as described below) prior to forwarding the message downstream.
             // We should remove the Content-Length request header in this case, for compatibility
             // reasons, include X-Content-Length so that the original Content-Length is still available.
-            if (RequestHeaders.ContentLength.HasValue)
+            if (RequestHeaders.TryGetValue(HeaderNames.ContentLength, out var contentLength))
             {
                 // if user already passed X-Content-Length, we won't overwrite it
-                _ = RequestHeaders.TryAdd("X-Content-Length", RequestHeaders[HeaderNames.ContentLength]);
+                _ = RequestHeaders.TryAdd("X-Content-Length", contentLength);
                 RequestHeaders.ContentLength = null;
+
+                if (!AllowKeepAliveAfterCLTE)
+                {
+                    // https://www.rfc-editor.org/rfc/rfc9112#section-6.1
+                    // A server MAY reject a request that contains both Content-Length
+                    // and Transfer-Encoding or process such a request in accordance
+                    // with the Transfer-Encoding alone. Regardless, the server MUST
+                    // close the connection after responding to such a request to
+                    // avoid the potential attacks.
+                    NativeMethods.HttpSetClose(_requestNativeHandle);
+                }
             }
             return true;
         }

src/Servers/IIS/IIS/src/NativeMethods.cs

@@ -143,6 +143,9 @@ private static unsafe partial int http_websockets_write_bytes(
     [LibraryImport(AspNetCoreModuleDll)]
     private static partial int http_close_connection(NativeSafeHandle pInProcessHandler);
 
+    [LibraryImport(AspNetCoreModuleDll)]
+    private static partial int http_set_close(NativeSafeHandle pInProcessHandler);
+
     [LibraryImport(AspNetCoreModuleDll)]
     private static partial int http_response_set_need_goaway(NativeSafeHandle pInProcessHandler);
 
@@ -306,6 +309,11 @@ public static void HttpCloseConnection(NativeSafeHandle pInProcessHandler)
         Validate(http_close_connection(pInProcessHandler));
     }
 
+    public static void HttpSetClose(NativeSafeHandle pInProcessHandler)
+    {
+        Validate(http_set_close(pInProcessHandler));
+    }
+
     public static unsafe void HttpResponseSetUnknownHeader(NativeSafeHandle pInProcessHandler, byte* pszHeaderName, byte* pszHeaderValue, ushort usHeaderValueLength, bool fReplace)
     {
         Validate(http_response_set_unknown_header(pInProcessHandler, pszHeaderName, pszHeaderValue, usHeaderValueLength, fReplace));

src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs

@@ -10,8 +10,8 @@
 using System.Net.Http.Headers;
 using System.Text;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.Server.IntegrationTesting;
 using Microsoft.AspNetCore.InternalTesting;
+using Microsoft.AspNetCore.Server.IntegrationTesting;
 using Xunit;
 
 #if !IIS_FUNCTIONALS
@@ -788,6 +788,62 @@ await connection.Receive(
         }
     }
 
+    [ConditionalFact]
+    public async Task CloseConnectionAfterProcessingContentLengthPlusChunkedRequest()
+    {
+        using (var connection = _fixture.CreateTestConnection())
+        {
+            for (var i = 0; i < 2; i++)
+            {
+                await connection.Send(
+                    "POST /ReadAndWriteEcho HTTP/1.1",
+                    "Host: localhost",
+                    "Transfer-Encoding: chunked",
+                    "Connection: keep-alive",
+                    "Content-Length: 25",
+                    "",
+                    "5", "Hello",
+                    "6", " World",
+                    "0",
+                    "",
+                    "");
+            }
+
+            await connection.Receive(
+                "HTTP/1.1 200 OK",
+                "");
+            var headers = await connection.ReceiveHeaders();
+
+            // RFC 9112 §6.1: the server MUST close the connection after responding
+            // to a request that contained both Content-Length and Transfer-Encoding.
+            Assert.Contains("Connection: close", headers);
+            Assert.Contains("Server: Microsoft-IIS/10.0", headers);
+
+            if (headers.Contains("Transfer-Encoding: chunked"))
+            {
+                await connection.Receive(
+                    "B",
+                    "Hello World",
+                    "");
+                await connection.Receive(
+                    "0",
+                    "",
+                    "");
+            }
+            else
+            {
+                // Either framed by Content-Length: 11 or by connection close
+                // (no framing header). Either way the body is exactly "Hello World"
+                // and WaitForConnectionClose below verifies nothing else follows.
+                await connection.Receive("Hello World");
+            }
+
+            // Verify the second request was not processed and that the server closed
+            // the connection (no extra bytes are sent).
+            await connection.WaitForConnectionClose();
+        }
+    }
+
     private async Task<(int Status, string Body)> SendSocketRequestAsync(string path)
     {
         using (var connection = _fixture.CreateTestConnection())

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

@@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
 
 internal abstract class Http1MessageBody : MessageBody
 {
+    private static readonly bool ContinueProcessingAfterCLTE = AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.AllowKeepAliveAfterCLTE", out var value) && value;
+
     protected readonly Http1Connection _context;
     private bool _readerCompleted;
 
@@ -166,17 +168,18 @@ public static MessageBody For(
                 KestrelBadHttpRequestException.Throw(RequestRejectionReason.FinalTransferCodingNotChunked, transferEncoding);
             }
 
-            // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2
+            // https://www.rfc-editor.org/rfc/rfc9112#section-6.2
             // A sender MUST NOT send a Content-Length header field in any message
             // that contains a Transfer-Encoding header field.
-            // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3
+            // https://www.rfc-editor.org/rfc/rfc9112#section-6.3-2.3
             // If a message is received with both a Transfer-Encoding and a
             // Content-Length header field, the Transfer-Encoding overrides the
-            // Content-Length.  Such a message might indicate an attempt to
-            // perform request smuggling (Section 9.5) or response splitting
-            // (Section 9.4) and ought to be handled as an error.  A sender MUST
-            // remove the received Content-Length field prior to forwarding such
-            // a message downstream.
+            // Content-Length. Such a message might indicate an attempt to
+            // perform request smuggling (Section 11.2) or response splitting
+            // (Section 11.1) and ought to be handled as an error. An intermediary
+            // that chooses to forward the message MUST first remove the received
+            // Content-Length field and process the Transfer-Encoding
+            // (as described below) prior to forwarding the message downstream.
             // We should remove the Content-Length request header in this case, for compatibility
             // reasons, include x-Content-Length so that the original Content-Length is still available.
             if (headers.ContentLength.HasValue)
@@ -186,6 +189,17 @@ public static MessageBody For(
                 // if user already passed X-Content-Length, we won't overwrite it
                 _ = headerDictionary.TryAdd("X-Content-Length", headerDictionary[HeaderNames.ContentLength]);
                 headers.ContentLength = null;
+
+                if (!ContinueProcessingAfterCLTE)
+                {
+                    // https://www.rfc-editor.org/rfc/rfc9112#section-6.1
+                    // A server MAY reject a request that contains both Content-Length
+                    // and Transfer-Encoding or process such a request in accordance
+                    // with the Transfer-Encoding alone. Regardless, the server MUST
+                    // close the connection after responding to such a request to
+                    // avoid the potential attacks.
+                    keepAlive = false;
+                }
             }
 
             // TODO may push more into the wrapper rather than just calling into the message body