Setting an X-Forwarded-For header overrides whitelist-source-range in nginx-controller #1000

octete · 2017-07-20T10:22:25Z

I have hit a problem with the nginx-controller that I believe might be a bug in the nginx configuration. I might be wrong, so please, do point it out if it's the case.

I am trying to set up an nginx ingress controller in GKE (version 1.6.4) which has a global configuration that includes:

whitelist-source-range: '123.123.123.123/32'

Where access to the endpoint would only be allowed from the IP 123.123.123.123. I have seen that it generates a config in the nginx like:

    geo $the_real_ip $deny_2e7cc38e-100e-4c3b-a5bb-9c0c5c53c34b {
        default 1;

        123.123.123.123/32 0;
    }

Which is then used by:

            if ($deny_2e7cc38e-100e-4c3b-a5bb-9c0c5c53c34b) {
                return 403;
            }

and it seems the the_real_ip variable is set like:

    map $http_x_forwarded_for $the_real_ip {
        default          $http_x_forwarded_for;
        ''               $remote_addr;
    }

The ingress I have defined it like:

---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    kubernetes.io/ingress.class: nginx
  labels:
    run: source-ip-app
  name: source-ip-app
spec:
  rules:
    - host: test-app.notreally.org
      http:
        paths:
        - path: /
          backend:
            serviceName: loadbalancer
            servicePort: 80

And it works:

mandarina :: ~ » curl -s -H 'Host: test-app.notreally.org'  35.189.93.54
<html>
<head><title>403 Forbidden</title></head>
<body bgcolor="white">
<center><h1>403 Forbidden</h1></center>
<hr><center>nginx/1.13.3</center>
</body>
</html>

However, if I pass the X-Forwarded-For header myself, I am able to override this restriction, as per:

mandarina :: ~ » curl -s -H 'X-Forwarded-For: 123.123.123.123' -H 'Host: test-app.notreally.org'  35.189.93.54
CLIENT VALUES:
client_address=10.24.0.12
command=GET
real path=/
query=nil
request_version=1.1
request_uri=http://test-app.notreally.org:8080/

SERVER VALUES:
server_version=nginx: 1.10.0 - lua: 10001
...

So, it seems to me that just by setting the X-Forwarded-For as a header, overrides any other X-Forwarded-For header that might be set there and then this code:

    map $http_x_forwarded_for $the_real_ip {
        default          $http_x_forwarded_for;
        ''               $remote_addr;
    }

sets the_real_ip variable to whatever I pass as a header, thus overriding the ACL.

Before I dig further into the code (of which I have not read), I'd like to raise it here in case there's an obvious problem with what I'm doing. If so, please let me know and I'll close the ticket.
I can post the extra config here if it's needed. I didn't want to on the initial issue not to overload this with lots of yaml.

Many thanks.

The text was updated successfully, but these errors were encountered:

aledbf · 2017-07-28T23:20:45Z

@octete please configure the option proxy-real-ip-cidr in the configmap with the IP addresses that you trust (your elb for instance) to avoid this issue.

octete · 2017-08-01T13:29:58Z

@aledbf Thanks for your reply, but this would only take effect if ProxyProtocol would be enabled, right? (which is not enabled by the load balancer in GCP)

If it's meant to work with ProxyProtocol off, it is not working for me (setting proxy-real-ip-cidr to the IP of the load balancer GCP has given me), I still get the same result.
Would you have a pointer for me to read more about this? (I am failing to find any useful information).
Thanks.

aledbf · 2017-08-01T13:32:41Z

@octete please post some logs

octete · 2017-08-01T14:07:52Z

Thanks.

I have set proxy protocol on, as per:

  proxy-real-ip-cidr: 35.189.93.54/32
  use-proxy-protocol: "true"
  whitelist-source-range: 123.123.123.123/32

I hit it as I specified in the description:

curl -v -H 'X-Forwarded-For: 123.123.123.123' -H 'Host: test-app.notreally.org'  35.189.93.54

, and this is what I get in the logs from nginx:

" while reading PROXY protocol, client: 123.138.41.132, server: 0.0.0.0:80
2017/08/01 13:19:11 [error] 24#24: *5 broken header: "GET / HTTP/1.1
Host: test-app.notreally.org
User-Agent: curl/7.52.1
Accept: */*
X-Forwarded-For: 123.123.123.123

" while reading PROXY protocol, client: 123.138.41.132, server: 0.0.0.0:80
2017/08/01 13:19:17 [error] 24#24: *6 broken header: "GET / HTTP/1.1
Host: test-app.notreally.org
User-Agent: curl/7.52.1
Accept: */*
X-Forwarded-For: 123.123.123.123

