diff --git a/puppet/zulip/files/nginx/zulip-include-common/proxy b/puppet/zulip/files/nginx/zulip-include-common/proxy index 017a169824..be2b698162 100644 --- a/puppet/zulip/files/nginx/zulip-include-common/proxy +++ b/puppet/zulip/files/nginx/zulip-include-common/proxy @@ -4,5 +4,6 @@ proxy_http_version 1.1; proxy_set_header Connection ""; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; +proxy_set_header X-Real-Ip $remote_addr; proxy_next_upstream off; proxy_redirect off; diff --git a/puppet/zulip/files/nginx/zulip-include-frontend/app b/puppet/zulip/files/nginx/zulip-include-frontend/app index 16631b6f7a..9717be02ef 100644 --- a/puppet/zulip/files/nginx/zulip-include-frontend/app +++ b/puppet/zulip/files/nginx/zulip-include-frontend/app @@ -52,6 +52,7 @@ location /api/v1/events { # Send everything else to Django via uWSGI location / { include uwsgi_params; + uwsgi_param HTTP_X_REAL_IP $remote_addr; uwsgi_pass django; } @@ -65,12 +66,14 @@ location /thumbnail { include /etc/nginx/zulip-include/api_headers; include uwsgi_params; + uwsgi_param HTTP_X_REAL_IP $remote_addr; uwsgi_pass django; } location /avatar { include /etc/nginx/zulip-include/api_headers; include uwsgi_params; + uwsgi_param HTTP_X_REAL_IP $remote_addr; uwsgi_pass django; } @@ -79,6 +82,7 @@ location /api/ { include /etc/nginx/zulip-include/api_headers; include uwsgi_params; + uwsgi_param HTTP_X_REAL_IP $remote_addr; uwsgi_pass django; } diff --git a/puppet/zulip/templates/accept-loadbalancer.conf.template.erb b/puppet/zulip/templates/accept-loadbalancer.conf.template.erb index d970102414..8ffe2704b3 100644 --- a/puppet/zulip/templates/accept-loadbalancer.conf.template.erb +++ b/puppet/zulip/templates/accept-loadbalancer.conf.template.erb @@ -1,7 +1,11 @@ # Configuration for making Zulip app frontends accept the -# X-Forwarded-For header used by the Zulip proxy. The accepted IP -# addresses here are set in /etc/zulip/zulip.conf. +# X-Forwarded-For header used by proxies. The trusted IP addresses +# here are set by `loadbalancer.ips` in /etc/zulip/zulip.conf. +# +# This causes us to update $remote_addr, which we use in logging, and +# also pass down to Zulip as X-Real-Ip. real_ip_header X-Forwarded-For; +real_ip_recursive on; <% @loadbalancers.each do |host| -%> set_real_ip_from <%= host %>; <% diff --git a/zerver/middleware.py b/zerver/middleware.py index 64d2046483..ad8afe4a3d 100644 --- a/zerver/middleware.py +++ b/zerver/middleware.py @@ -519,25 +519,29 @@ class HostDomainMiddleware(MiddlewareMixin): return None -class SetRemoteAddrFromForwardedFor(MiddlewareMixin): - """ - Middleware that sets REMOTE_ADDR based on the HTTP_X_FORWARDED_FOR. +class SetRemoteAddrFromRealIpHeader(MiddlewareMixin): + """Middleware that sets REMOTE_ADDR based on the X-Real-Ip header. + + This middleware is similar to Django's old + SetRemoteAddrFromForwardedFor middleware. We use X-Real-Ip, and + not X-Forwarded-For, because the latter is a list of proxies, some + number of which are trusted by us, and some of which could be + arbitrarily set by the user. nginx has already parsed which are + which, and has set X-Real-Ip to the first one, going right to + left, which is untrusted. + + Since we are always deployed behind nginx, we can trust the + X-Real-Ip which is so set. In development, we fall back to the + REMOTE_ADDR supplied by the server. - This middleware replicates Django's former SetRemoteAddrFromForwardedFor middleware. - Because Zulip sits behind a NGINX reverse proxy, if the HTTP_X_FORWARDED_FOR - is set in the request, then it has properly been set by NGINX. - Therefore HTTP_X_FORWARDED_FOR's value is trusted. """ def process_request(self, request: HttpRequest) -> None: try: - real_ip = request.META["HTTP_X_FORWARDED_FOR"] + real_ip = request.META["HTTP_X_REAL_IP"] except KeyError: return None else: - # HTTP_X_FORWARDED_FOR can be a comma-separated list of IPs. - # For NGINX reverse proxy servers, the client's IP will be the first one. - real_ip = real_ip.split(",")[0].strip() request.META["REMOTE_ADDR"] = real_ip diff --git a/zproject/computed_settings.py b/zproject/computed_settings.py index 511000b139..c0e307b262 100644 --- a/zproject/computed_settings.py +++ b/zproject/computed_settings.py @@ -172,7 +172,7 @@ MIDDLEWARE = ( # With the exception of it's dependencies, # our logging middleware should be the top middleware item. "zerver.middleware.TagRequests", - "zerver.middleware.SetRemoteAddrFromForwardedFor", + "zerver.middleware.SetRemoteAddrFromRealIpHeader", "zerver.middleware.RequestContext", "zerver.middleware.LogRequests", "zerver.middleware.JsonErrorHandler",