From ecbd97e9682dffbd4cf0435b74acc5a1440b728b Mon Sep 17 00:00:00 2001 From: Federico Kamelhar Date: Sun, 31 May 2026 14:40:22 -0400 Subject: [PATCH] fix(gateway): rate-limit bootstrap-token verification Gateway/security: rate-limits pre-auth bootstrap-token verification and serializes per-IP attempts to prevent mutex-stall DoS while preserving device-token fallback. Fixes #77978. Co-authored-by: Federico Kamelhar --- src/gateway/auth-rate-limit.ts | 7 + ...preauth-bootstrap-token-rate-limit.test.ts | 106 +++++++++ .../server/ws-connection/auth-context.test.ts | 203 +++++++++++++++++- .../server/ws-connection/auth-context.ts | 156 ++++++++++---- 4 files changed, 424 insertions(+), 48 deletions(-) create mode 100644 src/gateway/server.preauth-bootstrap-token-rate-limit.test.ts diff --git a/src/gateway/auth-rate-limit.ts b/src/gateway/auth-rate-limit.ts index d25731003a4..e8b97d0e4c5 100644 --- a/src/gateway/auth-rate-limit.ts +++ b/src/gateway/auth-rate-limit.ts @@ -39,6 +39,13 @@ export interface RateLimitConfig { export const AUTH_RATE_LIMIT_SCOPE_DEFAULT = "default"; export const AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET = "shared-secret"; export const AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN = "device-token"; +// Per-IP gate for the pre-auth bootstrap-token verify path. +// `verifyDeviceBootstrapToken` is `withLock`-serialized in +// `device-bootstrap.ts` and runs fs read + fs write on every attempt; +// without a scope-specific limiter, attackers presenting a valid +// device signature can queue the bootstrap-pairing flow behind their +// requests, blocking legitimate node onboarding during the attack. +export const AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN = "bootstrap-token"; export const AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH = "hook-auth"; const BROWSER_ORIGIN_RATE_LIMIT_KEY_PREFIX = "browser-origin:"; diff --git a/src/gateway/server.preauth-bootstrap-token-rate-limit.test.ts b/src/gateway/server.preauth-bootstrap-token-rate-limit.test.ts new file mode 100644 index 00000000000..6d40ff1f371 --- /dev/null +++ b/src/gateway/server.preauth-bootstrap-token-rate-limit.test.ts @@ -0,0 +1,106 @@ +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 { + connectReq, + installGatewayTestHooks, + testState, + trackConnectChallengeNonce, + withGatewayServer, +} from "./test-helpers.js"; + +installGatewayTestHooks({ scope: "suite" }); + +async function openWs(port: number) { + const ws = new WebSocket(`ws://127.0.0.1:${port}`); + trackConnectChallengeNonce(ws); + await new Promise((resolve) => ws.once("open", resolve)); + return ws; +} + +async function attemptForgedBootstrap(port: number, identityPath: string) { + const ws = await openWs(port); + try { + const res = await connectReq(ws, { + skipDefaultAuth: true, + bootstrapToken: "forged-bootstrap-token", + deviceIdentityPath: identityPath, + }); + return res; + } finally { + ws.close(); + await new Promise((resolve) => { + if (ws.readyState === WebSocket.CLOSED) { + resolve(); + return; + } + ws.once("close", () => resolve()); + }); + } +} + +describe("pre-auth bootstrap-token rate limit", () => { + test("locks out concurrent forged bootstrap-token attempts after maxAttempts", async () => { + // exemptLoopback:false ensures the limiter applies to loopback test + // clients. In production the same gate applies to remote clients via + // the per-IP bucket. + testState.gatewayAuth = { + mode: "token", + token: "secret", + rateLimit: { + maxAttempts: 3, + windowMs: 60_000, + lockoutMs: 60_000, + exemptLoopback: false, + }, + }; + await withGatewayServer(async ({ port }) => { + const identityPrefix = path.join(os.tmpdir(), `openclaw-preauth-bootstrap-${randomUUID()}`); + + const responses = await Promise.all( + Array.from( + { length: 8 }, + async (_, index) => await attemptForgedBootstrap(port, `${identityPrefix}-${index}.json`), + ), + ); + const reasons = responses.map((res) => { + expect(res.ok).toBe(false); + const detail = res.error?.details as { authReason?: string } | undefined; + return detail?.authReason; + }); + expect(reasons.filter((reason) => reason === "bootstrap_token_invalid")).toHaveLength(3); + expect(reasons.filter((reason) => reason === "rate_limited")).toHaveLength(5); + }); + }); + + test("forged bootstrap-token failures consume their own bucket independent of device-token", async () => { + testState.gatewayAuth = { + mode: "token", + token: "secret", + rateLimit: { + maxAttempts: 1, + windowMs: 60_000, + lockoutMs: 60_000, + exemptLoopback: false, + }, + }; + await withGatewayServer(async ({ port }) => { + const identityPath = path.join( + os.tmpdir(), + `openclaw-preauth-bootstrap-shared-${randomUUID()}.json`, + ); + + const first = await attemptForgedBootstrap(port, identityPath); + expect(first.ok).toBe(false); + const firstDetail = first.error?.details as { authReason?: string } | undefined; + expect(firstDetail?.authReason).toBe("bootstrap_token_invalid"); + + const second = await attemptForgedBootstrap(port, identityPath); + expect(second.ok).toBe(false); + const secondDetail = second.error?.details as { authReason?: string } | undefined; + expect(secondDetail?.authReason).toBe("rate_limited"); + }); + }); +}); diff --git a/src/gateway/server/ws-connection/auth-context.test.ts b/src/gateway/server/ws-connection/auth-context.test.ts index 572e62fae3c..38fbf6c964b 100644 --- a/src/gateway/server/ws-connection/auth-context.test.ts +++ b/src/gateway/server/ws-connection/auth-context.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import type { AuthRateLimiter } from "../../auth-rate-limit.js"; +import { createAuthRateLimiter, type AuthRateLimiter } from "../../auth-rate-limit.js"; import { resolveConnectAuthDecision, type ConnectAuthState } from "./auth-context.js"; type VerifyDeviceTokenFn = Parameters[0]["verifyDeviceToken"]; @@ -9,7 +9,9 @@ type VerifyBootstrapTokenFn = Parameters< function createRateLimiter(params?: { allowed?: boolean; retryAfterMs?: number }): { limiter: AuthRateLimiter; + check: ReturnType; reset: ReturnType; + recordFailure: ReturnType; } { const allowed = params?.allowed ?? true; const retryAfterMs = params?.retryAfterMs ?? 5_000; @@ -22,7 +24,31 @@ function createRateLimiter(params?: { allowed?: boolean; retryAfterMs?: number } reset, recordFailure, } as unknown as AuthRateLimiter, + check, reset, + recordFailure, + }; +} + +function createPerScopeRateLimiter( + scopes: Record, +): { + limiter: AuthRateLimiter; + check: ReturnType; + reset: ReturnType; + recordFailure: ReturnType; +} { + const check = vi.fn((_ip: string | undefined, scope?: string) => { + const cfg = scopes[scope ?? ""] ?? { allowed: true }; + return { allowed: cfg.allowed, retryAfterMs: cfg.retryAfterMs ?? 5_000 }; + }); + const reset = vi.fn(); + const recordFailure = vi.fn(); + return { + limiter: { check, reset, recordFailure } as unknown as AuthRateLimiter, + check, + reset, + recordFailure, }; } @@ -281,4 +307,179 @@ describe("resolveConnectAuthDecision", () => { expect(verifyBootstrapToken).toHaveBeenCalledOnce(); expect(verifyDeviceToken).not.toHaveBeenCalled(); }); + + it("gates bootstrap-token verify when the bootstrap-token bucket is exceeded", async () => { + const rateLimiter = createPerScopeRateLimiter({ + "bootstrap-token": { allowed: false, retryAfterMs: 30_000 }, + "device-token": { allowed: true }, + "shared-secret": { allowed: true }, + }); + const verifyBootstrapToken = vi.fn(async () => ({ ok: true })); + const verifyDeviceToken = vi.fn(async () => ({ ok: true })); + const decision = await resolveDeviceTokenDecision({ + verifyBootstrapToken, + verifyDeviceToken, + rateLimiter: rateLimiter.limiter, + clientIp: "203.0.113.20", + stateOverrides: { + bootstrapTokenCandidate: "bootstrap-token", + deviceTokenCandidate: undefined, + deviceTokenCandidateSource: undefined, + }, + }); + expect(decision.authOk).toBe(false); + expect(decision.authResult.reason).toBe("rate_limited"); + expect(decision.authResult.retryAfterMs).toBe(30_000); + // The verify path is mutex-locked + does fs I/O — confirm we never invoke + // it once the bucket is exhausted. + expect(verifyBootstrapToken).not.toHaveBeenCalled(); + }); + + it("still verifies the device token when only the bootstrap-token path is rate-limited", async () => { + const rateLimiter = createPerScopeRateLimiter({ + "bootstrap-token": { allowed: false, retryAfterMs: 30_000 }, + "device-token": { allowed: true }, + "shared-secret": { allowed: true }, + }); + const verifyBootstrapToken = vi.fn(async () => ({ ok: true })); + const verifyDeviceToken = vi.fn(async () => ({ ok: true })); + const decision = await resolveDeviceTokenDecision({ + verifyBootstrapToken, + verifyDeviceToken, + rateLimiter: rateLimiter.limiter, + clientIp: "203.0.113.20", + stateOverrides: { + bootstrapTokenCandidate: "bootstrap-token", + deviceTokenCandidate: "device-token", + }, + }); + expect(decision.authOk).toBe(true); + expect(decision.authMethod).toBe("device-token"); + expect(verifyBootstrapToken).not.toHaveBeenCalled(); + expect(verifyDeviceToken).toHaveBeenCalledOnce(); + }); + + it("records a bootstrap-token failure when final auth rejects", async () => { + const rateLimiter = createPerScopeRateLimiter({ + "bootstrap-token": { allowed: true }, + "device-token": { allowed: true }, + "shared-secret": { allowed: true }, + }); + const verifyBootstrapToken = vi.fn(async () => ({ + ok: false, + reason: "bootstrap_token_invalid", + })); + const verifyDeviceToken = vi.fn(async () => ({ ok: true })); + await resolveDeviceTokenDecision({ + verifyBootstrapToken, + verifyDeviceToken, + rateLimiter: rateLimiter.limiter, + clientIp: "203.0.113.20", + stateOverrides: { + bootstrapTokenCandidate: "bootstrap-token", + deviceTokenCandidate: undefined, + deviceTokenCandidateSource: undefined, + }, + }); + expect(rateLimiter.recordFailure).toHaveBeenCalledWith("203.0.113.20", "bootstrap-token"); + expect(rateLimiter.reset).not.toHaveBeenCalledWith("203.0.113.20", "bootstrap-token"); + }); + + it("does not record a bootstrap-token failure when device-token fallback succeeds", async () => { + const rateLimiter = createPerScopeRateLimiter({ + "bootstrap-token": { allowed: true }, + "device-token": { allowed: true }, + "shared-secret": { allowed: true }, + }); + const verifyBootstrapToken = vi.fn(async () => ({ + ok: false, + reason: "bootstrap_token_invalid", + })); + const verifyDeviceToken = vi.fn(async () => ({ ok: true })); + const decision = await resolveDeviceTokenDecision({ + verifyBootstrapToken, + verifyDeviceToken, + rateLimiter: rateLimiter.limiter, + clientIp: "203.0.113.20", + stateOverrides: { + bootstrapTokenCandidate: "bootstrap-token", + deviceTokenCandidate: "device-token", + }, + }); + expect(decision.authOk).toBe(true); + expect(decision.authMethod).toBe("device-token"); + expect(rateLimiter.recordFailure).not.toHaveBeenCalledWith("203.0.113.20", "bootstrap-token"); + }); + + it("serializes concurrent bootstrap-token failures before checking the next attempt", async () => { + const rateLimiter = createAuthRateLimiter({ + maxAttempts: 3, + windowMs: 60_000, + lockoutMs: 60_000, + exemptLoopback: false, + pruneIntervalMs: 0, + }); + let activeBootstrapChecks = 0; + let maxActiveBootstrapChecks = 0; + const verifyBootstrapToken = vi.fn(async () => { + activeBootstrapChecks += 1; + maxActiveBootstrapChecks = Math.max(maxActiveBootstrapChecks, activeBootstrapChecks); + await new Promise((resolve) => setTimeout(resolve, 5)); + activeBootstrapChecks -= 1; + return { ok: false, reason: "bootstrap_token_invalid" }; + }); + const verifyDeviceToken = vi.fn(async () => ({ ok: true })); + try { + const decisions = await Promise.all( + Array.from( + { length: 8 }, + async () => + await resolveDeviceTokenDecision({ + verifyBootstrapToken, + verifyDeviceToken, + rateLimiter, + clientIp: "203.0.113.20", + stateOverrides: { + bootstrapTokenCandidate: "bootstrap-token", + deviceTokenCandidate: undefined, + deviceTokenCandidateSource: undefined, + }, + }), + ), + ); + const reasons = decisions.map((decision) => decision.authResult.reason); + expect(reasons.filter((reason) => reason === "bootstrap_token_invalid")).toHaveLength(3); + expect(reasons.filter((reason) => reason === "rate_limited")).toHaveLength(5); + expect(verifyBootstrapToken).toHaveBeenCalledTimes(3); + expect(maxActiveBootstrapChecks).toBe(1); + expect(verifyDeviceToken).not.toHaveBeenCalled(); + } finally { + rateLimiter.dispose(); + } + }); + + it("resets the bootstrap-token bucket when the verify succeeds", async () => { + const rateLimiter = createPerScopeRateLimiter({ + "bootstrap-token": { allowed: true }, + "device-token": { allowed: true }, + "shared-secret": { allowed: true }, + }); + const verifyBootstrapToken = vi.fn(async () => ({ ok: true })); + const verifyDeviceToken = vi.fn(async () => ({ ok: true })); + const decision = await resolveDeviceTokenDecision({ + verifyBootstrapToken, + verifyDeviceToken, + rateLimiter: rateLimiter.limiter, + clientIp: "203.0.113.20", + stateOverrides: { + bootstrapTokenCandidate: "bootstrap-token", + deviceTokenCandidate: undefined, + deviceTokenCandidateSource: undefined, + }, + }); + expect(decision.authOk).toBe(true); + expect(decision.authMethod).toBe("bootstrap-token"); + expect(rateLimiter.reset).toHaveBeenCalledWith("203.0.113.20", "bootstrap-token"); + expect(rateLimiter.recordFailure).not.toHaveBeenCalledWith("203.0.113.20", "bootstrap-token"); + }); }); diff --git a/src/gateway/server/ws-connection/auth-context.ts b/src/gateway/server/ws-connection/auth-context.ts index ee025211991..81d1daa78f4 100644 --- a/src/gateway/server/ws-connection/auth-context.ts +++ b/src/gateway/server/ws-connection/auth-context.ts @@ -1,6 +1,7 @@ import type { IncomingMessage } from "node:http"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { + AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN, AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN, AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET, type AuthRateLimiter, @@ -11,6 +12,7 @@ import { type GatewayAuthResult, type ResolvedGatewayAuth, } from "../../auth.js"; +import { withSerializedRateLimitAttempt } from "../../rate-limit-attempt-serialization.js"; type HandshakeConnectAuth = { token?: string; @@ -52,6 +54,30 @@ export type ConnectAuthDecision = { deviceTokenSharedGatewaySessionGeneration?: string; }; +type ResolveConnectAuthDecisionParams = { + state: ConnectAuthState; + hasDeviceIdentity: boolean; + deviceId?: string; + publicKey?: string; + role: string; + scopes: string[]; + rateLimiter?: AuthRateLimiter; + clientIp?: string; + verifyBootstrapToken: (params: { + deviceId: string; + publicKey: string; + token: string; + role: string; + scopes: string[]; + }) => Promise; + verifyDeviceToken: (params: { + deviceId: string; + token: string; + role: string; + scopes: string[]; + }) => Promise; +}; + function mapDeviceTokenAuthFailureReason(params: { tokenCheckReason?: string; candidateSource?: DeviceTokenCandidateSource; @@ -157,59 +183,100 @@ export async function resolveConnectAuthState(params: { }; } -export async function resolveConnectAuthDecision(params: { - state: ConnectAuthState; - hasDeviceIdentity: boolean; - deviceId?: string; - publicKey?: string; - role: string; - scopes: string[]; - rateLimiter?: AuthRateLimiter; - clientIp?: string; - verifyBootstrapToken: (params: { - deviceId: string; - publicKey: string; - token: string; - role: string; - scopes: string[]; - }) => Promise; - verifyDeviceToken: (params: { - deviceId: string; - token: string; - role: string; - scopes: string[]; - }) => Promise; -}): Promise { +export async function resolveConnectAuthDecision( + params: ResolveConnectAuthDecisionParams, +): Promise { + const shouldSerializeBootstrapAttempt = Boolean( + params.rateLimiter && + params.hasDeviceIdentity && + params.deviceId && + params.publicKey && + params.state.bootstrapTokenCandidate, + ); + if (!shouldSerializeBootstrapAttempt) { + return await resolveConnectAuthDecisionCore(params); + } + return await withSerializedRateLimitAttempt({ + ip: params.clientIp, + scope: AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN, + run: async () => await resolveConnectAuthDecisionCore(params), + }); +} + +async function resolveConnectAuthDecisionCore( + params: ResolveConnectAuthDecisionParams, +): Promise { let authResult = params.state.authResult; let authOk = params.state.authOk; let authMethod = params.state.authMethod; let deviceTokenSharedGatewaySessionGeneration: string | undefined; + let pendingBootstrapFailure = false; + + function finish(): ConnectAuthDecision { + if (pendingBootstrapFailure && !authOk) { + params.rateLimiter?.recordFailure(params.clientIp, AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN); + } + return { + authResult, + authOk, + authMethod, + deviceTokenSharedGatewaySessionGeneration, + }; + } const bootstrapTokenCandidate = params.state.bootstrapTokenCandidate; if (params.hasDeviceIdentity && params.deviceId && params.publicKey && bootstrapTokenCandidate) { - const tokenCheck = await params.verifyBootstrapToken({ - deviceId: params.deviceId, - publicKey: params.publicKey, - token: bootstrapTokenCandidate, - role: params.role, - scopes: params.scopes, - }); - if (tokenCheck.ok) { - // Prefer an explicit valid bootstrap token even when another auth path - // (for example tailscale serve header auth) already succeeded. QR pairing - // relies on the server classifying the handshake as bootstrap-token so the - // initial node pairing can be silently auto-approved and the bootstrap - // token can be revoked after approval. - authOk = true; - authMethod = "bootstrap-token"; - } else if (!authOk) { - authResult = { ok: false, reason: tokenCheck.reason ?? "bootstrap_token_invalid" }; + // Per-IP gate on the bootstrap-token verify path. + // verifyDeviceBootstrapToken is mutex-serialized and runs fs read + fs + // write per attempt, so unrate-limited attackers can queue the bootstrap + // pairing flow behind their requests and block legitimate onboarding. + let bootstrapRateLimited = false; + if (params.rateLimiter) { + const bootstrapRateCheck = params.rateLimiter.check( + params.clientIp, + AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN, + ); + if (!bootstrapRateCheck.allowed) { + bootstrapRateLimited = true; + if (!authOk) { + authResult = { + ok: false, + reason: "rate_limited", + rateLimited: true, + retryAfterMs: bootstrapRateCheck.retryAfterMs, + }; + } + } + } + if (!bootstrapRateLimited) { + const tokenCheck = await params.verifyBootstrapToken({ + deviceId: params.deviceId, + publicKey: params.publicKey, + token: bootstrapTokenCandidate, + role: params.role, + scopes: params.scopes, + }); + if (tokenCheck.ok) { + // Prefer an explicit valid bootstrap token even when another auth path + // (for example tailscale serve header auth) already succeeded. QR pairing + // relies on the server classifying the handshake as bootstrap-token so the + // initial node pairing can be silently auto-approved and the bootstrap + // token can be revoked after approval. + authOk = true; + authMethod = "bootstrap-token"; + params.rateLimiter?.reset(params.clientIp, AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN); + } else { + pendingBootstrapFailure = true; + if (!authOk) { + authResult = { ok: false, reason: tokenCheck.reason ?? "bootstrap_token_invalid" }; + } + } } } const deviceTokenCandidate = params.state.deviceTokenCandidate; if (!params.hasDeviceIdentity || !params.deviceId || authOk || !deviceTokenCandidate) { - return { authResult, authOk, authMethod }; + return finish(); } let deviceTokenRateLimited = false; @@ -258,10 +325,5 @@ export async function resolveConnectAuthDecision(params: { } } - return { - authResult, - authOk, - authMethod, - deviceTokenSharedGatewaySessionGeneration, - }; + return finish(); }