" while reading PROXY protocol, client: 123.138.41.132, server: 0.0.0.0:80
2017/08/01 13:19:25 [error] 24#24: *7 broken header: "GET / HTTP/1.1
Host: test-app.notreally.org
User-Agent: curl/7.52.1
Accept: */*
X-Forwarded-For: 123.123.123.123

If I, however, change the config map to:

  proxy-real-ip-cidr: 35.189.93.54/32
  use-proxy-protocol: "false"
  whitelist-source-range: 123.123.123.123/32

I get the original behaviour I was describing:

mandarina :: github.com/kubernetes/ingress ‹master› » curl  -H 'X-Forwarded-For: 123.123.123.123' -H 'Host: test-app.notreally.org'  35.189.93.54
CLIENT VALUES:
client_address=10.24.0.14
command=GET
real path=/
query=nil
request_version=1.1
request_uri=http://test-app.notreally.org:8080/

SERVER VALUES:
server_version=nginx: 1.10.0 - lua: 10001

HEADERS RECEIVED:
accept=*/*
connection=close
host=test-app.notreally.org
user-agent=curl/7.52.1
x-forwarded-for=123.123.123.123
x-forwarded-host=test-app.notreally.org
x-forwarded-port=80
x-forwarded-proto=http
x-original-uri=/
x-real-ip=123.123.123.123
x-scheme=http
BODY:
-no body in request-%                                                                                                                mandarina :: github.com/kubernetes/ingress ‹master› » curl  -H 'Host: test-app.notreally.org'  35.189.93.54
<html>
<head><title>403 Forbidden</title></head>
<body bgcolor="white">
<center><h1>403 Forbidden</h1></center>
<hr><center>nginx/1.13.3</center>
</body>
</html>
mandarina :: github.com/kubernetes/ingress ‹master› »

logs from nginx, in this case look like:

123.138.41.132 - [123.138.41.132] - - [01/Aug/2017:14:01:13 +0000] "GET / HTTP/1.1" 403 169 "-" "curl/7.52.1" 86 0.000 [default-loadbalancer-80] - - - -
123.123.123.123 - [123.123.123.123] - - [01/Aug/2017:14:01:15 +0000] "GET / HTTP/1.1" 200 509 "-" "curl/7.52.1" 120 0.000 [default-loadbalancer-80] 10.24.0.3:8080 642 0.000 200
123.123.123.123 - [123.123.123.123] - - [01/Aug/2017:14:01:39 +0000] "GET / HTTP/1.1" 200 509 "-" "curl/7.52.1" 120 0.000 [default-loadbalancer-80] 10.24.0.3:8080 642 0.000 200
123.138.41.132 - [123.138.41.132] - - [01/Aug/2017:14:01:50 +0000] "GET / HTTP/1.1" 403 169 "-" "curl/7.52.1" 86 0.000 [default-loadbalancer-80] - - - -

Thanks.

PS: I have modified the logs to change my source IP to 123.138.41.132, but shouldn't matter for this issue.

octete · 2017-08-01T16:29:02Z

@aledbf sorry, forgot to ping you.

aledbf · 2017-08-01T17:35:51Z

if you are using the ingress controller you need to enable use-proxy-protocol in the controller and the elb.
Please check this example https://github.com/kubernetes/ingress/tree/master/examples/aws/nginx

octete · 2017-08-01T19:20:45Z

Except that this is in GKE running in GCP, and it doesn't seem to support proxied load balancers, right?
I am even trying with https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/#preserving-the-client-source-ip but it's only been configured with the beta annotations, I will have to test with the full spec when I get to the office tomorrow.

octete · 2017-08-02T09:55:46Z

It seems to me that GCP load balancers don't support the Proxy Protocol. @aledbf does this only work with ProxyProtocol enabled?

rikatz · 2017-08-21T14:34:37Z

@aledbf @octete this is something that also causes some issues with Logging.

I've installed a brand new ingress controller here (based in the latest release) and when calling SSL vhosts the logged IP is localhost (as the flow is 443 -> vhost:442) and the log cames from vhost:442.

I think an option would be to have an option to enable or disable the entire SNI Proxy configuration inside NGINX (and NGINX becames an HTTP/s server only)

I think this solves: 6ef6343

But have to wait until the next release

rikatz · 2017-08-21T20:38:10Z

@octete Using the latest 'master' version, I've replaced the lines to:

default          $realip_remote_addr;

This seems to work for me, as users cannot bypass the whitelist anymore.

Take a look to see if this solves, then I'll open a PR to fix this.

Thanks

aledbf · 2017-08-21T20:50:46Z

