#1869 closed defect (wontfix)
Wrong client IP in errors logged early in request processing when using PROXY protocol
| Reported by: | Dustin Breuer | Owned by: | |
|---|---|---|---|
| Priority: | minor | Milestone: | |
| Component: | nginx-core | Version: | 1.17.x |
| Keywords: | Cc: | ||
| uname -a: | Linux rancher-foobar 5.2.11-1.el7.elrepo.x86_64 #1 SMP Thu Aug 29 08:10:52 EDT 2019 x86_64 GNU/Linux | ||
| nginx -V: |
nginx version: nginx/1.17.3
built by gcc 8.3.0 (Alpine 8.3.0) built with OpenSSL 1.1.1b 26 Feb 2019 TLS SNI support enabled configure arguments: --prefix=/etc/nginx --sbin-path=/usr/sbin/nginx --modules-path=/usr/lib/nginx/modules --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --user=nginx --group=nginx --with-http_ssl_module --with-http_realip_module --with-http_addition_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_random_index_module --with-http_secure_link_module --with-http_stub_status_module --with-http_auth_request_module --with-http_xslt_module=dynamic --with-http_image_filter_module=dynamic --with-http_geoip_module=dynamic --with-threads --with-stream --with-stream_ssl_module --with-stream_ssl_preread_module --with-stream_realip_module --with-stream_geoip_module=dynamic --with-http_slice_module --with-mail --with-mail_ssl_module --with-compat --with-file-aio --with-http_v2_module --with-http_v2_hpack_enc --add-module=/usr/src/headers-more-nginx-module-0.33 --add-module=/usr/src/ngx_brotli-0.1.2 --with-openssl-opt=enable-tls1_3 |
||
Description
Setup
Client -[HTTP(S)]-> HAProxy -[PROXY]-> nginx -[HTTPS]-> Application
We noticed in our logs some strange errors like this:
2019/10/07 12:53:47 [info] 61#61: *2 client sent plain HTTP request to HTTPS port while reading client request headers, client: 172.23.186.8, server: _, request: "GET / HTTP/1.1", host: "example.org"
The problem with this error is not the message itself. The problem is that the client IP is not the real client IP. It‘s the IP of the proxy sitting ahead of nginx.
The communication over PROXY protocol works fine, and realip is configured well. All our other logs (access + error) containing the correct client IP except for a few like this one.
It seems that if you use the PROXY protocol errors, which occure early during the request processing (e.g., NGX_HTTP_TO_HTTPS), don‘t get logged with the client IP but with the proxy IP. At this point in processing, we already have the $proxy_protocol_addr/connection->proxy_protocol_addr but ngx_http_log_error always uses connection->addr_text.
This only matters for a few errors e.g., NGX_HTTP_TO_HTTPS and cert issues. After these checks, the realip module jumps in and replaces connection->addr_text with connection->proxy_protocol_addr. So all errors after this got logged correctly with the client IP.
Change History (4)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
Further, the realip module changes client address based on a configuration of a particular server and/or location, and configurations can be different in different servers. I don't think there is a good solution.
The realip module kicks in at the POST_READ phase. This is the first of all phases, but there's some processing even before that. The errors you mentioned happen before the phases start, and therefore the address substitution that realip does is still not done at that early stage.
It is true that proxy protocol address is already available at that moment. In fact, you can use it in the access.log. But error logging in nginx always uses c->addr_text. Checking for possible realip substitutions looks like a dirty solution since realip is a separate and independent nginx module and we'd like to keep it like that. While the issue does sound valid, currently I don't see a good way to resolve it.