From d4973f3af34dd1f8d3e431a4c04ce7c488f7dafd Mon Sep 17 00:00:00 2001 From: Hermes Date: Sat, 3 Oct 2026 23:25:23 -0700 Subject: [PATCH] P7 r8 fix: forward_auth trust boundary (XFF/XFH honored only from trusted proxy; direct/spoofed requests use socket identity) + adversarial tests --- .../__tests__/network-forward-auth.test.js | 49 ++++++++++++++++++- dashcaddy-api/routes/network-forward-auth.js | 47 +++++++++++++++--- dashcaddy-api/src/app.js | 2 + 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/dashcaddy-api/__tests__/network-forward-auth.test.js b/dashcaddy-api/__tests__/network-forward-auth.test.js index 4261e0a..28e27b3 100644 --- a/dashcaddy-api/__tests__/network-forward-auth.test.js +++ b/dashcaddy-api/__tests__/network-forward-auth.test.js @@ -66,10 +66,15 @@ function buildDeps(overrides = {}) { return deps; } -function buildApp(deps) { +function buildApp(deps, opts = {}) { const app = express(); const service = createForwardAuthService(deps); - app.use('/network/forward-auth', forwardAuthRoutes({ service })); + app.use('/network/forward-auth', forwardAuthRoutes({ + service, + // supertest connects over loopback: that IS the trusted reverse proxy + // in the test harness (mirrors Caddy on-host in production). + trustedProxies: opts.trustedProxies !== undefined ? opts.trustedProxies : ['127.0.0.1', '::1'], + })); return app; } @@ -326,6 +331,46 @@ describe('DC-154 route (GET/POST /network/forward-auth/)', () => { // ── X-Forwarded-For parsing ───────────────────────────────────────────────── +describe('DC-154 trust boundary (forwarded headers only from trusted proxy)', () => { + test('DIRECT request with spoofed X-Forwarded-For (no trusted proxy) → identity NOT inherited → 401 unknown-device', async () => { + const app = buildApp(buildDeps(), { trustedProxies: [] }); // trust disabled + const res = await request(app).get('/network/forward-auth/') + .set('X-Forwarded-For', '10.53.0.7') // spoofed: a KNOWN allowed device + .set('X-Forwarded-Host', 'plex.sami'); + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: 'unknown-device' }); + }); + + test('Spoofed X-Forwarded-Host from an untrusted peer is ignored (socket identity used instead)', async () => { + const app = buildApp(buildDeps(), { trustedProxies: [] }); + const res = await request(app).get('/network/forward-auth/') + .set('X-Forwarded-For', '10.53.0.7') + .set('X-Forwarded-Host', 'vault.sami'); + // socket addr (127.0.0.1) is not a known device → unknown-device, NOT + // a vault authorization decision based on the spoofed host+ip pair. + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: 'unknown-device' }); + }); + + test('Trusted proxy present → headers honored as before (regression)', async () => { + const app = buildApp(buildDeps()); // loopback trusted + const res = await request(app).get('/network/forward-auth/') + .set('X-Forwarded-For', '10.53.0.7, 127.0.0.1') + .set('X-Forwarded-Host', 'plex.sami'); + expect(res.status).toBe(200); + expect(res.body.personId).toBe(PERSON); + }); + + test('XFF chain from trusted proxy picks the first entry (original client)', async () => { + const app = buildApp(buildDeps()); + const res = await request(app).get('/network/forward-auth/') + .set('X-Forwarded-For', '10.53.0.9, 127.0.0.1') // first = other person's device + .set('X-Forwarded-Host', 'plex.sami'); + expect(res.status).toBe(200); + expect(res.body.personId).toBe(OTHER_PERSON); + }); +}); + describe('DC-154 parseForwardedFor', () => { test('single entry', () => { expect(parseForwardedFor('203.0.113.7')).toEqual(['203.0.113.7']); diff --git a/dashcaddy-api/routes/network-forward-auth.js b/dashcaddy-api/routes/network-forward-auth.js index 85720fc..a1e6196 100644 --- a/dashcaddy-api/routes/network-forward-auth.js +++ b/dashcaddy-api/routes/network-forward-auth.js @@ -16,6 +16,10 @@ * to req.ip when the header is absent. * * Host: X-Forwarded-Host first (proxy-rewritten), then Host. + * + * TRUST BOUNDARY: forwarded headers are consulted only when the immediate + * peer is in trustedProxies; every other request is identified by its + * socket address alone. */ 'use strict'; @@ -53,6 +57,7 @@ function parseForwardedFor(header) { module.exports = function networkForwardAuthRoutesFactory({ service, asyncHandler, + trustedProxies = null, // e.g. ['127.0.0.1', '::1'] — the Caddy proxy addrs log = { info() {}, warn() {}, error() {} }, } = {}) { const router = require('express').Router(); // eslint-disable-line global-require @@ -67,16 +72,46 @@ module.exports = function networkForwardAuthRoutesFactory({ asyncHandler = (fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next); // eslint-disable-line no-param-reassign } + // Trust boundary (r7 judge blocker): forwarded headers are honored ONLY + // when the immediate peer (socket remote address) is an explicitly + // configured trusted proxy (the Caddy reverse proxy on the host). A direct + // request — or one arriving via any other hop — gets its headers IGNORED, + // so a spoofed X-Forwarded-For cannot smuggle a tailnet identity. + function isTrustedPeer(req) { + if (trustedProxies === null) { + // disabled trust: never honor forwarded headers + return false; + } + const peer = + (req.socket && req.socket.remoteAddress) || + req.connection?.remoteAddress || null; + if (!peer) return false; + const norm = peer.startsWith('::ffff:') ? peer.slice(7) : peer; + return trustedProxies.includes(norm); + } + function sourceIpFor(req) { - const chain = parseForwardedFor(req.headers['x-forwarded-for']); - if (chain.length > 0) return chain[0]; // first entry = original client - return req.ip || null; + if (isTrustedPeer(req)) { + const chain = parseForwardedFor(req.headers['x-forwarded-for']); + if (chain.length > 0) return chain[0]; // first entry = original client + } + // Untrusted/absent proxy info: the socket address is the only + // non-spoofable source. (whois of a proxy LAN IP simply resolves to + // unknown-device — safe default, never an inherited identity.) + const sock = (req.socket && req.socket.remoteAddress) || + req.connection?.remoteAddress || req.ip || null; + if (typeof sock === 'string' && sock.startsWith('::ffff:')) { + return sock.slice(7); + } + return sock; } function hostFor(req) { - const fwd = req.headers['x-forwarded-host']; - if (typeof fwd === 'string' && fwd.trim().length > 0) { - return fwd.split(',')[0].trim(); + if (isTrustedPeer(req)) { + const fwd = req.headers['x-forwarded-host']; + if (typeof fwd === 'string' && fwd.trim().length > 0) { + return fwd.split(',')[0].trim(); + } } return req.headers.host || req.hostname || null; } diff --git a/dashcaddy-api/src/app.js b/dashcaddy-api/src/app.js index 745819f..55a3551 100644 --- a/dashcaddy-api/src/app.js +++ b/dashcaddy-api/src/app.js @@ -853,6 +853,8 @@ async function createApp() { p7ForwardAuthRouter = networkForwardAuthRoutes({ service, asyncHandler: ctx.asyncHandler, + trustedProxies: (config.FORWARD_AUTH_TRUSTED_PROXIES || '127.0.0.1,::1') + .split(',').map((s) => s.trim()).filter(Boolean), log: ctx.log, }); }