mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-04 06:12:10 +03:00
fix(security): don't trust loopback socket as local when behind reverse proxy (#4632)
Integrated into release/v3.8.36 — port rebuilt clean, release-green
This commit is contained in:
committed by
GitHub
parent
ec1c5df091
commit
486f01710c
@@ -20,6 +20,20 @@ import { randomUUID } from "node:crypto";
|
||||
*/
|
||||
export const PEER_IP_HEADER = "x-omniroute-peer-ip";
|
||||
|
||||
/**
|
||||
* Companion header to PEER_IP_HEADER: `<token>|1` when the inbound TCP request
|
||||
* carried forwarding headers (`x-forwarded-for` / `x-real-ip`), `<token>|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).
|
||||
*
|
||||
* 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";
|
||||
|
||||
/** Generate (once) and return the per-process stamp token, persisting it in env
|
||||
* so the middleware running in the same process reads the identical value. */
|
||||
export function ensurePeerStampToken() {
|
||||
@@ -27,17 +41,26 @@ export function ensurePeerStampToken() {
|
||||
return process.env.OMNIROUTE_PEER_STAMP_TOKEN;
|
||||
}
|
||||
|
||||
/** Strip any client-supplied PEER_IP_HEADER and stamp the real TCP peer IP,
|
||||
* token-prefixed. Never throws — a stamping failure must not block a request
|
||||
* (it degrades to "locality unknown" → fail closed in the middleware). */
|
||||
/** 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
|
||||
* unknown" → fail closed in the middleware). */
|
||||
export function stampPeerIp(req) {
|
||||
try {
|
||||
if (!req || !req.headers) return;
|
||||
// Node lowercases incoming header names; delete kills any client value.
|
||||
delete req.headers[PEER_IP_HEADER];
|
||||
delete req.headers[VIA_PROXY_HEADER];
|
||||
const ip = req.socket && req.socket.remoteAddress;
|
||||
if (ip) {
|
||||
req.headers[PEER_IP_HEADER] = `${ensurePeerStampToken()}|${ip}`;
|
||||
const token = ensurePeerStampToken();
|
||||
req.headers[PEER_IP_HEADER] = `${token}|${ip}`;
|
||||
// Forwarding headers present = request arrived via a reverse proxy; the
|
||||
// loopback socket is the proxy hop, not the end-user, so it must not be
|
||||
// trusted as local. Token-prefix the marker so a remote caller cannot
|
||||
// forge it (or its absence) on a non-proxied request.
|
||||
const viaProxy = !!(req.headers["x-forwarded-for"] || req.headers["x-real-ip"]);
|
||||
req.headers[VIA_PROXY_HEADER] = `${token}|${viaProxy ? "1" : "0"}`;
|
||||
}
|
||||
} catch {
|
||||
/* never block a request on peer stamping */
|
||||
|
||||
@@ -37,6 +37,22 @@ export const CLI_TOKEN_HEADER = "x-omniroute-cli-token";
|
||||
*/
|
||||
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 `<token>|1` when the
|
||||
* inbound TCP request carried forwarding headers (`x-forwarded-for` /
|
||||
* `x-real-ip`) and `<token>|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
|
||||
* otherwise let a leaked JWT over a public tunnel reach LOCAL_ONLY routes
|
||||
* that spawn child processes. Token-validated like PEER_IP_HEADER, so a
|
||||
* remote caller cannot forge it. Stripped from forwarded headers before
|
||||
* route handlers see it.
|
||||
* Keep in sync with VIA_PROXY_HEADER in scripts/dev/peer-stamp.mjs.
|
||||
*/
|
||||
export const VIA_PROXY_HEADER = "x-omniroute-via-proxy";
|
||||
|
||||
/**
|
||||
* Trusted locality verdict ("loopback" | "lan" | "remote") that the pipeline
|
||||
* computes from the stamped real peer IP and forwards to route handlers. Route
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { timingSafeEqual } from "node:crypto";
|
||||
import { classifyHostLocality } from "./routeGuard";
|
||||
|
||||
/**
|
||||
* Resolve the real peer IP from the trusted `<token>|<ip>` stamp that the custom
|
||||
@@ -32,3 +33,67 @@ export function resolveStampedPeer(
|
||||
}
|
||||
return ip;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 `<token>|1` when forwarding headers (`x-forwarded-for` / `x-real-ip`) were
|
||||
* present on the inbound TCP request, and `<token>|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
|
||||
* token, "0", junk — returns false (the safe default: assume no proxy hop).
|
||||
*
|
||||
* SECURITY: paired with `resolveStampedPeer()` to close the upstream
|
||||
* decolua/9router da667836 vulnerability — when OmniRoute itself runs behind
|
||||
* an external reverse proxy (nginx / Caddy / Cloudflare Tunnel),
|
||||
* `req.socket.remoteAddress` is the proxy hop (usually 127.0.0.1), not the
|
||||
* end-user. Without this signal, `classifyHostLocality()` would return
|
||||
* "loopback" for every remote caller arriving via the proxy, granting access
|
||||
* to the LOCAL_ONLY tier that gates spawn-capable routes (Hard Rules #15 +
|
||||
* #17). Consume via `classifyStampedPeerLocality()` below.
|
||||
*/
|
||||
export function resolveStampedViaProxy(
|
||||
headerValue: string | null,
|
||||
token: string | undefined
|
||||
): boolean {
|
||||
if (!headerValue || !token) return false;
|
||||
const sep = headerValue.indexOf("|");
|
||||
if (sep <= 0) return false;
|
||||
const provided = headerValue.slice(0, sep);
|
||||
const payload = headerValue.slice(sep + 1);
|
||||
if (provided.length !== token.length) return false;
|
||||
try {
|
||||
if (!timingSafeEqual(Buffer.from(provided), Buffer.from(token))) return false;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
return payload === "1";
|
||||
}
|
||||
|
||||
/**
|
||||
* The trusted locality verdict consumed by the LOCAL_ONLY route guard. Wraps
|
||||
* `resolveStampedPeer()` + `resolveStampedViaProxy()` + `classifyHostLocality()`
|
||||
* so the pipeline has a single boundary helper:
|
||||
*
|
||||
* 1. Resolve the real peer IP from PEER_IP_HEADER (or fail closed → remote).
|
||||
* 2. If the via-proxy marker is present, the loopback / private-LAN socket
|
||||
* is the proxy hop, not the end-user — downgrade to "remote".
|
||||
* (Public-IP sockets are already remote, so the marker is a no-op there.)
|
||||
* 3. Otherwise classify the raw IP normally (loopback / lan / remote).
|
||||
*
|
||||
* Pure; both header values are token-validated, so an attacker who knows the
|
||||
* header names but not the per-process token cannot influence the verdict in
|
||||
* any direction.
|
||||
*/
|
||||
export function classifyStampedPeerLocality(
|
||||
peerHeader: string | null,
|
||||
viaProxyHeader: string | null,
|
||||
token: string | undefined
|
||||
): "loopback" | "lan" | "remote" {
|
||||
const ip = resolveStampedPeer(peerHeader, token);
|
||||
const viaProxy = resolveStampedViaProxy(viaProxyHeader, token);
|
||||
const locality = classifyHostLocality(ip);
|
||||
if (viaProxy && locality !== "remote") return "remote";
|
||||
return locality;
|
||||
}
|
||||
|
||||
@@ -6,8 +6,7 @@ import { checkBodySize, getBodySizeLimit } from "../../shared/middleware/bodySiz
|
||||
import { generateRequestId } from "../../shared/utils/requestId";
|
||||
import { applyCorsHeaders } from "../cors/origins";
|
||||
import { classifyRoute } from "./classify";
|
||||
import { classifyHostLocality } from "./routeGuard";
|
||||
import { resolveStampedPeer } from "./peerStamp";
|
||||
import { classifyStampedPeerLocality } from "./peerStamp";
|
||||
import { clientApiPolicy } from "./policies/clientApi";
|
||||
import { managementPolicy } from "./policies/management";
|
||||
import { publicPolicy } from "./policies/public";
|
||||
@@ -21,6 +20,7 @@ import {
|
||||
AUTHZ_HEADER_ROUTE_CLASS,
|
||||
AUTHZ_TRUSTED_HEADERS,
|
||||
PEER_IP_HEADER,
|
||||
VIA_PROXY_HEADER,
|
||||
} from "./headers";
|
||||
import type { AuthSubject, RouteClass, RouteClassification } from "./types";
|
||||
import type { AuthOutcome, RoutePolicy } from "./context";
|
||||
@@ -232,21 +232,29 @@ export async function runAuthzPipeline(
|
||||
for (const trusted of AUTHZ_TRUSTED_HEADERS) {
|
||||
requestHeaders.delete(trusted);
|
||||
}
|
||||
// The trusted peer-IP stamp is read by the policy from the ORIGINAL request
|
||||
// (above); strip it from the forwarded headers so the per-process token never
|
||||
// reaches route handlers or upstream providers.
|
||||
// The trusted peer-IP + via-proxy stamps are read by the policy from the
|
||||
// ORIGINAL request (above); strip them from the forwarded headers so the
|
||||
// per-process token never reaches route handlers or upstream providers.
|
||||
requestHeaders.delete(PEER_IP_HEADER);
|
||||
requestHeaders.delete(VIA_PROXY_HEADER);
|
||||
|
||||
requestHeaders.set(AUTHZ_HEADER_ROUTE_CLASS, classification.routeClass);
|
||||
requestHeaders.set(AUTHZ_HEADER_REQUEST_ID, requestId);
|
||||
// Stamp a trusted, non-secret locality verdict derived from the real stamped
|
||||
// peer IP. Route handlers (e.g. cliTokenAuth) read this instead of re-deriving
|
||||
// locality from the spoofable Host header. The client-supplied value (if any)
|
||||
// was already removed by the AUTHZ_TRUSTED_HEADERS strip above.
|
||||
// peer IP AND the via-proxy marker. Route handlers (e.g. cliTokenAuth) read
|
||||
// this instead of re-deriving locality from the spoofable Host header. The
|
||||
// client-supplied values (if any) were already removed by the
|
||||
// AUTHZ_TRUSTED_HEADERS strip above. When the via-proxy marker is set, a
|
||||
// loopback socket is the proxy hop, not the end-user — verdict is downgraded
|
||||
// to "remote" so the LOCAL_ONLY gate is not bypassed by a request arriving
|
||||
// through an external reverse proxy (nginx / Caddy / Cloudflare Tunnel).
|
||||
// See peerStamp.ts and the upstream da667836 reference for the full rationale.
|
||||
requestHeaders.set(
|
||||
AUTHZ_HEADER_PEER_LOCALITY,
|
||||
classifyHostLocality(
|
||||
resolveStampedPeer(request.headers.get(PEER_IP_HEADER), process.env.OMNIROUTE_PEER_STAMP_TOKEN)
|
||||
classifyStampedPeerLocality(
|
||||
request.headers.get(PEER_IP_HEADER),
|
||||
request.headers.get(VIA_PROXY_HEADER),
|
||||
process.env.OMNIROUTE_PEER_STAMP_TOKEN
|
||||
)
|
||||
);
|
||||
|
||||
|
||||
@@ -8,8 +8,8 @@ import { extractApiKey, isValidApiKey } from "../../../sse/services/auth";
|
||||
import { getApiKeyMetadata } from "../../../lib/db/apiKeys";
|
||||
import { hasManageScope } from "../../../lib/api/requireManagementAuth";
|
||||
import { evaluateAccessTokenAuth } from "../accessTokenAuth";
|
||||
import { CLI_TOKEN_HEADER, PEER_IP_HEADER } from "../headers";
|
||||
import { resolveStampedPeer } from "../peerStamp";
|
||||
import { CLI_TOKEN_HEADER, PEER_IP_HEADER, VIA_PROXY_HEADER } from "../headers";
|
||||
import { resolveStampedPeer, resolveStampedViaProxy } from "../peerStamp";
|
||||
import {
|
||||
isAlwaysProtectedPath,
|
||||
isLocalOnlyBypassableByManageScope,
|
||||
@@ -35,15 +35,34 @@ function requestPeerAddress(ctx: PolicyContext): string | null {
|
||||
return ctx.request.ip ?? ctx.request.socket?.remoteAddress ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the inbound TCP request carried forwarding headers
|
||||
* (`x-forwarded-for` / `x-real-ip`), as stamped by the custom Node server. When
|
||||
* set, the socket peer is the reverse-proxy hop, not the end-user — so a
|
||||
* loopback / private-LAN socket must NOT be trusted as local (Hard Rules #15 +
|
||||
* #17, port of decolua/9router da667836). Token-validated; an attacker who
|
||||
* knows the header name but not the per-process token cannot influence it.
|
||||
*/
|
||||
function isViaProxyRequest(ctx: PolicyContext): boolean {
|
||||
return resolveStampedViaProxy(
|
||||
ctx.request.headers?.get?.(VIA_PROXY_HEADER) ?? null,
|
||||
process.env.OMNIROUTE_PEER_STAMP_TOKEN
|
||||
);
|
||||
}
|
||||
|
||||
function isLoopbackRequest(ctx: PolicyContext): boolean {
|
||||
if (isViaProxyRequest(ctx)) return false;
|
||||
const peerAddress = requestPeerAddress(ctx);
|
||||
return peerAddress ? isLoopbackHost(peerAddress) : false;
|
||||
}
|
||||
|
||||
// Owner-authorized (2026-05-30): allow LOCAL_ONLY *paths* from a trusted private
|
||||
// LAN, based on the real socket peer IP (not spoofable). Does NOT relax the
|
||||
// CLI-token gate, which stays strictly loopback.
|
||||
// CLI-token gate, which stays strictly loopback. Also falls back to "not LAN"
|
||||
// when a reverse-proxy hop is detected (the apparent LAN IP would be the proxy,
|
||||
// not the end-user — see isViaProxyRequest above).
|
||||
function isPrivateLanRequest(ctx: PolicyContext): boolean {
|
||||
if (isViaProxyRequest(ctx)) return false;
|
||||
const peerAddress = requestPeerAddress(ctx);
|
||||
return peerAddress ? isPrivateLanHost(peerAddress) : false;
|
||||
}
|
||||
|
||||
112
tests/unit/route-guard-loopback-via-proxy.test.ts
Normal file
112
tests/unit/route-guard-loopback-via-proxy.test.ts
Normal file
@@ -0,0 +1,112 @@
|
||||
/**
|
||||
* Security regression: when OmniRoute itself runs behind an external reverse
|
||||
* proxy (nginx / Caddy / Cloudflare Tunnel), `req.socket.remoteAddress` is the
|
||||
* proxy hop — usually 127.0.0.1 — not the real end-user.
|
||||
*
|
||||
* Previously, the custom server stamped the loopback socket as the trusted
|
||||
* peer IP, so `classifyHostLocality()` returned "loopback" for every remote
|
||||
* caller arriving via the proxy → the LOCAL_ONLY route guard (which gates
|
||||
* spawn-capable routes like `/api/cli-tools/runtime/*`, `/api/services/*`,
|
||||
* `/api/plugins/*`, `/api/system/version`) was effectively bypassed. A leaked
|
||||
* JWT over the public tunnel could trigger child-process spawning.
|
||||
*
|
||||
* Fix (mirrors upstream decolua/9router commit da667836): the custom server
|
||||
* detects forwarding headers (`x-forwarded-for` / `x-real-ip`) and stamps a
|
||||
* token-protected `via-proxy` marker. When the marker is present, locality
|
||||
* derived from a loopback socket is downgraded to "remote" (fail closed).
|
||||
*
|
||||
* Hard Rules #15, #17 + Rule #18 (TDD before fix).
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
resolveStampedPeer,
|
||||
resolveStampedViaProxy,
|
||||
classifyStampedPeerLocality,
|
||||
} from "../../src/server/authz/peerStamp.ts";
|
||||
|
||||
const TOK = "process-secret-token-abc";
|
||||
|
||||
test("resolveStampedViaProxy: returns true only for the correctly-tokened stamp", () => {
|
||||
assert.equal(resolveStampedViaProxy(`${TOK}|1`, TOK), true);
|
||||
assert.equal(resolveStampedViaProxy(`${TOK}|0`, TOK), false);
|
||||
assert.equal(resolveStampedViaProxy(null, TOK), false);
|
||||
assert.equal(resolveStampedViaProxy("", TOK), false);
|
||||
});
|
||||
|
||||
test("resolveStampedViaProxy: rejects forged / un-tokened values", () => {
|
||||
assert.equal(resolveStampedViaProxy("1", TOK), false, "raw client value (no token)");
|
||||
assert.equal(resolveStampedViaProxy("wrong-token|1", TOK), false, "forged token");
|
||||
assert.equal(resolveStampedViaProxy(`${TOK}|1`, undefined), false, "no process token");
|
||||
});
|
||||
|
||||
test("classifyStampedPeerLocality: loopback socket WITHOUT a proxy stamp stays loopback", () => {
|
||||
// Local CLI / dashboard hit — the normal happy path.
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|127.0.0.1`, null, TOK),
|
||||
"loopback"
|
||||
);
|
||||
assert.equal(classifyStampedPeerLocality(`${TOK}|::1`, null, TOK), "loopback");
|
||||
});
|
||||
|
||||
test("classifyStampedPeerLocality: loopback socket WITH a proxy stamp is REMOTE (fail closed)", () => {
|
||||
// OmniRoute is behind nginx/Caddy/Cloudflare; the socket peer is the proxy.
|
||||
// The real end-user is somewhere on the public internet → must not be trusted
|
||||
// as local, otherwise the LOCAL_ONLY spawn-capable surface is reachable from
|
||||
// a tunnel.
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|127.0.0.1`, `${TOK}|1`, TOK),
|
||||
"remote",
|
||||
"loopback socket + via-proxy stamp must NOT be classified as local"
|
||||
);
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|::1`, `${TOK}|1`, TOK),
|
||||
"remote"
|
||||
);
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|::ffff:127.0.0.1`, `${TOK}|1`, TOK),
|
||||
"remote"
|
||||
);
|
||||
});
|
||||
|
||||
test("classifyStampedPeerLocality: private-LAN socket WITH a proxy stamp is still REMOTE", () => {
|
||||
// Caddy/nginx running on a LAN box in front of OmniRoute. We do not know how
|
||||
// the proxy is exposed (it could be tunneled to the public internet), so any
|
||||
// proxy hop downgrades locality to remote.
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|192.168.0.15`, `${TOK}|1`, TOK),
|
||||
"remote"
|
||||
);
|
||||
});
|
||||
|
||||
test("classifyStampedPeerLocality: public-IP socket is remote regardless of stamp", () => {
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|8.8.8.8`, null, TOK),
|
||||
"remote"
|
||||
);
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|8.8.8.8`, `${TOK}|1`, TOK),
|
||||
"remote"
|
||||
);
|
||||
});
|
||||
|
||||
test("classifyStampedPeerLocality: missing / forged peer stamp fails closed to remote", () => {
|
||||
assert.equal(classifyStampedPeerLocality(null, null, TOK), "remote");
|
||||
assert.equal(classifyStampedPeerLocality("forged|127.0.0.1", null, TOK), "remote");
|
||||
});
|
||||
|
||||
test("classifyStampedPeerLocality: untrusted (un-tokened) via-proxy header is ignored", () => {
|
||||
// A remote attacker who guesses the via-proxy header name but cannot mint the
|
||||
// token must NOT be able to flip the locality verdict by themselves; the
|
||||
// safety direction is OK (downgrade), but the inverse — pretending no proxy
|
||||
// exists when one does — would only be exploitable if the attacker controlled
|
||||
// BOTH headers, which the token gate prevents. We assert the bypass attempt
|
||||
// (un-tokened via-proxy hint) does not leak into the verdict for the normal
|
||||
// local-CLI case.
|
||||
assert.equal(
|
||||
classifyStampedPeerLocality(`${TOK}|127.0.0.1`, "1", TOK),
|
||||
"loopback",
|
||||
"un-tokened via-proxy hint is ignored (== false) — local CLI keeps loopback"
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user