From 73fcbf1498fdefb359f26e37f2f7d552d3a83952 Mon Sep 17 00:00:00 2001 From: Chris Ross Date: Fri, 7 Jan 2022 12:58:40 -0800 Subject: [PATCH 1/6] Allow overriding the host header if doesn't match the absolute-form host (#39334) * Allow overriding the host header if doesn't match the absolute-form host * Apply suggestions from code review Co-authored-by: Stephen Halter --- .../Core/src/Internal/Http/Http1Connection.cs | 17 ++++++++++++- .../Kestrel/Core/src/KestrelServerOptions.cs | 15 +++++++++++ .../BadHttpRequestTests.cs | 25 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 3a86d96e9a57..8c823135fd82 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -626,7 +626,22 @@ private void ValidateNonOriginHostHeader(string hostText) if (!_absoluteRequestTarget.IsDefaultPort || hostText != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture)) { - KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); + // Superseded by RFC 7230, but notable for back-compat. + // https://datatracker.ietf.org/doc/html/rfc2616/#section-5.2 + // 1. If Request-URI is an absoluteURI, the host is part of the + // Request-URI. Any Host header field value in the request MUST be + // ignored. + // We don't want to leave the invalid value for the app to accidentally consume, + // replace it with the value from the request line. + if (_context.ServiceContext.ServerOptions.EnableInsecureAbsoluteFormHostOverride) + { + hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture); + HttpRequestHeaders.HeaderHost = hostText; + } + else + { + KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidHostHeader, hostText); + } } } } diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index b44e3b33adcf..7c5204f0af00 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -35,6 +35,21 @@ public class KestrelServerOptions private Func _responseHeaderEncodingSelector = DefaultHeaderEncodingSelector; + private bool? _enableInsecureAbsoluteFormHostOverride; + internal bool EnableInsecureAbsoluteFormHostOverride + { + get + { + if (!_enableInsecureAbsoluteFormHostOverride.HasValue) + { + _enableInsecureAbsoluteFormHostOverride = + AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.EnableInsecureAbsoluteFormHostOverride", out var enabled) && enabled; + } + return _enableInsecureAbsoluteFormHostOverride.Value; + } + set => _enableInsecureAbsoluteFormHostOverride = value; + } + // The following two lists configure the endpoints that Kestrel should listen to. If both lists are empty, the "urls" config setting (e.g. UseUrls) is used. internal List CodeBackedListenOptions { get; } = new List(); internal List ConfigurationBackedListenOptions { get; } = new List(); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs index 5bde02517fff..308378ef7330 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Testing; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; using Moq; using Xunit; using BadHttpRequestException = Microsoft.AspNetCore.Http.BadHttpRequestException; @@ -140,6 +141,30 @@ public Task BadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestTarget CoreStrings.FormatBadRequest_InvalidHostHeader_Detail(host.Trim())); } + [Fact] + public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget() + { + var receivedHost = StringValues.Empty; + await using var server = new TestServer(context => + { + receivedHost = context.Request.Headers.Host; + return Task.CompletedTask; + }, new TestServiceContext(LoggerFactory) + { + ServerOptions = new KestrelServerOptions() + { + EnableInsecureAbsoluteFormHostOverride = true, + } + }); + using var client = server.CreateConnection(); + + await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\nHost: www.foo.comConnection: keep-alive\r\n\r\n"); + + await client.Receive("HTTP/1.1 200 OK"); + + Assert.Equal("www.foo.com:80", receivedHost); + } + [Fact] public Task BadRequestFor10BadHostHeaderFormat() { From 2ca5c88b34b441df8f5369fbe8aa83c7399c6ac5 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 24 May 2023 12:07:44 -0700 Subject: [PATCH 2/6] Add explanatory comment --- src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 8c823135fd82..ab3767ca9929 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -633,6 +633,9 @@ private void ValidateNonOriginHostHeader(string hostText) // ignored. // We don't want to leave the invalid value for the app to accidentally consume, // replace it with the value from the request line. + // This is insecure in the sense that we would ordinarily regard a mismatched + // request target and host as suspicious (e.g. indicative of a spoofing attempt) + // and reject the request. if (_context.ServiceContext.ServerOptions.EnableInsecureAbsoluteFormHostOverride) { hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture); From 7dec71ad39f20557424ec79acc386ad7bb349f76 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Fri, 26 May 2023 14:37:35 -0700 Subject: [PATCH 3/6] Replace internal API and appcontext switch with public API The new public API is `KestrelServerOptions.AllowUnsafeHostHeaderOverride` and I've moved the explanatory comments there. The behavior remains opt-in. --- .../Core/src/Internal/Http/Http1Connection.cs | 12 +------ .../Kestrel/Core/src/KestrelServerOptions.cs | 33 +++++++++++-------- .../Kestrel/Core/src/PublicAPI.Unshipped.txt | 2 ++ .../BadHttpRequestTests.cs | 3 +- 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index ab3767ca9929..c7bc55ea19a0 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -626,17 +626,7 @@ private void ValidateNonOriginHostHeader(string hostText) if (!_absoluteRequestTarget.IsDefaultPort || hostText != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture)) { - // Superseded by RFC 7230, but notable for back-compat. - // https://datatracker.ietf.org/doc/html/rfc2616/#section-5.2 - // 1. If Request-URI is an absoluteURI, the host is part of the - // Request-URI. Any Host header field value in the request MUST be - // ignored. - // We don't want to leave the invalid value for the app to accidentally consume, - // replace it with the value from the request line. - // This is insecure in the sense that we would ordinarily regard a mismatched - // request target and host as suspicious (e.g. indicative of a spoofing attempt) - // and reject the request. - if (_context.ServiceContext.ServerOptions.EnableInsecureAbsoluteFormHostOverride) + if (_context.ServiceContext.ServerOptions.AllowUnsafeHostHeaderOverride) { hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture); HttpRequestHeaders.HeaderHost = hostText; diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index 7c5204f0af00..e1a2c6ac1452 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -35,20 +35,25 @@ public class KestrelServerOptions private Func _responseHeaderEncodingSelector = DefaultHeaderEncodingSelector; - private bool? _enableInsecureAbsoluteFormHostOverride; - internal bool EnableInsecureAbsoluteFormHostOverride - { - get - { - if (!_enableInsecureAbsoluteFormHostOverride.HasValue) - { - _enableInsecureAbsoluteFormHostOverride = - AppContext.TryGetSwitch("Microsoft.AspNetCore.Server.Kestrel.EnableInsecureAbsoluteFormHostOverride", out var enabled) && enabled; - } - return _enableInsecureAbsoluteFormHostOverride.Value; - } - set => _enableInsecureAbsoluteFormHostOverride = value; - } + /// + /// In HTTP/1.x, when a request target is in absolute-form (see RFC 9112 Section 3.2.2), + /// for example + /// + /// GET http://www.example.com/path/to/index.html HTTP/1.1 + /// + /// the Host header is redundant. In fact, the RFC says + /// + /// When an origin server receives a request with an absolute-form of request-target, + /// the origin server MUST ignore the received Host header field (if any) and instead + /// use the host information of the request-target. + /// + /// However, it is still sensible to check whether the request target and Host header match + /// because a mismatch might indicate, for example, a spoofing attempt. Setting this property + /// to true bypasses that check and unconditionally overwrites the Host header with the value + /// from the request target. + /// + /// + public bool AllowUnsafeHostHeaderOverride { get; set; } // The following two lists configure the endpoints that Kestrel should listen to. If both lists are empty, the "urls" config setting (e.g. UseUrls) is used. internal List CodeBackedListenOptions { get; } = new List(); diff --git a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt index 5a2c7ffb53b1..f3d10565a2b2 100644 --- a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt +++ b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt @@ -1,6 +1,8 @@ #nullable enable Microsoft.AspNetCore.Server.Kestrel.Core.Features.ISslStreamFeature Microsoft.AspNetCore.Server.Kestrel.Core.Features.ISslStreamFeature.SslStream.get -> System.Net.Security.SslStream! +Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowUnsafeHostHeaderOverride.get -> bool +Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowUnsafeHostHeaderOverride.set -> void Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.ListenNamedPipe(string! pipeName) -> void Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.ListenNamedPipe(string! pipeName, System.Action! configure) -> void Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions.PipeName.get -> string? \ No newline at end of file diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs index 308378ef7330..19ecf6e17bcd 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs @@ -153,11 +153,12 @@ public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget() { ServerOptions = new KestrelServerOptions() { - EnableInsecureAbsoluteFormHostOverride = true, + AllowUnsafeHostHeaderOverride = true, } }); using var client = server.CreateConnection(); + // Note the missing line-reak between the Host and Connection headers await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\nHost: www.foo.comConnection: keep-alive\r\n\r\n"); await client.Receive("HTTP/1.1 200 OK"); From 9a89e7e31b79cd2a3b4c429b648c83a9cc68133e Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 30 May 2023 12:02:11 -0700 Subject: [PATCH 4/6] Separate corruption and mismatch tests --- .../test/InMemory.FunctionalTests/BadHttpRequestTests.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs index 19ecf6e17bcd..2edf87a36525 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs @@ -141,8 +141,10 @@ public Task BadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestTarget CoreStrings.FormatBadRequest_InvalidHostHeader_Detail(host.Trim())); } - [Fact] - public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget() + [Theory] + [InlineData("Host: www.foo.comConnection: keep-alive")] // Corrupted - missing line-break + [InlineData("Host: www.notfoo.com")] // Syntactically correct but not matching + public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(string hostHeader) { var receivedHost = StringValues.Empty; await using var server = new TestServer(context => @@ -158,8 +160,7 @@ public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget() }); using var client = server.CreateConnection(); - // Note the missing line-reak between the Host and Connection headers - await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\nHost: www.foo.comConnection: keep-alive\r\n\r\n"); + await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\n{hostHeader}\r\n\r\n"); await client.Receive("HTTP/1.1 200 OK"); From 630269225528ee7b57db90ef726fac8955350015 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 31 Jul 2023 16:31:51 -0700 Subject: [PATCH 5/6] Rename property per API review --- .../Kestrel/Core/src/Internal/Http/Http1Connection.cs | 2 +- src/Servers/Kestrel/Core/src/KestrelServerOptions.cs | 5 ++++- src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt | 4 ++-- .../test/InMemory.FunctionalTests/BadHttpRequestTests.cs | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index c7bc55ea19a0..9de5568317a8 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -626,7 +626,7 @@ private void ValidateNonOriginHostHeader(string hostText) if (!_absoluteRequestTarget.IsDefaultPort || hostText != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture)) { - if (_context.ServiceContext.ServerOptions.AllowUnsafeHostHeaderOverride) + if (_context.ServiceContext.ServerOptions.AllowHostHeaderOverride) { hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture); HttpRequestHeaders.HeaderHost = hostText; diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index e1a2c6ac1452..e23208d39cdd 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -52,8 +52,11 @@ public class KestrelServerOptions /// to true bypasses that check and unconditionally overwrites the Host header with the value /// from the request target. /// + /// + /// This does not apply to HTTP/2 or HTTP/3 because the Host header is not sent in requests. + /// /// - public bool AllowUnsafeHostHeaderOverride { get; set; } + public bool AllowHostHeaderOverride { get; set; } // The following two lists configure the endpoints that Kestrel should listen to. If both lists are empty, the "urls" config setting (e.g. UseUrls) is used. internal List CodeBackedListenOptions { get; } = new List(); diff --git a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt index f3d10565a2b2..b9a42177f4ef 100644 --- a/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt +++ b/src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt @@ -1,8 +1,8 @@ #nullable enable Microsoft.AspNetCore.Server.Kestrel.Core.Features.ISslStreamFeature Microsoft.AspNetCore.Server.Kestrel.Core.Features.ISslStreamFeature.SslStream.get -> System.Net.Security.SslStream! -Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowUnsafeHostHeaderOverride.get -> bool -Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowUnsafeHostHeaderOverride.set -> void +Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowHostHeaderOverride.get -> bool +Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.AllowHostHeaderOverride.set -> void Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.ListenNamedPipe(string! pipeName) -> void Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerOptions.ListenNamedPipe(string! pipeName, System.Action! configure) -> void Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions.PipeName.get -> string? \ No newline at end of file diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs index 2edf87a36525..e85fa8f0ba1d 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/BadHttpRequestTests.cs @@ -155,7 +155,7 @@ public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(str { ServerOptions = new KestrelServerOptions() { - AllowUnsafeHostHeaderOverride = true, + AllowHostHeaderOverride = true, } }); using var client = server.CreateConnection(); From ec6b5f41446ebcc3739cbb557a7051ad4b79bc02 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 1 Aug 2023 09:13:19 -0700 Subject: [PATCH 6/6] Clarify comment. Co-authored-by: Chris Ross --- src/Servers/Kestrel/Core/src/KestrelServerOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index e23208d39cdd..c9e8db2f7a93 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -53,7 +53,7 @@ public class KestrelServerOptions /// from the request target. /// /// - /// This does not apply to HTTP/2 or HTTP/3 because the Host header is not sent in requests. + /// This option does not apply to HTTP/2 or HTTP/3. /// /// public bool AllowHostHeaderOverride { get; set; }