nginx: Set X-Forwarded-Proto based on trust from requesting source.

Django has a `SECURE_PROXY_SSL_HEADER` setting[^1] which controls if
it examines a header, usually provided by upstream proxies, to allow
it to treat requests as "secure" even if the proximal HTTP connection
was not encrypted.  This header is usually the `X-Forwarded-Proto`
header, and the Django configuration has large warnings about ensuring
that this setting is not enabled unless `X-Forwarded-Proto` is
explicitly controlled by the proxy, and cannot be supplied by the
end-user.

In the absence of this setting, Django checks the `wsgi.url_scheme`
property of the WSGI environment[^2].

Zulip did not control the value of the `X-Forwarded-Proto` header,
because it did not set the `SECURE_PROXY_SSL_HEADER` setting (though
see below).  However, uwsgi has undocumented code which silently
overrides the `wsgi.url_scheme` property based on the
`HTTP_X_FORWARDED_PROTO` property[^3] (and hence the
`X-Forwarded-Proto` header), thus doing the same as enabling the
Django `SECURE_PROXY_SSL_HEADER` setting, but in a way that cannot be
disabled.  It also sets `wsgi.url_scheme` to `https` if the
`X-Forwarded-SSL` header is set to `on` or `1`[^4], providing an
alternate route to deceive to Django.

These combine to make Zulip always trust `X-Forwarded-Proto` or
``X-Forwarded-SSL` headers from external sources, and thus able to
trick Django into thinking a request is "secure" when it is not.
However, Zulip is not accessible via unencrypted channels, since it
redirects all `http` requests to `https` at the nginx level; this
mitigates the vulnerability.

Regardless, we harden Zulip against this vulnerability provided by the
undocumented uwsgi feature, by stripping off `X-Forwarded-SSL` headers
before they reach uwsgi, and setting `X-Forwarded-Proto` only if the
request was received directly from a trusted proxy.

Tornado, because it does not use uwsgi, is an entirely separate
codepath.  It uses the `proxy_set_header` values from
`puppet/zulip/files/nginx/zulip-include-common/proxy`, which set
`X-Forwarded-Proto` to the scheme that nginx received the request
over.  As such, `SECURE_PROXY_SSL_HEADER` was set in Tornado, and only
Tornado; since the header was always set in nginx, this was safe.
However, it was also _incorrect_ in cases where nginx did not do SSL
termination, but an upstream proxy did -- it would mark those requests
as insecure when they were actually secure.  We adjust the
`proxy_set_header X-Forwarded-Proto` used to talk to Tornado to
respect the proxy if it is trusted, or the local scheme if not.

[^1]: https://docs.djangoproject.com/en/4.2/ref/settings/#secure-proxy-ssl-header
[^2]: https://wsgi.readthedocs.io/en/latest/definitions.html#envvar-wsgi.url_scheme
[^3]: 73efb013e9/core/protocol.c (L558-L561)
[^4]: 73efb013e9/core/protocol.c (L531-L534)

puppet/zulip/files/nginx/uwsgi_params

@@ -14,5 +14,7 @@ uwsgi_param SERVER_ADDR     $server_addr;
 uwsgi_param SERVER_PORT     $server_port;
 uwsgi_param SERVER_NAME     $server_name;
 uwsgi_param HTTP_X_REAL_IP  $remote_addr;
+uwsgi_param HTTP_X_FORWARDED_PROTO $trusted_x_forwarded_proto;
+uwsgi_param HTTP_X_FORWARDED_SSL "";
 
 uwsgi_pass django;

puppet/zulip/files/nginx/zulip-include-common/proxy

@@ -3,7 +3,7 @@ proxy_http_version 1.1;
 # http://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive
 proxy_set_header Connection "";
 proxy_set_header Host $host;
-proxy_set_header X-Forwarded-Proto $scheme;
+proxy_set_header X-Forwarded-Proto $trusted_x_forwarded_proto;
 proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
 proxy_set_header X-Real-Ip $remote_addr;
 proxy_next_upstream off;

puppet/zulip/manifests/nginx.pp

@@ -55,6 +55,17 @@ class zulip::nginx {
     content => template('zulip/nginx.conf.template.erb'),
   }
 
+  $loadbalancers = split(zulipconf('loadbalancer', 'ips', ''), ',')
+  file { '/etc/nginx/zulip-include/trusted-proto':
+    ensure  => file,
+    require => Package[$zulip::common::nginx],
+    owner   => 'root',
+    group   => 'root',
+    mode    => '0644',
+    notify  => Service['nginx'],
+    content => template('zulip/nginx/trusted-proto.template.erb'),
+  }
+
   file { '/etc/nginx/uwsgi_params':
     ensure  => file,
     require => Package[$zulip::common::nginx],

puppet/zulip/templates/nginx/trusted-proto.template.erb

@@ -0,0 +1,21 @@
+<% if @loadbalancers.empty? %>
+# "set" is not supported at the server level, so use a map:
+map $remote_addr $trusted_x_forwarded_proto {
+    default $scheme;
+}
+<% else %>
+# We do this in two steps because `geo` does not support variable
+# interpolation in the value, but does support CIDR notation,
+# which the loadbalancer list may use.
+geo $realip_remote_addr $is_x_forwarded_proto_trusted {
+    default 0;
+<% @loadbalancers.each do |host| -%>
+    <%= host %> 1;
+<% end -%>
+}
+
+map $is_x_forwarded_proto_trusted $trusted_x_forwarded_proto {
+    0 $scheme;
+    1 $http_x_forwarded_proto;
+}
+<% end %>

puppet/zulip/templates/nginx/zulip-enterprise.template.erb

@@ -12,6 +12,7 @@ server {
 }
 <% end -%>
 
+include /etc/nginx/zulip-include/trusted-proto;
 include /etc/nginx/zulip-include/s3-cache;
 include /etc/nginx/zulip-include/upstreams;
 include /etc/zulip/nginx_sharding_map.conf;