diff --git a/changelog.d/fixes/12571-vnc-cdp-bridge-auth.md b/changelog.d/fixes/12571-vnc-cdp-bridge-auth.md new file mode 100644 index 0000000000..83cbb6cf8e --- /dev/null +++ b/changelog.d/fixes/12571-vnc-cdp-bridge-auth.md @@ -0,0 +1 @@ +- fix(docker): require a per-session token on the VNC browser CDP bridge and isolate it on a dedicated Docker network (#12571) diff --git a/docker/vnc-browser/chromium/Dockerfile b/docker/vnc-browser/chromium/Dockerfile index 579244536e..0d79c494b2 100644 --- a/docker/vnc-browser/chromium/Dockerfile +++ b/docker/vnc-browser/chromium/Dockerfile @@ -8,6 +8,14 @@ # Chrome 150 ignores --remote-debugging-address and binds loopback only. # The OmniRoute server harvests cookies over the host-mapped 9223. # +# SECURITY (#12571): 9223 is gated by a per-session shared secret +# (CDP_BRIDGE_TOKEN, injected via `-e` by src/lib/vncSession/service.ts) that +# every caller must present as an `X-Omni-Cdp-Token` header before the bridge +# forwards a single byte to Chromium — see cdp-bridge.py for the check. The +# container also runs on a dedicated Docker network (not the default bridge) +# so sibling containers can't reach 9223 either. Do not remove either control +# or the CDP bridge reverts to an unauthenticated, full-session-takeover proxy. +# # Alpine/Debian package mirrors are unreachable from the build sandbox, so we # extend a prebuilt image rather than apt/apk-installing anything. FROM linuxserver/chromium:latest diff --git a/docker/vnc-browser/chromium/cdp-bridge.py b/docker/vnc-browser/chromium/cdp-bridge.py index 7158f8dc1c..251c9f1208 100644 --- a/docker/vnc-browser/chromium/cdp-bridge.py +++ b/docker/vnc-browser/chromium/cdp-bridge.py @@ -5,19 +5,64 @@ Chrome binds DevTools to 127.0.0.1 only and ignores --remote-debugging-address on recent versions, so the host can't reach it via `docker -p 9222:9222`. This tiny TCP bridge (run inside the container) exposes the same CDP on all interfaces so the OmniRoute server's VNC harvester can connect from the host. + +SECURITY (#12571): 9223 is reachable by any sibling container on the same +Docker bridge network, not just the host, and CDP grants full control over a +live, credential-bearing browser session (Runtime.evaluate, cookie theft, +etc). Every connection MUST present the shared secret in CDP_BRIDGE_TOKEN +(env, injected per-session by src/lib/vncSession/service.ts) as an +`X-Omni-Cdp-Token: ` header on its first HTTP request/WS-upgrade +before a single byte is forwarded upstream. A missing/invalid token gets the +connection closed immediately with no response, so probing gives no signal. """ -import socket, threading, sys +import os, socket, threading, sys SRC_HOST, SRC_PORT = "127.0.0.1", 9222 PUB_HOST, PUB_PORT = "0.0.0.0", 9223 +TOKEN = os.environ.get("CDP_BRIDGE_TOKEN", "") +TOKEN_HEADER = f"x-omni-cdp-token: {TOKEN}".lower() +PEEK_TIMEOUT_S = 5 +MAX_PEEK_BYTES = 8192 + + +def has_valid_token(initial_chunk: bytes) -> bool: + """Check whether the client's first bytes carry the configured secret. + + A missing/empty TOKEN always fails closed (no caller can present a valid + empty header line the way this check is written). + """ + if not TOKEN: + return False + try: + text = initial_chunk.decode("latin-1", errors="ignore").lower() + except (UnicodeDecodeError, LookupError): + return False + return TOKEN_HEADER in text + + +def read_initial_chunk(client): + client.settimeout(PEEK_TIMEOUT_S) + try: + return client.recv(MAX_PEEK_BYTES) + except OSError: + return b"" + finally: + client.settimeout(None) def bridge(client, target_addr): + initial = read_initial_chunk(client) + if not has_valid_token(initial): + client.close() + return + try: upstream = socket.create_connection(target_addr, timeout=10) + upstream.sendall(initial) except OSError: client.close() return + a = threading.Thread(target=pipe, args=(client, upstream), daemon=True) b = threading.Thread(target=pipe, args=(upstream, client), daemon=True) a.start(); b.start() diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index f357f73584..5708b56760 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -1440,6 +1440,7 @@ Containerized Chromium+VNC used for interactive browser-login credential capture | `OMNIROUTE_VNC_READY_MS` | `45000` | `src/lib/vncSession/manifest.ts` | Timeout (ms) waiting for the containerized browser to become CDP-ready. | | `OMNIROUTE_VNC_HARVEST_MS` | `20000` | `src/lib/vncSession/manifest.ts` | Timeout (ms) for harvesting the captured session/cookies after login completes. | | `OMNIROUTE_VNC_CHROMIUM_ARGS` | `--remote-debugging-port=9222 --no-first-run --no-default-browser-check` | `src/lib/vncSession/manifest.ts` | Extra command-line flags passed to the containerized Chromium. | +| `OMNIROUTE_VNC_NETWORK` | `omniroute-vnc-browser-login` | `src/lib/vncSession/manifest.ts` | Dedicated Docker network the VNC login container joins (#12571) instead of the default bridge, so sibling containers can't reach its CDP bridge port. | | `VIBEPROXY_DATA_DIR` | _(unset)_ | `open-sse/services/notionThreadSessions.ts` | **Legacy alias** for `DATA_DIR`, checked only after both `DATA_DIR` and `OMNIROUTE_DATA_DIR` are unset. Locates the Notion web-thread session cache (`/notion-web-thread-sessions.json`). | --- @@ -1578,6 +1579,7 @@ Used by `src/lib/vncSession/manifest.ts` to configure Docker-based headless Chro | `OMNIROUTE_VNC_MAX_SESSIONS` | `4` | `src/lib/vncSession/manifest.ts` | Maximum concurrent VNC sessions. | | `OMNIROUTE_VNC_READY_MS` | `45000` | `src/lib/vncSession/manifest.ts` | Browser readiness timeout (ms). | | `OMNIROUTE_VNC_HARVEST_MS` | `20000` | `src/lib/vncSession/manifest.ts` | Harvest/cleanup timeout (ms). | +| `OMNIROUTE_VNC_NETWORK` | `omniroute-vnc-browser-login` | `src/lib/vncSession/manifest.ts` | Dedicated Docker network the container joins (#12571), off the default bridge. | | `VIBEPROXY_DATA_DIR` | _(unset)_ | `open-sse/services/notionThreadSessions.ts` | Directory for Notion thread session persistence. | ### Internal service auth diff --git a/src/lib/vncSession/harvest.ts b/src/lib/vncSession/harvest.ts index b95c51735e..4791471395 100644 --- a/src/lib/vncSession/harvest.ts +++ b/src/lib/vncSession/harvest.ts @@ -17,6 +17,9 @@ export interface HarvestResult { hasCredential: boolean; } +/** Header name the CDP bridge (docker/vnc-browser/chromium/cdp-bridge.py) requires (#12571). */ +const CDP_TOKEN_HEADER = "X-Omni-Cdp-Token"; + interface Pending { resolve: (value: any) => void; reject: (error: Error) => void; @@ -36,8 +39,8 @@ class CdpClient { private sessionId: string | null = null; private closed = false; - constructor(wsUrl: string) { - this.ws = new WebSocket(wsUrl); + constructor(wsUrl: string, cdpToken: string) { + this.ws = new WebSocket(wsUrl, { headers: { [CDP_TOKEN_HEADER]: cdpToken } }); this.ws.on("message", (data) => this.onMessage(data)); this.ws.on("close", () => this.rejectAll(new Error("CDP websocket closed"))); this.ws.on("error", (error) => this.rejectAll(toError(error, "CDP websocket error"))); @@ -252,7 +255,11 @@ class CdpClient { } } -export async function waitForCdpReady(cdpPort: number, timeoutMs: number): Promise { +export async function waitForCdpReady( + cdpPort: number, + timeoutMs: number, + cdpToken: string +): Promise { const deadline = Date.now() + timeoutMs; let lastError: Error | null = null; @@ -260,7 +267,11 @@ export async function waitForCdpReady(cdpPort: number, timeoutMs: number): Promi const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 2_000); try { - const version = await fetchJson(`http://127.0.0.1:${cdpPort}/json/version`, controller.signal); + const version = await fetchJson( + `http://127.0.0.1:${cdpPort}/json/version`, + controller.signal, + cdpToken + ); if (version?.webSocketDebuggerUrl) return; lastError = new Error("CDP endpoint did not return a websocket URL"); } catch (error) { @@ -277,7 +288,8 @@ export async function waitForCdpReady(cdpPort: number, timeoutMs: number): Promi export async function harvestFromContainer( cdpPort: number, provider: VncProviderEntry, - timeoutMs = 20_000 + timeoutMs = 20_000, + cdpToken = "" ): Promise { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); @@ -286,14 +298,15 @@ export async function harvestFromContainer( try { const version = await fetchJson( `http://127.0.0.1:${cdpPort}/json/version`, - controller.signal + controller.signal, + cdpToken ); const debuggerUrl = version?.webSocketDebuggerUrl; if (typeof debuggerUrl !== "string" || !debuggerUrl) { throw new Error("No CDP websocket endpoint from browser container"); } - client = new CdpClient(rewriteDebuggerUrl(debuggerUrl, cdpPort)); + client = new CdpClient(rewriteDebuggerUrl(debuggerUrl, cdpPort), cdpToken); await client.ready(Math.min(timeoutMs, 15_000), controller.signal); const origin = new URL(provider.url).origin; @@ -403,8 +416,8 @@ function safeOrigin(value: string | undefined): string | null { } } -async function fetchJson(url: string, signal: AbortSignal): Promise { - const response = await fetch(url, { signal }); +async function fetchJson(url: string, signal: AbortSignal, cdpToken = ""): Promise { + const response = await fetch(url, { signal, headers: { [CDP_TOKEN_HEADER]: cdpToken } }); if (!response.ok) throw new Error(`CDP endpoint returned HTTP ${response.status}`); return response.json(); } diff --git a/src/lib/vncSession/manifest.ts b/src/lib/vncSession/manifest.ts index f419fc7e13..c4bf47e6eb 100644 --- a/src/lib/vncSession/manifest.ts +++ b/src/lib/vncSession/manifest.ts @@ -86,6 +86,12 @@ export const VNC_CONFIG = { maxSessionMs: Number(process.env.OMNIROUTE_VNC_MAX_MS || 30 * 60 * 1000), maxSessions: Number(process.env.OMNIROUTE_VNC_MAX_SESSIONS || 4), dockerBin: process.env.OMNIROUTE_DOCKER_BIN || "docker", + /** + * Dedicated bridge network for browser-login containers (#12571): keeps + * them off Docker's default bridge network so sibling containers can't + * reach the CDP bridge port over the container-to-container path. + */ + network: process.env.OMNIROUTE_VNC_NETWORK || "omniroute-vnc-browser-login", browserReadyTimeoutMs: Number(process.env.OMNIROUTE_VNC_READY_MS || 45_000), harvestTimeoutMs: Number(process.env.OMNIROUTE_VNC_HARVEST_MS || 20_000), chromiumArgs: diff --git a/src/lib/vncSession/service.ts b/src/lib/vncSession/service.ts index 92f57ce5f0..452913401f 100644 --- a/src/lib/vncSession/service.ts +++ b/src/lib/vncSession/service.ts @@ -1,5 +1,5 @@ import { spawn } from "node:child_process"; -import { randomUUID } from "node:crypto"; +import { randomBytes, randomUUID } from "node:crypto"; import { chmodSync, mkdirSync, rmSync } from "node:fs"; import { join } from "node:path"; import { isConnectionUnavailableToAuxiliaryActivity } from "@/lib/exclusiveLeaseIsolation"; @@ -17,6 +17,8 @@ export interface VncSession { containerName: string; profileDir: string; cdpPort: number; + /** Shared secret the CDP bridge (docker/vnc-browser/chromium/cdp-bridge.py) requires (#12571). */ + cdpToken: string; vncPort: number; url: string; status: VncSessionStatus; @@ -138,6 +140,67 @@ function createProfileDir(connectionId: string, sessionId: string): string { return profileDir; } +let networkEnsured = false; + +/** + * Creates the dedicated browser-login bridge network (#12571) if it does not + * already exist. Idempotent: `docker network create` failing because the + * network is already there is not an error. + */ +async function ensureNetwork(): Promise { + if (networkEnsured) return; + const result = await docker(["network", "create", VNC_CONFIG.network], { timeoutMs: 15_000 }); + if (result.code !== 0 && !/already exists/i.test(result.err)) { + throw new Error(result.err.trim() || `Could not create Docker network ${VNC_CONFIG.network}`); + } + networkEnsured = true; +} + +/** + * Builds the `docker run` argument array for a browser-login container. + * Pulled out as a pure function so the security-relevant shape (dedicated + * network + CDP_BRIDGE_TOKEN, #12571) is directly testable without spawning + * Docker or touching the DB. + */ +export function buildRunArgs(params: { + containerName: string; + sessionId: string; + connectionId: string; + profileDir: string; + chromeCli: string; + cdpToken: string; +}): string[] { + return [ + "run", + "-d", + "--name", + params.containerName, + "--restart", + "no", + "--network", + VNC_CONFIG.network, + "--label", + `${LABEL}=true`, + "--label", + `${LABEL}.session-id=${params.sessionId}`, + "--label", + `${LABEL}.connection-id=${params.connectionId}`, + "--shm-size", + "1gb", + "-p", + `127.0.0.1::${VNC_CONFIG.containerVncPort}`, + "-p", + `127.0.0.1::${VNC_CONFIG.containerCdpPort}`, + "-v", + `${params.profileDir}:${VNC_CONFIG.containerProfileDir}`, + "-e", + `CHROME_CLI=${params.chromeCli}`, + "-e", + `CDP_BRIDGE_TOKEN=${params.cdpToken}`, + VNC_CONFIG.image, + ]; +} + async function publishedPort(containerName: string, containerPort: number): Promise { const result = await docker(["port", containerName, `${containerPort}/tcp`], { timeoutMs: 10_000, @@ -176,6 +239,7 @@ export async function startSession(connectionId: string): Promise { const sessionId = randomUUID(); const containerName = sessionKey(sessionId); const profileDir = createProfileDir(connectionId, sessionId); + const cdpToken = randomBytes(24).toString("hex"); const state: VncSession = { sessionId, connectionId, @@ -183,6 +247,7 @@ export async function startSession(connectionId: string): Promise { containerName, profileDir, cdpPort: 0, + cdpToken, vncPort: 0, url: provider.url, status: "starting", @@ -193,33 +258,10 @@ export async function startSession(connectionId: string): Promise { SESSIONS.set(sessionId, state); try { + await ensureNetwork(); const chromeCli = `${VNC_CONFIG.chromiumArgs} ${provider.url}`; const result = await docker( - [ - "run", - "-d", - "--name", - containerName, - "--restart", - "no", - "--label", - `${LABEL}=true`, - "--label", - `${LABEL}.session-id=${sessionId}`, - "--label", - `${LABEL}.connection-id=${connectionId}`, - "--shm-size", - "1gb", - "-p", - `127.0.0.1::${VNC_CONFIG.containerVncPort}`, - "-p", - `127.0.0.1::${VNC_CONFIG.containerCdpPort}`, - "-v", - `${profileDir}:${VNC_CONFIG.containerProfileDir}`, - "-e", - `CHROME_CLI=${chromeCli}`, - VNC_CONFIG.image, - ], + buildRunArgs({ containerName, sessionId, connectionId, profileDir, chromeCli, cdpToken }), { timeoutMs: 120_000 } ); if (result.code !== 0) { @@ -234,7 +276,7 @@ export async function startSession(connectionId: string): Promise { state.vncPort = await publishedPort(containerName, VNC_CONFIG.containerVncPort); state.cdpPort = await publishedPort(containerName, VNC_CONFIG.containerCdpPort); - await waitForCdpReady(state.cdpPort, VNC_CONFIG.browserReadyTimeoutMs); + await waitForCdpReady(state.cdpPort, VNC_CONFIG.browserReadyTimeoutMs, state.cdpToken); state.status = "running"; scheduleIdleSweep(); @@ -275,7 +317,8 @@ export async function harvestSession( const harvest = await harvestFromContainer( session.cdpPort, provider, - VNC_CONFIG.harvestTimeoutMs + VNC_CONFIG.harvestTimeoutMs, + session.cdpToken ); session.lastHarvestAt = Date.now(); if (!harvest.hasCredential) { diff --git a/tests/unit/vnc-cdp-bridge-auth.test.ts b/tests/unit/vnc-cdp-bridge-auth.test.ts new file mode 100644 index 0000000000..195270a6df --- /dev/null +++ b/tests/unit/vnc-cdp-bridge-auth.test.ts @@ -0,0 +1,124 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import net from "node:net"; +import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const BRIDGE_SCRIPT = path.resolve(__dirname, "../../docker/vnc-browser/chromium/cdp-bridge.py"); +const UPSTREAM_PORT = 9222; // SRC_PORT in cdp-bridge.py +const BRIDGE_PORT = 9223; // PUB_PORT in cdp-bridge.py +const TOKEN = "test-secret-token-12571"; + +function waitForListening(server: net.Server): Promise { + return new Promise((resolve, reject) => { + server.once("listening", () => resolve()); + server.once("error", reject); + }); +} + +function waitForBridgeReady(child: ChildProcessWithoutNullStreams): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => reject(new Error("cdp-bridge.py did not report ready in time")), + 5000 + ); + child.stderr.on("data", (chunk: Buffer) => { + if (chunk.toString("utf8").includes("forwarding")) { + clearTimeout(timer); + resolve(); + } + }); + child.once("error", (err) => { + clearTimeout(timer); + reject(err); + }); + child.once("exit", (code) => { + clearTimeout(timer); + reject(new Error(`cdp-bridge.py exited early with code ${code}`)); + }); + }); +} + +function startUpstream(): Promise<{ server: net.Server; receivedAnyBytes: () => boolean }> { + let received = false; + const server = net.createServer((socket) => { + socket.on("data", () => { + received = true; + }); + }); + return waitForListening(server.listen(UPSTREAM_PORT, "127.0.0.1")).then(() => ({ + server, + receivedAnyBytes: () => received, + })); +} + +function startBridge(): ChildProcessWithoutNullStreams { + return spawn("python3", [BRIDGE_SCRIPT], { + stdio: ["ignore", "ignore", "pipe"], + env: { ...process.env, CDP_BRIDGE_TOKEN: TOKEN }, + }); +} + +test("cdp-bridge.py must not forward bytes from an unauthenticated peer (#12571)", async () => { + const upstream = await startUpstream(); + const bridge = startBridge(); + + try { + await waitForBridgeReady(bridge); + + await new Promise((resolve, reject) => { + const client = net.createConnection({ host: "127.0.0.1", port: BRIDGE_PORT }, () => { + client.write("GET /json/version HTTP/1.1\r\nHost: x\r\n\r\n"); + }); + client.once("error", reject); + setTimeout(() => { + client.destroy(); + resolve(); + }, 500); + }); + + assert.equal( + upstream.receivedAnyBytes(), + false, + "cdp-bridge.py forwarded traffic from an unauthenticated peer straight to Chromium's CDP " + + "port — the bridge has no auth/token check (see docker/vnc-browser/chromium/cdp-bridge.py)" + ); + } finally { + bridge.kill("SIGKILL"); + await new Promise((resolve) => upstream.server.close(() => resolve())); + } +}); + +test("cdp-bridge.py forwards bytes once the caller presents the configured token (#12571)", async () => { + const upstream = await startUpstream(); + const bridge = startBridge(); + + try { + await waitForBridgeReady(bridge); + + await new Promise((resolve, reject) => { + const client = net.createConnection({ host: "127.0.0.1", port: BRIDGE_PORT }, () => { + client.write( + `GET /json/version HTTP/1.1\r\nHost: x\r\nX-Omni-Cdp-Token: ${TOKEN}\r\n\r\n` + ); + }); + client.once("error", reject); + setTimeout(() => { + client.destroy(); + resolve(); + }, 500); + }); + + assert.equal( + upstream.receivedAnyBytes(), + true, + "cdp-bridge.py should forward traffic once the caller presents the correct " + + "CDP_BRIDGE_TOKEN" + ); + } finally { + bridge.kill("SIGKILL"); + await new Promise((resolve) => upstream.server.close(() => resolve())); + } +}); diff --git a/tests/unit/vnc-session-docker-args.test.ts b/tests/unit/vnc-session-docker-args.test.ts new file mode 100644 index 0000000000..5a40bd917c --- /dev/null +++ b/tests/unit/vnc-session-docker-args.test.ts @@ -0,0 +1,46 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { buildRunArgs, sessionKey } from "@/lib/vncSession/service"; +import { VNC_CONFIG } from "@/lib/vncSession/manifest"; + +test("buildRunArgs (#12571) injects the CDP bridge token and a non-default network", () => { + const args = buildRunArgs({ + containerName: sessionKey("session-abc"), + sessionId: "session-abc", + connectionId: "connection-xyz", + profileDir: "/tmp/profile", + chromeCli: "--remote-debugging-port=9222 https://example.com", + cdpToken: "super-secret-token", + }); + + const networkIndex = args.indexOf("--network"); + assert.ok(networkIndex >= 0, "docker run args must include --network"); + assert.equal(args[networkIndex + 1], VNC_CONFIG.network); + assert.notEqual( + args[networkIndex + 1], + "bridge", + "must not join Docker's default bridge network (#12571)" + ); + + const envFlags = args.filter((_value, index) => args[index - 1] === "-e"); + assert.ok( + envFlags.some((flag) => flag === "CDP_BRIDGE_TOKEN=super-secret-token"), + "docker run args must inject CDP_BRIDGE_TOKEN for the container's cdp-bridge.py" + ); +}); + +test("buildRunArgs (#12571) generates a distinct token per call so sessions cannot reuse each other's secret", () => { + const base = { + containerName: "c", + sessionId: "s", + connectionId: "conn", + profileDir: "/tmp/p", + chromeCli: "--x", + }; + const argsA = buildRunArgs({ ...base, cdpToken: "token-a" }); + const argsB = buildRunArgs({ ...base, cdpToken: "token-b" }); + + assert.ok(argsA.includes("CDP_BRIDGE_TOKEN=token-a")); + assert.ok(argsB.includes("CDP_BRIDGE_TOKEN=token-b")); + assert.notDeepEqual(argsA, argsB); +});