IP address calculated from X-Forwarded-For are insecure about request-ip HOT 7 OPEN

@pbojinov You are gladly accepting PR for 18 months now!

from request-ip.

@austince I will gladly accept a PR for this

from request-ip.

Would you accept a PR for this? What are the situations where the reverse format applies?

from request-ip.

This is also true of CF-Connecting-IP. Cloudflare sends the true client's IP in this header, but also appends it to X-Forwarded-For.

In general, the first IP is supposed to be the correct one, but in cases where third parties get involved (meaning LBs, CDNs and the like) the real IP is actually at the end.

That said, it might be wise to read CF-Connecting-IP before X-Forwarded-For, but I'm not sure if that will break anything. :(

from request-ip.

This is also incorrect for Google Cloud Run (or any Google Cloud service behind their load balancers). Docs are here: https://cloud.google.com/load-balancing/docs/https

Works basically the same as AWS. I think all the major cloud providers do this.

from request-ip.

TLDR: a pull request was merged to fix this but then reverted, the library remains insecure

To anyone who comes across this, what appears to have happened is that a PR was eventually merged that changed the behavior to taking the rightmost (last) comma delimited IP instead of the leftmost (first).

#51 (PR)

This was good for security but incorrect as per spec, since if a request passes through multiple chained load balancers / proxies and each one appends the previous one's IP, then the rightmost entry contains the IP of the last proxy, and it's actually an entry to the left of this that contains the clientIP (the leftmost if no spoofing). The pull request seemed to misunderstand this and think that the clientIP always ends up at the end: it doesn't, it's just that only the end is always[*] safe from spoofing.

X-Forwarded-For: [spoofed, spoofed, ..., spoofed,] clientIP, [proxy, proxy, ..., proxy]

To put it another way, spoofing inserts some arbitrary number of fake entries to the left of the actual IP, and chained proxies insert a number of useless entries to the right of the actual IP. The true clientIP could be anywhere in the list, and there is no algorithm that can be locate it robustly without any further information. The only hope is to know the number of proxies you expect to see on the right side (or the IP ranges of all of them) and keep moving to the left until you find the one true clientIP. Note that in the most common case[*] of only a single proxy, this reduces to taking the rightmost entry, but the point is that this is not a universal truth.

Accordingly just taking the rightmost entry broke some people's applications, who complained that it was against spec, and the pull request was reverted.

#60 (complaints)
#61 (reverting PR)

And so that is how things stand today, back to where we started from, with a parsing routine that is "correct" (against arbitrary numbers of proxies) but insecure (against spoofing).

[*]: Actually, in the arguably even more common case of zero proxies (ie the client spoke to the server directly, or at least, through no proxies of the type that set this header), nothing in that header will be correct, and anything in that header (including the rightmost entry) can only have been spoofed. But that's more the scope of this issue: #26

from request-ip.

Given how difficult an issue this is, I suggest just being upfront in the documentation that this library is not and cannot be secure against spoofing, and should not be relied upon for security purposes.

I would also mention that if you must parse the IP for security purposes, you need to either:

• only rely on the TCP Connection IP (if there aren't any load balancers in the way)
• if there are load balancers, first determine that the request passed through them, determine exactly how many, and parse the ip out manually from the correct entry (counting from the right) of the correct header. For most simple AWS ELB setups this will reduce to just taking eg the rightmost entry from the X-Forwarded-For header.

from request-ip.