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

@@ -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"

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;

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;

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(

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" }),

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<string, string[]>,