realip_remote_addr overwritten

[vladimir.golovin00@…]

Hello,

I noticed that $real_remote_addr gets overwritten when the first ip (left) and all ips in the header match ranges from set_real_ip_from.

My understanding is that it should never get overwritten.

Example:

nginx compiled with ./configure --with-http_realip_module

nginx.conf:

---

user www-data;
worker_processes 1;
pid /var/run/nginx.pid;

events {

worker_connections 1024;

}

http {

include mime.types;
default_type application/octet-stream;

log_format special '$remote_addr - $remote_user [$time_local] "$request" '

'$status $body_bytes_sent "$http_referer" '
'"$http_user_agent" "$http_x_forwarded_for" "$realip_remote_addr"';

access_log access.log special;

real_ip_header X-Forwarded-For;
set_real_ip_from 127.0.0.1/32;
set_real_ip_from 192.168.0.0/24;

server {

listen 80;
server_name localhost;

location / {

root html;
index index.html index.htm;

}

}

}

---

curl -H "X-Forwarded-For: 192.168.0.1" 127.0.0.1

Expected result:

192.168.0.1 - - [06/Oct/2016:16:56:33 +0000] "GET / HTTP/1.1" 200 612 "-" "curl/7.38.0" "192.168.0.1" "127.0.0.1"

Actual result:

192.168.0.1 - - [06/Oct/2016:16:56:33 +0000] "GET / HTTP/1.1" 200 612 "-" "curl/7.38.0" "192.168.0.1" "192.168.0.1"

"$realip_remote_addr
keeps the original client address"

Also if we do:

curl -H "X-Forwarded-For: 4.4.4.4, 192.168.0.1" 127.0.0.1

(adding one extra IP not from set_real_ip_from)

The log has 127.0.0.1 as it should:

4.4.4.4 - - [06/Oct/2016:16:59:44 +0000] "GET / HTTP/1.1" 200 612 "-" "curl/7.38.0" "4.4.4.4, 192.168.0.1" "127.0.0.1"

Tried latest nginx 1.10.1 and 1.11.4

[Maxim Dounin]

Thanks for reporting this. Please try the following patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1475777661 -10800
#      Thu Oct 06 21:14:21 2016 +0300
# Node ID bbd13797c9e213f92cf48fa8839d4899010df509
# Parent  880b5f75870f414165306829c036488672055a25
Realip: fixed duplicate processing on redirects (ticket #1098).

Duplicate processing was possible if the address set by realip was
listed in set_realip_from, and there was an internal redirect so module
context was cleared.  This resulted in exactly the same address set,
so previously this wasn't a problem, though now results in incorrect
$realip_remote_addr being picked.

Fix is to use ngx_http_realip_get_module_ctx() to lookup module context
even if was cleared.  Additionally, the order of checks was switched to
check the configuration first as it looks more effective.

diff --git a/src/http/modules/ngx_http_realip_module.c b/src/http/modules/ngx_http_realip_module.c
--- a/src/http/modules/ngx_http_realip_module.c
+++ b/src/http/modules/ngx_http_realip_module.c
@@ -141,15 +141,15 @@ ngx_http_realip_handler(ngx_http_request
     ngx_http_realip_ctx_t       *ctx;
     ngx_http_realip_loc_conf_t  *rlcf;
 
-    ctx = ngx_http_get_module_ctx(r, ngx_http_realip_module);
+    rlcf = ngx_http_get_module_loc_conf(r, ngx_http_realip_module);
 
-    if (ctx) {
+    if (rlcf->from == NULL) {
         return NGX_DECLINED;
     }
 
-    rlcf = ngx_http_get_module_loc_conf(r, ngx_http_realip_module);
+    ctx = ngx_http_realip_get_module_ctx(r);
 
-    if (rlcf->from == NULL) {
+    if (ctx) {
         return NGX_DECLINED;
     }
 

[vladimir.golovin00@…]

Works fine, thanks!

[Maxim Dounin <mdounin@…>]

In 6729:cecf415643d7/nginx:

Realip: fixed duplicate processing on redirects (ticket #1098).

Duplicate processing was possible if the address set by realip was
listed in set_realip_from, and there was an internal redirect so module
context was cleared. This resulted in exactly the same address being set,
so this wasn't a problem before the $realip_remote_addr variable was
introduced, though now results in incorrect $realip_remote_addr being
picked.

Fix is to use ngx_http_realip_get_module_ctx() to look up module context
even if it was cleared. Additionally, the order of checks was switched to
check the configuration first as it looks more effective.

[Maxim Dounin]

Committed, thanks for testing.