diff --git a/CHANGELOG.md b/CHANGELOG.md index 674821df1cf..80b1f9e12c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai - Security: fix unauthenticated Nostr profile API remote config tampering. (#13719) Thanks @coygeek. - Security: remove bundled soul-evil hook. (#14757) Thanks @Imccccc. - Security/Web tools: treat browser/web content as untrusted by default (wrapped outputs for browser snapshot/tabs/console and structured external-content metadata for web tools), and strip `toolResult.details` from model-facing transcript/compaction inputs to reduce prompt-injection replay risk. +- Security/Hooks: harden webhook and device token verification with shared constant-time secret comparison, and add per-client auth-failure throttling for hook endpoints (`429` + `Retry-After`). Thanks @akhmittra. - Gateway: raise WS payload/buffer limits so 5,000,000-byte image attachments work reliably. (#14486) Thanks @0xRaini. - Logging/CLI: use local timezone timestamps for console prefixing, and include `±HH:MM` offsets when using `openclaw logs --local-time` to avoid ambiguity. (#14771) Thanks @0xRaini. - Gateway: drain active turns before restart to prevent message loss. (#13931) Thanks @0xRaini. diff --git a/docs/automation/webhook.md b/docs/automation/webhook.md index 78fb7d63789..ccb2cbbeb86 100644 --- a/docs/automation/webhook.md +++ b/docs/automation/webhook.md @@ -122,6 +122,7 @@ Mapping options (summary): - `200` for `/hooks/wake` - `202` for `/hooks/agent` (async run started) - `401` on auth failure +- `429` after repeated auth failures from the same client (check `Retry-After`) - `400` on invalid payload - `413` on oversized payloads @@ -165,6 +166,7 @@ curl -X POST http://127.0.0.1:18789/hooks/gmail \ - Keep hook endpoints behind loopback, tailnet, or trusted reverse proxy. - Use a dedicated hook token; do not reuse gateway auth tokens. +- Repeated auth failures are rate-limited per client address to slow brute-force attempts. - If you use multi-agent routing, set `hooks.allowedAgentIds` to limit explicit `agentId` selection. - Avoid including sensitive raw payloads in webhook logs. - Hook payloads are treated as untrusted and wrapped with safety boundaries by default. diff --git a/src/gateway/auth.ts b/src/gateway/auth.ts index 9c7fb9acb60..dce9f7a8ecc 100644 --- a/src/gateway/auth.ts +++ b/src/gateway/auth.ts @@ -1,7 +1,7 @@ import type { IncomingMessage } from "node:http"; -import { timingSafeEqual } from "node:crypto"; import type { GatewayAuthConfig, GatewayTailscaleMode } from "../config/config.js"; import { readTailscaleWhoisIdentity, type TailscaleWhoisIdentity } from "../infra/tailscale.js"; +import { safeEqualSecret } from "../security/secret-equal.js"; import { isLoopbackAddress, isTrustedProxyAddress, @@ -37,13 +37,6 @@ type TailscaleUser = { type TailscaleWhoisLookup = (ip: string) => Promise; -function safeEqual(a: string, b: string): boolean { - if (a.length !== b.length) { - return false; - } - return timingSafeEqual(Buffer.from(a), Buffer.from(b)); -} - function normalizeLogin(login: string): string { return login.trim().toLowerCase(); } @@ -253,7 +246,7 @@ export async function authorizeGatewayConnect(params: { if (!connectAuth?.token) { return { ok: false, reason: "token_missing" }; } - if (!safeEqual(connectAuth.token, auth.token)) { + if (!safeEqualSecret(connectAuth.token, auth.token)) { return { ok: false, reason: "token_mismatch" }; } return { ok: true, method: "token" }; @@ -267,7 +260,7 @@ export async function authorizeGatewayConnect(params: { if (!password) { return { ok: false, reason: "password_missing" }; } - if (!safeEqual(password, auth.password)) { + if (!safeEqualSecret(password, auth.password)) { return { ok: false, reason: "password_mismatch" }; } return { ok: true, method: "password" }; diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index b6c4019f911..912463c1ba2 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -18,6 +18,7 @@ import { handleA2uiHttpRequest, } from "../canvas-host/a2ui.js"; import { loadConfig } from "../config/config.js"; +import { safeEqualSecret } from "../security/secret-equal.js"; import { handleSlackHttpRequest } from "../slack/http/index.js"; import { authorizeGatewayConnect, isLocalDirectRequest, type ResolvedGatewayAuth } from "./auth.js"; import { @@ -49,6 +50,11 @@ import { handleOpenResponsesHttpRequest } from "./openresponses-http.js"; import { handleToolsInvokeHttpRequest } from "./tools-invoke-http.js"; type SubsystemLogger = ReturnType; +type HookAuthFailure = { count: number; windowStartedAtMs: number }; + +const HOOK_AUTH_FAILURE_LIMIT = 20; +const HOOK_AUTH_FAILURE_WINDOW_MS = 60_000; +const HOOK_AUTH_FAILURE_TRACK_MAX = 2048; type HookDispatchers = { dispatchWakeHook: (value: { text: string; mode: "now" | "next-heartbeat" }) => void; @@ -140,6 +146,39 @@ export function createHooksRequestHandler( } & HookDispatchers, ): HooksRequestHandler { const { getHooksConfig, bindHost, port, logHooks, dispatchAgentHook, dispatchWakeHook } = opts; + const hookAuthFailures = new Map(); + + const resolveHookClientKey = (req: IncomingMessage): string => { + return req.socket?.remoteAddress?.trim() || "unknown"; + }; + + const recordHookAuthFailure = ( + clientKey: string, + nowMs: number, + ): { throttled: boolean; retryAfterSeconds?: number } => { + if (!hookAuthFailures.has(clientKey) && hookAuthFailures.size >= HOOK_AUTH_FAILURE_TRACK_MAX) { + hookAuthFailures.clear(); + } + const current = hookAuthFailures.get(clientKey); + const expired = !current || nowMs - current.windowStartedAtMs >= HOOK_AUTH_FAILURE_WINDOW_MS; + const next: HookAuthFailure = expired + ? { count: 1, windowStartedAtMs: nowMs } + : { count: current.count + 1, windowStartedAtMs: current.windowStartedAtMs }; + hookAuthFailures.set(clientKey, next); + if (next.count <= HOOK_AUTH_FAILURE_LIMIT) { + return { throttled: false }; + } + const retryAfterMs = Math.max(1, next.windowStartedAtMs + HOOK_AUTH_FAILURE_WINDOW_MS - nowMs); + return { + throttled: true, + retryAfterSeconds: Math.ceil(retryAfterMs / 1000), + }; + }; + + const clearHookAuthFailure = (clientKey: string) => { + hookAuthFailures.delete(clientKey); + }; + return async (req, res) => { const hooksConfig = getHooksConfig(); if (!hooksConfig) { @@ -161,12 +200,24 @@ export function createHooksRequestHandler( } const token = extractHookToken(req); - if (!token || token !== hooksConfig.token) { + const clientKey = resolveHookClientKey(req); + if (!safeEqualSecret(token, hooksConfig.token)) { + const throttle = recordHookAuthFailure(clientKey, Date.now()); + if (throttle.throttled) { + const retryAfter = throttle.retryAfterSeconds ?? 1; + res.statusCode = 429; + res.setHeader("Retry-After", String(retryAfter)); + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end("Too Many Requests"); + logHooks.warn(`hook auth throttled for ${clientKey}; retry-after=${retryAfter}s`); + return true; + } res.statusCode = 401; res.setHeader("Content-Type", "text/plain; charset=utf-8"); res.end("Unauthorized"); return true; } + clearHookAuthFailure(clientKey); if (req.method !== "POST") { res.statusCode = 405; diff --git a/src/gateway/server.hooks.e2e.test.ts b/src/gateway/server.hooks.e2e.test.ts index 1eb41e0f64e..0c35216bec6 100644 --- a/src/gateway/server.hooks.e2e.test.ts +++ b/src/gateway/server.hooks.e2e.test.ts @@ -318,4 +318,59 @@ describe("gateway server hooks", () => { await server.close(); } }); + + test("throttles repeated hook auth failures and resets after success", async () => { + testState.hooksConfig = { enabled: true, token: "hook-secret" }; + const port = await getFreePort(); + const server = await startGatewayServer(port); + try { + const firstFail = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer wrong", + }, + body: JSON.stringify({ text: "blocked" }), + }); + expect(firstFail.status).toBe(401); + + let throttled: Response | null = null; + for (let i = 0; i < 20; i++) { + throttled = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer wrong", + }, + body: JSON.stringify({ text: "blocked" }), + }); + } + expect(throttled?.status).toBe(429); + expect(throttled?.headers.get("retry-after")).toBeTruthy(); + + const allowed = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer hook-secret", + }, + body: JSON.stringify({ text: "auth reset" }), + }); + expect(allowed.status).toBe(200); + await waitForSystemEvent(); + drainSystemEvents(resolveMainKey()); + + const failAfterSuccess = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer wrong", + }, + body: JSON.stringify({ text: "blocked" }), + }); + expect(failAfterSuccess.status).toBe(401); + } finally { + await server.close(); + } + }); }); diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index c1605debdbd..5604047265d 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -7,6 +7,7 @@ import { getPairedDevice, requestDevicePairing, rotateDeviceToken, + verifyDeviceToken, } from "./device-pairing.js"; describe("device pairing tokens", () => { @@ -41,4 +42,40 @@ describe("device pairing tokens", () => { paired = await getPairedDevice("device-1", baseDir); expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]); }); + + test("verifies token and rejects mismatches", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); + const request = await requestDevicePairing( + { + deviceId: "device-1", + publicKey: "public-key-1", + role: "operator", + scopes: ["operator.read"], + }, + baseDir, + ); + await approveDevicePairing(request.request.requestId, baseDir); + const paired = await getPairedDevice("device-1", baseDir); + const token = paired?.tokens?.operator?.token; + expect(token).toBeTruthy(); + + const ok = await verifyDeviceToken({ + deviceId: "device-1", + token: token ?? "", + role: "operator", + scopes: ["operator.read"], + baseDir, + }); + expect(ok.ok).toBe(true); + + const mismatch = await verifyDeviceToken({ + deviceId: "device-1", + token: "x".repeat((token ?? "1234").length), + role: "operator", + scopes: ["operator.read"], + baseDir, + }); + expect(mismatch.ok).toBe(false); + expect(mismatch.reason).toBe("token-mismatch"); + }); }); diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index c2193af3f38..97d66886596 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; import path from "node:path"; import { resolveStateDir } from "../config/paths.js"; +import { safeEqualSecret } from "../security/secret-equal.js"; export type DevicePairingPendingRequest = { requestId: string; @@ -431,7 +432,7 @@ export async function verifyDeviceToken(params: { if (entry.revokedAtMs) { return { ok: false, reason: "token-revoked" }; } - if (entry.token !== params.token) { + if (!safeEqualSecret(params.token, entry.token)) { return { ok: false, reason: "token-mismatch" }; } const requestedScopes = normalizeScopes(params.scopes); diff --git a/src/security/secret-equal.test.ts b/src/security/secret-equal.test.ts new file mode 100644 index 00000000000..e6c30e354ca --- /dev/null +++ b/src/security/secret-equal.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import { safeEqualSecret } from "./secret-equal.js"; + +describe("safeEqualSecret", () => { + it("matches identical secrets", () => { + expect(safeEqualSecret("secret-token", "secret-token")).toBe(true); + }); + + it("rejects mismatched secrets", () => { + expect(safeEqualSecret("secret-token", "secret-tokEn")).toBe(false); + }); + + it("rejects different-length secrets", () => { + expect(safeEqualSecret("short", "much-longer")).toBe(false); + }); + + it("rejects missing values", () => { + expect(safeEqualSecret(undefined, "secret")).toBe(false); + expect(safeEqualSecret("secret", undefined)).toBe(false); + expect(safeEqualSecret(null, "secret")).toBe(false); + }); +}); diff --git a/src/security/secret-equal.ts b/src/security/secret-equal.ts new file mode 100644 index 00000000000..4ea80b321f1 --- /dev/null +++ b/src/security/secret-equal.ts @@ -0,0 +1,16 @@ +import { timingSafeEqual } from "node:crypto"; + +export function safeEqualSecret( + provided: string | undefined | null, + expected: string | undefined | null, +): boolean { + if (typeof provided !== "string" || typeof expected !== "string") { + return false; + } + const providedBuffer = Buffer.from(provided); + const expectedBuffer = Buffer.from(expected); + if (providedBuffer.length !== expectedBuffer.length) { + return false; + } + return timingSafeEqual(providedBuffer, expectedBuffer); +}