diff --git a/src/gateway/server.auth.browser-hardening.test.ts b/src/gateway/server.auth.browser-hardening.test.ts new file mode 100644 index 00000000000..070addbdc53 --- /dev/null +++ b/src/gateway/server.auth.browser-hardening.test.ts @@ -0,0 +1,155 @@ +import { randomUUID } from "node:crypto"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, test } from "vitest"; +import { WebSocket } from "ws"; +import { + loadOrCreateDeviceIdentity, + publicKeyRawBase64UrlFromPem, + signDevicePayload, +} from "../infra/device-identity.js"; +import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; +import { buildDeviceAuthPayload } from "./device-auth.js"; +import { + connectReq, + installGatewayTestHooks, + readConnectChallengeNonce, + testState, + trackConnectChallengeNonce, + withGatewayServer, +} from "./test-helpers.js"; + +installGatewayTestHooks({ scope: "suite" }); + +const TEST_OPERATOR_CLIENT = { + id: GATEWAY_CLIENT_NAMES.TEST, + version: "1.0.0", + platform: "test", + mode: GATEWAY_CLIENT_MODES.TEST, +}; + +const originForPort = (port: number) => `http://127.0.0.1:${port}`; + +const openWs = async (port: number, headers?: Record) => { + const ws = new WebSocket(`ws://127.0.0.1:${port}`, headers ? { headers } : undefined); + trackConnectChallengeNonce(ws); + await new Promise((resolve) => ws.once("open", resolve)); + return ws; +}; + +async function createSignedDevice(params: { + token: string; + scopes: string[]; + clientId: string; + clientMode: string; + identityPath?: string; + nonce: string; + signedAtMs?: number; +}) { + const identity = params.identityPath + ? loadOrCreateDeviceIdentity(params.identityPath) + : loadOrCreateDeviceIdentity(); + const signedAtMs = params.signedAtMs ?? Date.now(); + const payload = buildDeviceAuthPayload({ + deviceId: identity.deviceId, + clientId: params.clientId, + clientMode: params.clientMode, + role: "operator", + scopes: params.scopes, + signedAtMs, + token: params.token, + nonce: params.nonce, + }); + return { + identity, + device: { + id: identity.deviceId, + publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem), + signature: signDevicePayload(identity.privateKeyPem, payload), + signedAt: signedAtMs, + nonce: params.nonce, + }, + }; +} + +describe("gateway auth browser hardening", () => { + test("rejects non-local browser origins for non-control-ui clients", async () => { + testState.gatewayAuth = { mode: "token", token: "secret" }; + await withGatewayServer(async ({ port }) => { + const ws = await openWs(port, { origin: "https://attacker.example" }); + try { + const res = await connectReq(ws, { + token: "secret", + client: TEST_OPERATOR_CLIENT, + }); + expect(res.ok).toBe(false); + expect(res.error?.message ?? "").toContain("origin not allowed"); + } finally { + ws.close(); + } + }); + }); + + test("rate-limits browser-origin auth failures on loopback even when loopback exemption is enabled", async () => { + testState.gatewayAuth = { + mode: "token", + token: "secret", + rateLimit: { maxAttempts: 1, windowMs: 60_000, lockoutMs: 60_000, exemptLoopback: true }, + }; + await withGatewayServer(async ({ port }) => { + const firstWs = await openWs(port, { origin: originForPort(port) }); + try { + const first = await connectReq(firstWs, { token: "wrong" }); + expect(first.ok).toBe(false); + expect(first.error?.message ?? "").not.toContain("retry later"); + } finally { + firstWs.close(); + } + + const secondWs = await openWs(port, { origin: originForPort(port) }); + try { + const second = await connectReq(secondWs, { token: "wrong" }); + expect(second.ok).toBe(false); + expect(second.error?.message ?? "").toContain("retry later"); + } finally { + secondWs.close(); + } + }); + }); + + test("does not silently auto-pair non-control-ui browser clients on loopback", async () => { + const { listDevicePairing } = await import("../infra/device-pairing.js"); + testState.gatewayAuth = { mode: "token", token: "secret" }; + + await withGatewayServer(async ({ port }) => { + const browserWs = await openWs(port, { origin: originForPort(port) }); + try { + const nonce = await readConnectChallengeNonce(browserWs); + expect(typeof nonce).toBe("string"); + const { identity, device } = await createSignedDevice({ + token: "secret", + scopes: ["operator.admin"], + clientId: TEST_OPERATOR_CLIENT.id, + clientMode: TEST_OPERATOR_CLIENT.mode, + identityPath: path.join(os.tmpdir(), `openclaw-browser-device-${randomUUID()}.json`), + nonce: String(nonce ?? ""), + }); + const res = await connectReq(browserWs, { + token: "secret", + scopes: ["operator.admin"], + client: TEST_OPERATOR_CLIENT, + device, + }); + expect(res.ok).toBe(false); + expect(res.error?.message ?? "").toContain("pairing required"); + + const pairing = await listDevicePairing(); + const pending = pairing.pending.find((entry) => entry.deviceId === identity.deviceId); + expect(pending).toBeTruthy(); + expect(pending?.silent).toBe(false); + } finally { + browserWs.close(); + } + }); + }); +}); diff --git a/src/gateway/server.auth.test.ts b/src/gateway/server.auth.test.ts index 38668de7f40..83a97644d19 100644 --- a/src/gateway/server.auth.test.ts +++ b/src/gateway/server.auth.test.ts @@ -672,17 +672,6 @@ describe("gateway server auth/connect", () => { ws.close(); }); - test("rejects non-local browser origins for non-control-ui clients", async () => { - const ws = await openWs(port, { origin: "https://attacker.example" }); - const res = await connectReq(ws, { - token: "secret", - client: TEST_OPERATOR_CLIENT, - }); - expect(res.ok).toBe(false); - expect(res.error?.message ?? "").toContain("origin not allowed"); - ws.close(); - }); - test("returns control ui hint when token is missing", async () => { const ws = await openWs(port, { origin: originForPort(port) }); const res = await connectReq(ws, { @@ -712,27 +701,6 @@ describe("gateway server auth/connect", () => { ); ws.close(); }); - - test("rate-limits browser-origin auth failures on loopback even when loopback exemption is enabled", async () => { - testState.gatewayAuth = { - mode: "token", - token: "secret", - rateLimit: { maxAttempts: 1, windowMs: 60_000, lockoutMs: 60_000, exemptLoopback: true }, - }; - await withGatewayServer(async ({ port }) => { - const firstWs = await openWs(port, { origin: originForPort(port) }); - const first = await connectReq(firstWs, { token: "wrong" }); - expect(first.ok).toBe(false); - expect(first.error?.message ?? "").not.toContain("retry later"); - firstWs.close(); - - const secondWs = await openWs(port, { origin: originForPort(port) }); - const second = await connectReq(secondWs, { token: "wrong" }); - expect(second.ok).toBe(false); - expect(second.error?.message ?? "").toContain("retry later"); - secondWs.close(); - }); - }); }); describe("explicit none auth", () => { @@ -1246,43 +1214,6 @@ describe("gateway server auth/connect", () => { restoreGatewayToken(prevToken); }); - test("does not silently auto-pair non-control-ui browser clients on loopback", async () => { - const { listDevicePairing } = await import("../infra/device-pairing.js"); - const { randomUUID } = await import("node:crypto"); - const os = await import("node:os"); - const path = await import("node:path"); - const { server, ws, port, prevToken } = await startServerWithClient("secret"); - ws.close(); - - const browserWs = await openWs(port, { origin: originForPort(port) }); - const nonce = await readConnectChallengeNonce(browserWs); - const { identity, device } = await createSignedDevice({ - token: "secret", - scopes: ["operator.admin"], - clientId: TEST_OPERATOR_CLIENT.id, - clientMode: TEST_OPERATOR_CLIENT.mode, - identityPath: path.join(os.tmpdir(), `openclaw-browser-device-${randomUUID()}.json`), - nonce, - }); - const res = await connectReq(browserWs, { - token: "secret", - scopes: ["operator.admin"], - client: TEST_OPERATOR_CLIENT, - device, - }); - expect(res.ok).toBe(false); - expect(res.error?.message ?? "").toContain("pairing required"); - - const pairing = await listDevicePairing(); - const pending = pairing.pending.find((entry) => entry.deviceId === identity.deviceId); - expect(pending).toBeTruthy(); - expect(pending?.silent).toBe(false); - - browserWs.close(); - await server.close(); - restoreGatewayToken(prevToken); - }); - test("merges remote node/operator pairing requests for the same unpaired device", async () => { const { mkdtemp } = await import("node:fs/promises"); const { tmpdir } = await import("node:os"); diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index 8b368539469..3dbd86e1e5e 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -110,6 +110,21 @@ const logWsControl = log.child("ws"); const gatewayRuntime = runtimeForLogger(log); const canvasRuntime = runtimeForLogger(logCanvas); +type AuthRateLimitConfig = Parameters[0]; + +function createGatewayAuthRateLimiters(rateLimitConfig: AuthRateLimitConfig | undefined): { + rateLimiter?: AuthRateLimiter; + browserRateLimiter: AuthRateLimiter; +} { + const rateLimiter = rateLimitConfig ? createAuthRateLimiter(rateLimitConfig) : undefined; + // Browser-origin WS auth attempts always use loopback-non-exempt throttling. + const browserRateLimiter = createAuthRateLimiter({ + ...rateLimitConfig, + exemptLoopback: false, + }); + return { rateLimiter, browserRateLimiter }; +} + export type GatewayServer = { close: (opts?: { reason?: string; restartExpectedMs?: number | null }) => Promise; }; @@ -311,16 +326,10 @@ export async function startGatewayServer( let hooksConfig = runtimeConfig.hooksConfig; const canvasHostEnabled = runtimeConfig.canvasHostEnabled; - // Create auth rate limiter only when explicitly configured. + // Create auth rate limiters used by connect/auth flows. const rateLimitConfig = cfgAtStart.gateway?.auth?.rateLimit; - const authRateLimiter: AuthRateLimiter | undefined = rateLimitConfig - ? createAuthRateLimiter(rateLimitConfig) - : undefined; - // Always keep a browser-origin fallback limiter for WS auth attempts. - const browserAuthRateLimiter: AuthRateLimiter = createAuthRateLimiter({ - ...rateLimitConfig, - exemptLoopback: false, - }); + const { rateLimiter: authRateLimiter, browserRateLimiter: browserAuthRateLimiter } = + createGatewayAuthRateLimiters(rateLimitConfig); let controlUiRootState: ControlUiRootState | undefined; if (controlUiRootOverride) { diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index 0d694d12529..8feb385e8f9 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -83,6 +83,52 @@ import { isUnauthorizedRoleError, UnauthorizedFloodGuard } from "./unauthorized- type SubsystemLogger = ReturnType; const DEVICE_SIGNATURE_SKEW_MS = 2 * 60 * 1000; +const BROWSER_ORIGIN_LOOPBACK_RATE_LIMIT_IP = "198.18.0.1"; + +type HandshakeBrowserSecurityContext = { + hasBrowserOriginHeader: boolean; + enforceOriginCheckForAnyClient: boolean; + rateLimitClientIp: string | undefined; + authRateLimiter?: AuthRateLimiter; +}; + +function resolveHandshakeBrowserSecurityContext(params: { + requestOrigin?: string; + hasProxyHeaders: boolean; + clientIp: string | undefined; + rateLimiter?: AuthRateLimiter; + browserRateLimiter?: AuthRateLimiter; +}): HandshakeBrowserSecurityContext { + const hasBrowserOriginHeader = Boolean( + params.requestOrigin && params.requestOrigin.trim() !== "", + ); + return { + hasBrowserOriginHeader, + enforceOriginCheckForAnyClient: hasBrowserOriginHeader && !params.hasProxyHeaders, + rateLimitClientIp: + hasBrowserOriginHeader && isLoopbackAddress(params.clientIp) + ? BROWSER_ORIGIN_LOOPBACK_RATE_LIMIT_IP + : params.clientIp, + authRateLimiter: + hasBrowserOriginHeader && params.browserRateLimiter + ? params.browserRateLimiter + : params.rateLimiter, + }; +} + +function shouldAllowSilentLocalPairing(params: { + isLocalClient: boolean; + hasBrowserOriginHeader: boolean; + isControlUi: boolean; + isWebchat: boolean; + reason: "not-paired" | "role-upgrade" | "scope-upgrade"; +}): boolean { + return ( + params.isLocalClient && + (!params.hasBrowserOriginHeader || params.isControlUi || params.isWebchat) && + (params.reason === "not-paired" || params.reason === "scope-upgrade") + ); +} export function attachGatewayWsMessageHandler(params: { socket: WebSocket; @@ -195,12 +241,19 @@ export function attachGatewayWsMessageHandler(params: { const isWebchatConnect = (p: ConnectParams | null | undefined) => isWebchatClient(p?.client); const unauthorizedFloodGuard = new UnauthorizedFloodGuard(); - const hasBrowserOriginHeader = Boolean(requestOrigin && requestOrigin.trim() !== ""); - const enforceBrowserOriginForAnyClient = hasBrowserOriginHeader && !hasProxyHeaders; - const browserRateLimitClientIp = - hasBrowserOriginHeader && isLoopbackAddress(clientIp) ? "198.18.0.1" : clientIp; - const authRateLimiter = - hasBrowserOriginHeader && browserRateLimiter ? browserRateLimiter : rateLimiter; + const browserSecurity = resolveHandshakeBrowserSecurityContext({ + requestOrigin, + hasProxyHeaders, + clientIp, + rateLimiter, + browserRateLimiter, + }); + const { + hasBrowserOriginHeader, + enforceOriginCheckForAnyClient, + rateLimitClientIp: browserRateLimitClientIp, + authRateLimiter, + } = browserSecurity; socket.on("message", async (data) => { if (isClosed()) { @@ -338,7 +391,7 @@ export function attachGatewayWsMessageHandler(params: { const isControlUi = connectParams.client.id === GATEWAY_CLIENT_IDS.CONTROL_UI; const isWebchat = isWebchatConnect(connectParams); - if (enforceBrowserOriginForAnyClient || isControlUi || isWebchat) { + if (enforceOriginCheckForAnyClient || isControlUi || isWebchat) { const originCheck = checkBrowserOrigin({ requestHost, origin: requestOrigin, @@ -622,10 +675,13 @@ export function attachGatewayWsMessageHandler(params: { const requirePairing = async ( reason: "not-paired" | "role-upgrade" | "scope-upgrade", ) => { - const allowSilentLocalPairing = - isLocalClient && - (!hasBrowserOriginHeader || isControlUi || isWebchat) && - (reason === "not-paired" || reason === "scope-upgrade"); + const allowSilentLocalPairing = shouldAllowSilentLocalPairing({ + isLocalClient, + hasBrowserOriginHeader, + isControlUi, + isWebchat, + reason, + }); const pairing = await requestDevicePairing({ deviceId: device.id, publicKey: devicePublicKey,