X-Forwarded-For is not updated from proxy_protocol when terminating TLS

[abh]

Using nginx ingress controller .13 it appears the X-Forwarded-For header isn't updated on TLS connections when proxy_protocol is enabled.

I setup an httpbin that shows this, I hope:

No TLS, my IP is added to X-Forwarded-For:

$ curl -sH 'X-Forwarded-For: 10.0.0.1, fake.ip, 198.51.100.1'  http://httpbin.empty.us/get?show_env=1 | jq -r '.headers["X-Forwarded-For"]'
10.0.0.1, fake.ip, 198.51.100.1, 2601:647:4200:ffff:a93e:89fc:4684:c616

With TLS, it's not:

$ curl -sH 'X-Forwarded-For: 10.0.0.1, fake.ip, 198.51.100.1'  https://httpbin.empty.us/get?show_env=1 | jq -r '.headers["X-Forwarded-For"]'
10.0.0.1, fake.ip, 198.51.100.1

Note that if the client doesn't set an X-Forwarded-For header the right IP address is added to the header, but if the header already exists it's passed straight through without the "real IP" being added (oops!).

I have "proxy-real-ip-cidr" set to "10.0.100.0/24" (which is where our load balancers are). See also #1000.

[abh]

FWIW after reading #1381 I tried test image 0.240, but that seems even worse in that for both TLS and non-TLS it just passes on the left-most IP address in the X-Forwarded-For list (no matter with the other entries are).

[aledbf]

@abh when you use proxy_protocol X-Forwarded-For is omitted.

[aledbf]

@abh please use quay.io/aledbf/nginx-ingress-controller:0.243

[abh]

Yup, 0.243 fixes the security issue of trusting the X-Forwarded-For and the feature of getting the actual client IP there. It'd be nice if it could still be properly conforming to how the header should work (adding the IP) so chains of proxies can work. (For example the real IP might be a CDN server; if the application knows or can lookup the IPs of the CDN servers it can choose to trust the next IP in the X-Forwarded-For chain).

[aledbf]

@abh can you test the image with real traffic behind a proxy? This should work (set proxy-real-ip-cidr). There is no code for this. This is just nginx with the real_ip module

[aledbf]

if the application knows or can lookup the IPs of the CDN servers it can choose to trust the next IP in the X-Forwarded-For chain

We can choose between the correct IP address or let the app handle this. Why? If nginx c

[abh]

Your sentence got cut off ...

The proper way is to add the correct IP at the end of the list. Then the application can choose which if any of the "upstream" proxies it will trust; or it can just trust the last one (the ingress).

[aledbf]

@abh sorry :)
If we just pass the header as sent from the user then things like the authorization (and incorrect logging) in nginx can be pypassed

[abh]

Not if you add the correct IP address at the end. That's how the header works.

https://tools.ietf.org/html/rfc7239#section-5.2

[aledbf]

@abh sure, but that can be a fake IP address.

Edit: please try to see this from my point of view. I cannot fulfill all the uses cases. In this case I prefer to use the have the real IP address

[aledbf]

@abh can you provide a full working example of what you expect? (just to be able to reproduce what you see)

[abh]

Sorry, I use this for a hobby project and sometimes I don't have time on continuous days to fuss with it.

The user will get the real IP address, or rather all the real IP addresses if the header is implemented correctly. One use case, for example, is getting the end-user IP when the site is behind a CDN.

Cloudflare for example publish a list of their IPs. Processing the X-Forwarded-For header the application can say "yes, the next IP was from my CDN so we'll also look at the next IP after that".

https://support.cloudflare.com/hc/en-us/articles/200170706-How-do-I-restore-original-visitor-IP-with-Nginx-

(Also continued in #1489)