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

Provide a default Host header when it's missing/empty #5909

Open
Tratcher opened this issue Dec 8, 2017 · 19 comments
Open

Provide a default Host header when it's missing/empty #5909

Tratcher opened this issue Dec 8, 2017 · 19 comments
Labels
affected-very-few This issue impacts very few customers area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating. severity-nice-to-have This label is used by an internal tool
Milestone

Comments

@Tratcher
Copy link
Member

Tratcher commented Dec 8, 2017

RE: microsoft/ApplicationInsights-aspnetcore#278

HTTP/1.0 requests do not provide Host headers, and HTTP/1.1 requests are required to provide it but it may be empty. This causes failures for components like loggers that want to describe the request with a Uri, or middleware that create absolute links like redirect-to-https. This is a protocol level problem that applies to supported servers.

Proposal: Hosting can provide a default host setting (from config) and apply it whenever the host header is missing or empty. This would happen right when the request is received and before any diagnostics take place:
https://github.com/aspnet/Hosting/blob/9f1e6607dd1b3d15bc6c42146629677c6b455fd0/src/Microsoft.AspNetCore.Hosting/Internal/HostingApplication.cs#L35-L37

Updated proposal
In the absence of config, inspect IServerAddresses for a url that matches the request scheme and port. The new Https redirect middleware does something similar:
https://github.com/aspnet/BasicMiddleware/blob/dd038387285bf9fa8dc52910ef762b9843ff22e4/src/Microsoft.AspNetCore.HttpsPolicy/HttpsRedirectionMiddleware.cs#L122-L139

Workarounds:

  • The same logic could be applied via middleware but it wouldn't help these early diagnostic code paths.
  • All consumers would need to be aware of the possibility of a missing host and provide some fallback logic. For loggers this isn't too bad, they could leave it out or substitute "unspecified", but redirect-to-https would have no default recourse.
@charlessolar
Copy link

charlessolar commented Dec 16, 2017

As noted in other thread this would fix issues related to using HaProxy and aspnetcore - would like to have a fix sooner rather than later as its a slightly big problem for me right now.

All health checks from HaProxy are failing by default - currently investigating workarounds.. including having haproxy use http/1.1 or disabling logging somehow

Workaround for HaProxy is like so:

option  httpchk GET /health HTTP/1.1\r\nHost:\ www

or if like me you are using marathon-lb as part of DCOS then you should label your service like this:

"HAPROXY_0_BACKEND_HTTP_HEALTHCHECK_OPTIONS": "  option  httpchk GET {healthCheckPath} HTTP/1.1\\r\\nHost:\\ www\n  timeout check {healthCheckTimeoutSeconds}s\n"

@davidfowl
Copy link
Member

@Volak you can work around the issue right now by implementing your own IHttpContextFactory and setting the host header same as the above is proposing.

@muratg
Copy link
Contributor

muratg commented Jan 19, 2018

cc @glennc

@Tratcher
Copy link
Member Author

Note to self, Host includes a port that's scheme specific. In a reverse proxy scenario we would not know what that port would be, nor would Hosting be aware of the public scheme as forwarders would not be applied yet. Should apps rely on x-forwarded-host to override the default value in that scenario?

If the config value does not include a port then Hosting can add one by looking at the local port.

@Drawaes
Copy link
Contributor

Drawaes commented Jan 19, 2018 via email

@Tratcher
Copy link
Member Author

@glennc I found a way to provide an default host value. See the updated proposal above.

@davidfowl
Copy link
Member

Would that happen on every request?

@Tratcher
Copy link
Member Author

Only requests without a host header, and there'd be room for caching.

@muratg
Copy link
Contributor

muratg commented Feb 15, 2018

Bringing back to triage

@Tratcher
Copy link
Member Author

@glennc @shirhatti this feature is more relevant for HTTP/2 where the Host/Authority is optional.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Hosting Dec 18, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 18, 2018
@aspnet-hello aspnet-hello added area-hosting enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating. labels Dec 18, 2018
@Tratcher Tratcher added affected-very-few This issue impacts very few customers severity-nice-to-have This label is used by an internal tool labels Nov 6, 2020 — with ASP.NET Core Issue Ranking
@Tratcher Tratcher removed this from the Backlog milestone May 27, 2021
@Tratcher
Copy link
Member Author

