middleware: Detect reverse proxy misconfigurations.

Combine nginx and Django middlware to stop putting misleading warnings
about `CSRF_TRUSTED_ORIGINS` when the issue is untrusted proxies.
This attempts to, in the error logs, diagnose and suggest next steps
to fix common proxy misconfigurations.

See also #24599 and zulip/docker-zulip#403.
This commit is contained in:
Alex Vandiver 2023-06-16 16:41:29 +00:00 committed by Tim Abbott
parent cf0b803d50
commit 8a77cca341
5 changed files with 56 additions and 0 deletions

View File

@ -16,5 +16,6 @@ uwsgi_param SERVER_NAME $server_name;
uwsgi_param HTTP_X_REAL_IP $remote_addr; uwsgi_param HTTP_X_REAL_IP $remote_addr;
uwsgi_param HTTP_X_FORWARDED_PROTO $trusted_x_forwarded_proto; uwsgi_param HTTP_X_FORWARDED_PROTO $trusted_x_forwarded_proto;
uwsgi_param HTTP_X_FORWARDED_SSL ""; uwsgi_param HTTP_X_FORWARDED_SSL "";
uwsgi_param HTTP_X_PROXY_MISCONFIGURATION $x_proxy_misconfiguration;
uwsgi_pass django; uwsgi_pass django;

View File

@ -6,5 +6,6 @@ proxy_set_header Host $host;
proxy_set_header X-Forwarded-Proto $trusted_x_forwarded_proto; 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-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Real-Ip $remote_addr; proxy_set_header X-Real-Ip $remote_addr;
proxy_set_header X-Proxy-Misconfiguration $x_proxy_misconfiguration;
proxy_next_upstream off; proxy_next_upstream off;
proxy_redirect off; proxy_redirect off;

View File

@ -3,6 +3,10 @@
map $remote_addr $trusted_x_forwarded_proto { map $remote_addr $trusted_x_forwarded_proto {
default $scheme; default $scheme;
} }
map $http_x_forwarded_for $x_proxy_misconfiguration {
default "";
"~." "No proxies configured in Zulip, but proxy headers detected from proxy at $remote_addr; see https://zulip.readthedocs.io/en/latest/production/deployment.html#putting-the-zulip-application-behind-a-reverse-proxy";
}
<% else %> <% else %>
# We do this in two steps because `geo` does not support variable # We do this in two steps because `geo` does not support variable
# interpolation in the value, but does support CIDR notation, # interpolation in the value, but does support CIDR notation,
@ -18,4 +22,9 @@ map $is_x_forwarded_proto_trusted $trusted_x_forwarded_proto {
0 $scheme; 0 $scheme;
1 $http_x_forwarded_proto; 1 $http_x_forwarded_proto;
} }
map "$is_x_forwarded_proto_trusted:$http_x_forwarded_proto" $x_proxy_misconfiguration {
"~^0:" "Incorrect reverse proxy IPs set in Zulip (try $remote_addr?); see https://zulip.readthedocs.io/en/latest/production/deployment.html#putting-the-zulip-application-behind-a-reverse-proxy";
"~^1:$" "No X-Forwarded-Proto header sent from trusted proxy $realip_remote_addr; see example configurations in https://zulip.readthedocs.io/en/latest/production/deployment.html#putting-the-zulip-application-behind-a-reverse-proxy";
default "";
}
<% end %> <% end %>

View File

@ -600,6 +600,50 @@ class SetRemoteAddrFromRealIpHeader(MiddlewareMixin):
request.META["REMOTE_ADDR"] = real_ip request.META["REMOTE_ADDR"] = real_ip
class ProxyMisconfigurationError(JsonableError):
http_status_code = 500
data_fields = ["proxy_reason"]
def __init__(self, proxy_reason: str) -> None:
self.proxy_reason = proxy_reason
@staticmethod
def msg_format() -> str:
return _("Reverse proxy misconfiguration: {proxy_reason}")
class DetectProxyMisconfiguration(MiddlewareMixin):
def process_view(
self,
request: HttpRequest,
view_func: Callable[Concatenate[HttpRequest, ParamT], HttpResponseBase],
args: List[object],
kwargs: Dict[str, Any],
) -> None:
proxy_state_header = request.headers.get("X-Proxy-Misconfiguration", "")
# Our nginx configuration sets this header if:
# - there is an X-Forwarded-For set but no proxies configured in Zulip
# - proxies are configured but the request did not come from them
# - proxies are configured and the request came from them,
# but there was no X-Forwarded-Proto header
#
# Note that the first two may be false-positives. We only
# display the error if the request also came in over HTTP (and
# a trusted proxy didn't say they get it over HTTPS), which
# should be impossible because Zulip only supports external
# https:// URLs in production. nginx configuration ensures
# that request.is_secure() is only true if our nginx is
# serving the request over HTTPS, or it came from a trusted
# proxy which reports that it is doing so. This will result
# in false negatives if Zulip's nginx is serving responses
# over HTTPS to a proxy whose IP is not configured, or
# misconfigured, but we cannot distinguish this from a random
# client which is providing proxy headers to a correctly
# configured Zulip.
if proxy_state_header != "" and not request.is_secure():
raise ProxyMisconfigurationError(proxy_state_header)
def alter_content(request: HttpRequest, content: bytes) -> bytes: def alter_content(request: HttpRequest, content: bytes) -> bytes:
first_paragraph_text = get_content_description(content, request) first_paragraph_text = get_content_description(content, request)
placeholder_open_graph_description = RequestNotes.get_notes( placeholder_open_graph_description = RequestNotes.get_notes(

View File

@ -176,6 +176,7 @@ MIDDLEWARE = [
"django.middleware.common.CommonMiddleware", "django.middleware.common.CommonMiddleware",
"zerver.middleware.LocaleMiddleware", "zerver.middleware.LocaleMiddleware",
"zerver.middleware.HostDomainMiddleware", "zerver.middleware.HostDomainMiddleware",
"zerver.middleware.DetectProxyMisconfiguration",
"django.middleware.csrf.CsrfViewMiddleware", "django.middleware.csrf.CsrfViewMiddleware",
# Make sure 2FA middlewares come after authentication middleware. # Make sure 2FA middlewares come after authentication middleware.
"django_otp.middleware.OTPMiddleware", # Required by two factor auth. "django_otp.middleware.OTPMiddleware", # Required by two factor auth.