diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c778ebad1c..5fd42829b46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -192,6 +192,7 @@ Docs: https://docs.openclaw.ai - Clawdock: avoid Zsh readonly variable collisions in helper scripts. (#15501) Thanks @nkelner. - Memory: switch default local embedding model to the QAT `embeddinggemma-300m-qat-Q8_0` variant for better quality at the same footprint. (#15429) Thanks @azade-c. - Docs/Mermaid: remove hardcoded Mermaid init theme blocks from four docs diagrams so dark mode inherits readable theme defaults. (#15157) Thanks @heytulsiprasad. +- Security/Pairing: generate 256-bit base64url device and node pairing tokens and use byte-safe constant-time verification to avoid token-compare edge-case failures. (#16535) Thanks @FaizanKolega, @gumadeiras. ## 2026.2.12 diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index 5604047265d..2335c2f7d74 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -10,19 +10,41 @@ import { verifyDeviceToken, } from "./device-pairing.js"; +async function setupPairedOperatorDevice(baseDir: string, scopes: string[]) { + const request = await requestDevicePairing( + { + deviceId: "device-1", + publicKey: "public-key-1", + role: "operator", + scopes, + }, + baseDir, + ); + await approveDevicePairing(request.request.requestId, baseDir); +} + +function requireToken(token: string | undefined): string { + expect(typeof token).toBe("string"); + if (typeof token !== "string") { + throw new Error("expected operator token to be issued"); + } + return token; +} + describe("device pairing tokens", () => { + test("generates base64url device tokens with 256-bit entropy output length", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); + await setupPairedOperatorDevice(baseDir, ["operator.admin"]); + + const paired = await getPairedDevice("device-1", baseDir); + const token = requireToken(paired?.tokens?.operator?.token); + expect(token).toMatch(/^[A-Za-z0-9_-]{43}$/); + expect(Buffer.from(token, "base64url")).toHaveLength(32); + }); + test("preserves existing token scopes when rotating without scopes", 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.admin"], - }, - baseDir, - ); - await approveDevicePairing(request.request.requestId, baseDir); + await setupPairedOperatorDevice(baseDir, ["operator.admin"]); await rotateDeviceToken({ deviceId: "device-1", @@ -45,23 +67,13 @@ describe("device pairing tokens", () => { 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); + await setupPairedOperatorDevice(baseDir, ["operator.read"]); const paired = await getPairedDevice("device-1", baseDir); - const token = paired?.tokens?.operator?.token; - expect(token).toBeTruthy(); + const token = requireToken(paired?.tokens?.operator?.token); const ok = await verifyDeviceToken({ deviceId: "device-1", - token: token ?? "", + token, role: "operator", scopes: ["operator.read"], baseDir, @@ -70,7 +82,7 @@ describe("device pairing tokens", () => { const mismatch = await verifyDeviceToken({ deviceId: "device-1", - token: "x".repeat((token ?? "1234").length), + token: "x".repeat(token.length), role: "operator", scopes: ["operator.read"], baseDir, @@ -78,4 +90,23 @@ describe("device pairing tokens", () => { expect(mismatch.ok).toBe(false); expect(mismatch.reason).toBe("token-mismatch"); }); + + test("treats multibyte same-length token input as mismatch without throwing", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); + await setupPairedOperatorDevice(baseDir, ["operator.read"]); + const paired = await getPairedDevice("device-1", baseDir); + const token = requireToken(paired?.tokens?.operator?.token); + const multibyteToken = "é".repeat(token.length); + expect(Buffer.from(multibyteToken).length).not.toBe(Buffer.from(token).length); + + await expect( + verifyDeviceToken({ + deviceId: "device-1", + token: multibyteToken, + role: "operator", + scopes: ["operator.read"], + baseDir, + }), + ).resolves.toEqual({ ok: false, reason: "token-mismatch" }); + }); }); diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 24379870811..41d786238db 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -1,5 +1,4 @@ import { randomUUID } from "node:crypto"; -import { safeEqualSecret } from "../security/secret-equal.js"; import { createAsyncLock, pruneExpiredPending, @@ -7,6 +6,7 @@ import { resolvePairingPaths, writeJsonAtomic, } from "./pairing-files.js"; +import { generatePairingToken, verifyPairingToken } from "./pairing-token.js"; export type DevicePairingPendingRequest = { requestId: string; @@ -176,7 +176,7 @@ function scopesAllow(requested: string[], allowed: string[]): boolean { } function newToken() { - return randomUUID().replaceAll("-", ""); + return generatePairingToken(); } export async function listDevicePairing(baseDir?: string): Promise { @@ -375,7 +375,7 @@ export async function verifyDeviceToken(params: { if (entry.revokedAtMs) { return { ok: false, reason: "token-revoked" }; } - if (!safeEqualSecret(params.token, entry.token)) { + if (!verifyPairingToken(params.token, entry.token)) { return { ok: false, reason: "token-mismatch" }; } const requestedScopes = normalizeScopes(params.scopes); diff --git a/src/infra/node-pairing.test.ts b/src/infra/node-pairing.test.ts new file mode 100644 index 00000000000..17c83c03500 --- /dev/null +++ b/src/infra/node-pairing.test.ts @@ -0,0 +1,60 @@ +import { mkdtemp } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, test } from "vitest"; +import { + approveNodePairing, + getPairedNode, + requestNodePairing, + verifyNodeToken, +} from "./node-pairing.js"; + +async function setupPairedNode(baseDir: string): Promise { + const request = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run"], + }, + baseDir, + ); + await approveNodePairing(request.request.requestId, baseDir); + const paired = await getPairedNode("node-1", baseDir); + expect(paired).not.toBeNull(); + if (!paired) { + throw new Error("expected node to be paired"); + } + return paired.token; +} + +describe("node pairing tokens", () => { + test("generates base64url node tokens with 256-bit entropy output length", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-node-pairing-")); + const token = await setupPairedNode(baseDir); + expect(token).toMatch(/^[A-Za-z0-9_-]{43}$/); + expect(Buffer.from(token, "base64url")).toHaveLength(32); + }); + + test("verifies token and rejects mismatches", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-node-pairing-")); + const token = await setupPairedNode(baseDir); + await expect(verifyNodeToken("node-1", token, baseDir)).resolves.toEqual({ + ok: true, + node: expect.objectContaining({ nodeId: "node-1" }), + }); + await expect(verifyNodeToken("node-1", "x".repeat(token.length), baseDir)).resolves.toEqual({ + ok: false, + }); + }); + + test("treats multibyte same-length token input as mismatch without throwing", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-node-pairing-")); + const token = await setupPairedNode(baseDir); + const multibyteToken = "é".repeat(token.length); + expect(Buffer.from(multibyteToken).length).not.toBe(Buffer.from(token).length); + + await expect(verifyNodeToken("node-1", multibyteToken, baseDir)).resolves.toEqual({ + ok: false, + }); + }); +}); diff --git a/src/infra/node-pairing.ts b/src/infra/node-pairing.ts index 0b1bf245b1d..88c428df13d 100644 --- a/src/infra/node-pairing.ts +++ b/src/infra/node-pairing.ts @@ -6,6 +6,7 @@ import { resolvePairingPaths, writeJsonAtomic, } from "./pairing-files.js"; +import { generatePairingToken, verifyPairingToken } from "./pairing-token.js"; export type NodePairingPendingRequest = { requestId: string; @@ -87,7 +88,7 @@ function normalizeNodeId(nodeId: string) { } function newToken() { - return randomUUID().replaceAll("-", ""); + return generatePairingToken(); } export async function listNodePairing(baseDir?: string): Promise { @@ -217,7 +218,7 @@ export async function verifyNodeToken( if (!node) { return { ok: false }; } - return node.token === token ? { ok: true, node } : { ok: false }; + return verifyPairingToken(token, node.token) ? { ok: true, node } : { ok: false }; } export async function updatePairedNodeMetadata( diff --git a/src/infra/pairing-token.ts b/src/infra/pairing-token.ts new file mode 100644 index 00000000000..96960da53b8 --- /dev/null +++ b/src/infra/pairing-token.ts @@ -0,0 +1,12 @@ +import { randomBytes } from "node:crypto"; +import { safeEqualSecret } from "../security/secret-equal.js"; + +export const PAIRING_TOKEN_BYTES = 32; + +export function generatePairingToken(): string { + return randomBytes(PAIRING_TOKEN_BYTES).toString("base64url"); +} + +export function verifyPairingToken(provided: string, expected: string): boolean { + return safeEqualSecret(provided, expected); +}