gplessis/dotdeb-nginx

After upgrade from nginx-naxsi 1.8 to nginx-extras 1.10 wrong ip in naxsi logs when using reverse proxy

Closed this issue ยท 24 comments

I'm using nginx with naxsi and realip module.
nginx is behind a loadbalancer (haproxy), realip module is configured to show in logs "real" client ip.

In nginx-naxsi 1.8 in naxsi error log there is real ip:

2016/10/20 10:26:18 [error] 26568#26568: *91 NAXSI_EXLOG: ip=REALIP&server=URL&uri=/xxx&id=1302&zone=ARGS&var_name=&content=<%3Fphp%20%3F>, client: REALIP, server: URL, request: "GET /xxx?returnUrl=https%3A%2F%2FURL%2F&%3C?php%20?%3E HTTP/1.1", host: "URL"

After upgrade to nginx-extras 1.10 in normal logs i have real ip, but in naxsi logs i have:

2016/10/20 12:09:27 [error] 32550#32550: *13 NAXSI_EXLOG: ip=LBIP&server=URL&uri=/xxx&id=1303&zone=ARGS&var_name=&content=<%3Fphp%20%3F>, client: LBIP, server: URL, request: "GET /xxx?returnUrl=https%3A%2F%2FURL%2F&%3C?php%20?%3E HTTP/1.1", host: "URL"

Though I'm using the nginx-full package I can confirm this issue. It seems like the X-Forwarded-For header is ignored in some way. I first experienced this issue with version 1.10.1 of the nginx-full package. I was hoping for it to be fixed in version 1.10.2 but using this package it still seems broken.

I'm not sure if it's an issue with the package or with nginx. @gplessis If you need help or any debugging info I could provide you some.

I can confirm this also - 1.10.1 and 1.10.2 both affected.

I found this ticket - same issue? - patch is not in 1.10.2 from what I can see https://trac.nginx.org/nginx/ticket/1098

I tried the above patch (also applied this first so the other patch applied cleanly without changes nginx/nginx@97495b6) and it didn't help.

To see if it's an Nginx bug I tried to confirm this behaviour on a Gentoo linux server running nginx v1.10.1. That server nicely shows the content of the X-Forwarded-For header as the source IP in the log file so I'm not sure it's an Nginx bug.

https://trac.nginx.org/nginx/ticket/1098 (#1098) is about $realip_remote_addr. For what I can tell from the documentation there is no relation between $realip_remote_addr and $remote_addr although there might, of course, be from a code point of view.
Given that a patch for #1098 has already been applied to 1.11.5, which was released about 7 days earlier than 1.10.2, it seems unlikely that this issue is related to realip being completely broken. I would then have expected it to be merged to 1.10.2 already.

@joolswills Based on the results on a Gentoo linux server I ran some more tests on a debian server running nginx v1.10.2. It seems like set_real_ip_from is working fine as long as I'm not using $remote_addr any further in the config.

With this config in use, realip works as expected and the X-Forward-For content shows up in the log files as the source IP (detail and crit both use $remote_addr):

server {
    listen       81;
    server_name   _;
    access_log   /var/log/nginx/access.log detail;
    error_log    /var/log/nginx/error.log crit;
    root         /var/www

    set_real_ip_from    0.0.0.0/0;
    real_ip_header     X-Forwarded-For;

    location / {
    }
}

This config seems to break realip functionality:

server {
    listen       81;
    server_name   _;
    access_log   /var/log/nginx/access.log detail;
    error_log    /var/log/nginx/error.log crit;
    root         /var/www

    set_real_ip_from    0.0.0.0/0;
    real_ip_header     X-Forwarded-For;

    set $test123 $remote_addr;

    location / {
    }
}

Thanks - Please can you add this to the nginx ticket so we can get this fixed upstream ?

I'll copy your post there. Thanks.

I just remember that nginx offers Debian packages as well. Using nginx-1.10.2-1~jessie from this repository realip works as expected so I think the behaviour we're seeing is dotdeb specific.

nginx closed the bug as they couldn't reproduce it so I suspect you are right but I've not had a chance to test yet. Hopefully this can be reproduced / looked at by the dotdeb developer.

Thanks for your inputs, guys. I'll take a look at this issue asap.

@gplessis many thanks. I just installed the dotdeb nginx-light 1.10.2 and from initial testing I think it's ok, which could point to some issue with a module that is included in nginx-full but not light

@gplessis I figured that one of the configure parameters introduced this bug so I excluded them one by one when compiling nginx by hand. It seems like this configure parameter causes realip not to function properly: --add-module=nginx-1.10.2/debian/modules/ngx_http_pinba_module

Any news on this ? Am currently running own built nginx-extras package without ngx_http_pinba_module which works fine but would like to go back to using the dotdeb built packages.

@gplessis realise you may be busy, would you accept a patch for this to disable ngx_http_pinba_module ? Do you have an alternative plan? I would prefer not to maintain my own custom nginx debs if possible.

  1. I have some issue reproducing the problem
  2. I find it odd that pinba would cause such an issue, there is no code in it manipulating remote_addr or real_ip
  3. Did you have the opportunity to try this patch for Naxsi instead?
  1. Happens to me just proxying from Varnish to nginx - I put info on this ticket (when I thought it was a nginx issue) - https://trac.nginx.org/nginx/ticket/1127
  2. Perhaps it doesn't need to - perhaps the way the module is implemented just causes the behaviour. I can verify disabling the module sorts it.
  3. I am not using Naxsi.

Ok, I'll provide a build without the Pinba module for you to test. Stay tuned.

Here is the build without Pinba. Could you please test and advise?

I've tested this build and realip seems to work perfectly fine.

FWIW: I have used the Chrome modheader extension to set the x_forwarded_for header to a random IP.

Thank you.

@theundefined @joolswills could you please test and confirm the fix before I publish it?

Packages of Nginx 1.10.3 have been released, without Pinba support. Please confirm that your problem is fixed after upgrading.

Sorry I didn't have time to get back to you before - updated and all seems good here. Thanks.