Skip to content

Commit 3c75b2e

Browse files
jonpsprimadhav165
jonpspri
authored and
madhav165
committed
fix(security): CSRF origin validation and forwarded-header pass-through for reverse-proxy deployments (#3431)
Behind layered reverse proxies, X-Forwarded-Proto / X-Forwarded-Host headers may reflect internal hops rather than the external scheme, causing the strict same-origin CSRF check to reject legitimate admin mutations. - Extend _request_origin_matches() to fall back to settings.allowed_origins when the forwarded-header comparison fails; wildcard entries (*, null, "") are excluded to preserve fail-closed behavior - Normalize allowed_origins entries (strip whitespace, case-insensitive null) - Add map block to nginx.conf and Helm nginx configmap to preserve upstream X-Forwarded-Proto instead of overwriting with $scheme; pass X-Forwarded-Host Closes #3431 Signed-off-by: Jonathan Springer <jps@s390x.com>
1 parent 8cf7bc8 commit 3c75b2e

File tree

4 files changed

+215
-23
lines changed

4 files changed

+215
-23
lines changed

‎charts/mcp-stack/templates/configmap-nginx-proxy.yaml‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ data:
2929
proxy_cache_path {{ .Values.nginxProxy.config.cache.path }} levels=1:2 keys_zone=mcp_cache:100m max_size={{ .Values.nginxProxy.config.cache.maxSize }} inactive={{ .Values.nginxProxy.config.cache.inactive }} use_temp_path=off;
3030
{{- end }}
3131
32+
# Preserve X-Forwarded-Proto from upstream proxy (e.g. ALB, ingress
33+
# controller); fall back to $scheme when nginx is the outermost proxy.
34+
map $http_x_forwarded_proto $forwarded_proto {
35+
default $http_x_forwarded_proto;
36+
"" $scheme;
37+
}
38+
3239
upstream gateway_upstream {
3340
server {{ $gatewayServiceName }}:{{ $gatewayServicePort }};
3441
keepalive 32;
@@ -51,10 +58,11 @@ data:
5158
5259
location / {
5360
proxy_http_version 1.1;
54-
proxy_set_header Host $host;
61+
proxy_set_header Host $http_host;
5562
proxy_set_header X-Real-IP $remote_addr;
5663
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
57-
proxy_set_header X-Forwarded-Proto $scheme;
64+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
65+
proxy_set_header X-Forwarded-Host $http_host;
5866
proxy_set_header Connection "";
5967
6068
proxy_connect_timeout 30s;

‎infra/nginx/nginx.conf‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,13 @@ http {
185185
# # proxy_ssl_trusted_certificate /app/certs/cert.pem;
186186
# proxy_ssl_session_reuse on;
187187

188+
# Preserve X-Forwarded-Proto from upstream proxy (e.g. ALB, ingress
189+
# controller); fall back to $scheme when nginx is the outermost proxy.
190+
map $http_x_forwarded_proto $forwarded_proto {
191+
default $http_x_forwarded_proto;
192+
"" $scheme;
193+
}
194+
188195
# Cache bypass conditions
189196
map $request_method $skip_cache {
190197
default 0;
@@ -285,7 +292,8 @@ http {
285292
proxy_set_header Host $http_host;
286293
proxy_set_header X-Real-IP $remote_addr;
287294
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
288-
proxy_set_header X-Forwarded-Proto $scheme;
295+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
296+
proxy_set_header X-Forwarded-Host $http_host;
289297
proxy_set_header Connection "";
290298
proxy_http_version 1.1;
291299
}
@@ -313,7 +321,8 @@ http {
313321
proxy_set_header Host $http_host;
314322
proxy_set_header X-Real-IP $remote_addr;
315323
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
316-
proxy_set_header X-Forwarded-Proto $scheme;
324+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
325+
proxy_set_header X-Forwarded-Host $http_host;
317326
proxy_set_header Connection "";
318327
proxy_http_version 1.1;
319328
}
@@ -361,7 +370,8 @@ http {
361370
proxy_set_header Host $http_host;
362371
proxy_set_header X-Real-IP $remote_addr;
363372
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
364-
proxy_set_header X-Forwarded-Proto $scheme;
373+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
374+
proxy_set_header X-Forwarded-Host $http_host;
365375
proxy_set_header Connection "";
366376
proxy_http_version 1.1;
367377

@@ -384,7 +394,8 @@ http {
384394
proxy_set_header Host $http_host;
385395
proxy_set_header X-Real-IP $remote_addr;
386396
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
387-
proxy_set_header X-Forwarded-Proto $scheme;
397+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
398+
proxy_set_header X-Forwarded-Host $http_host;
388399
proxy_set_header Connection "";
389400
proxy_http_version 1.1;
390401

@@ -450,7 +461,8 @@ http {
450461
proxy_set_header Host $http_host;
451462
proxy_set_header X-Real-IP $remote_addr;
452463
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
453-
proxy_set_header X-Forwarded-Proto $scheme;
464+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
465+
proxy_set_header X-Forwarded-Host $http_host;
454466
proxy_set_header Connection "";
455467
proxy_http_version 1.1;
456468

@@ -477,7 +489,8 @@ http {
477489
proxy_set_header Host $http_host;
478490
proxy_set_header X-Real-IP $remote_addr;
479491
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
480-
proxy_set_header X-Forwarded-Proto $scheme;
492+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
493+
proxy_set_header X-Forwarded-Host $http_host;
481494

482495
# Extended timeouts for SSE
483496
proxy_connect_timeout 1h;
@@ -500,7 +513,8 @@ http {
500513
proxy_set_header Host $http_host;
501514
proxy_set_header X-Real-IP $remote_addr;
502515
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
503-
proxy_set_header X-Forwarded-Proto $scheme;
516+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
517+
proxy_set_header X-Forwarded-Host $http_host;
504518

505519
# Extended timeouts for SSE
506520
proxy_connect_timeout 1h;
@@ -518,7 +532,8 @@ http {
518532
proxy_set_header Host $http_host;
519533
proxy_set_header X-Real-IP $remote_addr;
520534
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
521-
proxy_set_header X-Forwarded-Proto $scheme;
535+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
536+
proxy_set_header X-Forwarded-Host $http_host;
522537
}
523538

524539
location ~ ^/servers/.*/ws$ {
@@ -534,7 +549,8 @@ http {
534549
proxy_set_header Host $http_host;
535550
proxy_set_header X-Real-IP $remote_addr;
536551
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
537-
proxy_set_header X-Forwarded-Proto $scheme;
552+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
553+
proxy_set_header X-Forwarded-Host $http_host;
538554

539555
# Extended timeouts for WebSocket
540556
proxy_connect_timeout 1h;
@@ -564,7 +580,8 @@ http {
564580
proxy_set_header Host $http_host;
565581
proxy_set_header X-Real-IP $remote_addr;
566582
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
567-
proxy_set_header X-Forwarded-Proto $scheme;
583+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
584+
proxy_set_header X-Forwarded-Host $http_host;
568585
proxy_set_header Connection "";
569586
proxy_http_version 1.1;
570587

@@ -596,7 +613,8 @@ http {
596613
proxy_set_header Host $http_host;
597614
proxy_set_header X-Real-IP $remote_addr;
598615
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
599-
proxy_set_header X-Forwarded-Proto $scheme;
616+
proxy_set_header X-Forwarded-Proto $forwarded_proto;
617+
proxy_set_header X-Forwarded-Host $http_host;
600618
proxy_set_header Connection "";
601619
proxy_http_version 1.1;
602620

‎mcpgateway/admin.py‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,16 +1080,20 @@ def _normalize_origin_parts(scheme: str, netloc: str) -> tuple[str, str, int]:
10801080
def _request_origin_matches(request: Request) -> bool:
10811081
"""Return ``True`` when Origin/Referer matches this request origin.
10821082

1083+
The function first performs an exact same-origin comparison using the
1084+
request's forwarded headers (``X-Forwarded-Proto`` / ``X-Forwarded-Host``).
1085+
When that fails — common behind layered reverse proxies where forwarded
1086+
headers reflect internal hops rather than the external scheme — it falls
1087+
back to checking whether the candidate origin is explicitly listed in
1088+
``settings.allowed_origins``. Wildcard entries (``*``, ``null``, ``""``)
1089+
are excluded from the fallback to preserve fail-closed behavior.
1090+
10831091
Args:
10841092
request: Incoming request carrying Origin/Referer and host headers.
10851093

10861094
Returns:
1087-
``True`` when candidate origin exactly matches request origin; otherwise ``False``.
1088-
1089-
Note:
1090-
When the service is deployed behind a reverse proxy, this check relies on
1091-
``X-Forwarded-Proto`` / ``X-Forwarded-Host`` values emitted by that proxy.
1092-
The deployment boundary must sanitize and overwrite forwarded headers.
1095+
``True`` when candidate origin matches either the request origin or an
1096+
entry in ``settings.allowed_origins``; otherwise ``False``.
10931097
"""
10941098
origin = request.headers.get("origin")
10951099
referer = request.headers.get("referer")
@@ -1117,7 +1121,29 @@ def _request_origin_matches(request: Request) -> bool:
11171121

11181122
candidate_parts = _normalize_origin_parts(parsed_candidate.scheme, parsed_candidate.netloc)
11191123
request_parts = _normalize_origin_parts(request_scheme, request_netloc)
1120-
return candidate_parts == request_parts
1124+
if candidate_parts == request_parts:
1125+
return True
1126+
1127+
# Fallback: accept origins explicitly listed in settings.allowed_origins.
1128+
# Handles reverse-proxy deployments where forwarded headers may not
1129+
# accurately reflect the external scheme/host.
1130+
for allowed in settings.allowed_origins:
1131+
# Normalize each allowed origin to avoid config surprises such as
1132+
# ["https://a.com "] or [" null "], which could otherwise be
1133+
# mis-parsed or skipped.
1134+
allowed_normalized = str(allowed).strip()
1135+
if not allowed_normalized or allowed_normalized == "*" or allowed_normalized.casefold() == "null":
1136+
continue
1137+
try:
1138+
allowed_parsed = urllib.parse.urlparse(allowed_normalized)
1139+
if not allowed_parsed.scheme or not allowed_parsed.netloc:
1140+
continue
1141+
if candidate_parts == _normalize_origin_parts(allowed_parsed.scheme, allowed_parsed.netloc):
1142+
return True
1143+
except Exception: # nosec B112 - malformed allowed_origins entry should not crash
1144+
continue
1145+
1146+
return False
11211147

11221148

11231149
def _set_admin_csrf_cookie(request: Request, response: Response) -> str:

0 commit comments

Comments
 (0)