dotnet/aspnetcore

Support the new "Forwarded" header (RFC 7239)

cwe1ss opened this issue ยท 18 comments

There is now a standard for the most common X-Forwarded-* headers. It was introduced with RFC 7239: "Forwarded HTTP Extension".

Example: Forwarded: for=192.0.2.60;proto=http;by=203.0.113.43

It would be great if this could be supported.

Yes, we've seen it. Are you aware of any proxies that actually set that new header?

No - but I haven't looked either. I recently built a small ASP.NET gateway proxy for Azure Service Fabric and that's why I stumbled upon this RFC. I guess this definitely isn't high priority since there won't be anyone who's setting just this header for a looong time. It would just be good to support it someday since it is a standard and will eventually get more common.

I have a scenario whereby we have a reverse proxy (internally) passing requests to backend apiโ€™s and web apps; and one of the api's is ASP.NET 5 and using the BasicMiddleware classes to handle the headers from the upstream proxies. The team had implemented the newer RFC so BasicMiddleware can't handle Forwarded headers from the RFC.

We're using Forwarded and plan to implement it in the proxies we use, such as NGINX. The header is becoming more mainstream and it would be good of Microsoft to advocate for its use over the (non-standard) X-Forwarded-* headers.

Would also like to have ability to extend the 'Forwared' header with a PathBase parameter as well as support for that with the current implementation i.e. X-Forwarded-PathBase

(somewhat) Related: https://github.com/aspnet/Hosting/issues/1120

@damianh do you expect PathBase to be different per request? That Hosting issue is about getting one value at startup.

Yes, I would certainly like to be able to do so on a per-request basis. This will allow two scenarios where generated URLs and paths (fully qualified, rooted, relative) would "just work":

  1. Re-configuring the proxy to a new path base (e.g. http://proxy/foo/ -> http://backend/ to http://proxy/bar/ -> http://backend/ ) without having to reconfigure and restart the backend server.

  2. Having one backend server responding on two separate paths (e.g. http://proxy/foo/ | http://proxy/bar/ -> http://backend/). Useful if "migrating" to a new route without breaking clients in situ.

@Tratcher This is what I use near top of my Configure(IApplicationBuilder app):

        var forwardedHeadersOptions = new ForwardedHeadersOptions
        {
            ForwardedHeaders = ForwardedHeaders.All
        };
        app.UseForwardedHeaders(forwardedHeadersOptions);
        app.Use((context, next) => {
            if (context.Request.Headers.TryGetValue("X-Forwarded-PathBase", out var pathBases))
            {
                context.Request.PathBase = pathBases.First();
            }
            return next();
        });

"Are you aware of any proxies that actually set that new header?"

Arguably that point is somewhat circular, i.e. maybe if ASP.NET Core supported the 'Forwarded' header there would be more pressure on the reverse proxy implementations to support it. It is after all the only approach that actually has a formal specification (rfc7239).

Would also like to have ability to extend the 'Forwared' header with a PathBase parameter as well as support for that with the current implementation i.e. X-Forwarded-PathBase

Just to mention that I've shipped a package that supports this https://github.com/ProxyKit/HttpOverrides

"Are you aware of any proxies that actually set that new header?"

Arguably that point is somewhat circular, i.e. maybe if ASP.NET Core supported the 'Forwarded' header there would be more pressure on the reverse proxy implementations to support it. It is after all the only approach that actually has a formal specification (rfc7239).

Indeed. If no one starts implementing the standard, it will never gain traction. Also: Spring has support for this Forwarded header since 2016...

Perhaps someone could reach out to the YARP team to get their views, i.e. e.g. are they planning to implement rfc7239

microsoft/reverse-proxy

Yes, we've seen it. Are you aware of any proxies that actually set that new header?

@Tratcher, it is the documented way for Azure APIM : https://docs.microsoft.com/en-us/azure/api-management/policies/set-header-to-enable-backend-to-construct-urls

Perhaps someone could reach out to the YARP team to get their views, i.e. e.g. are they planning to implement rfc7239

microsoft/reverse-proxy

YARP is being built by this team. We did add support for the new header in YARP but it's off by default. See https://microsoft.github.io/reverse-proxy/articles/transforms.html#forwarded

Design note: this probably should be implemented in AspNetCore as a whole new middleware, it takes very different options. I'm also not sure what to do with the randomized client ids, stash them in a new request feature?

Design note: this probably should be implemented in AspNetCore as a whole new middleware, it takes very different options.

@Tratcher I think there is value in keeping it in the same middleware, if possible, as the headers are conceptually the same. A class called ForwardedHeadersMiddleware is where I would expect to find the standards-compliant implementation, I wouldn't keep looking for a different middleware.

What if Forwarded was added to the ForwardedHeaders enum? To restrict the processing, in line with how X-Forwarded-* is configured, the enum could include ForwardedBy, ForwardedFor, ForwardedHost and ForwardedProto as well.

Support for both Forwarded and X-Forwarded-* could then be configured together, such as:

app.UseForwardedHeaders(new ForwardedHeadersOptions
{
    ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.ForwardedFor
});

There are some options that only make sense for the X-Forwarded-* headers, but is that a problem? They can either be ignored or cause an exception if they are set when only support for the Forwarded header is configured.

I'm also not sure what to do with the randomized client ids, stash them in a new request feature?

Would it be a plausible alternative to just provide a class for parsing the Forwarded header and leave the rest up to the developer when using randomized client ids?

Support for both Forwarded and X-Forwarded-* could then be configured together, such as:

app.UseForwardedHeaders(new ForwardedHeadersOptions
{
    ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.ForwardedFor
});

That's a good example of why these shouldn't be combine into one middleware. If a request has both headers then what order should they be processed in? It's ambiguous.

Thanks for contacting us.

We're moving this issue to the .NET 8 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.

That's a good example of why these shouldn't be combine into one middleware. If a request has both headers then what order should they be processed in? It's ambiguous.

I'm not disagreeing with you @Tratcher, but I do think it's easy to argue that Forwarded should take precedence over X-Forwarded-*, both due to chronology and precision.

Chronology

Since X-Forwarded-* existed before Forwarded, it is most likely that Forwarded was added after X-Forwarded-* in order to accommodate middleware that supports Forwarded. Middleware that doesn't support Forwarded will just ignore it and process X-Forwarded-* as before.

If adding Forwarded to an HTTP request doesn't do anything because X-Forwarded-* takes precedence, it makes it very hard to evolve beyond the status quo since every new header will have to be added as a clean break with existing infrastructure.

Precision

Forwarded also has a precisely defined processing model, which I assume most implementers prefer over the undefined behavior of X-Forwarded-*.

What is a proxy supposed to do when there are an unequal amount of items added to X-Forwarded-For, X-Forwarded-Host and X-Forwareded-Proto, for instance? Which item corresponds to which between the three headers? Such situations may occur if a reverse proxy doesn't support all 3 headers and only modifies one or two of them.

There's also the question of whether new items should be appended or prepended to existing headers. There are numerous edge cases that are unresolved since these headers have been invented separately and never thoroughly specified in terms of a single, coherent standard.

Since Forwarded groups for, by, host and proto together for each item and clearly defines that items should be appended to an existing header, it doesn't suffer from these problems. If issues with Forwarded arises, IETF has a clearly defined process to improve upon the published RFC, which will be made available in a canonical RFC that all implementers can use and reference.

Because of all of these benefits, I think it's obvious that Forwarded should take precedence over X-Forwarded-*, when both exist in an HTTP request.