Skip to content

Commit

Permalink
middleware: Detect reverse proxy misconfigurations.
Browse files Browse the repository at this point in the history
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 zulip#24599 and zulip/docker-zulip#403.
  • Loading branch information
alexmv committed Jun 28, 2023
1 parent 024cfae commit c9586d0
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 0 deletions.
1 change: 1 addition & 0 deletions puppet/zulip/files/nginx/uwsgi_params
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ 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_param HTTP_X_PROXY_MISCONFIGURATION $x_proxy_misconfiguration;

uwsgi_pass django;
1 change: 1 addition & 0 deletions puppet/zulip/files/nginx/zulip-include-common/proxy
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ proxy_set_header Host $host;
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_set_header X-Proxy-Misconfiguration $x_proxy_misconfiguration;
proxy_next_upstream off;
proxy_redirect off;
9 changes: 9 additions & 0 deletions puppet/zulip/templates/nginx/trusted-proto.template.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
map $remote_addr $trusted_x_forwarded_proto {
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 %>
# We do this in two steps because `geo` does not support variable
# interpolation in the value, but does support CIDR notation,
Expand All @@ -18,4 +22,9 @@ map $is_x_forwarded_proto_trusted $trusted_x_forwarded_proto {
0 $scheme;
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 %>
44 changes: 44 additions & 0 deletions zerver/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,50 @@ def process_request(self, request: HttpRequest) -> None:
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:
first_paragraph_text = get_content_description(content, request)
placeholder_open_graph_description = RequestNotes.get_notes(
Expand Down
1 change: 1 addition & 0 deletions zproject/computed_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@
"django.middleware.common.CommonMiddleware",
"zerver.middleware.LocaleMiddleware",
"zerver.middleware.HostDomainMiddleware",
"zerver.middleware.DetectProxyMisconfiguration",
"django.middleware.csrf.CsrfViewMiddleware",
# Make sure 2FA middlewares come after authentication middleware.
"django_otp.middleware.OTPMiddleware", # Required by two factor auth.
Expand Down

0 comments on commit c9586d0

Please sign in to comment.