fix(gateway): prevent auth bypass when behind unconfigured reverse proxy (#1795)
* fix(gateway): prevent auth bypass when behind unconfigured reverse proxy When proxy headers (X-Forwarded-For, X-Real-IP) are present but gateway.trustedProxies is not configured, the gateway now treats connections as non-local. This prevents a scenario where all proxied requests appear to come from localhost and receive automatic trust. Previously, running behind nginx/Caddy without configuring trustedProxies would cause isLocalClient=true for all external connections, potentially bypassing authentication and auto-approving device pairing. The gateway now logs a warning when this condition is detected, guiding operators to configure trustedProxies for proper client IP detection. Also adds documentation for reverse proxy security configuration. * fix: harden reverse proxy auth (#1795) (thanks @orlyjamie) --------- Co-authored-by: orlyjamie <orlyjamie@users.noreply.github.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
committed by
GitHub
parent
1c606fdb57
commit
6aec34bc60
@@ -351,6 +351,27 @@ describe("gateway server auth/connect", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects proxied connections without auth when proxy headers are untrusted", async () => {
|
||||
const prevToken = process.env.CLAWDBOT_GATEWAY_TOKEN;
|
||||
delete process.env.CLAWDBOT_GATEWAY_TOKEN;
|
||||
const port = await getFreePort();
|
||||
const server = await startGatewayServer(port);
|
||||
const ws = new WebSocket(`ws://127.0.0.1:${port}`, {
|
||||
headers: { "x-forwarded-for": "203.0.113.10" },
|
||||
});
|
||||
await new Promise<void>((resolve) => ws.once("open", resolve));
|
||||
const res = await connectReq(ws);
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.message ?? "").toContain("gateway auth required");
|
||||
ws.close();
|
||||
await server.close();
|
||||
if (prevToken === undefined) {
|
||||
delete process.env.CLAWDBOT_GATEWAY_TOKEN;
|
||||
} else {
|
||||
process.env.CLAWDBOT_GATEWAY_TOKEN = prevToken;
|
||||
}
|
||||
});
|
||||
|
||||
test("accepts device token auth for paired device", async () => {
|
||||
const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js");
|
||||
const { approveDevicePairing, getPairedDevice, listDevicePairing } =
|
||||
|
||||
@@ -26,7 +26,7 @@ import type { ResolvedGatewayAuth } from "../../auth.js";
|
||||
import { authorizeGatewayConnect } from "../../auth.js";
|
||||
import { loadConfig } from "../../../config/config.js";
|
||||
import { buildDeviceAuthPayload } from "../../device-auth.js";
|
||||
import { isLocalGatewayAddress, resolveGatewayClientIp } from "../../net.js";
|
||||
import { isLocalGatewayAddress, isTrustedProxyAddress, resolveGatewayClientIp } from "../../net.js";
|
||||
import { resolveNodeCommandAllowlist } from "../../node-command-policy.js";
|
||||
import {
|
||||
type ConnectParams,
|
||||
@@ -177,7 +177,24 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
const configSnapshot = loadConfig();
|
||||
const trustedProxies = configSnapshot.gateway?.trustedProxies ?? [];
|
||||
const clientIp = resolveGatewayClientIp({ remoteAddr, forwardedFor, realIp, trustedProxies });
|
||||
const isLocalClient = isLocalGatewayAddress(clientIp);
|
||||
|
||||
// If proxy headers are present but the remote address isn't trusted, don't treat
|
||||
// the connection as local. This prevents auth bypass when running behind a reverse
|
||||
// proxy without proper configuration - the proxy's loopback connection would otherwise
|
||||
// cause all external requests to be treated as trusted local clients.
|
||||
const hasProxyHeaders = Boolean(forwardedFor || realIp);
|
||||
const remoteIsTrustedProxy = isTrustedProxyAddress(remoteAddr, trustedProxies);
|
||||
const hasUntrustedProxyHeaders = hasProxyHeaders && !remoteIsTrustedProxy;
|
||||
const isLocalClient = !hasUntrustedProxyHeaders && isLocalGatewayAddress(clientIp);
|
||||
const reportedClientIp = hasUntrustedProxyHeaders ? undefined : clientIp;
|
||||
|
||||
if (hasUntrustedProxyHeaders) {
|
||||
logWsControl.warn(
|
||||
"Proxy headers detected from untrusted address. " +
|
||||
"Connection will not be treated as local. " +
|
||||
"Configure gateway.trustedProxies to restore local client detection behind your proxy.",
|
||||
);
|
||||
}
|
||||
|
||||
const isWebchatConnect = (p: ConnectParams | null | undefined) => isWebchatClient(p?.client);
|
||||
|
||||
@@ -322,6 +339,31 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
const isControlUi = connectParams.client.id === GATEWAY_CLIENT_IDS.CONTROL_UI;
|
||||
const allowInsecureControlUi =
|
||||
isControlUi && configSnapshot.gateway?.controlUi?.allowInsecureAuth === true;
|
||||
if (hasUntrustedProxyHeaders && resolvedAuth.mode === "none") {
|
||||
setHandshakeState("failed");
|
||||
setCloseCause("proxy-auth-required", {
|
||||
client: connectParams.client.id,
|
||||
clientDisplayName: connectParams.client.displayName,
|
||||
mode: connectParams.client.mode,
|
||||
version: connectParams.client.version,
|
||||
});
|
||||
send({
|
||||
type: "res",
|
||||
id: frame.id,
|
||||
ok: false,
|
||||
error: errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
"gateway auth required behind reverse proxy",
|
||||
{
|
||||
details: {
|
||||
hint: "set gateway.auth or configure gateway.trustedProxies",
|
||||
},
|
||||
},
|
||||
),
|
||||
});
|
||||
close(1008, "gateway auth required");
|
||||
return;
|
||||
}
|
||||
|
||||
if (!device) {
|
||||
const canSkipDevice = allowInsecureControlUi ? hasSharedAuth : hasTokenAuth;
|
||||
@@ -581,7 +623,7 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
clientMode: connectParams.client.mode,
|
||||
role,
|
||||
scopes,
|
||||
remoteIp: clientIp,
|
||||
remoteIp: reportedClientIp,
|
||||
silent: isLocalClient,
|
||||
});
|
||||
const context = buildRequestContext();
|
||||
@@ -665,7 +707,7 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
clientMode: connectParams.client.mode,
|
||||
role,
|
||||
scopes,
|
||||
remoteIp: clientIp,
|
||||
remoteIp: reportedClientIp,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -714,7 +756,7 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
if (presenceKey) {
|
||||
upsertPresence(presenceKey, {
|
||||
host: connectParams.client.displayName ?? connectParams.client.id ?? os.hostname(),
|
||||
ip: isLocalClient ? undefined : clientIp,
|
||||
ip: isLocalClient ? undefined : reportedClientIp,
|
||||
version: connectParams.client.version,
|
||||
platform: connectParams.client.platform,
|
||||
deviceFamily: connectParams.client.deviceFamily,
|
||||
@@ -773,7 +815,9 @@ export function attachGatewayWsMessageHandler(params: {
|
||||
setHandshakeState("connected");
|
||||
if (role === "node") {
|
||||
const context = buildRequestContext();
|
||||
const nodeSession = context.nodeRegistry.register(nextClient, { remoteIp: clientIp });
|
||||
const nodeSession = context.nodeRegistry.register(nextClient, {
|
||||
remoteIp: reportedClientIp,
|
||||
});
|
||||
const instanceIdRaw = connectParams.client.instanceId;
|
||||
const instanceId = typeof instanceIdRaw === "string" ? instanceIdRaw.trim() : "";
|
||||
const nodeIdsForPairing = new Set<string>([nodeSession.nodeId]);
|
||||
|
||||
@@ -53,6 +53,30 @@ describe("security audit", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("warns when loopback control UI lacks trusted proxies", async () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
gateway: {
|
||||
bind: "loopback",
|
||||
controlUi: { enabled: true },
|
||||
},
|
||||
};
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: false,
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "gateway.trusted_proxies_missing",
|
||||
severity: "warn",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("flags logging.redactSensitive=off", async () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
logging: { redactSensitive: "off" },
|
||||
|
||||
@@ -207,6 +207,10 @@ function collectGatewayConfigFindings(cfg: ClawdbotConfig): SecurityAuditFinding
|
||||
const bind = typeof cfg.gateway?.bind === "string" ? cfg.gateway.bind : "loopback";
|
||||
const tailscaleMode = cfg.gateway?.tailscale?.mode ?? "off";
|
||||
const auth = resolveGatewayAuth({ authConfig: cfg.gateway?.auth, tailscaleMode });
|
||||
const controlUiEnabled = cfg.gateway?.controlUi?.enabled !== false;
|
||||
const trustedProxies = Array.isArray(cfg.gateway?.trustedProxies)
|
||||
? cfg.gateway.trustedProxies
|
||||
: [];
|
||||
|
||||
if (bind !== "loopback" && auth.mode === "none") {
|
||||
findings.push({
|
||||
@@ -218,6 +222,20 @@ function collectGatewayConfigFindings(cfg: ClawdbotConfig): SecurityAuditFinding
|
||||
});
|
||||
}
|
||||
|
||||
if (bind === "loopback" && controlUiEnabled && trustedProxies.length === 0) {
|
||||
findings.push({
|
||||
checkId: "gateway.trusted_proxies_missing",
|
||||
severity: "warn",
|
||||
title: "Reverse proxy headers are not trusted",
|
||||
detail:
|
||||
"gateway.bind is loopback and gateway.trustedProxies is empty. " +
|
||||
"If you expose the Control UI through a reverse proxy, configure trusted proxies " +
|
||||
"so local-client checks cannot be spoofed.",
|
||||
remediation:
|
||||
"Set gateway.trustedProxies to your proxy IPs or keep the Control UI local-only.",
|
||||
});
|
||||
}
|
||||
|
||||
if (tailscaleMode === "funnel") {
|
||||
findings.push({
|
||||
checkId: "gateway.tailscale_funnel",
|
||||
|
||||
Reference in New Issue
Block a user