nginx real_ip_header and X-Forwarded-For seems wrong

Solution 1:

I believe the key to solving X-Forwarded-For woes when multiple IPs are chained is the recently introduced configuration option, real_ip_recursive (added in nginx 1.2.1 and 1.3.0). From the nginx realip docs:

If recursive search is enabled, an original client address that matches one of the trusted addresses is replaced by the last non-trusted address sent in the request header field.

nginx was grabbing the last IP address in the chain by default because that was the only one that was assumed to be trusted. But with the new real_ip_recursive enabled and with multiple set_real_ip_from options, you can define multiple trusted proxies and it will fetch the last non-trusted IP.

For example, with this config:

set_real_ip_from 127.0.0.1;
set_real_ip_from 192.168.2.1;
real_ip_header X-Forwarded-For;
real_ip_recursive on;

And an X-Forwarded-For header resulting in:

X-Forwarded-For: 123.123.123.123, 192.168.2.1, 127.0.0.1

nginx will now pick out 123.123.123.123 as the client's IP address.

As for why nginx doesn't just pick the left-most IP address and requires you to explicitly define trusted proxies, it's to prevent easy IP spoofing.

Let's say a client's real IP address is 123.123.123.123. Let's also say the client is up to no good, and they're trying to spoof their IP address to be 11.11.11.11. They send a request to the server with this header already in place:

X-Forwarded-For: 11.11.11.11

Since reverse proxies simply add IPs to this X-Forwarded-For chain, let's say it ends up looking like this when nginx gets to it:

X-Forwarded-For: 11.11.11.11, 123.123.123.123, 192.168.2.1, 127.0.0.1

If you simply grabbed the left-most address, that would allow the client to easily spoof their IP address. But with the above example nginx config, nginx will only trust the last two addresses as proxies. This means nginx will correctly pick 123.123.123.123 as the IP address, despite that spoofed IP actually being the left-most.

Solution 2:

The parsing of the X-Forwarded-For header is indeed flawed in the nginx real_ip module.

len = r->headers_in.x_forwarded_for->value.len;
ip = r->headers_in.x_forwarded_for->value.data;

for (p = ip + len - 1; p > ip; p--) {
  if (*p == ' ' || *p == ',') {
    p++;
    len -= p - ip;
    ip = p;
    break;
  }
}

It starts on the far right of the header string, and as soon as it sees a space or comma, it stops looking and sticks the part to the right of the space or comma in the IP variable. So, it's treating the most recent proxy address as the original client address.

It's not playing nice according to the spec; this is the danger of not having it spelled out in painfully obvious terms in an RFC.

Aside: It's hard to even find a good primary source on the format, which was originally defined by Squid - a dig through their documentation confirms the ordering; leftmost is original client, rightmost is the most recent append. I'm sorely tempted to add a [citation needed] to that wikipedia page. One anonymous edit seems to be the internet's authority on the subject.

If possible, can you have your intermediate proxies stop adding themselves to the end of the header, just leaving it with the real client address only?

---

Solution 3:

X-Real-IP is the IP address of the actual client the server is talking to (the "real" client of the server), which, in the case of a proxied connection, is the proxy server. That's why X-Real-IP will contain the last IP in the X-Forwarded-For header.