mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(gateway): enforce caller-scope subsetting in device.token.rotate
device.token.rotate accepted attacker-controlled scopes and forwarded them to rotateDeviceToken without verifying the caller held those scopes. A pairing-scoped token could rotate up to operator.admin on any already-paired device whose approvedScopes included admin. Add a caller-scope subsetting check before rotateDeviceToken: the requested scopes must be a subset of client.connect.scopes via the existing roleScopesAllow helper. Reject with missing scope: <scope> if not. Also add server.device-token-rotate-authz.test.ts covering both the priv-esc path and the admin-to-node-invoke chain. Fixes GHSA-4jpw-hj22-2xmc
This commit is contained in:
@@ -161,6 +161,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Cron/owner-only tools: pass trusted isolated cron runs into the embedded agent with owner context so `cron`/`gateway` tooling remains available after the owner-auth hardening narrowed direct-message ownership inference.
|
- Cron/owner-only tools: pass trusted isolated cron runs into the embedded agent with owner context so `cron`/`gateway` tooling remains available after the owner-auth hardening narrowed direct-message ownership inference.
|
||||||
- Browser/SSRF: block private-network intermediate redirect hops in strict browser navigation flows and fail closed when remote tab-open paths cannot inspect redirect chains. Thanks @zpbrent.
|
- Browser/SSRF: block private-network intermediate redirect hops in strict browser navigation flows and fail closed when remote tab-open paths cannot inspect redirect chains. Thanks @zpbrent.
|
||||||
- MS Teams/authz: keep `groupPolicy: "allowlist"` enforcing sender allowlists even when a team/channel route allowlist is configured, so route matches no longer widen group access to every sender in that route. Thanks @zpbrent.
|
- MS Teams/authz: keep `groupPolicy: "allowlist"` enforcing sender allowlists even when a team/channel route allowlist is configured, so route matches no longer widen group access to every sender in that route. Thanks @zpbrent.
|
||||||
|
- Security/Gateway: block `device.token.rotate` from minting operator scopes broader than the caller session already holds, closing the critical paired-device token privilege escalation reported as GHSA-4jpw-hj22-2xmc.
|
||||||
- Security/system.run: bind approved `bun` and `deno run` script operands to on-disk file snapshots so post-approval script rewrites are denied before execution.
|
- Security/system.run: bind approved `bun` and `deno run` script operands to on-disk file snapshots so post-approval script rewrites are denied before execution.
|
||||||
- Skills/download installs: pin the validated per-skill tools root before writing downloaded archives, so rebinding the lexical tools path cannot redirect download writes outside the intended tools directory. Thanks @tdjackey.
|
- Skills/download installs: pin the validated per-skill tools root before writing downloaded archives, so rebinding the lexical tools path cannot redirect download writes outside the intended tools directory. Thanks @tdjackey.
|
||||||
- Control UI/Debug: replace the Manual RPC free-text method field with a sorted dropdown sourced from gateway-advertised methods, and stack the form vertically for narrower layouts. (#14967) thanks @rixau.
|
- Control UI/Debug: replace the Manual RPC free-text method field with a sorted dropdown sourced from gateway-advertised methods, and stack the form vertically for narrower layouts. (#14967) thanks @rixau.
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import {
|
import {
|
||||||
approveDevicePairing,
|
approveDevicePairing,
|
||||||
|
getPairedDevice,
|
||||||
listDevicePairing,
|
listDevicePairing,
|
||||||
removePairedDevice,
|
removePairedDevice,
|
||||||
type DeviceAuthToken,
|
type DeviceAuthToken,
|
||||||
@@ -8,6 +9,8 @@ import {
|
|||||||
rotateDeviceToken,
|
rotateDeviceToken,
|
||||||
summarizeDeviceTokens,
|
summarizeDeviceTokens,
|
||||||
} from "../../infra/device-pairing.js";
|
} from "../../infra/device-pairing.js";
|
||||||
|
import { normalizeDeviceAuthScopes } from "../../shared/device-auth.js";
|
||||||
|
import { roleScopesAllow } from "../../shared/operator-scope-compat.js";
|
||||||
import {
|
import {
|
||||||
ErrorCodes,
|
ErrorCodes,
|
||||||
errorShape,
|
errorShape,
|
||||||
@@ -31,6 +34,25 @@ function redactPairedDevice(
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function resolveMissingRequestedScope(params: {
|
||||||
|
role: string;
|
||||||
|
requestedScopes: readonly string[];
|
||||||
|
callerScopes: readonly string[];
|
||||||
|
}): string | null {
|
||||||
|
for (const scope of params.requestedScopes) {
|
||||||
|
if (
|
||||||
|
!roleScopesAllow({
|
||||||
|
role: params.role,
|
||||||
|
requestedScopes: [scope],
|
||||||
|
allowedScopes: params.callerScopes,
|
||||||
|
})
|
||||||
|
) {
|
||||||
|
return scope;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
export const deviceHandlers: GatewayRequestHandlers = {
|
export const deviceHandlers: GatewayRequestHandlers = {
|
||||||
"device.pair.list": async ({ params, respond }) => {
|
"device.pair.list": async ({ params, respond }) => {
|
||||||
if (!validateDevicePairListParams(params)) {
|
if (!validateDevicePairListParams(params)) {
|
||||||
@@ -146,7 +168,7 @@ export const deviceHandlers: GatewayRequestHandlers = {
|
|||||||
context.logGateway.info(`device pairing removed device=${removed.deviceId}`);
|
context.logGateway.info(`device pairing removed device=${removed.deviceId}`);
|
||||||
respond(true, removed, undefined);
|
respond(true, removed, undefined);
|
||||||
},
|
},
|
||||||
"device.token.rotate": async ({ params, respond, context }) => {
|
"device.token.rotate": async ({ params, respond, context, client }) => {
|
||||||
if (!validateDeviceTokenRotateParams(params)) {
|
if (!validateDeviceTokenRotateParams(params)) {
|
||||||
respond(
|
respond(
|
||||||
false,
|
false,
|
||||||
@@ -165,6 +187,28 @@ export const deviceHandlers: GatewayRequestHandlers = {
|
|||||||
role: string;
|
role: string;
|
||||||
scopes?: string[];
|
scopes?: string[];
|
||||||
};
|
};
|
||||||
|
const pairedDevice = await getPairedDevice(deviceId);
|
||||||
|
if (!pairedDevice) {
|
||||||
|
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role"));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
|
||||||
|
const requestedScopes = normalizeDeviceAuthScopes(
|
||||||
|
scopes ?? pairedDevice.tokens?.[role.trim()]?.scopes ?? pairedDevice.scopes,
|
||||||
|
);
|
||||||
|
const missingScope = resolveMissingRequestedScope({
|
||||||
|
role,
|
||||||
|
requestedScopes,
|
||||||
|
callerScopes,
|
||||||
|
});
|
||||||
|
if (missingScope) {
|
||||||
|
respond(
|
||||||
|
false,
|
||||||
|
undefined,
|
||||||
|
errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${missingScope}`),
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
const entry = await rotateDeviceToken({ deviceId, role, scopes });
|
const entry = await rotateDeviceToken({ deviceId, role, scopes });
|
||||||
if (!entry) {
|
if (!entry) {
|
||||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role"));
|
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role"));
|
||||||
|
|||||||
284
src/gateway/server.device-token-rotate-authz.test.ts
Normal file
284
src/gateway/server.device-token-rotate-authz.test.ts
Normal file
@@ -0,0 +1,284 @@
|
|||||||
|
import os from "node:os";
|
||||||
|
import path from "node:path";
|
||||||
|
import { describe, expect, test } from "vitest";
|
||||||
|
import { WebSocket } from "ws";
|
||||||
|
import {
|
||||||
|
loadOrCreateDeviceIdentity,
|
||||||
|
publicKeyRawBase64UrlFromPem,
|
||||||
|
type DeviceIdentity,
|
||||||
|
} from "../infra/device-identity.js";
|
||||||
|
import {
|
||||||
|
approveDevicePairing,
|
||||||
|
getPairedDevice,
|
||||||
|
requestDevicePairing,
|
||||||
|
rotateDeviceToken,
|
||||||
|
} from "../infra/device-pairing.js";
|
||||||
|
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
|
||||||
|
import { GatewayClient } from "./client.js";
|
||||||
|
import {
|
||||||
|
connectOk,
|
||||||
|
installGatewayTestHooks,
|
||||||
|
rpcReq,
|
||||||
|
startServerWithClient,
|
||||||
|
trackConnectChallengeNonce,
|
||||||
|
} from "./test-helpers.js";
|
||||||
|
|
||||||
|
installGatewayTestHooks({ scope: "suite" });
|
||||||
|
|
||||||
|
function resolveDeviceIdentityPath(name: string): string {
|
||||||
|
const root = process.env.OPENCLAW_STATE_DIR ?? process.env.HOME ?? os.tmpdir();
|
||||||
|
return path.join(root, "test-device-identities", `${name}.json`);
|
||||||
|
}
|
||||||
|
|
||||||
|
function loadDeviceIdentity(name: string): {
|
||||||
|
identityPath: string;
|
||||||
|
identity: DeviceIdentity;
|
||||||
|
publicKey: string;
|
||||||
|
} {
|
||||||
|
const identityPath = resolveDeviceIdentityPath(name);
|
||||||
|
const identity = loadOrCreateDeviceIdentity(identityPath);
|
||||||
|
return {
|
||||||
|
identityPath,
|
||||||
|
identity,
|
||||||
|
publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
async function pairDevice(params: {
|
||||||
|
name: string;
|
||||||
|
role: "node" | "operator";
|
||||||
|
scopes: string[];
|
||||||
|
clientId?: string;
|
||||||
|
clientMode?: string;
|
||||||
|
}): Promise<{
|
||||||
|
identityPath: string;
|
||||||
|
identity: DeviceIdentity;
|
||||||
|
}> {
|
||||||
|
const loaded = loadDeviceIdentity(params.name);
|
||||||
|
const request = await requestDevicePairing({
|
||||||
|
deviceId: loaded.identity.deviceId,
|
||||||
|
publicKey: loaded.publicKey,
|
||||||
|
role: params.role,
|
||||||
|
scopes: params.scopes,
|
||||||
|
clientId: params.clientId,
|
||||||
|
clientMode: params.clientMode,
|
||||||
|
});
|
||||||
|
await approveDevicePairing(request.request.requestId);
|
||||||
|
return {
|
||||||
|
identityPath: loaded.identityPath,
|
||||||
|
identity: loaded.identity,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
async function issuePairingScopedTokenForAdminApprovedDevice(name: string): Promise<{
|
||||||
|
deviceId: string;
|
||||||
|
identityPath: string;
|
||||||
|
pairingToken: string;
|
||||||
|
}> {
|
||||||
|
const paired = await pairDevice({
|
||||||
|
name,
|
||||||
|
role: "operator",
|
||||||
|
scopes: ["operator.admin"],
|
||||||
|
clientId: GATEWAY_CLIENT_NAMES.TEST,
|
||||||
|
clientMode: GATEWAY_CLIENT_MODES.TEST,
|
||||||
|
});
|
||||||
|
const rotated = await rotateDeviceToken({
|
||||||
|
deviceId: paired.identity.deviceId,
|
||||||
|
role: "operator",
|
||||||
|
scopes: ["operator.pairing"],
|
||||||
|
});
|
||||||
|
expect(rotated?.token).toBeTruthy();
|
||||||
|
return {
|
||||||
|
deviceId: paired.identity.deviceId,
|
||||||
|
identityPath: paired.identityPath,
|
||||||
|
pairingToken: String(rotated?.token ?? ""),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
async function openTrackedWs(port: number): Promise<WebSocket> {
|
||||||
|
const ws = new WebSocket(`ws://127.0.0.1:${port}`);
|
||||||
|
trackConnectChallengeNonce(ws);
|
||||||
|
await new Promise<void>((resolve, reject) => {
|
||||||
|
const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), 5_000);
|
||||||
|
ws.once("open", () => {
|
||||||
|
clearTimeout(timer);
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
ws.once("error", (error) => {
|
||||||
|
clearTimeout(timer);
|
||||||
|
reject(error);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
return ws;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function connectPairingScopedOperator(params: {
|
||||||
|
port: number;
|
||||||
|
identityPath: string;
|
||||||
|
deviceToken: string;
|
||||||
|
}): Promise<WebSocket> {
|
||||||
|
const ws = await openTrackedWs(params.port);
|
||||||
|
await connectOk(ws, {
|
||||||
|
skipDefaultAuth: true,
|
||||||
|
deviceToken: params.deviceToken,
|
||||||
|
deviceIdentityPath: params.identityPath,
|
||||||
|
scopes: ["operator.pairing"],
|
||||||
|
});
|
||||||
|
return ws;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function connectApprovedNode(params: {
|
||||||
|
port: number;
|
||||||
|
name: string;
|
||||||
|
onInvoke: (payload: unknown) => void;
|
||||||
|
}): Promise<GatewayClient> {
|
||||||
|
const paired = await pairDevice({
|
||||||
|
name: params.name,
|
||||||
|
role: "node",
|
||||||
|
scopes: [],
|
||||||
|
clientId: GATEWAY_CLIENT_NAMES.NODE_HOST,
|
||||||
|
clientMode: GATEWAY_CLIENT_MODES.NODE,
|
||||||
|
});
|
||||||
|
|
||||||
|
let readyResolve: (() => void) | null = null;
|
||||||
|
const ready = new Promise<void>((resolve) => {
|
||||||
|
readyResolve = resolve;
|
||||||
|
});
|
||||||
|
|
||||||
|
const client = new GatewayClient({
|
||||||
|
url: `ws://127.0.0.1:${params.port}`,
|
||||||
|
connectDelayMs: 2_000,
|
||||||
|
token: "secret",
|
||||||
|
role: "node",
|
||||||
|
clientName: GATEWAY_CLIENT_NAMES.NODE_HOST,
|
||||||
|
clientVersion: "1.0.0",
|
||||||
|
platform: "linux",
|
||||||
|
mode: GATEWAY_CLIENT_MODES.NODE,
|
||||||
|
scopes: [],
|
||||||
|
commands: ["system.run"],
|
||||||
|
deviceIdentity: paired.identity,
|
||||||
|
onHelloOk: () => readyResolve?.(),
|
||||||
|
onEvent: (event) => {
|
||||||
|
if (event.event !== "node.invoke.request") {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
params.onInvoke(event.payload);
|
||||||
|
const payload = event.payload as { id?: string; nodeId?: string };
|
||||||
|
if (!payload.id || !payload.nodeId) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
void client.request("node.invoke.result", {
|
||||||
|
id: payload.id,
|
||||||
|
nodeId: payload.nodeId,
|
||||||
|
ok: true,
|
||||||
|
payloadJSON: JSON.stringify({ ok: true }),
|
||||||
|
});
|
||||||
|
},
|
||||||
|
});
|
||||||
|
client.start();
|
||||||
|
await Promise.race([
|
||||||
|
ready,
|
||||||
|
new Promise<never>((_, reject) => {
|
||||||
|
setTimeout(() => reject(new Error("timeout waiting for node hello")), 5_000);
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
return client;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function getConnectedNodeId(ws: WebSocket): Promise<string> {
|
||||||
|
const nodes = await rpcReq<{ nodes?: Array<{ nodeId: string; connected?: boolean }> }>(
|
||||||
|
ws,
|
||||||
|
"node.list",
|
||||||
|
{},
|
||||||
|
);
|
||||||
|
expect(nodes.ok).toBe(true);
|
||||||
|
const nodeId = nodes.payload?.nodes?.find((node) => node.connected)?.nodeId ?? "";
|
||||||
|
expect(nodeId).toBeTruthy();
|
||||||
|
return nodeId;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function waitForMacrotasks(): Promise<void> {
|
||||||
|
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||||
|
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("gateway device.token.rotate caller scope guard", () => {
|
||||||
|
test("rejects rotating an admin-approved device token above the caller session scopes", async () => {
|
||||||
|
const started = await startServerWithClient("secret");
|
||||||
|
const attacker = await issuePairingScopedTokenForAdminApprovedDevice("rotate-attacker");
|
||||||
|
|
||||||
|
let pairingWs: WebSocket | undefined;
|
||||||
|
try {
|
||||||
|
pairingWs = await connectPairingScopedOperator({
|
||||||
|
port: started.port,
|
||||||
|
identityPath: attacker.identityPath,
|
||||||
|
deviceToken: attacker.pairingToken,
|
||||||
|
});
|
||||||
|
|
||||||
|
const rotate = await rpcReq(pairingWs, "device.token.rotate", {
|
||||||
|
deviceId: attacker.deviceId,
|
||||||
|
role: "operator",
|
||||||
|
scopes: ["operator.admin"],
|
||||||
|
});
|
||||||
|
expect(rotate.ok).toBe(false);
|
||||||
|
expect(rotate.error?.message).toBe("missing scope: operator.admin");
|
||||||
|
|
||||||
|
const paired = await getPairedDevice(attacker.deviceId);
|
||||||
|
expect(paired?.tokens?.operator?.scopes).toEqual(["operator.pairing"]);
|
||||||
|
expect(paired?.approvedScopes).toEqual(["operator.admin"]);
|
||||||
|
} finally {
|
||||||
|
pairingWs?.close();
|
||||||
|
started.ws.close();
|
||||||
|
await started.server.close();
|
||||||
|
started.envSnapshot.restore();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("blocks the pairing-token to admin-node-invoke escalation chain", async () => {
|
||||||
|
const started = await startServerWithClient("secret");
|
||||||
|
const attacker = await issuePairingScopedTokenForAdminApprovedDevice("rotate-rce-attacker");
|
||||||
|
|
||||||
|
let sawInvoke = false;
|
||||||
|
let pairingWs: WebSocket | undefined;
|
||||||
|
let nodeClient: GatewayClient | undefined;
|
||||||
|
|
||||||
|
try {
|
||||||
|
await connectOk(started.ws);
|
||||||
|
nodeClient = await connectApprovedNode({
|
||||||
|
port: started.port,
|
||||||
|
name: "rotate-rce-node",
|
||||||
|
onInvoke: () => {
|
||||||
|
sawInvoke = true;
|
||||||
|
},
|
||||||
|
});
|
||||||
|
await getConnectedNodeId(started.ws);
|
||||||
|
|
||||||
|
pairingWs = await connectPairingScopedOperator({
|
||||||
|
port: started.port,
|
||||||
|
identityPath: attacker.identityPath,
|
||||||
|
deviceToken: attacker.pairingToken,
|
||||||
|
});
|
||||||
|
|
||||||
|
const rotate = await rpcReq<{ token?: string }>(pairingWs, "device.token.rotate", {
|
||||||
|
deviceId: attacker.deviceId,
|
||||||
|
role: "operator",
|
||||||
|
scopes: ["operator.admin"],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(rotate.ok).toBe(false);
|
||||||
|
expect(rotate.error?.message).toBe("missing scope: operator.admin");
|
||||||
|
await waitForMacrotasks();
|
||||||
|
expect(sawInvoke).toBe(false);
|
||||||
|
|
||||||
|
const paired = await getPairedDevice(attacker.deviceId);
|
||||||
|
expect(paired?.tokens?.operator?.scopes).toEqual(["operator.pairing"]);
|
||||||
|
expect(paired?.tokens?.operator?.token).toBe(attacker.pairingToken);
|
||||||
|
} finally {
|
||||||
|
pairingWs?.close();
|
||||||
|
nodeClient?.stop();
|
||||||
|
started.ws.close();
|
||||||
|
await started.server.close();
|
||||||
|
started.envSnapshot.restore();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user