Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/9.0] Fix Kestrel host header mismatch handling when port in Url #59362

Open
wants to merge 2 commits into
base: release/9.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Buffers;
using System.Diagnostics;
using System.Globalization;
using System.IO.Pipelines;
using Microsoft.AspNetCore.Connections;
using Microsoft.AspNetCore.Connections.Features;
Expand Down Expand Up @@ -644,16 +643,24 @@ private void ValidateNonOriginHostHeader(string hostText)
// authority component, excluding any userinfo subcomponent and its "@"
// delimiter.

// Accessing authority always allocates, store it in a local to only allocate once
var authority = _absoluteRequestTarget!.Authority;

// System.Uri doesn't not tell us if the port was in the original string or not.
// When IsDefaultPort = true, we will allow Host: with or without the default port
if (hostText != _absoluteRequestTarget!.Authority)
if (hostText != authority)
{
if (!_absoluteRequestTarget.IsDefaultPort
|| hostText != _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture))
|| hostText != $"{authority}:{_absoluteRequestTarget.Port}")
{
if (_context.ServiceContext.ServerOptions.AllowHostHeaderOverride)
{
hostText = _absoluteRequestTarget.Authority + ":" + _absoluteRequestTarget.Port.ToString(CultureInfo.InvariantCulture);
// No need to include the port here, it's either already in the Authority
// or it's the default port
// see: https://datatracker.ietf.org/doc/html/rfc2616/#section-14.23
// A "host" without any trailing port information implies the default
// port for the service requested (e.g., "80" for an HTTP URL).
hostText = authority;
HttpRequestHeaders.HeaderHost = hostText;
}
else
Expand Down
7 changes: 6 additions & 1 deletion src/Servers/Kestrel/shared/test/HttpParsingData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,10 @@ public static TheoryData<string, string> HostHeaderData
{ "GET /pub/WWW/", "www.example.org" },
{ "GET http://localhost/", "localhost" },
{ "GET http://localhost:80/", "localhost:80" },
{ "GET http://localhost:80/", "localhost" },
{ "GET https://localhost/", "localhost" },
{ "GET https://localhost:443/", "localhost:443" },
{ "GET https://localhost:443/", "localhost" },
{ "CONNECT asp.net:80", "asp.net:80" },
{ "CONNECT asp.net:443", "asp.net:443" },
{ "CONNECT user-images.githubusercontent.com:443", "user-images.githubusercontent.com:443" },
Expand Down Expand Up @@ -534,10 +536,13 @@ public static TheoryData<string, string> HostHeaderInvalidData
data.Add("CONNECT contoso.com", host);
}

// port mismatch when target contains port
// port mismatch when target contains default https port
data.Add("GET https://contoso.com:443/", "contoso.com:5000");
data.Add("CONNECT contoso.com:443", "contoso.com:5000");

// port mismatch when target contains default http port
data.Add("GET http://contoso.com:80/", "contoso.com:5000");

return data;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,12 @@ public Task BadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestTarget
}

[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)
[InlineData("http://www.foo.com", "Host: www.foo.comConnection: keep-alive", "www.foo.com")] // Corrupted - missing line-break
[InlineData("http://www.foo.com/", "Host: www.notfoo.com", "www.foo.com")] // Syntactically correct but not matching
[InlineData("http://www.foo.com:80", "Host: www.notfoo.com", "www.foo.com")] // Explicit default port in request string
[InlineData("http://www.foo.com:5129", "Host: www.foo.com", "www.foo.com:5129")] // Non-default port in request string
[InlineData("http://www.foo.com:5129", "Host: www.foo.com:5128", "www.foo.com:5129")] // Different port in host header
public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(string requestString, string hostHeader, string expectedHost)
{
var testMeterFactory = new TestMeterFactory();
using var connectionDuration = new MetricCollector<double>(testMeterFactory, "Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration");
Expand All @@ -175,13 +178,13 @@ public async Task CanOptOutOfBadRequestIfHostHeaderDoesNotMatchRequestTarget(str
{
using (var client = server.CreateConnection())
{
await client.SendAll($"GET http://www.foo.com/api/data HTTP/1.1\r\n{hostHeader}\r\n\r\n");
await client.SendAll($"GET {requestString} HTTP/1.1\r\n{hostHeader}\r\n\r\n");

await client.Receive("HTTP/1.1 200 OK");
}
}

Assert.Equal("www.foo.com:80", receivedHost);
Assert.Equal(expectedHost, receivedHost);

Assert.Collection(connectionDuration.GetMeasurementSnapshot(), m => MetricsAssert.NoError(m.Tags));
}
Expand Down
Loading