@rikatz that replacement works but you are not respecting the X-Forwarder-For header.

rikatz · 2017-08-21T20:57:33Z

But the meaning to respect the X-Forwarded-For is when you're using a proxy.

As this is the case that Proxy is not being used, I've replaced here so this works. Isn't this the case?

This is the whole case:

    {{ if $cfg.UseProxyProtocol }}
    map $http_x_forwarded_for $the_real_ip {
        default          $http_x_forwarded_for;
        ''               $proxy_protocol_addr;
    }
    {{ else }}
    map $http_x_forwarded_for $the_real_ip {
        default          $realip_remote_addr;
    }
    {{ end }}

rikatz · 2017-08-22T12:17:55Z

@aledbf Any consideration about this? That's the easiest (don't know if that's the most correct) way I could manage to make both the log and whitelist being respected :)

rikatz · 2017-08-22T12:56:31Z

Doing some more research here, this might also be a problem:

Set a X-Forwarded-Proto = http (using some extension like modify-header in Firefox)
Set an Ingress with SSL Redirection (http -> https). This will be added to nginx conf.

Try to access this ingress with http:// (instead of https). Because of these lines you're not going to be redirected, instead you're going to access the site through http.

boazy · 2017-09-01T02:52:44Z

@aledbf
Correct me if I'm wrong, but I don't see how setting both proxy-real-ip-cidr and use-proxy-protocol: true could help if your load balancer is on a public IP (which would usually be the case).
Only the load balancer would be trusting for setting the Real IP through the proxy protocol, but any user can connect to the load balancer and override that value by setting the X-Forwarded-For header and thereby spoofing their IP address. Since we're usually talking about a TCP load balancer when dealing with the proxy protocol, it means that the load balancer won't clear or enforce values for the X-Forwarded-For header.

mettjus · 2017-10-04T16:17:25Z

Shouldn't $the_real_ip always equal $proxy_protocol_addr in case $cfg.UseProxyProtocol is true? How it works now anybody would be able to overwrite X-Real-IP by injecting X-Forwarded-For header, which shouldn't be allowed unless there was another trusted proxy in front of the PROXY_PROTOCOL proxy.

aledbf closed this as completed Jul 28, 2017

aledbf reopened this Aug 1, 2017

rikatz mentioned this issue Aug 21, 2017

formatIP issue #1202

Closed

rikatz mentioned this issue Aug 22, 2017

Correct nginx tmpl #1222

Closed

aledbf mentioned this issue Sep 17, 2017

Refactor X-Forwarded-* headers #1381

Merged

abh mentioned this issue Sep 29, 2017

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

Closed

aledbf closed this as completed in #1381 Sep 30, 2017

novohool mentioned this issue Jan 16, 2023

circumvent whitelist block #9502

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setting an X-Forwarded-For header overrides whitelist-source-range in nginx-controller #1000

Setting an X-Forwarded-For header overrides whitelist-source-range in nginx-controller #1000

octete commented Jul 20, 2017

aledbf commented Jul 28, 2017

octete commented Aug 1, 2017

aledbf commented Aug 1, 2017

octete commented Aug 1, 2017

octete commented Aug 1, 2017

aledbf commented Aug 1, 2017

octete commented Aug 1, 2017

octete commented Aug 2, 2017

rikatz commented Aug 21, 2017

rikatz commented Aug 21, 2017

aledbf commented Aug 21, 2017

rikatz commented Aug 21, 2017 •

edited

Loading

rikatz commented Aug 22, 2017

rikatz commented Aug 22, 2017

boazy commented Sep 1, 2017

mettjus commented Oct 4, 2017 •

edited

Loading

Setting an X-Forwarded-For header overrides whitelist-source-range in nginx-controller #1000

Setting an X-Forwarded-For header overrides whitelist-source-range in nginx-controller #1000

Comments

octete commented Jul 20, 2017

aledbf commented Jul 28, 2017

octete commented Aug 1, 2017

aledbf commented Aug 1, 2017

octete commented Aug 1, 2017

octete commented Aug 1, 2017

aledbf commented Aug 1, 2017

octete commented Aug 1, 2017

octete commented Aug 2, 2017

rikatz commented Aug 21, 2017

rikatz commented Aug 21, 2017

aledbf commented Aug 21, 2017

rikatz commented Aug 21, 2017 • edited Loading

rikatz commented Aug 22, 2017

rikatz commented Aug 22, 2017

boazy commented Sep 1, 2017

mettjus commented Oct 4, 2017 • edited Loading

rikatz commented Aug 21, 2017 •

edited

Loading

mettjus commented Oct 4, 2017 •

edited

Loading