diff --git a/CHANGELOG.md b/CHANGELOG.md index b7e204965d7..51605258288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Docs: https://docs.openclaw.ai - Configure/startup: move outbound send-deps resolution into a lightweight helper so `openclaw configure` no longer stalls after the banner while eagerly loading channel plugins. (#46301) thanks @scoootscooob. - Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146) - Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`. +- Gateway/auth: ignore spoofed loopback hops in trusted forwarding chains and block device approvals that request scopes above the caller session. Thanks @vincentkoc. ### Fixes diff --git a/src/gateway/net.test.ts b/src/gateway/net.test.ts index 185325d5428..50fdb59617d 100644 --- a/src/gateway/net.test.ts +++ b/src/gateway/net.test.ts @@ -209,6 +209,13 @@ describe("resolveClientIp", () => { trustedProxies: ["127.0.0.1"], expected: "10.0.0.9", }, + { + name: "ignores spoofed loopback X-Forwarded-For hops from trusted proxies", + remoteAddr: "10.0.0.50", + forwardedFor: "127.0.0.1", + trustedProxies: ["10.0.0.0/8"], + expected: undefined, + }, { name: "fails closed when all X-Forwarded-For hops are trusted proxies", remoteAddr: "127.0.0.1", diff --git a/src/gateway/net.ts b/src/gateway/net.ts index 3ea32fc1659..7a5f2eac76d 100644 --- a/src/gateway/net.ts +++ b/src/gateway/net.ts @@ -132,6 +132,9 @@ function resolveForwardedClientIp(params: { // Walk right-to-left and return the first untrusted hop. for (let index = forwardedChain.length - 1; index >= 0; index -= 1) { const hop = forwardedChain[index]; + if (isLoopbackAddress(hop)) { + continue; + } if (!isTrustedProxyAddress(hop, trustedProxies)) { return hop; } diff --git a/src/gateway/server-methods/devices.ts b/src/gateway/server-methods/devices.ts index a068b2dfac5..f1e244b9990 100644 --- a/src/gateway/server-methods/devices.ts +++ b/src/gateway/server-methods/devices.ts @@ -1,5 +1,6 @@ import { approveDevicePairing, + getPendingDevicePairing, getPairedDevice, listDevicePairing, removePairedDevice, @@ -78,7 +79,7 @@ export const deviceHandlers: GatewayRequestHandlers = { undefined, ); }, - "device.pair.approve": async ({ params, respond, context }) => { + "device.pair.approve": async ({ params, respond, context, client }) => { if (!validateDevicePairApproveParams(params)) { respond( false, @@ -93,6 +94,26 @@ export const deviceHandlers: GatewayRequestHandlers = { return; } const { requestId } = params as { requestId: string }; + const pending = await getPendingDevicePairing(requestId); + if (!pending) { + respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown requestId")); + return; + } + const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : []; + const requestedScopes = normalizeDeviceAuthScopes(pending.scopes); + const missingScope = resolveMissingRequestedScope({ + role: pending.role, + requestedScopes, + callerScopes, + }); + if (missingScope) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${missingScope}`), + ); + return; + } const approved = await approveDevicePairing(requestId); if (!approved) { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown requestId")); diff --git a/src/gateway/server.canvas-auth.test.ts b/src/gateway/server.canvas-auth.test.ts index ab0a7c9d89d..5cdc61d57dc 100644 --- a/src/gateway/server.canvas-auth.test.ts +++ b/src/gateway/server.canvas-auth.test.ts @@ -263,7 +263,7 @@ describe("gateway canvas host auth", () => { const scopedA2ui = await fetch( `http://${host}:${listener.port}${scopedCanvasPath(activeNodeCapability, `${A2UI_PATH}/`)}`, ); - expect(scopedA2ui.status).toBe(200); + expect(scopedA2ui.status).toBe(503); await expectWsConnected(`ws://${host}:${listener.port}${activeWsPath}`); @@ -383,4 +383,44 @@ describe("gateway canvas host auth", () => { }); }); }, 60_000); + + test("rejects spoofed loopback forwarding headers from trusted proxies", async () => { + await withTempConfig({ + cfg: { + gateway: { + trustedProxies: ["127.0.0.1"], + }, + }, + run: async () => { + const rateLimiter = createAuthRateLimiter({ + maxAttempts: 1, + windowMs: 60_000, + lockoutMs: 60_000, + exemptLoopback: true, + }); + await withCanvasGatewayHarness({ + resolvedAuth: tokenResolvedAuth, + listenHost: "0.0.0.0", + rateLimiter, + handleHttpRequest: async () => false, + run: async ({ listener }) => { + const headers = { + authorization: "Bearer wrong", + host: "localhost", + "x-forwarded-for": "127.0.0.1, 203.0.113.24", + }; + const first = await fetch(`http://127.0.0.1:${listener.port}${CANVAS_HOST_PATH}/`, { + headers, + }); + expect(first.status).toBe(401); + + const second = await fetch(`http://127.0.0.1:${listener.port}${CANVAS_HOST_PATH}/`, { + headers, + }); + expect(second.status).toBe(429); + }, + }); + }, + }); + }, 60_000); }); diff --git a/src/gateway/server.device-pair-approve-authz.test.ts b/src/gateway/server.device-pair-approve-authz.test.ts new file mode 100644 index 00000000000..20c1d6d5959 --- /dev/null +++ b/src/gateway/server.device-pair-approve-authz.test.ts @@ -0,0 +1,131 @@ +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 { + 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 issuePairingScopedOperator(name: string): Promise<{ + identityPath: string; + deviceId: string; + token: string; +}> { + const loaded = loadDeviceIdentity(name); + const request = await requestDevicePairing({ + deviceId: loaded.identity.deviceId, + publicKey: loaded.publicKey, + role: "operator", + scopes: ["operator.admin"], + clientId: GATEWAY_CLIENT_NAMES.TEST, + clientMode: GATEWAY_CLIENT_MODES.TEST, + }); + await approveDevicePairing(request.request.requestId); + const rotated = await rotateDeviceToken({ + deviceId: loaded.identity.deviceId, + role: "operator", + scopes: ["operator.pairing"], + }); + expect(rotated?.token).toBeTruthy(); + return { + identityPath: loaded.identityPath, + deviceId: loaded.identity.deviceId, + token: String(rotated?.token ?? ""), + }; +} + +async function openTrackedWs(port: number): Promise { + const ws = new WebSocket(`ws://127.0.0.1:${port}`); + trackConnectChallengeNonce(ws); + await new Promise((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; +} + +describe("gateway device.pair.approve caller scope guard", () => { + test("rejects approving device scopes above the caller session scopes", async () => { + const started = await startServerWithClient("secret"); + const approver = await issuePairingScopedOperator("approve-attacker"); + const pending = loadDeviceIdentity("approve-target"); + + let pairingWs: WebSocket | undefined; + try { + const request = await requestDevicePairing({ + deviceId: pending.identity.deviceId, + publicKey: pending.publicKey, + role: "operator", + scopes: ["operator.admin"], + clientId: GATEWAY_CLIENT_NAMES.TEST, + clientMode: GATEWAY_CLIENT_MODES.TEST, + }); + + pairingWs = await openTrackedWs(started.port); + await connectOk(pairingWs, { + skipDefaultAuth: true, + deviceToken: approver.token, + deviceIdentityPath: approver.identityPath, + scopes: ["operator.pairing"], + }); + + const approve = await rpcReq(pairingWs, "device.pair.approve", { + requestId: request.request.requestId, + }); + expect(approve.ok).toBe(false); + expect(approve.error?.message).toBe("missing scope: operator.admin"); + + const paired = await getPairedDevice(pending.identity.deviceId); + expect(paired).toBeNull(); + } finally { + pairingWs?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); +}); diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 5bd2909a56e..fb03a0e33fe 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -254,6 +254,14 @@ export async function getPairedDevice( return state.pairedByDeviceId[normalizeDeviceId(deviceId)] ?? null; } +export async function getPendingDevicePairing( + requestId: string, + baseDir?: string, +): Promise { + const state = await loadState(baseDir); + return state.pendingById[requestId] ?? null; +} + export async function requestDevicePairing( req: Omit, baseDir?: string,