If proxy-real-ip-cidr isn't explicitly set, real_ip_recursive should be off #4073
Description
Is this a BUG REPORT or FEATURE REQUEST? (choose one):
BUG REPORT
NGINX Ingress controller version:
0.24.1 (also happens in older versions)
Environment:
- Cloud provider or hardware configuration: AWS
What happened:
I am on AWS with L7 ELB in front of ingress-nginx.
The ELB and ingress controller are configured with the default configuration documented here: https://kubernetes.github.io/ingress-nginx/deploy/#aws
Especially I did not touch the following line:
proxy-real-ip-cidr: "0.0.0.0/0" # restrict this to the IP addresses of ELB
This was first introduced in the file in 0.24.0 so long-time users will surely oversee this.
Not setting this will lead to the same value being the default.
What you expected to happen:
I expect the X-Forwarded-For and the X-Real-IP headers to be populated with the IP of the client, even when the client itself sends an X-Forwarded-For header.
How to reproduce it (as minimally and precisely as possible):
I wrote a small service which spits out the headers (you could use <?php phpinfo(); ?>).
I then simulate the client sitting behind a proxy:
curl -H 'X-Forwarded-For: 10.1.1.1' -v https://example.com/ip
This request leads to the ELB sending the following X-Forwarded-For header where 34.230.47.162 is my real IP:
X-Forwarded-For: 10.1.1.1, 34.230.47.162
All is good, we are following the spec and I expect that Nginx gives 34.230.47.162 as the client IP.
But the headers received by the application look like this:
X-Forwarded-For: 10.1.1.1X-Original-Forwarded-For: 10.1.1.1, 34.230.47.162X-Real-IP: 10.1.1.1
The reason is that real_ip_recursive on with set_real_ip_from 0.0.0.0/0 causes all IPs in the chain to be trusted. That means that it considers 34.230.47.162 as a proxy we operate and follows the chain all the way to the first IP in the list.
We can do better than this little comment in the configmap documentation!
If the user didn't set this up correctly (0.0.0.0/0 is not a value I consider correct) real_ip_recursive should be set to off. This would only evaluate the last IP in the X-Forwarded-For header and I can't see why we wouldn't want this to be the default behavior. The only time set_real_ip_from is needed is when you have a proxy which adds its own IP to X-Forwarded-For and you want to exclude that.
Please also note that the documentation is not helpful, this parameter is independent of use-proxy-protocol.
proxy-real-ip-cidr
If use-proxy-protocol is enabled, proxy-real-ip-cidr defines the default the IP/network address of your external load balancer.
In addition to that I also had to put the Kubernetes internal IP range (100.64.0.0/10 in my case) into the proxy-real-ip-cidr list because I was seeing a few cases of Nginx reporting these cluster internal IPs. I am not sure what causes this.