diff --git a/.env.example b/.env.example index 8ba3be7d0..462459b44 100644 --- a/.env.example +++ b/.env.example @@ -234,6 +234,12 @@ API_TOKEN_HMAC_KEY="REPLACE-WITH-OUTPUT-OF-openssl-rand-hex-32" # refuse the headers, log one warning, and share one rate-limit bucket across # all anonymous callers. # TRUST_PROXY_HOPS="1" +# The addresses the reverse proxy connects from (comma-separated CIDRs). Set, +# only those connections may report the client's address; unset, any private +# address may, which behind Docker Desktop, OrbStack or docker-proxy includes +# every caller of the published port. Same host: 127.0.0.1/32,::1/128; Docker +# network: the proxy's subnet. See docs/self-hosting/reverse-proxy.md. +# TRUSTED_PROXY_CIDRS="" # TRUST_CF_CONNECTING_IP="" diff --git a/.env.production.example b/.env.production.example index 81356bfb9..4c88afe04 100644 --- a/.env.production.example +++ b/.env.production.example @@ -565,6 +565,23 @@ API_TOKEN_HMAC_KEY="" # anonymous callers into one rate-limit bucket. # TRUST_PROXY_HOPS="1" # +# The addresses your reverse proxy connects from, as comma-separated CIDRs +# (IPv4 or IPv6; a bare address is one host). When set, only those +# connections may report the client's address; every other connection is +# its own client. When unset, any private address may, which is the old +# behaviour and is NOT safe behind Docker Desktop, OrbStack or docker-proxy: +# there every caller of the published port arrives from the gateway, a +# private address, and could name its own client address. Set it whenever a +# reverse proxy is in front: +# proxy on the same host (proxying to 127.0.0.1:3000): 127.0.0.1/32,::1/128 +# proxy container on a Docker network: that network's subnet (docker +# network inspect ), e.g. 172.18.0.0/16 +# proxy on another machine with a public address: that address, /32 +# A proxy on a public address that is not listed is not believed: every +# request through it shares the proxy's address and one rate-limit bucket. +# The boot log names the rule in force in one [proxy-trust] line. +# TRUSTED_PROXY_CIDRS="127.0.0.1/32,::1/128" +# # Behind Cloudflare the real visitor IP arrives in the `cf-connecting-ip` # header; the X-Forwarded-For chain otherwise yields the Cloudflare edge IP, # so the audit-log overview stores the CF datacentre address and the geo diff --git a/docker-compose.yml b/docker-compose.yml index c73d135f1..cbf7e5aab 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -108,7 +108,14 @@ x-healthlog-env: &healthlog_env # collapses every anonymous caller into one rate-limit bucket. Listed here # so an operator value in .env reaches the container (the `environment:` # block is a whitelist). + # TRUSTED_PROXY_CIDRS names the addresses the reverse proxy connects from + # (comma-separated CIDRs). Set, only those connections may report a client + # address; unset, any private address may, which behind Docker Desktop, + # OrbStack or docker-proxy includes every caller of the published port. + # Same host: 127.0.0.1/32,::1/128. Docker network: the proxy's subnet. + # See docs/self-hosting/reverse-proxy.md. TRUST_PROXY_HOPS: "${TRUST_PROXY_HOPS:-}" + TRUSTED_PROXY_CIDRS: "${TRUSTED_PROXY_CIDRS:-}" TRUST_CF_CONNECTING_IP: "${TRUST_CF_CONNECTING_IP:-}" IP_GEO_LOOKUP_URL: "${IP_GEO_LOOKUP_URL:-}" IP_GEO_LOOKUP_DISABLED: "${IP_GEO_LOOKUP_DISABLED:-}" diff --git a/docs/self-hosting/reverse-proxy.md b/docs/self-hosting/reverse-proxy.md index 5dee21854..3cceebaac 100644 --- a/docs/self-hosting/reverse-proxy.md +++ b/docs/self-hosting/reverse-proxy.md @@ -21,13 +21,14 @@ tailnet with no extra container. See | `NEXT_PUBLIC_APP_URL` | Every public-facing absolute URL the UI emits | Must match the hostname the proxy serves | | `APP_URL` | Server-side absolute URLs from background jobs | Mirror of the above; both must match | | `TRUST_PROXY_HOPS` | `X-Forwarded-For` chain trust | Defaults to `1` — one trusted hop | +| `TRUSTED_PROXY_CIDRS` | Which connections may report a client address | Unset: any private address; see below | | `TRUST_CF_CONNECTING_IP` | Reading the visitor address Cloudflare sends | Off unless `1`; see the warning below | Set the URL pair to the public origin (`https://your-instance.example.com`) and restart the `app` container. The image reads both at startup; no rebuild needed. -All four variables are on the shipped `docker-compose.yml` +All five variables are on the shipped `docker-compose.yml` `environment:` whitelist, so a value in `.env` reaches the container without editing the compose file. (Before v1.37.33 the whitelist was missing `TRUST_PROXY_HOPS`; on those versions add @@ -68,13 +69,56 @@ The reserved and multicast blocks are in the list because no host on the internet can open a connection from them, so a peer there sits on a network you run. A request that reaches HealthLog straight from a public address carries headers its sender wrote, so HealthLog ignores them and -uses the connection's own address, and says so once in the log. A proxy on -the same host or Docker network connects from a private address, so the -usual setups above need nothing. If your proxy reaches HealthLog over a -public address, put it on a private network with HealthLog instead. The log +uses the connection's own address, and says so once in the log. The log line also shows up when the published port is open to the internet beside the proxy; bind it to the proxy's network or to `127.0.0.1` then. +That list is the default when `TRUSTED_PROXY_CIDRS` is unset, and it is +only as good as the network in front of the port. Read the next section +before relying on it. + +### `TRUSTED_PROXY_CIDRS` — name the proxy + +Set `TRUSTED_PROXY_CIDRS` to the addresses your reverse proxy connects +from, as comma-separated CIDRs (IPv4 or IPv6; a bare address counts as a +single host). Once it is set, only a connection from inside those ranges +may report the client's address. Every other connection is its own client, +whatever its address, private ones included, and the log says so once. + +Why it matters: some setups make every caller look like a private address. +Docker Desktop, OrbStack, and Docker's `docker-proxy` (the userland proxy +behind a published port such as `3000:3000`) forward each connection from +the gateway address of the Docker network, which is private. On those +hosts, someone who can reach the published port directly, from the LAN or +from the internet if the port is open, passes the private-address check +and can again choose the address that every per-IP rate limit, the audit +log and the sign-in notices see, a new one per request if they like. +Naming your proxy closes that. The default port binding in +`docker-compose.yml` stays as it is (LAN installs rely on it); this +variable is the fix. + +What to set: + +| Where the proxy runs | `TRUSTED_PROXY_CIDRS` | +| ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| On the same host, proxying to `127.0.0.1:3000` (bare Nginx, Caddy) | `127.0.0.1/32,::1/128` | +| In a container on a Docker network with HealthLog (Traefik, NPM, Coolify) | that network's subnet, e.g. `172.18.0.0/16` (`docker network inspect ` shows it under `IPAM`) | +| On another machine that connects from a public address | that machine's address, e.g. `198.51.100.7/32` | +| No proxy, or `tailscale serve` only | leave it unset | + +At boot the app logs one `[proxy-trust]` line naming the rule in force: +which connections it reads forwarding headers from and how many hops deep. +A value that is not a CIDR is logged as an error, and until it is fixed no +connection may report a client address. + +**A proxy that connects from a public address.** Without +`TRUSTED_PROXY_CIDRS`, such a proxy is not believed: every request through +it resolves to the proxy's own address, so all your users share one +rate-limit bucket and one person mistyping a password can hold sign-in back +for everyone. Releases before v1.46 believed any peer, so this shows up after +the upgrade. Set `TRUSTED_PROXY_CIDRS` to the proxy's address (or put the proxy +on a private network with HealthLog) and restart. + ### `TRUST_CF_CONNECTING_IP` — only when the origin takes Cloudflare only With `TRUST_CF_CONNECTING_IP=1` HealthLog takes the visitor's address from @@ -283,6 +327,15 @@ The one-shot stderr warning `[getClientIp] TRUST_PROXY_HOPS=N but X-Forwarded-For carried M entries` confirms a mismatch. Adjust the env var down to the actual chain length and restart. +If every request logs the same client address, the proxy's own, the proxy +is not one HealthLog believes: the `[getClientIp]` warning about a +connection "from a public address" or "from outside TRUSTED_PROXY_CIDRS" +names the case. Add the proxy's address to `TRUSTED_PROXY_CIDRS`. + +A `TRUST_PROXY_HOPS` value that is not a whole number resolves no client +address at all (requests keep working) and is named in the `[proxy-trust]` +boot line. + ## Running over plain HTTP on a LAN (no reverse proxy) The session cookie carries the `Secure` flag by default under diff --git a/scripts/env-manifest.json b/scripts/env-manifest.json index 75339c7fd..51477220b 100644 --- a/scripts/env-manifest.json +++ b/scripts/env-manifest.json @@ -432,6 +432,11 @@ "purpose": "Trusted reverse-proxy hop count; the client IP is read that many entries from the right of X-Forwarded-For. Forwarding headers are read only from a peer whose own address is not public (loopback, private, CGNAT, unique-local, link-local, reserved, multicast).", "example": "1" }, + { + "name": "TRUSTED_PROXY_CIDRS", + "purpose": "Comma-separated CIDRs (IPv4 or IPv6; a bare address is one host) the reverse proxy connects from. Set, only a connection from inside them may report the client address in X-Forwarded-For or X-Real-IP; every other connection is its own client. Unset, any non-public address may, which behind Docker Desktop, OrbStack or docker-proxy includes every caller of a published port. Same host: 127.0.0.1/32,::1/128; Docker network: the proxy's subnet. An entry that is not a CIDR lets no connection report an address and is named at boot.", + "example": "127.0.0.1/32,::1/128" + }, { "name": "TRUST_CF_CONNECTING_IP", "purpose": "\"1\" takes the client IP from Cloudflare's CF-Connecting-IP header, from every connection and before the peer check. Safe only when the origin accepts connections from Cloudflare alone (a Tunnel, or a firewall admitting only Cloudflare's ranges); otherwise any caller forges the address rate limits and the audit log record. Off by default.", diff --git a/src/instrumentation.ts b/src/instrumentation.ts index e7cd50243..96bf1f6c9 100644 --- a/src/instrumentation.ts +++ b/src/instrumentation.ts @@ -15,6 +15,18 @@ async function registerOfflineGeoNoticeAtBoot(): Promise { } } +/** + * Which connections may report a client address (TRUSTED_PROXY_CIDRS, + * TRUST_PROXY_HOPS), said once, with a bad value named. + */ +async function logProxyTrustAtBootSafely(): Promise { + try { + (await import("@/lib/http/proxy-trust")).logProxyTrustAtBoot(); + } catch { + // A log line must never break boot. + } +} + export async function register() { // Only start the worker on the Node.js server runtime (not Edge, not build) if (process.env.NEXT_RUNTIME === "nodejs") { @@ -54,6 +66,7 @@ export async function register() { } await registerOfflineGeoNoticeAtBoot(); + await logProxyTrustAtBootSafely(); // Does the configured encryption key open this database? A process whose // key does not refuses every API request with one clear 503 and starts diff --git a/src/lib/__tests__/api-handler-invalid-proxy-hops.test.ts b/src/lib/__tests__/api-handler-invalid-proxy-hops.test.ts new file mode 100644 index 000000000..b051dc4cf --- /dev/null +++ b/src/lib/__tests__/api-handler-invalid-proxy-hops.test.ts @@ -0,0 +1,52 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ prisma: {} })); +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { NextRequest } from "next/server"; +import { apiHandler } from "@/lib/api-handler"; +import { getClientIp } from "@/lib/api-response"; + +/** + * A typo in `TRUST_PROXY_HOPS` used to make every request that asked for the + * client address throw, and `apiHandler` asks for it on every request to + * record it: every route answered 500, the health probe included, so the + * container looked dead for a configuration slip. It now resolves no address + * and the boot log names the value. + */ +afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); +}); + +describe("apiHandler with an invalid TRUST_PROXY_HOPS", () => { + it("answers the route instead of 500, with no client address", async () => { + vi.stubEnv("TRUST_PROXY_HOPS", "two"); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const seen: Array = []; + const route = apiHandler(async (request: NextRequest) => { + seen.push(getClientIp(request)); + return Response.json({ data: { status: "ok" }, error: null }); + }); + const response = await route( + new NextRequest("https://health.example.com/api/health", { + headers: { "x-forwarded-for": "203.0.113.4" }, + }), + ); + expect(response.status).toBe(200); + expect(seen).toEqual([null]); + }); +}); diff --git a/src/lib/__tests__/get-client-ip.test.ts b/src/lib/__tests__/get-client-ip.test.ts index 6ef8182b0..9d57d4c25 100644 --- a/src/lib/__tests__/get-client-ip.test.ts +++ b/src/lib/__tests__/get-client-ip.test.ts @@ -10,16 +10,22 @@ import { import { getClientIp, getClientIpOrTrustWarning, + _resetInvalidHopsWarningForTests, + _resetPublicPeerForwardingWarningForTests, _resetTrustViolationWarningForTests, _resetUnreadProxyHeadersWarningForTests, } from "../api-response"; +import { describeProxyTrust } from "../http/proxy-trust"; const ORIGINAL_HOPS = process.env.TRUST_PROXY_HOPS; const ORIGINAL_CF_FLAG = process.env.TRUST_CF_CONNECTING_IP; +const ORIGINAL_CIDRS = process.env.TRUSTED_PROXY_CIDRS; beforeEach(() => { delete process.env.TRUST_PROXY_HOPS; delete process.env.TRUST_CF_CONNECTING_IP; + delete process.env.TRUSTED_PROXY_CIDRS; + _resetPublicPeerForwardingWarningForTests(); _resetTrustViolationWarningForTests(); _resetUnreadProxyHeadersWarningForTests(); }); @@ -29,6 +35,8 @@ afterEach(() => { else process.env.TRUST_PROXY_HOPS = ORIGINAL_HOPS; if (ORIGINAL_CF_FLAG === undefined) delete process.env.TRUST_CF_CONNECTING_IP; else process.env.TRUST_CF_CONNECTING_IP = ORIGINAL_CF_FLAG; + if (ORIGINAL_CIDRS === undefined) delete process.env.TRUSTED_PROXY_CIDRS; + else process.env.TRUSTED_PROXY_CIDRS = ORIGINAL_CIDRS; }); function makeRequest(headers: Record): Request { @@ -64,11 +72,23 @@ describe("getClientIp trusted-proxy semantics (V3 audit)", () => { expect(ip).toBeNull(); }); - it("throws at boot when TRUST_PROXY_HOPS is unparseable (fixes review L-1)", () => { + it("resolves no address for an unparseable TRUST_PROXY_HOPS instead of failing the request", () => { + // It used to throw on every request that asked for the client address, + // so every route answered 500, the health probe included. Reading the + // headers anyway would silently trust a chain the operator never + // described (review L-1), so: no address, a trust violation, one warning. process.env.TRUST_PROXY_HOPS = "garbage"; - expect(() => - getClientIp(makeRequest({ "x-forwarded-for": "5.6.7.8" })), - ).toThrow(/TRUST_PROXY_HOPS/); + _resetInvalidHopsWarningForTests(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + onTestFinished(() => warn.mockRestore()); + const request = makeRequest({ "x-forwarded-for": "5.6.7.8" }); + expect(getClientIpOrTrustWarning(request)).toEqual({ + ip: null, + trustViolation: true, + }); + expect(getClientIp(request)).toBeNull(); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]![0]).toMatch(/TRUST_PROXY_HOPS="garbage"/); }); it.each(["", " "])( @@ -480,3 +500,93 @@ describe("getClientIp against the connection's own peer", () => { ).toBeNull(); }); }); + +/** + * Behind Docker Desktop, OrbStack or `docker-proxy`, every connection to a + * published port arrives from the gateway, a private address, so the + * private-range default believed a direct caller's forwarding headers again. + * `TRUSTED_PROXY_CIDRS` names the proxies; no other peer may forward. + */ +describe("getClientIp with TRUSTED_PROXY_CIDRS", () => { + const stamped = (peer: string, headers: Record = {}) => + makeRequest({ ...headers, "x-healthlog-peer-address": peer }); + + it("reads forwarding headers only from a peer inside the list", () => { + process.env.TRUSTED_PROXY_CIDRS = "172.18.0.0/16, 127.0.0.1"; + expect( + getClientIp(stamped("172.18.0.5", { "x-forwarded-for": "203.0.113.5" })), + ).toBe("203.0.113.5"); + expect( + getClientIp(stamped("127.0.0.1", { "x-real-ip": "203.0.113.6" })), + ).toBe("203.0.113.6"); + }); + + it("uses the gateway's own address for a direct caller from a private network outside the list", () => { + process.env.TRUSTED_PROXY_CIDRS = "172.18.0.0/16"; + vi.spyOn(console, "warn").mockImplementation(() => {}); + const forged = ["203.0.113.1", "203.0.113.2"].map((ip) => + getClientIp( + stamped("192.168.65.1", { "x-forwarded-for": ip, "x-real-ip": ip }), + ), + ); + expect(forged).toEqual(["192.168.65.1", "192.168.65.1"]); + vi.restoreAllMocks(); + }); + + it("believes a proxy on a public address once it is listed", () => { + process.env.TRUSTED_PROXY_CIDRS = "198.51.100.7/32"; + expect( + getClientIp( + stamped("198.51.100.7", { "x-forwarded-for": "203.0.113.9" }), + ), + ).toBe("203.0.113.9"); + }); + + it("takes IPv6 ranges and bare addresses", () => { + process.env.TRUSTED_PROXY_CIDRS = "fd00::/8,2001:db8::1"; + expect( + getClientIp(stamped("fd12::3", { "x-forwarded-for": "203.0.113.1" })), + ).toBe("203.0.113.1"); + expect( + getClientIp(stamped("2001:db8::1", { "x-forwarded-for": "203.0.113.2" })), + ).toBe("203.0.113.2"); + expect( + getClientIp(stamped("2001:db8::2", { "x-forwarded-for": "203.0.113.3" })), + ).toBe("2001:db8::2"); + }); + + it("lets no peer forward when an entry is not a CIDR", () => { + process.env.TRUSTED_PROXY_CIDRS = "172.18.0.0/16, not-a-range"; + vi.spyOn(console, "warn").mockImplementation(() => {}); + expect( + getClientIp(stamped("172.18.0.5", { "x-forwarded-for": "203.0.113.5" })), + ).toBe("172.18.0.5"); + vi.restoreAllMocks(); + }); +}); + +describe("the boot line naming the proxy rule", () => { + it("names the private-range default, and what it lets in behind a port forwarder", () => { + expect(describeProxyTrust()).toMatchObject({ + level: "info", + message: expect.stringMatching(/TRUSTED_PROXY_CIDRS unset.*docker-proxy/), + }); + }); + + it("names the list when one is set", () => { + process.env.TRUSTED_PROXY_CIDRS = "127.0.0.1/32"; + expect(describeProxyTrust()).toMatchObject({ + level: "info", + message: expect.stringContaining("only connections from 127.0.0.1/32"), + }); + }); + + it("names a bad TRUST_PROXY_HOPS and a bad CIDR as errors", () => { + process.env.TRUST_PROXY_HOPS = "one"; + process.env.TRUSTED_PROXY_CIDRS = "10.0.0.0/33"; + const line = describeProxyTrust(); + expect(line.level).toBe("error"); + expect(line.message).toContain('TRUST_PROXY_HOPS="one"'); + expect(line.message).toContain('"10.0.0.0/33"'); + }); +}); diff --git a/src/lib/api-response.ts b/src/lib/api-response.ts index f7f8f058c..afc629b15 100644 --- a/src/lib/api-response.ts +++ b/src/lib/api-response.ts @@ -7,7 +7,11 @@ import { peerAddressStampInstalled, readPeerAddress, } from "@/lib/http/peer-address"; -import { isPublicIp } from "@/lib/validations/notifications"; +import { + parseTrustProxyHops, + parseTrustedProxyCidrs, + peerMayForward, +} from "@/lib/http/proxy-trust"; export function apiSuccess(data: T, status = 200) { return NextResponse.json({ data, error: null }, { status }); @@ -508,13 +512,16 @@ export async function safeJson( * observed when the request arrived. * - "N" (>1) → trust N hops; read the Nth-from-rightmost XFF entry. * - * Forwarding headers are read only from a connection whose peer could be a - * proxy: loopback, a private or unique-local network, CGNAT (Tailscale). A - * connection from a public address reached the server directly, so whatever - * forwarding headers it sends are its own, and its own address is the client - * IP. The peer comes from `@/lib/http/peer-address`. Without that check, the - * default of one trusted hop let any direct caller name a fresh address per - * request and step around every per-IP limit. + * Forwarding headers are read only from a connection whose peer may be a + * proxy (`peerMayForward` in `@/lib/http/proxy-trust`): one inside + * `TRUSTED_PROXY_CIDRS` when that is set, otherwise any loopback, private, + * unique-local or CGNAT (Tailscale) address. Any other connection reached the + * server directly, so whatever forwarding headers it sends are its own, and + * its own address is the client IP. The peer comes from + * `@/lib/http/peer-address`. Without that check, the default of one trusted + * hop let any direct caller name a fresh address per request and step around + * every per-IP limit. An invalid `TRUST_PROXY_HOPS` resolves no address + * (and is named at boot) instead of failing the request. * * Cloudflare opt-in (`TRUST_CF_CONNECTING_IP=1`): when the env flag is * set, the helper prefers the `cf-connecting-ip` header before walking @@ -561,22 +568,24 @@ function readCfConnectingIp(request: Request): string | null { return looksLikeIp(trimmed) ? trimmed : null; } -function parseTrustProxyHops(raw: string | undefined): number { - // Empty is unset. The shipped docker-compose.yml passes - // `TRUST_PROXY_HOPS: "${TRUST_PROXY_HOPS:-}"`, which is an empty string for - // every operator who never set it; reading that as invalid threw on every - // request that asked for the client IP. - const trimmed = raw?.trim() ?? ""; - if (trimmed === "") return 1; - if (!/^\d+$/.test(trimmed)) { - // Reject explicitly-invalid values so an operator typo doesn't silently - // switch a real-proxy deployment to "no XFF trust" mode (review - // finding L-1: TRUST_PROXY_HOPS=garbage degraded to hops=0 silently). - throw new Error( - `TRUST_PROXY_HOPS must be a non-negative integer, got: ${JSON.stringify(raw)}`, - ); - } - return parseInt(trimmed, 10); +/** + * Once per process: `TRUST_PROXY_HOPS` is not a number. Every request that + * asks for the client address used to throw on it, so every route answered + * 500, `/api/health` included. It resolves no address instead, and says so. + */ +let invalidHopsWarned = false; + +function warnInvalidHopsOnce(raw: string | undefined): void { + if (invalidHopsWarned) return; + invalidHopsWarned = true; + console.warn( + `[getClientIp] TRUST_PROXY_HOPS=${JSON.stringify(raw)} is not a non-negative integer; no client address is read from forwarding headers until it is fixed, and every anonymous caller shares one rate-limit bucket.`, + ); +} + +/** Test-only reset for {@link warnInvalidHopsOnce}. */ +export function _resetInvalidHopsWarningForTests(): void { + invalidHopsWarned = false; } /** @@ -646,12 +655,23 @@ let publicPeerForwardingWarned = false; function warnPublicPeerForwardingOnce(): void { if (publicPeerForwardingWarned) return; publicPeerForwardingWarned = true; + const listed = + parseTrustedProxyCidrs(process.env.TRUSTED_PROXY_CIDRS).kind !== "unset"; console.warn( - "[getClientIp] a connection from a public address sent X-Forwarded-For or X-Real-IP; those headers are ignored and the connection's own address is used. " + - "If HealthLog is meant to be reached only through a reverse proxy, the published port is open beyond it.", + listed + ? "[getClientIp] a connection from outside TRUSTED_PROXY_CIDRS sent X-Forwarded-For or X-Real-IP; those headers are ignored and the connection's own address is used. " + + "If it was your proxy, add its address to TRUSTED_PROXY_CIDRS; otherwise the published port is reachable beyond the proxy." + : "[getClientIp] a connection from a public address sent X-Forwarded-For or X-Real-IP; those headers are ignored and the connection's own address is used. " + + "If HealthLog is meant to be reached only through a reverse proxy, the published port is open beyond it. " + + "If the proxy connects from a public address, set TRUSTED_PROXY_CIDRS to it.", ); } +/** Test-only reset for {@link warnPublicPeerForwardingOnce}. */ +export function _resetPublicPeerForwardingWarningForTests(): void { + publicPeerForwardingWarned = false; +} + /** * Tagged return shape so a caller can apply a tighter universal * rate-limit when the trust chain is misconfigured. F-6 (mobile security @@ -681,6 +701,10 @@ export function getClientIpOrTrustWarning(request: Request): { if (cfIp) return { ip: cfIp, trustViolation: false }; const hops = parseTrustProxyHops(process.env.TRUST_PROXY_HOPS); + if (hops === null) { + warnInvalidHopsOnce(process.env.TRUST_PROXY_HOPS); + return { ip: null, trustViolation: true }; + } // The connection's own peer, stamped by the server for every request it // receives. `undefined` only for a request the server did not receive (a @@ -693,9 +717,10 @@ export function getClientIpOrTrustWarning(request: Request): { if (peer !== undefined) { // No proxy declared: the peer is the client. if (hops === 0) return { ip: peer, trustViolation: false }; - // A peer on a public address reached the server directly; its - // forwarding headers are its own choice, not a proxy's report. - if (isPublicIp(peer)) { + // A peer that is not a proxy (outside TRUSTED_PROXY_CIDRS when it is + // set, on a public address when it is not) reached the server directly; + // its forwarding headers are its own choice, not a proxy's report. + if (!peerMayForward(peer)) { if ( request.headers.has("x-real-ip") || request.headers.get("x-forwarded-for")?.trim() !== peer diff --git a/src/lib/http/proxy-trust.ts b/src/lib/http/proxy-trust.ts new file mode 100644 index 000000000..d7cc9128d --- /dev/null +++ b/src/lib/http/proxy-trust.ts @@ -0,0 +1,175 @@ +/** + * Which connections may tell the server who the client is. + * + * A reverse proxy reports the client's address in `X-Forwarded-For` or + * `X-Real-IP`. Anyone else who sends those headers is choosing the address + * every per-IP rate limit, the audit log and the sign-in notices see, so the + * client-IP resolver (`getClientIpOrTrustWarning` in `@/lib/api-response`) + * reads them only from a peer this module says may forward. + * + * Two rules: + * + * - `TRUSTED_PROXY_CIDRS` set (comma-separated CIDRs, IPv4 or IPv6): only + * a peer inside one of them may forward. Every other connection is its + * own client, whatever address it comes from. + * - unset: any peer whose address is not public (loopback, private, + * CGNAT, unique-local, link-local) may forward. This is the long-standing + * default and stays for compatibility, but it is only as good as the + * network: behind Docker Desktop, OrbStack or `docker-proxy` every + * connection to a published port arrives from the gateway address, a + * private one, so a caller that reaches the port directly is believed + * again. Setting the list closes that. + * + * `TRUST_PROXY_HOPS` says how many proxies the `X-Forwarded-For` chain + * passed through. A value that is not a non-negative integer used to throw + * on every request that asked for the client address, `/api/health` + * included; it now resolves no address and is named once at boot. + * + * Both are read per call, so the suites can change them; the parsed CIDR + * list is cached by its raw value. + */ +import { BlockList, isIP } from "node:net"; + +import { isPublicIp } from "@/lib/validations/notifications"; + +// ── TRUST_PROXY_HOPS ────────────────────────────────────────────────────── + +/** + * The configured hop count, `1` when unset or empty, or `null` when the + * value is not a non-negative integer. + */ +export function parseTrustProxyHops(raw: string | undefined): number | null { + // Empty is unset. The shipped docker-compose.yml passes + // `TRUST_PROXY_HOPS: "${TRUST_PROXY_HOPS:-}"`, which is an empty string for + // every operator who never set it. + const trimmed = raw?.trim() ?? ""; + if (trimmed === "") return 1; + if (!/^\d+$/.test(trimmed)) return null; + return parseInt(trimmed, 10); +} + +// ── TRUSTED_PROXY_CIDRS ─────────────────────────────────────────────────── + +export type TrustedProxyList = + /** Unset: the private-address default applies. */ + | { kind: "unset" } + | { kind: "list"; entries: string[]; blockList: BlockList } + /** Set, but an entry is not a CIDR. No peer may forward. */ + | { kind: "invalid"; invalid: string[] }; + +/** `10.0.0.0/8`, `127.0.0.1`, `fd00::/8`; a bare address is a /32 or /128. */ +function parseCidr( + entry: string, +): { address: string; prefix: number; family: "ipv4" | "ipv6" } | null { + const [address = "", prefixText, extra] = entry.split("/"); + if (extra !== undefined) return null; + const version = isIP(address); + if (version === 0) return null; + const max = version === 4 ? 32 : 128; + if (prefixText !== undefined && !/^\d{1,3}$/.test(prefixText)) return null; + const prefix = prefixText === undefined ? max : Number(prefixText); + if (prefix > max) return null; + return { address, prefix, family: version === 4 ? "ipv4" : "ipv6" }; +} + +let cached: { raw: string; list: TrustedProxyList } | null = null; + +export function parseTrustedProxyCidrs( + raw: string | undefined, +): TrustedProxyList { + const value = raw?.trim() ?? ""; + if (cached && cached.raw === value) return cached.list; + let list: TrustedProxyList; + if (value === "") { + list = { kind: "unset" }; + } else { + const entries = value + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); + const blockList = new BlockList(); + const invalid: string[] = []; + for (const entry of entries) { + const cidr = parseCidr(entry); + if (!cidr) { + invalid.push(entry); + continue; + } + blockList.addSubnet(cidr.address, cidr.prefix, cidr.family); + } + list = + invalid.length > 0 || entries.length === 0 + ? { kind: "invalid", invalid: invalid.length > 0 ? invalid : [value] } + : { kind: "list", entries, blockList }; + } + cached = { raw: value, list }; + return list; +} + +/** Whether a connection from `peer` may report the client's address. */ +export function peerMayForward(peer: string): boolean { + const list = parseTrustedProxyCidrs(process.env.TRUSTED_PROXY_CIDRS); + if (list.kind === "unset") return !isPublicIp(peer); + if (list.kind === "invalid") return false; + const family = isIP(peer) === 6 ? "ipv6" : "ipv4"; + return list.blockList.check(peer, family); +} + +// ── The boot line ───────────────────────────────────────────────────────── + +/** + * One line naming the rule in force, for the boot log: which peers may + * forward, how many hops are read, and what is wrong with a value that + * cannot be used. + */ +export function describeProxyTrust(): { + level: "info" | "error"; + message: string; +} { + const hopsRaw = process.env.TRUST_PROXY_HOPS; + const hops = parseTrustProxyHops(hopsRaw); + const list = parseTrustedProxyCidrs(process.env.TRUSTED_PROXY_CIDRS); + const problems: string[] = []; + if (hops === null) { + problems.push( + `TRUST_PROXY_HOPS=${JSON.stringify(hopsRaw)} is not a non-negative integer, so no client address is read from forwarding headers`, + ); + } + if (list.kind === "invalid") { + problems.push( + `TRUSTED_PROXY_CIDRS has entries that are not CIDRs (${list.invalid.map((e) => JSON.stringify(e)).join(", ")}), so no connection may report a client address`, + ); + } + const peers = + list.kind === "list" + ? `only connections from ${list.entries.join(", ")} (TRUSTED_PROXY_CIDRS)` + : list.kind === "unset" + ? "connections from any loopback, private, CGNAT, unique-local or link-local address (TRUSTED_PROXY_CIDRS unset; behind Docker Desktop, OrbStack or docker-proxy that includes every caller of a published port)" + : "no connection"; + if (problems.length > 0) { + return { + level: "error", + message: `[proxy-trust] ${problems.join("; ")}. Fix the value; see docs/self-hosting/reverse-proxy.md.`, + }; + } + return { + level: "info", + message: + hops === 0 + ? "[proxy-trust] TRUST_PROXY_HOPS=0: forwarding headers are never read; the client address is the connection's own." + : `[proxy-trust] forwarding headers are read from ${peers}, ${hops} hop${hops === 1 ? "" : "s"} deep (TRUST_PROXY_HOPS).`, + }; +} + +/** + * Log the rule in force once, at boot (`src/instrumentation.ts`). The + * logger is a parameter, as in the readiness summary: the production build + * strips direct `console.log` calls. + */ +export function logProxyTrustAtBoot( + log: Pick = console, +): void { + const { level, message } = describeProxyTrust(); + if (level === "error") log.error(message); + else log.log(message); +}