microsoft / reverse-proxy

A toolkit for developing high-performance HTTP reverse proxy applications.
https://microsoft.github.io/reverse-proxy
MIT License
8.49k stars 836 forks source link

YARP with ForwardedHeadersMiddleware doesn't produce expected backend request headers #1331

Open Bouke opened 2 years ago

Bouke commented 2 years ago

Describe the bug

Using {"X-Forwarded": "Append"} with ForwardedHeadersMiddleware doesn't produce expected backend request headers.

My application is running behind a reverse proxy (HAProxy). HAProxy sets the X-Forwarded-For and related headers. I've configured my application to use the ForwardedHeadersMiddleware, which updates HttpContext.Connection.RemoteIpAddress as expected. I expect that having a transform {"X-Forwarded": "Append"} would append the original IP address. However the XFF header isn't changed.

Imagine the following request: Client -> HAProxy -> Application -> Backend

The request received from HAProxy by Application is:

RemoteIpAddress: IP(HAProxy)
X-Forwarded-For: IP(Client)

After applying ForwardedHeadersMiddleware the request looks like this:

RemoteIpAddress: IP(Client)
X-Forwarded-For: IP(Client)
X-Original-For: IP(HAProxy):Port

The request created by YARP for Backend becomes:

X-Forwarded-For: IP(Client)
X-Original-For: IP(HAProxy):Port

However I expected the request to be:

X-Forwarded-For: IP(Client), IP(HAProxy)
X-Original-For: IP(HAProxy):Port   (this header is not needed, but could be filtered through a transform)

Further technical details

Tratcher commented 2 years ago

ForwardedHeadersMiddleware wasn't designed for use in the proxy (YARP), it's designed for use in the backend. You'll get the expected results if you remove that middleware. Is there something you needed the client IP for in the proxy?

Bouke commented 2 years ago

I'm using YARP alongside an ASP.NET Core MVC application; any route not matched by the MVC endpoint is forwarded to the backend. This is a variant of the Strangler Fig pattern combining the Strangler Façade with the Modern application (inspired by this talk). The Core application (Façade + Modern) will replace the Legacy (.NET Framework) backend application. Once the migration is done (eventually) the YARP endpoint will be removed.

Tratcher commented 2 years ago

Ah, ok. Since YARP is the temporary component here then your best option is probably to use ForwardedHeadersMiddleware, {"X-Forwarded": "Set"}, and { "RequestHeaderRemove": "X-Original-For" }. You should end up with X-Forwarded-For: IP(Client). The destination doesn't need to be aware of the HAProxy IP.

Bouke commented 2 years ago

Indeed that works although the HAProxy IP won't be visible. I think it would be good to mention the incompatibility with ForwardedHeadersMiddleware in the docs.

karelz commented 2 years ago

Triage: The only thing left is to mention in docs that ForwardedHeadersMiddleware is not compatible with YARP.

chrfin commented 6 months ago

I am using incremental migration of ASP.NET to ASP.NET Core and had a similar problem (not using HAProxy, but Azure Application Gateway) and solved it with the current YARP version by supplying context => context.AddXForwarded(ForwardedTransformActions.Off) as the configureTransform-parameter to the MapForwarder-call.