fix: harden device pairing token generation and verification (#16535)
Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: bcbb50e3683b12643d8eb2ef3fde74dd3a3ac4a7 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
This commit is contained in:
parent
b97191b81a
commit
48b3d7096c
@ -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
|
||||
|
||||
|
||||
@ -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" });
|
||||
});
|
||||
});
|
||||
|
||||
@ -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<DevicePairingList> {
|
||||
@ -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);
|
||||
|
||||
60
src/infra/node-pairing.test.ts
Normal file
60
src/infra/node-pairing.test.ts
Normal file
@ -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<string> {
|
||||
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,
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -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<NodePairingList> {
|
||||
@ -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(
|
||||
|
||||
12
src/infra/pairing-token.ts
Normal file
12
src/infra/pairing-token.ts
Normal file
@ -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);
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user