P7 r8 fix: forward_auth trust boundary (XFF/XFH honored only from trusted proxy; direct/spoofed requests use socket identity) + adversarial tests
This commit is contained in:
3 files changed
+90
-8
No files matched your search
@@ -66,10 +66,15 @@ function buildDeps(overrides = {}) {
|
|||||||
return deps;
|
return deps;
|
||||||
}
|
}
|
||||||
|
|
||||||
function buildApp(deps) {
|
function buildApp(deps, opts = {}) {
|
||||||
const app = express();
|
const app = express();
|
||||||
const service = createForwardAuthService(deps);
|
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;
|
return app;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -326,6 +331,46 @@ describe('DC-154 route (GET/POST /network/forward-auth/)', () => {
|
|||||||
|
|
||||||
// ── X-Forwarded-For parsing ─────────────────────────────────────────────────
|
// ── 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', () => {
|
describe('DC-154 parseForwardedFor', () => {
|
||||||
test('single entry', () => {
|
test('single entry', () => {
|
||||||
expect(parseForwardedFor('203.0.113.7')).toEqual(['203.0.113.7']);
|
expect(parseForwardedFor('203.0.113.7')).toEqual(['203.0.113.7']);
|
||||||
|
|||||||
@@ -16,6 +16,10 @@
|
|||||||
* to req.ip when the header is absent.
|
* to req.ip when the header is absent.
|
||||||
*
|
*
|
||||||
* Host: X-Forwarded-Host first (proxy-rewritten), then Host.
|
* 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';
|
'use strict';
|
||||||
@@ -53,6 +57,7 @@ function parseForwardedFor(header) {
|
|||||||
module.exports = function networkForwardAuthRoutesFactory({
|
module.exports = function networkForwardAuthRoutesFactory({
|
||||||
service,
|
service,
|
||||||
asyncHandler,
|
asyncHandler,
|
||||||
|
trustedProxies = null, // e.g. ['127.0.0.1', '::1'] — the Caddy proxy addrs
|
||||||
log = { info() {}, warn() {}, error() {} },
|
log = { info() {}, warn() {}, error() {} },
|
||||||
} = {}) {
|
} = {}) {
|
||||||
const router = require('express').Router(); // eslint-disable-line global-require
|
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
|
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) {
|
function sourceIpFor(req) {
|
||||||
const chain = parseForwardedFor(req.headers['x-forwarded-for']);
|
if (isTrustedPeer(req)) {
|
||||||
if (chain.length > 0) return chain[0]; // first entry = original client
|
const chain = parseForwardedFor(req.headers['x-forwarded-for']);
|
||||||
return req.ip || null;
|
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) {
|
function hostFor(req) {
|
||||||
const fwd = req.headers['x-forwarded-host'];
|
if (isTrustedPeer(req)) {
|
||||||
if (typeof fwd === 'string' && fwd.trim().length > 0) {
|
const fwd = req.headers['x-forwarded-host'];
|
||||||
return fwd.split(',')[0].trim();
|
if (typeof fwd === 'string' && fwd.trim().length > 0) {
|
||||||
|
return fwd.split(',')[0].trim();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return req.headers.host || req.hostname || null;
|
return req.headers.host || req.hostname || null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -853,6 +853,8 @@ async function createApp() {
|
|||||||
p7ForwardAuthRouter = networkForwardAuthRoutes({
|
p7ForwardAuthRouter = networkForwardAuthRoutes({
|
||||||
service,
|
service,
|
||||||
asyncHandler: ctx.asyncHandler,
|
asyncHandler: ctx.asyncHandler,
|
||||||
|
trustedProxies: (config.FORWARD_AUTH_TRUSTED_PROXIES || '127.0.0.1,::1')
|
||||||
|
.split(',').map((s) => s.trim()).filter(Boolean),
|
||||||
log: ctx.log,
|
log: ctx.log,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user