fix(security): harden hook and device token auth
This commit is contained in:
parent
54513f4240
commit
113ebfd6a2
@ -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.
|
||||
|
||||
@ -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.
|
||||
|
||||
@ -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<TailscaleWhoisIdentity | null>;
|
||||
|
||||
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" };
|
||||
|
||||
@ -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<typeof createSubsystemLogger>;
|
||||
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<string, HookAuthFailure>();
|
||||
|
||||
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;
|
||||
|
||||
@ -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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
@ -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);
|
||||
|
||||
22
src/security/secret-equal.test.ts
Normal file
22
src/security/secret-equal.test.ts
Normal file
@ -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);
|
||||
});
|
||||
});
|
||||
16
src/security/secret-equal.ts
Normal file
16
src/security/secret-equal.ts
Normal file
@ -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);
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user