Introduce KestrelServerOptions.AllowUnsafeHostHeaderOverride

[amcasey]

Background and Motivation

At the request of an internal partner, #39334 introduced an internal API and corresponding appcontext switch that made it possible to overwrite an incorrect Host header with a value derived from an absolute-form request target to handle the surprisingly common client behavior of missing the line-break after the host header, as IIS/Http.Sys did. This PR upgrades it to a public API since those clients aren't going away. The behavior is the same, it just has a name and a doc comment intended for broader consumption.

Issue: #39335
PR: #48460

Proposed API

namespace Microsoft.AspNetCore.Server.Kestrel.Core;

public class KestrelServerOptions
{
+    public bool AllowUnsafeHostHeaderOverride { get; set; }
}

Usage Examples

var builder = WebApplication.CreateBuilder(args);

builder.WebHost.UseKestrel(serverOptions => {
    serverOptions.AllowUnsafeHostHeaderOverride = true
});

Alternative Designs

We could add a new configuration setting, but we don't really want this to be widely used (since it can hide security threats) and the only existing consumer configured it in code.

Risks

Opting in to this setting makes it easier to write a less secure server.

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[JamesNK]

IMO "Override" in the name is unnecessary.

[amcasey]

IMO "Override" in the name is unnecessary.

Tell me more? Without it, I have trouble reading the name as indicating that the original host header will be replaced.

[JamesNK]

Setting AllowUnsafeHostHeader = true to allow an unsafe host header doesn't seem confusing.

builder.WebHost.UseKestrel(serverOptions =>
{
    serverOptions.AllowUnsafeHostHeader = true
});

By default, Kestrel doesn't allow unsafe host headers, and you're changing that to true.

Also, there is consistency to consider. KestrelServerOptions has other "Allow" properties:

• AllowSynchronousIO
• AllowResponseHeaderCompression
• AllowAlternateSchemes

They don't have override on them. I don't think adding "Override" on AllowUnsafeHostHeader adds any value and is inconsistent with other properties.

[amcasey]

To confirm, it's not the setting that's being overridden, it's the actual header. When the setting is true,

GET http://www.foo.com/api/data
Host: http://www.bar.com

is rewritten to

GET http://www.foo.com/api/data
Host: http://www.foo.com

What do you think would be a clear way to express that that will happen?

[halter73]

API Review Notes:

• https://datatracker.ietf.org/doc/html/rfc9112#section-3.2.2-8 says you MUST use the host from the absolute form start line rather than HOST header if they do not match, so that's what this does.

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. Note that if the request-target does not have an authority component, an empty Host header field will be sent in this case.

• The host from the start line will be validated just like the HOST header would normally be validated when it's overridden.
• Should we just follow the RFC and override any time there's a disagreement by default?
  • Technically we're not complying with the RFC if we don't. Do we really need to opt-in to follow the spec and be less strict.
  • That seems sketchy. They almost always match for any conforming client. Not many people have asked for this.
• If this is the default IIS behavior and "required" by RFC, do we need Unsafe in the name?
  • No.
• What about PreferRequestTargetHost?
  • This is more clear about what overrides what.
  • It's less clear overall that it's about the HostHeader.
• Note in doc comments. This only applies to HTTP/1, because there are no more absolute form request targets.

API Approved!

namespace Microsoft.AspNetCore.Server.Kestrel.Core;

public class KestrelServerOptions
{
+    public bool AllowHostHeaderOverride { get; set; }
}

[amcasey]

Obviously, I have completely forgotten what we discussed. If anyone remembers an action item beyond "rename the property", speak now. (The notes seem pretty clear, but I'm not sure why I wouldn't have just done the rename during the meeting.)

Edit: I skimmed the recording and couldn't find more work. I assume I was just trying to land something more urgent.