Kestrel is getting used more as an edge server. Bumping from backlog for re-triage.

What do IIS/Http.Sys do? I assume they require a host for routing.

@ghost
Copy link

ghost commented Jun 2, 2021

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@Tratcher Tratcher removed their assignment Aug 11, 2021
@ladenedge
Copy link

Just ran into this with a pretty basic Kestrel server on our VMs:

Microsoft.AspNetCore.Server.Kestrel: Connection id "0HMB4B4V678A2", Request id "0HMB4B4V678A2:00000002": An unhandled exception was thrown by the application. System.UriFormatException: Invalid URI: The hostname could not be parsed.
 at System.Uri.CreateThis(String uri, Boolean dontEscape, UriKind uriKind)
 at Microsoft.AspNetCore.Components.NavigationManager.set_BaseUri(String value)
 at Microsoft.AspNetCore.Components.NavigationManager.Initialize(String baseUri, String uri)
 ...

Fiddler confirms this occurs due to requests without a Host header. Eg:

GET https:///?healthcheck HTTP/1.0
User-Agent: Fiddler

My new workaround, in case it helps anyone, is a small piece of middleware:

// Usage: app.UseMiddleware<AddHostHeader>();
public class AddHostHeader
{
    public AddHostHeader(RequestDelegate next)
    {
        Next = next;
    }

    RequestDelegate Next { get; }

    public async Task Invoke(HttpContext httpContext)
    {
        if (!httpContext.Request.Headers.ContainsKey("Host"))
            httpContext.Request.Headers["Host"] = "UNKNOWN-HOST";

        await Next(httpContext);
    }
}

"UNKNOWN-HOST" is what the AppInsights team used in their issue Requests with the url http:/// causes the exception.

@Tratcher Tratcher modified the milestones: .NET 7 Planning, Backlog Sep 13, 2022
@ghost
Copy link

ghost commented Sep 13, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@ericsampson
Copy link

For the empty case, isn't the required behavior called out in the HTTP RFCs as copied below?
Otherwise there are security issues.

When a proxy receives a request with an absolute-form of request-target, the proxy MUST ignore the received Host header field (if any) and instead replace it with the host information of the request-target. A proxy that forwards such a request MUST generate a new Host field value based on the received request-target rather than forward the received Host field value.

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.

@ericsampson
Copy link

Also fwiw, the RFCs also call out that compliant implementations MUST reject http/https URIs without a specified authority, like in the GET http:/// example originally linked to:

A sender MUST NOT generate an "http" URI with an empty host identifier.
A recipient that processes such a URI reference MUST reject it as invalid.

@Tratcher
Copy link
Member Author

Those are referring to requests formatted like this:

GET http://host:port/path HTTP/1.1

but the more common format looks like this:

GET /path HTTP/1.1
Host: host:port

In this case the Host header is required to be present, but it's allowed to be empty:

GET /path HTTP/1.1
Host:

Luckily few clients actually do this.

@ericsampson
Copy link

ericsampson commented Mar 16, 2023

Ah OK, the original post in that appinsights thread was misleading - they initially mentioned that it was doing something like GET http:///, which is invalid according to the RFC.

So that's why I was confused, I thought they were talking about an absolute-type request.

In a later comment that I missed reading, they clarified that the actual call was more like what you showed.
Thanks

@ericsampson
Copy link

Just for my own education I went looking, and it seems that the RFC covers the actual scenario that you're talking about, including substituting a default when the Host header is empty (if that's safe in the given situation)
Cheers

If the target URI's authority component is empty and its URI scheme requires a non-empty authority (as is the case for "http" and "https"), the server can reject the request or determine whether a configured default applies that is consistent with the incoming connection's context. (snip)

An empty authority is replaced with the configured default before further processing of the request.

Supplying a default name for authority within the context of a secured connection is inherently unsafe if there is any chance that the user agent's intended authority might differ from the default. A server that can uniquely identify an authority from the request context MAY use that identity as a default without this risk. Alternatively, it might be better to redirect the request to a safe resource that explains how to obtain a new client.

@amcasey amcasey added area-hosting Includes Hosting and removed feature-hosting labels Jun 1, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-very-few This issue impacts very few customers area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating. severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests