Skip to content

Commit ecbd97e

Browse files
fede-kamelfede-kamel
authored
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 <federico.kamelhar@oracle.com>
1 parent ef04c72 commit ecbd97e

4 files changed

Lines changed: 424 additions & 48 deletions

File tree

‎src/gateway/auth-rate-limit.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ export interface RateLimitConfig {
3939
export const AUTH_RATE_LIMIT_SCOPE_DEFAULT = "default";
4040
export const AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET = "shared-secret";
4141
export const AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN = "device-token";
42+
// Per-IP gate for the pre-auth bootstrap-token verify path.
43+
// `verifyDeviceBootstrapToken` is `withLock`-serialized in
44+
// `device-bootstrap.ts` and runs fs read + fs write on every attempt;
45+
// without a scope-specific limiter, attackers presenting a valid
46+
// device signature can queue the bootstrap-pairing flow behind their
47+
// requests, blocking legitimate node onboarding during the attack.
48+
export const AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN = "bootstrap-token";
4249
export const AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH = "hook-auth";
4350
const BROWSER_ORIGIN_RATE_LIMIT_KEY_PREFIX = "browser-origin:";
4451

‎src/gateway/server.preauth-bootstrap-token-rate-limit.test.ts‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { randomUUID } from "node:crypto";
2+
import os from "node:os";
3+
import path from "node:path";
4+
import { describe, expect, test } from "vitest";
5+
import { WebSocket } from "ws";
6+
import {
7+
connectReq,
8+
installGatewayTestHooks,
9+
testState,
10+
trackConnectChallengeNonce,
11+
withGatewayServer,
12+
} from "./test-helpers.js";
13+
14+
installGatewayTestHooks({ scope: "suite" });
15+
16+
async function openWs(port: number) {
17+
const ws = new WebSocket(`ws://127.0.0.1:${port}`);
18+
trackConnectChallengeNonce(ws);
19+
await new Promise<void>((resolve) => ws.once("open", resolve));
20+
return ws;
21+
}
22+
23+
async function attemptForgedBootstrap(port: number, identityPath: string) {
24+
const ws = await openWs(port);
25+
try {
26+
const res = await connectReq(ws, {
27+
skipDefaultAuth: true,
28+
bootstrapToken: "forged-bootstrap-token",
29+
deviceIdentityPath: identityPath,
30+
});
31+
return res;
32+
} finally {
33+
ws.close();
34+
await new Promise<void>((resolve) => {
35+
if (ws.readyState === WebSocket.CLOSED) {
36+
resolve();
37+
return;
38+
}
39+
ws.once("close", () => resolve());
40+
});
41+
}
42+
}
43+
44+
describe("pre-auth bootstrap-token rate limit", () => {
45+
test("locks out concurrent forged bootstrap-token attempts after maxAttempts", async () => {
46+
// exemptLoopback:false ensures the limiter applies to loopback test
47+
// clients. In production the same gate applies to remote clients via
48+
// the per-IP bucket.
49+
testState.gatewayAuth = {
50+
mode: "token",
51+
token: "secret",
52+
rateLimit: {
53+
maxAttempts: 3,
54+
windowMs: 60_000,
55+
lockoutMs: 60_000,
56+
exemptLoopback: false,
57+
},
58+
};
59+
await withGatewayServer(async ({ port }) => {
60+
const identityPrefix = path.join(os.tmpdir(), `openclaw-preauth-bootstrap-${randomUUID()}`);
61+
62+
const responses = await Promise.all(
63+
Array.from(
64+
{ length: 8 },
65+
async (_, index) => await attemptForgedBootstrap(port, `${identityPrefix}-${index}.json`),
66+
),
67+
);
68+
const reasons = responses.map((res) => {
69+
expect(res.ok).toBe(false);
70+
const detail = res.error?.details as { authReason?: string } | undefined;
71+
return detail?.authReason;
72+
});
73+
expect(reasons.filter((reason) => reason === "bootstrap_token_invalid")).toHaveLength(3);
74+
expect(reasons.filter((reason) => reason === "rate_limited")).toHaveLength(5);
75+
});
76+
});
77+
78+
test("forged bootstrap-token failures consume their own bucket independent of device-token", async () => {
79+
testState.gatewayAuth = {
80+
mode: "token",
81+
token: "secret",
82+
rateLimit: {
83+
maxAttempts: 1,
84+
windowMs: 60_000,
85+
lockoutMs: 60_000,
86+
exemptLoopback: false,
87+
},
88+
};
89+
await withGatewayServer(async ({ port }) => {
90+
const identityPath = path.join(
91+
os.tmpdir(),
92+
`openclaw-preauth-bootstrap-shared-${randomUUID()}.json`,
93+
);
94+
95+
const first = await attemptForgedBootstrap(port, identityPath);
96+
expect(first.ok).toBe(false);
97+
const firstDetail = first.error?.details as { authReason?: string } | undefined;
98+
expect(firstDetail?.authReason).toBe("bootstrap_token_invalid");
99+
100+
const second = await attemptForgedBootstrap(port, identityPath);
101+
expect(second.ok).toBe(false);
102+
const secondDetail = second.error?.details as { authReason?: string } | undefined;
103+
expect(secondDetail?.authReason).toBe("rate_limited");
104+
});
105+
});
106+
});

‎src/gateway/server/ws-connection/auth-context.test.ts‎

Lines changed: 202 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it, vi } from "vitest";
2-
import type { AuthRateLimiter } from "../../auth-rate-limit.js";
2+
import { createAuthRateLimiter, type AuthRateLimiter } from "../../auth-rate-limit.js";
33
import { resolveConnectAuthDecision, type ConnectAuthState } from "./auth-context.js";
44

55
type VerifyDeviceTokenFn = Parameters<typeof resolveConnectAuthDecision>[0]["verifyDeviceToken"];
@@ -9,7 +9,9 @@ type VerifyBootstrapTokenFn = Parameters<
99

1010
function createRateLimiter(params?: { allowed?: boolean; retryAfterMs?: number }): {
1111
limiter: AuthRateLimiter;
12+
check: ReturnType<typeof vi.fn>;
1213
reset: ReturnType<typeof vi.fn>;
14+
recordFailure: ReturnType<typeof vi.fn>;
1315
} {
1416
const allowed = params?.allowed ?? true;
1517
const retryAfterMs = params?.retryAfterMs ?? 5_000;
@@ -22,7 +24,31 @@ function createRateLimiter(params?: { allowed?: boolean; retryAfterMs?: number }
2224
reset,
2325
recordFailure,
2426
} as unknown as AuthRateLimiter,
27+
check,
2528
reset,
29+
recordFailure,
30+
};
31+
}
32+
33+
function createPerScopeRateLimiter(
34+
scopes: Record<string, { allowed: boolean; retryAfterMs?: number }>,
35+
): {
36+
limiter: AuthRateLimiter;
37+
check: ReturnType<typeof vi.fn>;
38+
reset: ReturnType<typeof vi.fn>;
39+
recordFailure: ReturnType<typeof vi.fn>;
40+
} {
41+
const check = vi.fn((_ip: string | undefined, scope?: string) => {
42+
const cfg = scopes[scope ?? ""] ?? { allowed: true };
43+
return { allowed: cfg.allowed, retryAfterMs: cfg.retryAfterMs ?? 5_000 };
44+
});
45+
const reset = vi.fn();
46+
const recordFailure = vi.fn();
47+
return {
48+
limiter: { check, reset, recordFailure } as unknown as AuthRateLimiter,
49+
check,
50+
reset,
51+
recordFailure,
2652
};
2753
}
2854

