gplessis / dotdeb-nginx

Dotdeb : Nginx
http://www.dotdeb.org/
BSD 2-Clause "Simplified" License
63 stars 31 forks source link

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

Closed theundefined closed 7 years ago

theundefined commented 8 years ago

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"

fzwart commented 8 years ago

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.

joolswills commented 8 years ago

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

joolswills commented 8 years ago

I tried the above patch (also applied this first so the other patch applied cleanly without changes https://github.com/nginx/nginx/commit/97495b662f378bad2a9df05d4c4b4e0977b1d309) and it didn't help.

joolswills commented 8 years ago

Have opened a ticket here - https://trac.nginx.org/nginx/ticket/1127

fzwart commented 8 years ago

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.

fzwart commented 8 years ago

@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 / {
    }
}
joolswills commented 8 years ago

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

joolswills commented 8 years ago

I'll copy your post there. Thanks.

fzwart commented 8 years ago

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.

joolswills commented 8 years ago

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.

gplessis commented 8 years ago

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

joolswills commented 8 years ago

@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

fzwart commented 8 years ago

@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

joolswills commented 7 years ago

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.

joolswills commented 7 years ago

@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.

gplessis commented 7 years ago
  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?
joolswills commented 7 years ago
  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.
gplessis commented 7 years ago

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

joolswills commented 7 years ago

@gplessis thanks.

gplessis commented 7 years ago

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

fzwart commented 7 years ago

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.

gplessis commented 7 years ago

Thank you.

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

gplessis commented 7 years ago

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

joolswills commented 7 years ago

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