From 872e5eba73bfa454a8212c9420a271327e3b486d Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Wed, 14 Oct 2026 08:12:58 +0800 Subject: [PATCH] fix: check trusted proxy login against nginx's peer, not nginx (#1625) The backend only sees the bundled nginx on loopback, so the allowlist could only work by trusting 127.0.0.1, which let any client that reached nginx log in as anyone by sending the identity headers. nginx now passes its own TCP peer (before real_ip rewriting) in X-Termix-Proxy-Peer, and loopback connections are judged by that address instead. --- docker/docker-compose.yml | 6 +++- docker/nginx-https.conf | 3 ++ docker/nginx.conf | 3 ++ src/backend/database/routes/users.ts | 3 +- .../tests/utils/trusted-proxy-auth.test.ts | 30 +++++++++++++++++++ src/backend/utils/trusted-proxy-auth.ts | 26 ++++++++++++++++ 6 files changed, 69 insertions(+), 2 deletions(-) diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 601ae1808..10d35d746 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -28,7 +28,11 @@ services: # Trusted reverse-proxy authentication is disabled by default. When # enabled, do not expose this container directly to untrusted clients. # TRUSTED_PROXY_AUTH_ENABLED: "true" - # TRUSTED_PROXY_AUTH_TRUSTED_PROXIES: "172.16.0.0/12" + # List the address your authenticating proxy connects from, as this + # container sees it (for a proxy container on termix-net, its IP there). + # Keep it narrow: published ports can reach the container from the + # Docker gateway, so do not trust whole private ranges. + # TRUSTED_PROXY_AUTH_TRUSTED_PROXIES: "172.20.0.10" # TRUSTED_PROXY_AUTH_ROLE_MAP: '{"operators":["user"]}' # TRUSTED_PROXY_AUTH_USERNAME_HEADER: "x-forwarded-username" # TRUSTED_PROXY_AUTH_ROLE_HEADER: "x-forwarded-role" diff --git a/docker/nginx-https.conf b/docker/nginx-https.conf index dbcdfbc9c..182fee0e0 100644 --- a/docker/nginx-https.conf +++ b/docker/nginx-https.conf @@ -200,6 +200,9 @@ http { proxy_http_version 1.1; proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; + # The TCP peer before set_real_ip_from rewrites $remote_addr from + # X-Forwarded-For; trusted proxy login checks this address. + proxy_set_header X-Termix-Proxy-Peer $realip_remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; proxy_set_header X-Forwarded-Port $proxy_x_forwarded_port; diff --git a/docker/nginx.conf b/docker/nginx.conf index fa369e170..4f539e692 100644 --- a/docker/nginx.conf +++ b/docker/nginx.conf @@ -169,6 +169,9 @@ http { proxy_http_version 1.1; proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; + # The TCP peer before set_real_ip_from rewrites $remote_addr from + # X-Forwarded-For; trusted proxy login checks this address. + proxy_set_header X-Termix-Proxy-Peer $realip_remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; proxy_set_header X-Forwarded-Port $proxy_x_forwarded_port; diff --git a/src/backend/database/routes/users.ts b/src/backend/database/routes/users.ts index 1c8942b67..aba089a02 100644 --- a/src/backend/database/routes/users.ts +++ b/src/backend/database/routes/users.ts @@ -44,6 +44,7 @@ import { import type { UserRecord } from "../repositories/user-repository.js"; import { getTrustedProxyAuthConfig, + getTrustedProxySourceAddress, isTrustedProxyAddress, isTrustedProxyAuthEnabled, resolveTrustedProxyRoles, @@ -304,7 +305,7 @@ router.post("/proxy-login", async (req, res) => { } if (!config.enabled) return res.json({ enabled: false }); - const sourceAddress = req.socket.remoteAddress; + const sourceAddress = getTrustedProxySourceAddress(req); try { if (!isTrustedProxyAddress(sourceAddress, config.trustedProxies)) { authLogger.warn( diff --git a/src/backend/tests/utils/trusted-proxy-auth.test.ts b/src/backend/tests/utils/trusted-proxy-auth.test.ts index 34b06d3e0..1adfd01ea 100644 --- a/src/backend/tests/utils/trusted-proxy-auth.test.ts +++ b/src/backend/tests/utils/trusted-proxy-auth.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { getTrustedProxyAuthConfig, + getTrustedProxySourceAddress, isTrustedProxyAddress, parseTrustedProxyRoleMap, resolveTrustedProxyRoles, @@ -28,6 +29,35 @@ describe("trusted proxy authentication config", () => { expect(isTrustedProxyAddress("2001:db8::5", trusted)).toBe(true); }); + it("judges requests relayed by the bundled nginx by nginx's own peer", () => { + const viaNginx = (peer?: string) => ({ + headers: peer ? { "x-termix-proxy-peer": peer } : {}, + socket: { remoteAddress: "::ffff:127.0.0.1" }, + }); + expect(getTrustedProxySourceAddress(viaNginx("172.20.0.10"))).toBe( + "172.20.0.10", + ); + // A client reaching nginx directly is reported as itself, so trusting + // the auth proxy no longer means trusting nginx on loopback. + expect( + isTrustedProxyAddress( + getTrustedProxySourceAddress(viaNginx("203.0.113.7")), + ["127.0.0.1"], + ), + ).toBe(false); + // A local reverse proxy that does not set the header keeps working. + expect(getTrustedProxySourceAddress(viaNginx())).toBe("::ffff:127.0.0.1"); + }); + + it("ignores the peer header on connections that are not from loopback", () => { + expect( + getTrustedProxySourceAddress({ + headers: { "x-termix-proxy-peer": "172.20.0.10" }, + socket: { remoteAddress: "198.51.100.9" }, + }), + ).toBe("198.51.100.9"); + }); + it("fails closed when a supplied external role is not mapped", () => { const roleMap = parseTrustedProxyRoleMap( JSON.stringify({ operators: ["operator"], viewers: "readonly" }), diff --git a/src/backend/utils/trusted-proxy-auth.ts b/src/backend/utils/trusted-proxy-auth.ts index a2e207b26..444315a8c 100644 --- a/src/backend/utils/trusted-proxy-auth.ts +++ b/src/backend/utils/trusted-proxy-auth.ts @@ -1,3 +1,4 @@ +import type { IncomingHttpHeaders } from "node:http"; import { BlockList, isIP } from "node:net"; export interface TrustedProxyAuthConfig { @@ -108,6 +109,31 @@ export function isTrustedProxyAddress( ); } +function isLoopbackAddress(address: string): boolean { + const normalized = normalizeAddress(address); + return normalized === "::1" || normalized.startsWith("127."); +} + +/** + * The address checked against the allowlist. The bundled nginx always + * connects from loopback and sets X-Termix-Proxy-Peer to its own TCP peer, + * overwriting whatever the client sent, so behind it the proxy that counts + * is that peer rather than nginx. Any other connection is judged by its + * socket address. + */ +export function getTrustedProxySourceAddress(req: { + headers: IncomingHttpHeaders; + socket: { remoteAddress?: string }; +}): string | undefined { + const socketAddress = req.socket.remoteAddress; + const peer = req.headers["x-termix-proxy-peer"]; + return socketAddress && + isLoopbackAddress(socketAddress) && + typeof peer === "string" + ? peer + : socketAddress; +} + export function resolveTrustedProxyRoles( header: string, roleMap: Map,