@@ -281,4 +307,179 @@ describe("resolveConnectAuthDecision", () => {
281307
expect(verifyBootstrapToken).toHaveBeenCalledOnce();
282308
expect(verifyDeviceToken).not.toHaveBeenCalled();
283309
});
310+
311+
it("gates bootstrap-token verify when the bootstrap-token bucket is exceeded", async () => {
312+
const rateLimiter = createPerScopeRateLimiter({
313+
"bootstrap-token": { allowed: false, retryAfterMs: 30_000 },
314+
"device-token": { allowed: true },
315+
"shared-secret": { allowed: true },
316+
});
317+
const verifyBootstrapToken = vi.fn<VerifyBootstrapTokenFn>(async () => ({ ok: true }));
318+
const verifyDeviceToken = vi.fn<VerifyDeviceTokenFn>(async () => ({ ok: true }));
319+
const decision = await resolveDeviceTokenDecision({
320+
verifyBootstrapToken,
321+
verifyDeviceToken,
322+
rateLimiter: rateLimiter.limiter,
323+
clientIp: "203.0.113.20",
324+
stateOverrides: {
325+
bootstrapTokenCandidate: "bootstrap-token",
326+
deviceTokenCandidate: undefined,
327+
deviceTokenCandidateSource: undefined,
328+
},
329+
});
330+
expect(decision.authOk).toBe(false);
331+
expect(decision.authResult.reason).toBe("rate_limited");
332+
expect(decision.authResult.retryAfterMs).toBe(30_000);
333+
// The verify path is mutex-locked + does fs I/O — confirm we never invoke
334+
// it once the bucket is exhausted.
335+
expect(verifyBootstrapToken).not.toHaveBeenCalled();
336+
});
337+
338+
it("still verifies the device token when only the bootstrap-token path is rate-limited", async () => {
339+
const rateLimiter = createPerScopeRateLimiter({
340+
"bootstrap-token": { allowed: false, retryAfterMs: 30_000 },
341+
"device-token": { allowed: true },
342+
"shared-secret": { allowed: true },
343+
});
344+
const verifyBootstrapToken = vi.fn<VerifyBootstrapTokenFn>(async () => ({ ok: true }));
345+
const verifyDeviceToken = vi.fn<VerifyDeviceTokenFn>(async () => ({ ok: true }));
346+
const decision = await resolveDeviceTokenDecision({
347+
verifyBootstrapToken,
348+
verifyDeviceToken,
349+
rateLimiter: rateLimiter.limiter,
350+
clientIp: "203.0.113.20",
351+
stateOverrides: {
352+
bootstrapTokenCandidate: "bootstrap-token",
353+
deviceTokenCandidate: "device-token",
354+
},
355+
});
356+
expect(decision.authOk).toBe(true);
357+
expect(decision.authMethod).toBe("device-token");
358+
expect(verifyBootstrapToken).not.toHaveBeenCalled();
359+
expect(verifyDeviceToken).toHaveBeenCalledOnce();
360+
});
361+
362+
it("records a bootstrap-token failure when final auth rejects", async () => {
363+
const rateLimiter = createPerScopeRateLimiter({
364+
"bootstrap-token": { allowed: true },
365+
"device-token": { allowed: true },
366+
"shared-secret": { allowed: true },
367+
});
368+
const verifyBootstrapToken = vi.fn<VerifyBootstrapTokenFn>(async () => ({
369+
ok: false,
370+
reason: "bootstrap_token_invalid",
371+
}));
372+
const verifyDeviceToken = vi.fn<VerifyDeviceTokenFn>(async () => ({ ok: true }));
373+
await resolveDeviceTokenDecision({
374+
verifyBootstrapToken,
375+
verifyDeviceToken,
376+
rateLimiter: rateLimiter.limiter,
377+
clientIp: "203.0.113.20",
378+
stateOverrides: {
379+
bootstrapTokenCandidate: "bootstrap-token",
380+
deviceTokenCandidate: undefined,
381+
deviceTokenCandidateSource: undefined,
382+
},
383+
});
384+
expect(rateLimiter.recordFailure).toHaveBeenCalledWith("203.0.113.20", "bootstrap-token");
385+
expect(rateLimiter.reset).not.toHaveBeenCalledWith("203.0.113.20", "bootstrap-token");
386+
});
387+
388+
it("does not record a bootstrap-token failure when device-token fallback succeeds", async () => {
389+
const rateLimiter = createPerScopeRateLimiter({
390+
"bootstrap-token": { allowed: true },
391+
"device-token": { allowed: true },
392+
"shared-secret": { allowed: true },
393+
});
394+
const verifyBootstrapToken = vi.fn<VerifyBootstrapTokenFn>(async () => ({
395+
ok: false,
396+
reason: "bootstrap_token_invalid",
397+
}));
398+
const verifyDeviceToken = vi.fn<VerifyDeviceTokenFn>(async () => ({ ok: true }));
399+
const decision = await resolveDeviceTokenDecision({
400+
verifyBootstrapToken,
401+
verifyDeviceToken,
402+
rateLimiter: rateLimiter.limiter,
403+
clientIp: "203.0.113.20",
404+
stateOverrides: {
405+
bootstrapTokenCandidate: "bootstrap-token",
406+
deviceTokenCandidate: "device-token",
407+
},
408+
});
409+
expect(decision.authOk).toBe(true);
410+
expect(decision.authMethod).toBe("device-token");
411+
expect(rateLimiter.recordFailure).not.toHaveBeenCalledWith("203.0.113.20", "bootstrap-token");
412+
});
413+
414+
it("serializes concurrent bootstrap-token failures before checking the next attempt", async () => {
415+
const rateLimiter = createAuthRateLimiter({
416+
maxAttempts: 3,
417+
windowMs: 60_000,
418+
lockoutMs: 60_000,
419+
exemptLoopback: false,
420+
pruneIntervalMs: 0,
421+
});
422+
let activeBootstrapChecks = 0;
423+
let maxActiveBootstrapChecks = 0;
424+
const verifyBootstrapToken = vi.fn<VerifyBootstrapTokenFn>(async () => {
425+
activeBootstrapChecks += 1;
426+
maxActiveBootstrapChecks = Math.max(maxActiveBootstrapChecks, activeBootstrapChecks);
427+
await new Promise((resolve) => setTimeout(resolve, 5));
428+
activeBootstrapChecks -= 1;
429+
return { ok: false, reason: "bootstrap_token_invalid" };
430+
});
431+
const verifyDeviceToken = vi.fn<VerifyDeviceTokenFn>(async () => ({ ok: true }));
432+
try {
433+
const decisions = await Promise.all(
434+
Array.from(
435+
{ length: 8 },
436+
async () =>
437+
await resolveDeviceTokenDecision({
438+
verifyBootstrapToken,
439+
verifyDeviceToken,
440+
rateLimiter,
441+
clientIp: "203.0.113.20",
442+
stateOverrides: {
443+
bootstrapTokenCandidate: "bootstrap-token",
444+
deviceTokenCandidate: undefined,
445+
deviceTokenCandidateSource: undefined,
446+
},
447+
}),
448+
),
449+
);
450+
const reasons = decisions.map((decision) => decision.authResult.reason);
451+
expect(reasons.filter((reason) => reason === "bootstrap_token_invalid")).toHaveLength(3);
452+
expect(reasons.filter((reason) => reason === "rate_limited")).toHaveLength(5);
453+
expect(verifyBootstrapToken).toHaveBeenCalledTimes(3);
454+
expect(maxActiveBootstrapChecks).toBe(1);
455+
expect(verifyDeviceToken).not.toHaveBeenCalled();
456+
} finally {
457+
rateLimiter.dispose();
458+
}
459+
});
460+
461+
it("resets the bootstrap-token bucket when the verify succeeds", async () => {
462+
const rateLimiter = createPerScopeRateLimiter({
463+
"bootstrap-token": { allowed: true },
464+
"device-token": { allowed: true },
465+
"shared-secret": { allowed: true },
466+
});
467+
const verifyBootstrapToken = vi.fn<VerifyBootstrapTokenFn>(async () => ({ ok: true }));
468+
const verifyDeviceToken = vi.fn<VerifyDeviceTokenFn>(async () => ({ ok: true }));
469+
const decision = await resolveDeviceTokenDecision({
470+
verifyBootstrapToken,
471+
verifyDeviceToken,
472+
rateLimiter: rateLimiter.limiter,
473+
clientIp: "203.0.113.20",
474+
stateOverrides: {
475+
bootstrapTokenCandidate: "bootstrap-token",
476+
deviceTokenCandidate: undefined,
477+
deviceTokenCandidateSource: undefined,
478+
},
479+
});
480+
expect(decision.authOk).toBe(true);
481+
expect(decision.authMethod).toBe("bootstrap-token");
482+
expect(rateLimiter.reset).toHaveBeenCalledWith("203.0.113.20", "bootstrap-token");
483+
expect(rateLimiter.recordFailure).not.toHaveBeenCalledWith("203.0.113.20", "bootstrap-token");
484+
});
284485
});

0 commit comments

Comments
 (0)