Skip to content

Commit

Permalink
Fix Kestrel host header mismatch handling when port in Url (#59352)
Browse files Browse the repository at this point in the history
  • Loading branch information
BrennanConroy authored Dec 9, 2024
1 parent 633add6 commit 4655dc9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 10 deletions.
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

0 comments on commit 4655dc9

Please sign in to comment.