diff --git a/.env.example b/.env.example index 201a60d3793..c2996f0a133 100644 --- a/.env.example +++ b/.env.example @@ -749,6 +749,13 @@ NEXT_PUBLIC_CLOUD_URL= # Values: true/loopback (trust loopback proxy peers), private/lan (also trust LAN peers). # OMNIROUTE_TRUST_PROXY= +# Proxy addresses whose X-Forwarded-For / X-Real-IP the IP allow/deny list may believe, on top +# of loopback, private-network addresses and Cloudflare edges, which are always trusted. Needed +# only when the reverse proxy in front of OmniRoute has any other (public) address; without +# it that proxy's own address is what the filter judges. Comma-separated IPs or CIDR ranges. +# Used by: scripts/dev/peer-stamp.mjs. +# OMNIROUTE_TRUSTED_PROXIES=198.51.100.7,203.0.113.0/24 + # Public callback URL for asynchronous image/audio jobs (kie.ai, etc.). # Used by: open-sse/utils/kieTask.ts — overrides callbackUrlFromBaseUrl(). # Honor order: KIE_CALLBACK_URL → OMNIROUTE_KIE_CALLBACK_URL → OMNIROUTE_PUBLIC_URL. diff --git a/changelog.d/fixes/ip-filter-forged-forwarding-headers.md b/changelog.d/fixes/ip-filter-forged-forwarding-headers.md new file mode 100644 index 00000000000..ae76878d82b --- /dev/null +++ b/changelog.d/fixes/ip-filter-forged-forwarding-headers.md @@ -0,0 +1 @@ +- **fix(authz):** the IP allow/deny list judges the address of the connection unless a loopback, private-network or Cloudflare proxy fronts the request, and behind one it reads the client from what that proxy added rather than from what the client sent; a proxy on any other public address is named in the new `OMNIROUTE_TRUSTED_PROXIES` diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index ff6c7a7b962..b1e19730fdd 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -63,6 +63,7 @@ These **must** be set before the first run. Without them, the application will e | `SOURCE_VERSION` | No | _(unset)_ | `next.config.mjs`, `scripts/build/assembleStandalone.mjs` | Second in the chain — set by PaaS builders (e.g. Heroku-style) as the deployed commit. | | `NEXT_PUBLIC_SW_BUILD_ID` | No | _(derived)_ | `src/shared/components/PwaRegister.tsx` | Build-time public value the client uses to register `/sw.js?v=…`; derived from the two above, then the git SHA. | | `OMNIROUTE_PEER_STAMP_TOKEN` | No (auto) | _(auto per boot)_ | `src/server/authz/policies/management.ts` | Per-process secret proving the trusted peer-IP stamp came from OmniRoute's own HTTP server (`scripts/dev/peer-stamp.mjs`). The authz middleware trusts request locality (loopback/LAN gating of LOCAL_ONLY routes) only when the stamp carries this token. Auto-generated each boot — leave unset; only pin it for multi-process setups that must share the stamp. | +| `OMNIROUTE_TRUSTED_PROXIES` | No | _(unset)_ | `scripts/dev/peer-stamp.mjs` | Comma-separated IPs or CIDR ranges of reverse proxies whose `X-Forwarded-For` / `X-Real-IP` the IP allow/deny list may believe, on top of loopback, private-network addresses and Cloudflare edges, which are always trusted. Needed only for a proxy on any other public address; otherwise that proxy's own address is what the filter judges. | ### Generation Commands diff --git a/scripts/dev/peer-stamp.mjs b/scripts/dev/peer-stamp.mjs index ddac032ad3d..25fdf5949c9 100644 --- a/scripts/dev/peer-stamp.mjs +++ b/scripts/dev/peer-stamp.mjs @@ -1,4 +1,4 @@ -import { isIPv4, isIPv6 } from "node:net"; +import { isIP, isIPv4, isIPv6 } from "node:net"; import { randomUUID } from "node:crypto"; /** @@ -23,19 +23,33 @@ export const PEER_IP_HEADER = "x-omniroute-peer-ip"; /** * Companion header to PEER_IP_HEADER: `|1` when the inbound TCP request - * carried forwarding headers (`x-forwarded-for` / `x-real-ip`) or arrived from - * a Cloudflare edge IP with `cf-connecting-ip`, `|0` otherwise. Required - * so the middleware can tell that a loopback socket is the reverse-proxy hop - * (nginx / Caddy / Cloudflare Tunnel) and NOT trust it as local — without this, - * a leaked JWT over a public tunnel would reach the LOCAL_ONLY routes that - * spawn child processes (Hard Rules #15 + #17; port of upstream decolua/9router - * commit da667836). + * came from a peer that may be a reverse proxy (this host, a private-network + * address, a Cloudflare edge or an address named in OMNIROUTE_TRUSTED_PROXIES) + * and carried forwarding headers (`x-forwarded-for` / `x-real-ip`), or arrived + * from a Cloudflare edge IP with `cf-connecting-ip`; `|0` otherwise. + * Required so the middleware can tell that a loopback socket is the + * reverse-proxy hop (nginx / Caddy / Cloudflare Tunnel) and NOT trust it as + * local — without this, a leaked JWT over a public tunnel would reach the + * LOCAL_ONLY routes that spawn child processes (Hard Rules #15 + #17; port of + * upstream decolua/9router commit da667836). A peer on any other address can + * write forwarding headers itself, so from it they do not set the marker. * * Keep VIA_PROXY_HEADER in sync with VIA_PROXY_HEADER in * src/server/authz/headers.ts (the TS side cannot import this .mjs). */ export const VIA_PROXY_HEADER = "x-omniroute-via-proxy"; +/** + * The address the IP allow/deny list should judge, as `|`: the TCP + * peer itself, or, when the peer is a proxy that may be trusted, the client it + * reports (see resolveClientIp). Derived here, where the socket is known, so the + * middleware never has to choose between forwarding headers a client can write. + * + * Keep CLIENT_IP_HEADER in sync with CLIENT_IP_HEADER in + * src/server/authz/headers.ts (the TS side cannot import this .mjs). + */ +export const CLIENT_IP_HEADER = "x-omniroute-client-ip"; + /** * Cloudflare IPv4 ranges used to authenticate the `cf-connecting-ip` header. * @@ -171,9 +185,112 @@ export function isCloudflareIP(ip) { return false; } -/** Strip any client-supplied PEER_IP_HEADER + VIA_PROXY_HEADER and stamp the - * real TCP peer IP plus a token-protected via-proxy marker. Never throws — a - * stamping failure must not block a request (it degrades to "locality +// Same ranges as PRIVATE_LAN_PATTERNS in src/server/authz/routeGuard.ts +// (tests/unit/authz/peer-stamp.test.ts checks they agree). +const PRIVATE_LAN_PATTERNS = [ + /^10\.\d{1,3}\.\d{1,3}\.\d{1,3}$/, + /^100\.(6[4-9]|[78]\d|9\d|1[01]\d|12[0-7])\.\d{1,3}\.\d{1,3}$/, + /^192\.168\.\d{1,3}\.\d{1,3}$/, + /^172\.(1[6-9]|2\d|3[01])\.\d{1,3}\.\d{1,3}$/, + /^f[cd][0-9a-f]{2}:/i, + /^fe80:/i, +]; + +/** + * A plain IP address from a header value, or null. Accepts the forms proxies write: + * `::ffff:a.b.c.d`, `a.b.c.d:port` and `[v6]:port`. + */ +function normalizeIp(value) { + if (typeof value !== "string") return null; + let candidate = value.trim().replace(/^::ffff:/i, ""); + if (isIP(candidate)) return candidate; + const bracketed = /^\[([^\]]+)\](?::\d+)?$/.exec(candidate); + if (bracketed) candidate = bracketed[1].replace(/^::ffff:/i, ""); + else { + const withPort = /^(\d{1,3}(?:\.\d{1,3}){3}):\d+$/.exec(candidate); + if (withPort) candidate = withPort[1]; + } + return isIP(candidate) ? candidate : null; +} + +let configuredProxiesSource; +let configuredProxies = []; + +/** Addresses and CIDR ranges the operator names in OMNIROUTE_TRUSTED_PROXIES. */ +function getConfiguredProxies() { + const source = process.env.OMNIROUTE_TRUSTED_PROXIES || ""; + if (source === configuredProxiesSource) return configuredProxies; + configuredProxiesSource = source; + configuredProxies = []; + for (const part of source.split(",")) { + const [address, bits] = part.trim().split("/"); + const ip = normalizeIp(address); + if (!ip) continue; + const max = isIPv4(ip) ? 32 : 128; + const prefix = bits === undefined ? max : Number(bits); + if (!Number.isInteger(prefix) || prefix < 0 || prefix > max) continue; + configuredProxies.push({ ip, cidr: `${ip}/${prefix}` }); + } + return configuredProxies; +} + +function isConfiguredProxy(ip) { + return getConfiguredProxies().some((entry) => + isIPv4(ip) && isIPv4(entry.ip) + ? matchesIPv4Cidr(ip, entry.cidr) + : isIPv6(ip) && isIPv6(entry.ip) && matchesIPv6Cidr(ip, entry.cidr) + ); +} + +/** + * True when a forwarding header from this TCP peer may be believed: the peer is this host, a + * private-network proxy, a Cloudflare edge, or an address the operator lists in + * OMNIROUTE_TRUSTED_PROXIES (a proxy on any other public address has to be named there). A + * client on any other address can write those headers itself, so they say nothing about who + * it is. + */ +export function isTrustedProxyPeer(ip) { + const normalized = normalizeIp(ip); + if (!normalized) return false; + if (normalized === "::1" || normalized.startsWith("127.")) return true; + if (PRIVATE_LAN_PATTERNS.some((re) => re.test(normalized))) return true; + return isCloudflareIP(normalized) || isConfiguredProxy(normalized); +} + +/** + * The client address to judge for a request from `peerIp`. A peer that is not a trusted proxy + * is judged as itself. Behind a trusted proxy the client is what that proxy reported, read the + * way a proxy chain is built: a Cloudflare edge's `cf-connecting-ip`, else the right-most + * `x-forwarded-for` entry that is not itself a trusted proxy (the left-most entries are + * whatever the client sent), else `x-real-ip`. When nothing usable was reported the peer is + * judged. + */ +export function resolveClientIp(headers, peerIp) { + const peer = normalizeIp(peerIp); + if (!peer) return null; + if (!isTrustedProxyPeer(peer)) return peer; + + if (isCloudflareIP(peer)) { + const edgeClient = normalizeIp(String(headers["cf-connecting-ip"] || "").split(",")[0]); + if (edgeClient) return edgeClient; + } + + const chain = String(headers["x-forwarded-for"] || "").split(","); + let innermostProxy = null; + for (let index = chain.length - 1; index >= 0; index -= 1) { + const hop = normalizeIp(chain[index]); + if (!hop) break; + if (!isTrustedProxyPeer(hop)) return hop; + innermostProxy = hop; + } + if (innermostProxy) return innermostProxy; + + return normalizeIp(String(headers["x-real-ip"] || "").split(",")[0]) || peer; +} + +/** Strip any client-supplied PEER_IP_HEADER, VIA_PROXY_HEADER and CLIENT_IP_HEADER and stamp + * the real TCP peer IP, a token-protected via-proxy marker and the client address to judge. + * Never throws — a stamping failure must not block a request (it degrades to "locality * unknown" → fail closed in the middleware). */ export function stampPeerIp(req) { try { @@ -181,6 +298,7 @@ export function stampPeerIp(req) { // Node lowercases incoming header names; delete kills any client value. delete req.headers[PEER_IP_HEADER]; delete req.headers[VIA_PROXY_HEADER]; + delete req.headers[CLIENT_IP_HEADER]; const ip = req.socket && req.socket.remoteAddress; if (ip) { const token = ensurePeerStampToken(); @@ -190,14 +308,21 @@ export function stampPeerIp(req) { // trusted as local. Token-prefix the marker so a remote caller cannot // forge it (or its absence) on a non-proxied request. // + // x-forwarded-for / x-real-ip only count from a peer that may be a proxy: from + // any other peer they are just client-supplied text, and must not flip the marker + // and make the IP filter judge the header instead of the peer. + // // `cf-connecting-ip` is Cloudflare-specific and trivially forged by a // direct client. Only treat it as a proxy marker when the TCP peer itself // is a Cloudflare edge IP; otherwise a direct forger could flip the // via-proxy bit and force the middleware to ignore the real peer IP. - const hasGenericProxyHeaders = !!(req.headers["x-forwarded-for"] || req.headers["x-real-ip"]); - const hasCloudflareHeader = !!(req.headers["cf-connecting-ip"] && isCloudflareIP(ip)); - const viaProxy = hasGenericProxyHeaders || hasCloudflareHeader; + const hasForwardingHeaders = !!(req.headers["x-forwarded-for"] || req.headers["x-real-ip"]); + const viaProxy = + (hasForwardingHeaders && isTrustedProxyPeer(ip)) || + (!!req.headers["cf-connecting-ip"] && isCloudflareIP(ip)); req.headers[VIA_PROXY_HEADER] = `${token}|${viaProxy ? "1" : "0"}`; + const clientIp = viaProxy ? resolveClientIp(req.headers, ip) : normalizeIp(ip); + if (clientIp) req.headers[CLIENT_IP_HEADER] = `${token}|${clientIp}`; } } catch { /* never block a request on peer stamping */ diff --git a/src/server/authz/headers.ts b/src/server/authz/headers.ts index 2bbf95bf616..cb6d155813c 100644 --- a/src/server/authz/headers.ts +++ b/src/server/authz/headers.ts @@ -50,8 +50,11 @@ export const PEER_IP_HEADER = "x-omniroute-peer-ip"; /** * Trusted "request arrived via a reverse proxy" marker stamped by the custom * Node server alongside PEER_IP_HEADER, formatted as `|1` when the - * inbound TCP request carried forwarding headers (`x-forwarded-for` / - * `x-real-ip`) and `|0` otherwise. The middleware combines this with + * inbound TCP request came from a peer that may be a proxy (this host, a + * private-network address, a Cloudflare edge or an address named in + * OMNIROUTE_TRUSTED_PROXIES) and carried forwarding headers (`x-forwarded-for` / + * `x-real-ip`), or came from a Cloudflare edge with `cf-connecting-ip`, and + * `|0` otherwise. The middleware combines this with * the stamped peer IP so a loopback / private-LAN socket that is actually the * proxy hop (e.g. OmniRoute behind nginx / Caddy / Cloudflare Tunnel) is NOT * trusted as local — closing the upstream da667836 vulnerability that would @@ -63,6 +66,14 @@ export const PEER_IP_HEADER = "x-omniroute-peer-ip"; */ export const VIA_PROXY_HEADER = "x-omniroute-via-proxy"; +/** + * The address the IP allow/deny list judges, stamped by the custom Node server as + * `|`: the TCP peer, or the client a trusted proxy reported for it. Token-validated + * like PEER_IP_HEADER and stripped from forwarded headers in pipeline.ts. + * Keep in sync with CLIENT_IP_HEADER in scripts/dev/peer-stamp.mjs. + */ +export const CLIENT_IP_HEADER = "x-omniroute-client-ip"; + /** * Trusted locality verdict ("loopback" | "lan" | "remote") that the pipeline * computes from the stamped real peer IP and forwards to route handlers. Route diff --git a/src/server/authz/peerStamp.ts b/src/server/authz/peerStamp.ts index ba80df01896..92067d65773 100644 --- a/src/server/authz/peerStamp.ts +++ b/src/server/authz/peerStamp.ts @@ -38,7 +38,8 @@ export function resolveStampedPeer( * Resolve the trusted "request arrived via a reverse proxy" marker stamped by * the custom Node server (`scripts/dev/peer-stamp.mjs::stampPeerIp`). The stamp * is `|1` when forwarding headers (`x-forwarded-for` / `x-real-ip`) were - * present on the inbound TCP request, and `|0` otherwise. + * present on an inbound TCP request from a peer that may be a proxy (loopback, private + * network, Cloudflare edge or a configured trusted proxy), and `|0` otherwise. * * Returns true ONLY when the token constant-time-matches this process's stamp * token AND the payload is exactly "1". Any other value — no stamp, forged diff --git a/src/server/authz/pipeline.ts b/src/server/authz/pipeline.ts index c9e73c1630a..92cad186db1 100644 --- a/src/server/authz/pipeline.ts +++ b/src/server/authz/pipeline.ts @@ -35,6 +35,7 @@ import { CLI_TOKEN_HEADER, PEER_IP_HEADER, VIA_PROXY_HEADER, + CLIENT_IP_HEADER, } from "./headers"; import type { AuthSubject, RouteClass, RouteClassification } from "./types"; import type { AuthOutcome, RoutePolicy } from "./context"; @@ -315,6 +316,7 @@ export async function runAuthzPipeline( // per-process token never reaches route handlers or upstream providers. requestHeaders.delete(PEER_IP_HEADER); requestHeaders.delete(VIA_PROXY_HEADER); + requestHeaders.delete(CLIENT_IP_HEADER); requestHeaders.set(AUTHZ_HEADER_ROUTE_CLASS, classification.routeClass); requestHeaders.set(AUTHZ_HEADER_REQUEST_ID, requestId); @@ -385,7 +387,13 @@ export async function runAuthzPipeline( request.headers.get(VIA_PROXY_HEADER), process.env.OMNIROUTE_PEER_STAMP_TOKEN ); - const ipVerdict = checkRequestIP(request, viaProxy ? null : trustedPeerIp); + // The server stamps the client address to judge (the peer, or what a trusted proxy + // reported for it). Requests without that stamp keep the earlier rule. + const stampedClientIp = resolveStampedPeer( + request.headers.get(CLIENT_IP_HEADER), + process.env.OMNIROUTE_PEER_STAMP_TOKEN + ); + const ipVerdict = checkRequestIP(request, stampedClientIp ?? (viaProxy ? null : trustedPeerIp)); if (!ipVerdict.allowed) { const blocked = NextResponse.json( { error: ipVerdict.reason || "Access denied" }, diff --git a/tests/unit/authz/ip-filter-forwarded-headers.test.ts b/tests/unit/authz/ip-filter-forwarded-headers.test.ts new file mode 100644 index 00000000000..950df35545e --- /dev/null +++ b/tests/unit/authz/ip-filter-forwarded-headers.test.ts @@ -0,0 +1,211 @@ +// The IP allow/deny list has to judge the address of the connection unless a proxy that may be +// trusted sits in front: a loopback or private-network proxy, a Cloudflare edge, or an address +// the operator names in OMNIROUTE_TRUSTED_PROXIES. A direct client on a public address can put +// any value in X-Forwarded-For, X-Real-IP or CF-Connecting-IP, so those headers must not change +// who the filter thinks it is talking to; behind a trusted proxy, only what the proxy itself +// added counts, not whatever the client sent ahead of it. +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { NextRequest } from "next/server"; +import { wrapRequestListenerWithPeerStamp } from "../../../scripts/dev/peer-stamp.mjs"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ipfilter-fwd-")); +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.JWT_SECRET = "test-secret-ipfilter-forwarded"; + +const core = await import("../../../src/lib/db/core.ts"); +const ipFilter = await import("../../../open-sse/services/ipFilter.ts"); +const pipeline = await import("../../../src/server/authz/pipeline.ts"); + +const ORIGINAL_STAMP_TOKEN = process.env.OMNIROUTE_PEER_STAMP_TOKEN; +const ORIGINAL_TRUSTED_PROXIES = process.env.OMNIROUTE_TRUSTED_PROXIES; + +function restoreEnv(name: string, value: string | undefined) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; +} + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + restoreEnv("DATA_DIR", ORIGINAL_DATA_DIR); + restoreEnv("OMNIROUTE_PEER_STAMP_TOKEN", ORIGINAL_STAMP_TOKEN); + restoreEnv("OMNIROUTE_TRUSTED_PROXIES", ORIGINAL_TRUSTED_PROXIES); +}); + +test.beforeEach(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + ipFilter.resetIPFilter(); + process.env.OMNIROUTE_PEER_STAMP_TOKEN = "stamp-tok"; + delete process.env.OMNIROUTE_TRUSTED_PROXIES; +}); + +const BLOCKED = "203.0.113.9"; +const ALLOWED = "203.0.113.10"; + +// What the custom server does before the request reaches the pipeline: stamp the socket peer. +function stampedRequest(peer: string, headers: Record) { + const stamped: Record = {}; + wrapRequestListenerWithPeerStamp((req: { headers: Record }) => { + Object.assign(stamped, req.headers); + })({ headers: { ...headers }, socket: { remoteAddress: peer } } as never, {} as never); + return new NextRequest("http://localhost/v1/models", { headers: stamped }); +} + +async function blockedByIpFilter(request: NextRequest): Promise { + const res = await pipeline.runAuthzPipeline(request, { enforce: true }); + if (res.status !== 403) return false; + const body = (await res.clone().json()) as { error?: unknown }; + return typeof body.error === "string" && /blacklist|not in whitelist|banned/i.test(body.error); +} + +function blacklist(...ips: string[]) { + ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" }); + for (const ip of ips) ipFilter.addToBlacklist(ip); +} + +function whitelist(...ips: string[]) { + ipFilter.configureIPFilter({ enabled: true, mode: "whitelist" }); + for (const ip of ips) ipFilter.addToWhitelist(ip); +} + +test("a direct client cannot dodge the blacklist with a forged forwarding header", async () => { + blacklist(BLOCKED); + + for (const forged of [ + { "x-forwarded-for": ALLOWED }, + { "x-real-ip": ALLOWED }, + { "cf-connecting-ip": ALLOWED }, + ]) { + assert.equal( + await blockedByIpFilter(stampedRequest(BLOCKED, forged)), + true, + `forged ${Object.keys(forged)[0]}` + ); + } +}); + +test("a direct client cannot pass the whitelist with a forged forwarding header", async () => { + whitelist(ALLOWED); + + assert.equal( + await blockedByIpFilter(stampedRequest(BLOCKED, { "x-forwarded-for": ALLOWED })), + true + ); +}); + +test("a loopback proxy is still trusted to report the client address", async () => { + blacklist(BLOCKED); + + assert.equal( + await blockedByIpFilter(stampedRequest("127.0.0.1", { "x-forwarded-for": BLOCKED })), + true + ); + assert.equal( + await blockedByIpFilter(stampedRequest("127.0.0.1", { "x-forwarded-for": ALLOWED })), + false + ); +}); + +test("a proxy on a private network is still trusted to report the client address", async () => { + blacklist(BLOCKED); + + for (const proxy of ["10.1.2.3", "172.18.0.2", "192.168.1.5", "100.64.0.9"]) { + assert.equal( + await blockedByIpFilter(stampedRequest(proxy, { "x-forwarded-for": BLOCKED })), + true, + proxy + ); + } + + ipFilter.resetIPFilter(); + whitelist(ALLOWED); + assert.equal( + await blockedByIpFilter(stampedRequest("10.1.2.3", { "x-forwarded-for": ALLOWED })), + false + ); +}); + +test("a Cloudflare edge is trusted to report the client address", async () => { + blacklist(BLOCKED); + + assert.equal( + await blockedByIpFilter(stampedRequest("172.71.150.1", { "cf-connecting-ip": BLOCKED })), + true + ); + assert.equal( + await blockedByIpFilter(stampedRequest("172.71.150.1", { "cf-connecting-ip": ALLOWED })), + false + ); +}); + +test("behind a proxy that appends to X-Forwarded-For, an address the client put first is ignored", async () => { + blacklist(BLOCKED); + + // nginx: proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for + assert.equal( + await blockedByIpFilter( + stampedRequest("127.0.0.1", { "x-forwarded-for": `${ALLOWED}, ${BLOCKED}` }) + ), + true + ); + assert.equal( + await blockedByIpFilter( + stampedRequest("127.0.0.1", { "x-forwarded-for": `${BLOCKED}, ${ALLOWED}` }) + ), + false + ); +}); + +test("a forged CF-Connecting-IP behind a proxy that is not Cloudflare is ignored", async () => { + blacklist(BLOCKED); + + assert.equal( + await blockedByIpFilter( + stampedRequest("127.0.0.1", { "x-forwarded-for": BLOCKED, "cf-connecting-ip": ALLOWED }) + ), + true + ); + + ipFilter.resetIPFilter(); + whitelist(ALLOWED); + assert.equal( + await blockedByIpFilter( + stampedRequest("127.0.0.1", { "x-forwarded-for": BLOCKED, "cf-connecting-ip": ALLOWED }) + ), + true + ); +}); + +test("a private-network client whose forwarding header holds no address is judged as itself", async () => { + blacklist("192.168.1.50"); + + for (const junk of [{ "x-forwarded-for": "x" }, { "x-real-ip": "unknown" }]) { + assert.equal( + await blockedByIpFilter(stampedRequest("192.168.1.50", junk)), + true, + Object.keys(junk)[0] + ); + } +}); + +test("a proxy on any other public address is judged as itself until the operator names it", async () => { + const proxy = "198.51.100.7"; + whitelist(ALLOWED); + const viaProxy = () => stampedRequest(proxy, { "x-forwarded-for": ALLOWED }); + + assert.equal(await blockedByIpFilter(viaProxy()), true); + + process.env.OMNIROUTE_TRUSTED_PROXIES = "198.51.100.0/24"; + assert.equal(await blockedByIpFilter(viaProxy()), false); + assert.equal( + await blockedByIpFilter(stampedRequest(proxy, { "x-forwarded-for": BLOCKED })), + true + ); +}); diff --git a/tests/unit/authz/peer-stamp.test.ts b/tests/unit/authz/peer-stamp.test.ts index e8a5585a97e..f8412ab57e4 100644 --- a/tests/unit/authz/peer-stamp.test.ts +++ b/tests/unit/authz/peer-stamp.test.ts @@ -16,9 +16,14 @@ const { matchesIPv4Cidr, matchesIPv6Cidr, isCloudflareIP, + isTrustedProxyPeer, + resolveClientIp, + CLIENT_IP_HEADER, } = peerStamp; +const { isPrivateLanHost } = await import("../../../src/server/authz/routeGuard.ts"); const ORIGINAL_STAMP_TOKEN = process.env.OMNIROUTE_PEER_STAMP_TOKEN; +const ORIGINAL_TRUSTED_PROXIES = process.env.OMNIROUTE_TRUSTED_PROXIES; function makeReq(remoteAddress: string, headers: Record = {}) { return { @@ -36,11 +41,14 @@ function getViaProxy(req: ReturnType) { } test.after(() => { + if (ORIGINAL_TRUSTED_PROXIES === undefined) delete process.env.OMNIROUTE_TRUSTED_PROXIES; + else process.env.OMNIROUTE_TRUSTED_PROXIES = ORIGINAL_TRUSTED_PROXIES; if (ORIGINAL_STAMP_TOKEN === undefined) delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; else process.env.OMNIROUTE_PEER_STAMP_TOKEN = ORIGINAL_STAMP_TOKEN; }); test.beforeEach(() => { + delete process.env.OMNIROUTE_TRUSTED_PROXIES; delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; process.env.OMNIROUTE_PEER_STAMP_TOKEN = "stamp-tok"; }); @@ -96,14 +104,32 @@ test("Cloudflare bypass guard: direct forger sending cf-connecting-ip keeps via- ); }); -test("x-forwarded-for still wins via-proxy=1 even when cf-connecting-ip is forged", () => { +test("x-forwarded-for from a public, non-proxy peer keeps via-proxy=0", () => { + // Anyone can write x-forwarded-for, so from a peer that is neither this host, a private + // network nor a Cloudflare edge it must not make the middleware trust the header over the peer. const req = makeReq("203.0.113.7", { "x-forwarded-for": "203.0.113.99", + "x-real-ip": "203.0.113.99", "cf-connecting-ip": "198.51.100.1", }); stampPeerIp(req); - assert.equal(getViaProxy(req), "stamp-tok|1", "x-forwarded-for must still set via-proxy=1"); + assert.equal(getPeerIp(req), "stamp-tok|203.0.113.7", "peer-ip must stay the real direct IP"); + assert.equal(getViaProxy(req), "stamp-tok|0", "forged forwarding headers must not set via-proxy"); +}); + +test("x-forwarded-for from a private-network proxy sets via-proxy=1", () => { + for (const peer of ["10.1.2.3", "172.18.0.2", "192.168.1.5", "100.64.0.9", "fd00::2"]) { + const req = makeReq(peer, { "x-forwarded-for": "203.0.113.99" }); + stampPeerIp(req); + assert.equal(getViaProxy(req), "stamp-tok|1", peer); + } +}); + +test("x-forwarded-for from a Cloudflare edge sets via-proxy=1", () => { + const req = makeReq("172.71.150.1", { "x-forwarded-for": "203.0.113.99" }); + stampPeerIp(req); + assert.equal(getViaProxy(req), "stamp-tok|1"); }); test("F-05: non-Cloudflare proxy (x-forwarded-for only, no cf-connecting-ip) marks via-proxy=1", () => { @@ -178,3 +204,141 @@ test("F2-03: full-form IPv4-mapped address is normalized", () => { "full-form IPv4-mapped non-Cloudflare address must not match" ); }); + +const getClient = (req: ReturnType) => req.headers[CLIENT_IP_HEADER] ?? null; + +test("the trusted-proxy address classes agree with the LAN classes the route guard uses", () => { + const samples = [ + "10.0.0.1", + "10.255.255.254", + "100.64.0.1", + "100.127.255.254", + "100.63.255.255", + "100.128.0.1", + "172.15.255.255", + "172.16.0.1", + "172.31.255.254", + "172.32.0.1", + "192.167.1.1", + "192.168.0.1", + "fc00::1", + "fd12:3456::1", + "fe80::1", + "fec0::1", + "2001:db8::1", + "8.8.8.8", + "::ffff:10.1.2.3", + "::ffff:8.8.8.8", + ]; + for (const ip of samples) { + const plain = ip.replace(/^::ffff:/i, ""); + // isTrustedProxyPeer also accepts loopback, Cloudflare and configured proxies, none of + // which are in the samples, so for these it must equal the route guard's LAN verdict. + assert.equal(isTrustedProxyPeer(ip), isPrivateLanHost(plain), ip); + } +}); + +test("a proxy on an address the operator names is trusted, one that is not named is not", () => { + const forwarded = { "x-forwarded-for": "203.0.113.99" }; + const unnamed = makeReq("198.51.100.7", forwarded); + stampPeerIp(unnamed); + assert.equal(getViaProxy(unnamed), "stamp-tok|0"); + assert.equal(getClient(unnamed), "stamp-tok|198.51.100.7"); + + process.env.OMNIROUTE_TRUSTED_PROXIES = + "192.0.2.1, 198.51.100.0/24 ,2001:db8::/32,not-an-ip,10.0.0.0/99"; + for (const peer of ["198.51.100.7", "2001:db8::42"]) { + const req = makeReq(peer, forwarded); + stampPeerIp(req); + assert.equal(getViaProxy(req), "stamp-tok|1", peer); + assert.equal(getClient(req), "stamp-tok|203.0.113.99", peer); + } + assert.equal(isTrustedProxyPeer("192.0.2.1"), true); + assert.equal(isTrustedProxyPeer("192.0.2.2"), false); + assert.equal(isTrustedProxyPeer("2001:db9::1"), false); +}); + +test("the client address stamp is the peer itself unless a trusted proxy fronts the request", () => { + const direct = makeReq("203.0.113.9", { + "x-forwarded-for": "203.0.113.10", + "x-real-ip": "203.0.113.10", + }); + stampPeerIp(direct); + assert.equal(getClient(direct), "stamp-tok|203.0.113.9"); + + const noHeaders = makeReq("::ffff:203.0.113.9"); + stampPeerIp(noHeaders); + assert.equal(getClient(noHeaders), "stamp-tok|203.0.113.9"); +}); + +test("a client-supplied client address header is replaced", () => { + const req = makeReq("203.0.113.9", { [CLIENT_IP_HEADER]: "stamp-tok|203.0.113.10" }); + stampPeerIp(req); + assert.equal(getClient(req), "stamp-tok|203.0.113.9"); +}); + +test("behind a trusted proxy the client is the right-most forwarded address that is not a proxy", () => { + // nginx appends the connecting address to whatever the client sent. + assert.equal( + resolveClientIp({ "x-forwarded-for": "203.0.113.10, 203.0.113.9" }, "127.0.0.1"), + "203.0.113.9" + ); + assert.equal( + resolveClientIp({ "x-forwarded-for": "6.6.6.6, 203.0.113.9, 10.0.0.4" }, "172.18.0.2"), + "203.0.113.9" + ); + assert.equal( + resolveClientIp({ "x-forwarded-for": "junk, 203.0.113.9" }, "127.0.0.1"), + "203.0.113.9" + ); + assert.equal( + resolveClientIp({ "x-forwarded-for": "203.0.113.9:51234" }, "127.0.0.1"), + "203.0.113.9" + ); + assert.equal( + resolveClientIp({ "x-forwarded-for": "[2001:db8::7]:443" }, "127.0.0.1"), + "2001:db8::7" + ); +}); + +test("when every forwarded address is a proxy, the outermost one is the client", () => { + assert.equal(resolveClientIp({ "x-forwarded-for": "10.1.1.5" }, "172.18.0.2"), "10.1.1.5"); + assert.equal( + resolveClientIp({ "x-forwarded-for": "192.168.1.9, 10.0.0.2" }, "127.0.0.1"), + "192.168.1.9" + ); +}); + +test("a forwarded header with nothing usable leaves the peer as the client", () => { + for (const headers of [ + { "x-forwarded-for": "x" }, + { "x-forwarded-for": "unknown" }, + { "x-real-ip": "not-an-ip" }, + {}, + ]) { + assert.equal(resolveClientIp(headers, "192.168.1.50"), "192.168.1.50", JSON.stringify(headers)); + } +}); + +test("x-real-ip is only read when there is no usable x-forwarded-for", () => { + assert.equal(resolveClientIp({ "x-real-ip": "203.0.113.9" }, "127.0.0.1"), "203.0.113.9"); + assert.equal( + resolveClientIp({ "x-real-ip": "6.6.6.6", "x-forwarded-for": "203.0.113.9" }, "127.0.0.1"), + "203.0.113.9" + ); +}); + +test("cf-connecting-ip is believed only from a Cloudflare edge", () => { + assert.equal( + resolveClientIp({ "cf-connecting-ip": "203.0.113.9" }, "172.71.150.1"), + "203.0.113.9" + ); + assert.equal( + resolveClientIp( + { "cf-connecting-ip": "6.6.6.6", "x-forwarded-for": "203.0.113.9" }, + "127.0.0.1" + ), + "203.0.113.9" + ); + assert.equal(resolveClientIp({ "cf-connecting-ip": "6.6.6.6" }, "127.0.0.1"), "127.0.0.1"); +});