diff --git a/.env.example b/.env.example index 15c41eae..d350f60b 100644 --- a/.env.example +++ b/.env.example @@ -74,6 +74,20 @@ BCRYPT_COST_FACTOR=12 # ----------------------------------------------------------------------------- # Proxy / Gateway # ----------------------------------------------------------------------------- +# Number of trusted reverse-proxy hops in front of the app. Controls how the +# client IP is derived from the X-Forwarded-For header (Express "trust proxy" +# semantics). +# +# TRUST_PROXY_HOPS=0 (default) — ignore X-Forwarded-For entirely and use the +# socket address. Safe when the app is directly +# exposed to clients. +# TRUST_PROXY_HOPS=1 — one trusted proxy; the client IP is taken one entry +# from the right of X-Forwarded-For (the value appended +# by that proxy). Leftmost entries are client-controlled +# and are never trusted. +# TRUST_PROXY_HOPS=2 — two trusted proxies (e.g. CDN + load balancer), etc. +# +# TRUST_PROXY_HOPS=0 # Comma-separated allowlist of upstream hosts the gateway may proxy to. # REQUIRED in production — startup fails if this variable is missing or empty. # In development/test, localhost and loopback entries are permitted by default. diff --git a/FORWARDED_HEADER_POLICY.md b/FORWARDED_HEADER_POLICY.md index 7aee328b..1dd324e6 100644 --- a/FORWARDED_HEADER_POLICY.md +++ b/FORWARDED_HEADER_POLICY.md @@ -75,6 +75,22 @@ Header stripping is performed case-insensitively. All header name variations (e. - Request IDs are included in error responses for debugging - UUID v4 format ensures global uniqueness +## Client IP Trust Boundary + +When the service sits behind one or more reverse proxies, client IP resolution follows Express's `trust proxy` semantics: + +- **No trust (default)**: all forwarded headers are ignored and the direct socket address is used. This is spoof-proof. +- **Hop count**: the client address is taken N entries from the right of the forwarded chain. With one trusted hop, `X-Forwarded-For: 1.1.1.1, 2.2.2.2` yields `2.2.2.2`. The leftmost entry is fully client-controlled and must not be trusted. +- **Trust all** (`true`): legacy behaviour that trusts every hop. Only use this when every proxy in the chain is controlled by the operator. + +### Configuration + +- `TRUST_PROXY_HEADERS=true`: trust all hops (legacy). +- `TRUST_PROXY_HOPS=N`: trust the last N hops. Takes precedence over `TRUST_PROXY_HEADERS` when set to a positive integer. +- Unset: no trust; the socket address is used. + +The IP-allowlist middleware and the request logger both call the same helper in `src/lib/clientIp.ts`, so the trust boundary is applied consistently across the stack. + ## Implementation Details The header policy is implemented in `src/routes/proxyRoutes.ts`: @@ -94,6 +110,8 @@ const DEFAULT_STRIP_HEADERS = [ ]; ``` +`x-forwarded-for` and `x-real-ip` are also stripped before forwarding to upstream services. + Headers are processed case-insensitively using lowercase comparison: ```typescript @@ -113,5 +131,6 @@ Comprehensive tests verify: - Case-insensitive header stripping works - Response headers are filtered appropriately - Request ID correlation is maintained +- Client IP resolution honours the trusted hop count and falls back to the socket address -See `src/__tests__/proxy.integration.test.ts` for detailed test coverage. +See `src/__tests__/proxy.integration.test.ts` and `src/lib/__tests__/clientIp.test.ts` for detailed test coverage. diff --git a/src/__tests__/ipAllowlist.test.ts b/src/__tests__/ipAllowlist.test.ts index 7a933726..a209a909 100644 --- a/src/__tests__/ipAllowlist.test.ts +++ b/src/__tests__/ipAllowlist.test.ts @@ -397,7 +397,7 @@ describe('IP Allowlist Middleware', () => { expect(mockLogger.info).toHaveBeenCalledWith( { allowedRangesCount: 1, - trustProxy: true, + trustProxyHops: Number.POSITIVE_INFINITY, proxyHeaders: expect.any(Array), enabled: true, }, diff --git a/src/config/env.ts b/src/config/env.ts index 95523758..6c534b9b 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -79,6 +79,30 @@ export const envSchema = z JWT_SECRET: z.string().min(1, "JWT_SECRET is required"), ADMIN_API_KEY: z.string().min(1, "ADMIN_API_KEY is required"), METRICS_API_KEY: z.string().min(1, "METRICS_API_KEY is required"), + /** + * TRUST_PROXY_HOPS — number of trusted reverse-proxy hops in front of the + * application. When greater than zero, the client IP is derived from the + * X-Forwarded-For header by selecting the entry that many positions from + * the right (matching Express `trust proxy` semantics). When zero (the + * default), the socket address is used and X-Forwarded-For is ignored. + * + * Example: TRUST_PROXY_HOPS=1 with "X-Forwarded-For: 1.1.1.1, 2.2.2.2" + * yields 2.2.2.2 (the rightmost entry, i.e. the address appended by the + * single trusted proxy). The leftmost entry is fully client-controlled + * and must never be trusted. + */ + TRUST_PROXY_HOPS: z.coerce.number().int().min(0).default(0), + /** + * TRUST_PROXY_HEADERS — legacy boolean flag. Retained for backwards + * compatibility: when true and TRUST_PROXY_HOPS is unset/zero, it is + * treated as a single trusted hop. New deployments should prefer + * TRUST_PROXY_HOPS. + */ + TRUST_PROXY_HEADERS: z + .string() + .optional() + .transform((v) => v === "true") + .default(false), TRUST_FORWARDED_USER_ID: z .string() .optional() diff --git a/src/lib/__tests__/clientIp.test.ts b/src/lib/__tests__/clientIp.test.ts index eab579cd..5113c484 100644 --- a/src/lib/__tests__/clientIp.test.ts +++ b/src/lib/__tests__/clientIp.test.ts @@ -45,7 +45,36 @@ describe('getClientIp', () => { assert.equal(getClientIp(req, false), '1.2.3.4'); }); - test('uses x-forwarded-for leftmost IP when trustProxy is true', () => { + test('defaults to socket address when trustProxy is omitted', () => { + const req = makeReq({ + headers: { 'x-forwarded-for': '9.9.9.9' }, + socket: { remoteAddress: '1.2.3.4' } as never, + }); + assert.equal(getClientIp(req), '1.2.3.4'); + }); + + test('with one trusted hop, selects the rightmost forwarded entry', () => { + const req = makeReq({ + headers: { 'x-forwarded-for': '1.1.1.1, 2.2.2.2' }, + }); + assert.equal(getClientIp(req, 1), '2.2.2.2'); + }); + + test('with two trusted hops, selects the entry two from the right', () => { + const req = makeReq({ + headers: { 'x-forwarded-for': '1.1.1.1, 2.2.2.2, 3.3.3.3' }, + }); + assert.equal(getClientIp(req, 2), '2.2.2.2'); + }); + + test('spoofed leftmost entries cannot influence the result', () => { + const req = makeReq({ + headers: { 'x-forwarded-for': '10.0.0.1, 10.0.0.2, 2.2.2.2' }, + }); + assert.equal(getClientIp(req, 1), '2.2.2.2'); + }); + + test('trustProxy true trusts all hops and uses leftmost entry', () => { const req = makeReq({ headers: { 'x-forwarded-for': '5.5.5.5, 10.0.0.1, 172.16.0.1' }, }); @@ -57,7 +86,22 @@ describe('getClientIp', () => { headers: { 'x-forwarded-for': 'not-an-ip' }, socket: { remoteAddress: '1.2.3.4' } as never, }); - assert.equal(getClientIp(req, true), '1.2.3.4'); + assert.equal(getClientIp(req, 1), '1.2.3.4'); + }); + + test('falls back to socket when hop count exceeds chain length and leftmost is invalid', () => { + const req = makeReq({ + headers: { 'x-forwarded-for': 'not-an-ip, 2.2.2.2' }, + socket: { remoteAddress: '1.2.3.4' } as never, + }); + assert.equal(getClientIp(req, 5), '1.2.3.4'); + }); + + test('uses leftmost entry when hop count exceeds chain length', () => { + const req = makeReq({ + headers: { 'x-forwarded-for': '2.2.2.2, 3.3.3.3' }, + }); + assert.equal(getClientIp(req, 5), '2.2.2.2'); }); test('falls back to req.ip when socket is absent', () => { @@ -75,16 +119,16 @@ describe('getClientIp', () => { const reqBoth = makeReq({ headers: { 'x-forwarded-for': '5.5.5.5', 'x-real-ip': '6.6.6.6' }, }); - assert.equal(getClientIp(reqBoth, true), '5.5.5.5'); + assert.equal(getClientIp(reqBoth, 1), '5.5.5.5'); // Only x-real-ip present const reqReal = makeReq({ headers: { 'x-real-ip': '6.6.6.6' } }); - assert.equal(getClientIp(reqReal, true), '6.6.6.6'); + assert.equal(getClientIp(reqReal, 1), '6.6.6.6'); }); test('accepts custom proxy header list', () => { const req = makeReq({ headers: { 'x-custom-ip': '7.7.7.7' } }); - assert.equal(getClientIp(req, true, ['x-custom-ip']), '7.7.7.7'); + assert.equal(getClientIp(req, 1, ['x-custom-ip']), '7.7.7.7'); }); test('DEFAULT_PROXY_HEADERS includes x-forwarded-for', () => { diff --git a/src/lib/clientIp.ts b/src/lib/clientIp.ts index 45d26f61..a0ef53b4 100644 --- a/src/lib/clientIp.ts +++ b/src/lib/clientIp.ts @@ -1,5 +1,7 @@ import type { Request } from 'express'; +export type TrustProxyOption = boolean | number; + /** * Proxy headers checked when trustProxy is enabled, ordered by reliability. * The same list is used by the IP-allowlist middleware and the request logger @@ -25,33 +27,64 @@ export function isValidIp(ip: string): boolean { /** * Extracts the real client IP from an Express request. * - * When `trustProxy` is false (the default) the direct socket address is - * returned, making IP spoofing via headers impossible. + * Trust semantics follow Express' `trust proxy` model: + * - `false` (default): all forwarded headers are ignored and the direct + * socket address is returned, making header spoofing impossible. + * - `true`: trust all hops (equivalent to a hop count of `Infinity`). + * - `number N >= 1`: trust the last N hops. The client address is taken + * N entries from the right of the forwarded chain. With one trusted hop, + * `'1.1.1.1, 2.2.2.2'` yields `2.2.2.2`. This prevents a client from + * spoofing the leftmost entry to bypass the admin IP-allowlist or per-IP + * rate limits. * - * When `trustProxy` is true the proxy headers listed in `proxyHeaders` are - * consulted in order; the first valid IP wins. For `x-forwarded-for` only - * the leftmost entry is used because that is the original client address — - * subsequent entries are added by intermediary proxies and must not be trusted - * as the client origin. + * Because the client address is selected from the right of the chain, + * spoofed leftmost entries cannot influence the result as long as the + * configured hop count matches the actual number of trusted proxies. * * @param req Express request object - * @param trustProxy Whether to honour proxy forwarding headers + * @param trustProxy False (no trust), true (trust all), or a hop count >= 1 * @param proxyHeaders Ordered list of headers to inspect (defaults to {@link DEFAULT_PROXY_HEADERS}) */ export function getClientIp( req: Request, - trustProxy = false, + trustProxy: TrustProxyOption = false, proxyHeaders: readonly string[] = DEFAULT_PROXY_HEADERS, ): string { - if (trustProxy) { + const hops = normalizeTrustProxy(trustProxy); + + if (hops > 0) { for (const header of proxyHeaders) { const value = req.headers[header.toLowerCase()]; - if (typeof value === 'string' && value.trim()) { - const firstIp = value.split(',')[0].trim(); - if (isValidIp(firstIp)) return firstIp; - } + if (typeof value !== 'string' || !value.trim()) continue; + + const entries = value + .split(',') + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); + + if (entries.length === 0) continue; + + // Select the entry `hops` positions from the right. When the chain is + // shorter than the configured hop count, the leftmost entry is the + // best available candidate. + const index = Math.max(0, entries.length - hops); + const candidate = entries[index]; + if (candidate && isValidIp(candidate)) return candidate; } } return req.ip ?? req.socket?.remoteAddress ?? ''; } + +/** + * Normalises the `trustProxy` option into a non-negative hop count. + * `false` -> 0, `true` -> Infinity, and any number >= 1 -> that number. + */ +function normalizeTrustProxy(trustProxy: TrustProxyOption): number { + if (trustProxy === true) return Number.POSITIVE_INFINITY; + if (trustProxy === false) return 0; + if (typeof trustProxy === 'number' && Number.isFinite(trustProxy) && trustProxy >= 1) { + return Math.floor(trustProxy); + } + return 0; +} diff --git a/src/middleware/ipAllowlist.ts b/src/middleware/ipAllowlist.ts index 45cab559..75af16de 100644 --- a/src/middleware/ipAllowlist.ts +++ b/src/middleware/ipAllowlist.ts @@ -10,13 +10,23 @@ export interface IpAllowlistConfig { /** List of allowed IP ranges in CIDR notation */ allowedRanges: string[]; /** - * Whether to trust proxy headers for IP resolution. + * Number of trusted reverse-proxy hops ahead of the application. + * + * The client IP is resolved from the right side of the forwarded chain using + * this hop count (Express `trust proxy` semantics), so client-injected + * leftmost entries cannot satisfy the allowlist. `0` (the default) means no + * trust: the direct socket address is used and forwarded headers are ignored. * - * Security note: set this to `true` only when the service sits behind a - * trusted reverse proxy that you control. When `false` (the default) the - * direct socket address is used, making header-spoofing impossible. * See FORWARDED_HEADER_POLICY.md for the full trust-boundary policy. */ + trustProxyHops?: number; + /** + * Legacy boolean trust flag, kept for backwards compatibility. + * + * `true` trusts all hops (equivalent to `trustProxyHops: Infinity`, i.e. the + * leftmost forwarded entry); `false` means no trust. Prefer + * `trustProxyHops` for new call sites. + */ trustProxy?: boolean; /** Custom proxy headers to check (in order of priority) */ proxyHeaders?: string[]; @@ -28,13 +38,16 @@ export interface IpAllowlistConfig { * Creates IP allowlist middleware for protecting sensitive endpoints. * * IP resolution follows the trust-boundary policy in FORWARDED_HEADER_POLICY.md: - * - When trustProxy is false, the direct socket address is used (spoof-proof). - * - When trustProxy is true, only the leftmost entry of X-Forwarded-For is - * used, as subsequent entries are added by intermediary proxies. + * - With `trustProxyHops` 0 (the default) the direct socket address is used + * (spoof-proof); forwarded headers are ignored. + * - With `trustProxyHops` N >= 1 the N-th entry from the right of + * `X-Forwarded-For` is used, aligning with Express `trust proxy` semantics. + * - The legacy `trustProxy: true` flag still trusts all hops. */ export function createIpAllowlist(config: IpAllowlistConfig) { const { allowedRanges, + trustProxyHops, trustProxy = false, proxyHeaders = DEFAULT_PROXY_HEADERS, enabled = true, @@ -44,10 +57,21 @@ export function createIpAllowlist(config: IpAllowlistConfig) { throw new Error('IP allowlist must have at least one allowed range'); } + if ( + trustProxyHops !== undefined && + (!Number.isInteger(trustProxyHops) || trustProxyHops < 0) + ) { + throw new Error('trustProxyHops must be a non-negative integer'); + } + + // A numeric hop count takes precedence; otherwise fall back to the legacy + // boolean so existing callers keep their previous semantics. + const trustOption = trustProxyHops !== undefined ? trustProxyHops : trustProxy; + logger.info( { allowedRangesCount: allowedRanges.length, - trustProxy, + trustProxyHops: trustProxyHops ?? (trustProxy ? Number.POSITIVE_INFINITY : 0), proxyHeaders, enabled, }, @@ -60,9 +84,9 @@ export function createIpAllowlist(config: IpAllowlistConfig) { return; } - // Resolve client IP per trust-boundary policy: when trustProxy is false - // getClientIp returns req.ip (socket address), ignoring all forwarded headers. - const clientIp = getClientIp(req, trustProxy, proxyHeaders); + // Resolve client IP per trust-boundary policy: when trust is disabled + // getClientIp returns req.ip (socket address), ignoring forwarded headers. + const clientIp = getClientIp(req, trustOption, proxyHeaders); if (!isValidIp(clientIp)) { logger.warn( @@ -111,13 +135,39 @@ export function createIpAllowlist(config: IpAllowlistConfig) { }; } +/** + * Resolve the configured number of trusted proxy hops from the environment. + * + * `TRUST_PROXY_HOPS` (a non-negative integer) takes precedence. The legacy + * boolean `TRUST_PROXY_HEADERS` is kept for backwards compatibility and, when + * true and no hop count is set, is treated as a single trusted hop (the + * spoofable leftmost entry is no longer trusted). Otherwise 0 (no trust). + */ +function resolveTrustProxyHops(): number { + const rawHops = process.env.TRUST_PROXY_HOPS; + if (rawHops !== undefined && rawHops !== '') { + const parsed = Number(rawHops); + if (Number.isInteger(parsed) && parsed >= 0) { + return parsed; + } + logger.warn( + { rawHops }, + 'Invalid TRUST_PROXY_HOPS value; falling back to default', + ); + } + if (process.env.TRUST_PROXY_HEADERS === 'true') { + return 1; + } + return 0; +} + /** * Pre-configured IP allowlist for admin endpoints. * Uses environment variables for configuration. */ export function createAdminIpAllowlist() { const allowedRanges = process.env.ADMIN_IP_ALLOWED_RANGES?.split(',').map(r => r.trim()) ?? []; - const trustProxy = process.env.TRUST_PROXY_HEADERS === 'true'; + const trustProxyHops = resolveTrustProxyHops(); const enabled = process.env.ADMIN_IP_ALLOWLIST_ENABLED !== 'false'; if (allowedRanges.length === 0) { @@ -125,7 +175,7 @@ export function createAdminIpAllowlist() { return (_req: Request, _res: Response, next: NextFunction): void => next(); } - return createIpAllowlist({ allowedRanges, trustProxy, enabled }); + return createIpAllowlist({ allowedRanges, trustProxyHops, enabled }); } /** @@ -134,7 +184,7 @@ export function createAdminIpAllowlist() { */ export function createGatewayIpAllowlist() { const allowedRanges = process.env.GATEWAY_IP_ALLOWED_RANGES?.split(',').map(r => r.trim()) ?? []; - const trustProxy = process.env.TRUST_PROXY_HEADERS === 'true'; + const trustProxyHops = resolveTrustProxyHops(); const enabled = process.env.GATEWAY_IP_ALLOWLIST_ENABLED !== 'false'; if (allowedRanges.length === 0) { @@ -142,5 +192,5 @@ export function createGatewayIpAllowlist() { return (_req: Request, _res: Response, next: NextFunction): void => next(); } - return createIpAllowlist({ allowedRanges, trustProxy, enabled }); + return createIpAllowlist({ allowedRanges, trustProxyHops, enabled }); }