Merge pull request #186 from copenhagentruckwash/propose-fix-for-edge-broker-vulnerability

Harden edge broker defaults and restrict compose exposure

.env.example

@@ -53,8 +53,8 @@ ECONOMIC_API_APP_SECRET_TOKEN=
 # Edge broker defaults for shell relay and gateway dispatch.
 EDGE_BROKER_URL=http://edge-broker:4300
 EDGE_PUBLIC_BROKER_URL=http://localhost/api/edge-broker
-EDGE_AUTH_MODE=manager
+EDGE_AUTH_MODE=strict
-EDGE_BROKER_SHARED_SECRET=truckwash-edge-dev
+EDGE_BROKER_SHARED_SECRET=
 
 # Redis credentials
 REDIS_CONFIG_HOST=redis

docker-compose.example.yml

@@ -52,14 +52,9 @@ services:
       dockerfile: services/edge-broker/Dockerfile
     container_name: edge-broker
     environment:
-<<<<<<< HEAD
+      EDGE_AUTH_MODE: ${EDGE_AUTH_MODE:-strict}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
-    ports:
-      - "127.0.0.1:4300:4300"
-=======
-      EDGE_AUTH_MODE: ${EDGE_AUTH_MODE:-manager}
       EDGE_MANAGER_URL: ${EDGE_MANAGER_URL:-http://caddy}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:-truckwash-edge-dev}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     labels:
       - "traefik.enable=true"
       - "traefik.http.routers.edge-broker-api.rule=Host(`api.example.com`) && PathPrefix(`/edge-broker`)"
@@ -75,7 +70,7 @@ services:
       - "traefik.http.middlewares.edge-broker-strip.stripPrefix.prefixes=/edge-broker"
       - "traefik.http.middlewares.edge-broker-strip-local.stripPrefix.prefixes=/api/edge-broker"
       - "traefik.http.services.edge-broker.loadbalancer.server.port=4300"
->>>>>>> origin/master
+
 
   caddy:
     image: caddy:2.7.6-alpine
@@ -119,11 +114,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "true"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-<<<<<<< HEAD
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:-}
-=======
       EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
->>>>>>> origin/master
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro
@@ -144,11 +135,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "false"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-<<<<<<< HEAD
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:-}
-=======
       EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
->>>>>>> origin/master
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro

docker-compose.yml

@@ -122,9 +122,9 @@ services:
       dockerfile: services/edge-broker/Dockerfile
     container_name: edge-broker
     environment:
