X-Forwarded-For allows spoofing client address

[sorenisanerd]

Issue

X-Forwarded-For is passed through unfiltered, allowing anyone to spoof their origin IP.

Context

X-Forwarded-For records the path a given request has taken. The first IP is the origin client, each subsequent IP denotes a path along the way (proxies, load balancers, whatever). It's the only way for a backend service to determine the original IP, since the incoming connection is from the gorouter.

However, blindly trusting the header obviously allows anyone to spoof the origin IP. Common ways to address this security problem is to only trust X-Forwarded-For headers from trusted sources.

Examples of how to mitigate this problem:
https://httpd.apache.org/docs/current/mod/mod_remoteip.html#remoteiptrustedproxy
http://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from

Steps to Reproduce

Pass a X-Forwarded-For header with someone else's IP (e.g. 8.8.8.8) in it to your application, and it'll appear as though that's where the request came from.

Expected result

Since I'm not a trusted client, my X-Forwarded-For header should have been discarded, and my IP should be the first in the X-Forwarded-For header received by the backend.

Current result

The backend service will see an X-Forwarded-For header reading "8.8.8.8, [my real IP], [gorouter IP]" (possibly more, if there's a load balancer or something in the path). It will think the request came from 8.8.8.8.

Possible Fix

Allow specifying a list of IPs (or CIDR's) of trusted proxies and load balancers. If the request didn't come from one of them, discard the X-Forwarded-For. For bonus points, do what nginx does: Go through the list (starting from the last entry) and check each entry to see if it's the list of trusted proxies. When one is encountered that's not on the list, discard everything before it.

All of the above also applies to X-Forwarded-Proto. We shouldn't trust people to say that their request was ever HTTPS if it never was.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/147715417

The labels on this github issue will be updated when the story is started.

[shashwathi]

@sorenh :

Thank you for filing this issue. There is a story in our backlog which allows gorouter job to be configured with trusted proxies for given domains. This story address a part of the issue where gorouter expects the request to come from a trusted proxy otherwise the request is rejected. Will this story address your concern ?

Regards
Shash

[cppforlife]

assuming healthcheck api is private, how about automatically picking up trusted ips from whoever calls healthcheck api? that will allow for more dynamic configuration.

…

Sent from my iPhone

On Jun 30, 2017, at 3:27 PM, Shashwathi ***@***.***> wrote: @sorenh : Thank you for filing this issue. There is a story in our backlog which allows gorouter job to be configured with trusted proxies for given domains. This story address a part of the issue where gorouter expects the request to come from a trusted proxy otherwise the request is rejected. Will this story address your concern ? Regards Shash — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[sorenisanerd]

@shashwathi It helps very, very little. If the proxy in question can be relied upon to perform this filtering (discarding X-Forwarded-For entries from untrusted sources) then we'd be in good shape, but generally that's not something we can expect and there won't always be a proxy in front of the gorouter. I think this is something that needs to be addressed in gorouter.

[shalako]

@sorenh You will always need some proxy in front of Gorouter to maintain HA and horizontal scaling.

Gorouter currently supports the PROXY protocol. Using this you can have your LB send the client IP to gorouter. Gorouter will set x-forwarded-for to this value.

[sorenisanerd]

Is using DNS to spread the request load across a gorouter cluster not supported?

Anyway, if I'm reading the code correctly, using PROXY only helps identify the immediately preceding hop.

If an app wants to identify the client, all it has to go on is the X-Forwarded-For header, and since a client can put any old nonsense in there, it's utterly unreliable.

If a user is on a network with a proxy, connecting over the internet through an ISP who also has a proxy, in through your front end load balancer and the gorouter before reaching the app and everyone is a well-behaved, good citizen, the X-Forwarded-For header will read:

(Client IP), (Client network proxy IP), (ISP proxy IP), (Load balancer IP), (Gorouter IP)

I, as the platform provider, can't really trust anything before the ISP proxy IP (which was added by the load balancer). The application developer doesn't (or at least shouldn't) know anything about the network layout, so they don't know what to trust or how many of the final hops to discard to find something trustworthy or useful.

The gorouter really seems like the right place to address this problem. It can trust the load balancer to correctly identify its (immedite) client, but it can't trust any prior information, so it should be discarded and the X-Forwarded-For header should read only:

(ISP proxy IP), (Load balancer IP), (Gorouter IP)

[shashwathi]

@sorenh :

I see a potential problem with this approach. Lets take a CF deployment which has multiple layers of Load balancers configured, in this case the X-Forwarded-For header will read

ISP proxy IP), (Load balancer IP_1)....(Load balancer IP_n), (Gorouter IP)

This will require gorouter to have knowledge of upstream LB/Haproxy components configured to decide which part of X-Forwarded-For could potentially be corrupted.

Referring documentation on X-Forwarded-For on mozilla developer site standard format is

X-Forwarded-For: <client>, <proxy1>, <proxy2>

If gorouter alters the X-Forwarded-For header by removing previous IPs, it is passing partial information about the communication chain which breaks existing behavior. Probably a better approach to solve this problem is to verify if there is any fake IPs in that header and that might be expensive.

Thoughts @shalako ?

Regards
Shash

[shashwathi]

@cppforlife :

That does not solve the problem of passing fake IPs in X-forwarded-for header. We could consider this approach for adding trusted proxies.
I think GCP changes their LB private IP address so the pool of trusted proxies keep increasing.

Regards
Shash

[sorenisanerd]

You're exactly right that gorouter will need information about trustworthy components. The alternative is allowing clients to spoof their IP.

The typical scenario is simply: (Client IP), (My 1st load balancer), (My 2nd load balancer), (My nth load balancer), (Gorouter IP)

All the load balancers in that case would be listed as trusted and the client IP will be passed along to the application unharmed.

I'd by far prefer partial, trustworthy information over untrustworthy information (partial or not).

All of GCP's load balancers (and health checkers) are in two well-known subnets. The list of trustworthy proxies should be able to include subnets, not just individual IP's. Again, nginx's docs on why and how are pretty good.

[shubhaat]

We are currently working on replacing the current routing subsystem with Istio/Envoy, read more here. Envoy handling of the XFF header is described here. Does this address your concerns?

[mohitmehral]

We have tested in our lab environment.
Our incoming request is coming through WAF (Fortinet). And in this case server received remote_IP address parameter value as same as my own gateways address. Remote Host IP is coming in X-Forwarded-for / X-Real-IP header based parameter which can be spoof very easily through intruder (burp suite tools). In this case header based parameter value are not trusted till it is encrypted with your own private key+slat.

For solution: We put rule in WAF if remoteIP other then X-Forwarded-for IP then packet drop on WAF and assuming spoofing attack.
In case your incoming traffic is routed through Akamai(fast dns) then host machine IP is coming in header parameter TrueClientIP, this again can be spoof up. In this case policy made at akamai end where if trueclient IP is not matched with remote ip address then packet will be dropped.

[shalako]

@mohitmehral
Do I understand correctly that you are using your WAF to drop or modify the X-Forwarded-For header as @sorenh suggested?

[utako]

I'm closing this, as it appears your issue has been resolved by your workaround. Please feel free to open another issue if it has not been resolved.

Thanks,
@utako, CF Routing Team