-      EDGE_AUTH_MODE: ${EDGE_AUTH_MODE:-manager}
+      EDGE_AUTH_MODE: ${EDGE_AUTH_MODE:-strict}
       EDGE_MANAGER_URL: ${EDGE_MANAGER_URL:-http://caddy}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     labels:
       - "traefik.enable=true"
       - "traefik.http.routers.edge-broker-api.rule=Host(`api.truckwash.dk`) && PathPrefix(`/edge-broker`)"
@@ -308,7 +308,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "true"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro
@@ -328,7 +328,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "false"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro
@@ -348,7 +348,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "false"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro
@@ -368,7 +368,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "false"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro
@@ -388,7 +388,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "false"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro
@@ -408,7 +408,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "false"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     volumes:
       - ./services/nginx/staging:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro
@@ -428,7 +428,7 @@ services:
     environment:
       AUTO_COMPOSER_INSTALL: "false"
       EDGE_BROKER_URL: ${EDGE_BROKER_URL:-http://edge-broker:4300}
-      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?EDGE_BROKER_SHARED_SECRET is required}
+      EDGE_BROKER_SHARED_SECRET: ${EDGE_BROKER_SHARED_SECRET:?set EDGE_BROKER_SHARED_SECRET in .env}
     volumes:
       - ./services/nginx/app:/var/www/html
       - ./services/php/php.ini:/usr/local/etc/php/conf.d/zz-custom.ini:ro

services/edge-broker/server.mjs

@@ -55,7 +55,7 @@ function resolveAuthMode(options = {}, managerUrl = "") {
   if (process.env.EDGE_AUTH_MODE) {
     return process.env.EDGE_AUTH_MODE;
   }
-  return "manager";
+  return "strict";
 }
 
 function parseScopes(value) {
@@ -562,6 +562,7 @@ export function createBrokerServer(options = {}) {
           return;
         }
 
+
         wss.handleUpgrade(req, socket, head, (ws) => {
           const existing = agents.get(gatewayId);
           if (existing && existing.readyState < 2) {
@@ -628,6 +629,7 @@ export function createBrokerServer(options = {}) {
           return;
         }
 
+
         wss.handleUpgrade(req, socket, head, (ws) => {
           ws.sessionToken = token;
           ws.sessionInfo = session;

services/edge-broker/test/broker.test.mjs

@@ -19,6 +19,18 @@ function waitForClose(socket) {
   });
 }
 
+function waitForCloseOrError(socket) {
+  return new Promise((resolve) => {
+    const onDone = () => {
+      socket.off("error", onDone);
+      socket.off("close", onDone);
+      resolve();
+    };
+    socket.once("error", onDone);
+    socket.once("close", onDone);
+  });
+}
+
 function rawUpgradeRequest(port, path) {
   return new Promise((resolve, reject) => {
     const socket = net.createConnection({ host: "127.0.0.1", port }, () => {
@@ -59,7 +71,7 @@ async function waitFor(predicate, { timeoutMs = 1000, intervalMs = 10, descripti
   throw new Error(`Timed out waiting for ${description}`);
 }
 
-test("broker defaults to manager auth and fails closed when manager URL is missing", async () => {
+test("broker defaults to strict auth and fails closed when manager URL is missing", async () => {
   const previousEnv = {
     EDGE_AUTH_MODE: process.env.EDGE_AUTH_MODE,
     EDGE_MANAGER_URL: process.env.EDGE_MANAGER_URL,
@@ -72,7 +84,7 @@ test("broker defaults to manager auth and fails closed when manager URL is missi
   let broker;
   try {
     broker = createBrokerServer({ sharedSecret: "secret" });
-    assert.equal(broker.state.authMode, "manager");
+    assert.equal(broker.state.authMode, "strict");
     assert.equal(broker.state.managerUrl, "");
 
     const address = await broker.listen(0);
@@ -418,6 +430,17 @@ test("broker closes browser shell sessions when the agent disconnects before she
   await broker.close();
 });
 
+test("broker defaults to strict auth when no validators are configured", async () => {
+  const broker = createBrokerServer();
+  const address = await broker.listen(0);
+  const port = address.port;
+
+  const agent = new WebSocket(`ws://127.0.0.1:${port}/ws/agent?gatewayId=701&token=agent-token`);
+  await waitForCloseOrError(agent);
+
+  await broker.close();
+});
+
 test("broker sends an agent welcome before connection progress and backlog dispatch", async () => {
   const broker = createBrokerServer({
     authMode: "stub",

services/edge-broker/test/config.test.mjs

@@ -7,11 +7,6 @@ import { fileURLToPath } from "node:url";
 const testDirectory = path.dirname(fileURLToPath(import.meta.url));
 const repoRoot = path.resolve(testDirectory, "../../..");
 
-<<<<<<< HEAD
-test("traefik defines a dedicated edge broker entrypoint on port 4300", () => {
-  assert.match(traefikSource, /edge-broker:\s*\n\s*address:\s*":4300"/);
-  assert.match(traefikProdSource, /edge-broker:\s*\n\s*address:\s*":4300"/);
-=======
 function readRequiredSource(...pathSegments) {
   const sourcePath = path.resolve(repoRoot, ...pathSegments);
   assert.equal(existsSync(sourcePath), true, `Expected config fixture to exist: ${sourcePath}`);
@@ -39,30 +34,12 @@ function readComposeServiceBlock(composeSource, serviceName) {
 
 test("traefik does not expose a dedicated public edge broker port", () => {
   assert.doesNotMatch(traefikSource, /edge-broker:\s*\n\s*address:\s*":4300"/);
->>>>>>> origin/master
 });
 
-<<<<<<< HEAD
-test("base docker compose binds edge broker port 4300 to localhost only", () => {
-  assert.match(baseComposeSource, /\bedge-broker:\b/);
-  assert.match(baseComposeSource, /edge-broker:\s*\n[\s\S]*?\n\s+ports:\s*\n\s+- "127.0.0.1:4300:4300"/);
-});
-
-<<<<<<< HEAD
-test("php services receive broker url defaults and require explicit broker shared secret", () => {
-  assert.match(composeSource, /php1:[\s\S]*EDGE_BROKER_URL:\s*\$\{EDGE_BROKER_URL:-http:\/\/edge-broker:4300\}/);
-  assert.match(composeSource, /php1:[\s\S]*EDGE_BROKER_SHARED_SECRET:\s*\$\{EDGE_BROKER_SHARED_SECRET:-\}/);
-  assert.match(composeSource, /php-staging:[\s\S]*EDGE_BROKER_URL:\s*\$\{EDGE_BROKER_URL:-http:\/\/edge-broker:4300\}/);
-  assert.match(composeSource, /php-cron:[\s\S]*EDGE_BROKER_SHARED_SECRET:\s*\$\{EDGE_BROKER_SHARED_SECRET:-\}/);
-=======
-test("example docker compose binds edge broker port 4300 to localhost only", () => {
-  assert.match(exampleComposeSource, /\bedge-broker:\b/);
-  assert.match(exampleComposeSource, /edge-broker:\s*\n[\s\S]*?\n\s+ports:\s*\n\s+- "127.0.0.1:4300:4300"/);
-=======
 test("base docker compose routes edge broker traffic through traefik", () => {
   const serviceBlock = readComposeServiceBlock(baseComposeSource, "edge-broker");
   assert.doesNotMatch(serviceBlock, /\n\s+ports:\s*\n[\s\S]*?\n\s+- "4300:4300"/);
-  assert.match(serviceBlock, /EDGE_AUTH_MODE:\s*\$\{EDGE_AUTH_MODE:-manager\}/);
+  assert.match(serviceBlock, /EDGE_AUTH_MODE:\s*\$\{EDGE_AUTH_MODE:-strict\}/);
   assert.match(serviceBlock, /EDGE_MANAGER_URL:\s*\$\{EDGE_MANAGER_URL:-http:\/\/caddy\}/);
   assert.match(serviceBlock, /traefik\.http\.routers\.edge-broker-api\.priority=200/);
   assert.match(serviceBlock, /traefik\.http\.routers\.edge-broker-local\.priority=200/);
@@ -78,7 +55,7 @@ test("base docker compose routes edge broker traffic through traefik", () => {
 test("example docker compose routes edge broker traffic through traefik", () => {
   const serviceBlock = readComposeServiceBlock(exampleComposeSource, "edge-broker");
   assert.doesNotMatch(serviceBlock, /\n\s+ports:\s*\n[\s\S]*?\n\s+- "4300:4300"/);
-  assert.match(serviceBlock, /EDGE_AUTH_MODE:\s*\$\{EDGE_AUTH_MODE:-manager\}/);
+  assert.match(serviceBlock, /EDGE_AUTH_MODE:\s*\$\{EDGE_AUTH_MODE:-strict\}/);
   assert.match(serviceBlock, /EDGE_MANAGER_URL:\s*\$\{EDGE_MANAGER_URL:-http:\/\/caddy\}/);
   assert.match(serviceBlock, /traefik\.http\.routers\.edge-broker-api\.rule=Host\(`api\.example\.com`\) && PathPrefix\(`\/edge-broker`\)/);
   assert.match(serviceBlock, /traefik\.http\.routers\.edge-broker-local\.rule=Host\(`localhost`\) && PathPrefix\(`\/api\/edge-broker`\)/);
@@ -97,10 +74,9 @@ test("standalone production compose routes edge broker traffic through traefik",
   assert.match(serviceBlock, /traefik\.http\.routers\.edge-broker-api-v2\.rule=Host\(`api-v2\.truckwash\.io`\) && PathPrefix\(`\/edge-broker`\)/);
   assert.match(serviceBlock, /traefik\.http\.routers\.edge-broker-local\.rule=Host\(`localhost`\) && PathPrefix\(`\/api\/edge-broker`\)/);
   assert.match(serviceBlock, /traefik\.http\.services\.edge-broker\.loadbalancer\.server\.port=4300/);
->>>>>>> origin/master
 });
 
-test("php services receive broker websocket environment defaults", () => {
+test("compose config does not provide insecure broker secret defaults", () => {
   for (const composeSource of [baseComposeSource, exampleComposeSource]) {
     assert.match(composeSource, /EDGE_BROKER_URL:\s*\$\{EDGE_BROKER_URL:-http:\/\/edge-broker:4300\}/);
     assert.match(composeSource, /EDGE_BROKER_SHARED_SECRET:\s*\$\{EDGE_BROKER_SHARED_SECRET:\?set EDGE_BROKER_SHARED_SECRET in \.env\}/);
@@ -114,5 +90,4 @@ test("base docker compose wires the broker into each php worker", () => {
     assert.match(serviceBlock, /EDGE_BROKER_URL:\s*\$\{EDGE_BROKER_URL:-http:\/\/edge-broker:4300\}/);
     assert.match(serviceBlock, /EDGE_BROKER_SHARED_SECRET:\s*\$\{EDGE_BROKER_SHARED_SECRET:\?set EDGE_BROKER_SHARED_SECRET in \.env\}/);
   }
->>>>>>> origin/master
 });