Merge pull request #220 from copenhagentruckwash/fix-empty-edge-broker-secret-vulnerability

Fail closed when edge broker secret is missing

services/edge-broker/server.mjs

@@ -58,6 +58,32 @@ function resolveAuthMode(options = {}, managerUrl = "") {
   return "strict";
 }
 
+function resolveSharedSecret(options = {}) {
+  return String(options.sharedSecret ?? process.env.EDGE_BROKER_SHARED_SECRET ?? "").trim();
+}
+
+function requireSharedSecret(req, res, sharedSecret) {
+  if (sharedSecret === "") {
+    jsonResponse(res, 503, {
+      ok: false,
+      error: "Edge broker shared secret is not configured",
+      shared_secret_required: true,
+    });
+    return false;
+  }
+
+  if (req.headers["x-edge-broker-secret"] !== sharedSecret) {
+    jsonResponse(res, 403, {
+      ok: false,
+      error: "Forbidden",
+      shared_secret_required: true,
+    });
+    return false;
+  }
+
+  return true;
+}
+
 function parseScopes(value) {
   if (!Array.isArray(value)) {
     return [];
@@ -150,7 +176,7 @@ function rejectUpgrade(socket, statusCode, errorCode, message, details = {}) {
 }
 
 export function createBrokerServer(options = {}) {
-  const sharedSecret = options.sharedSecret ?? process.env.EDGE_BROKER_SHARED_SECRET ?? "";
+  const sharedSecret = resolveSharedSecret(options);
   const managerUrl = resolveManagerUrl(options);
   const authMode = resolveAuthMode(options, managerUrl);
   const commandTimeoutMs = options.commandTimeoutMs ?? 10000;
@@ -460,25 +486,19 @@ export function createBrokerServer(options = {}) {
       }
 
       if (req.method === "POST" && url.pathname === "/api/diagnostics/shared-secret") {
-        if (sharedSecret && req.headers["x-edge-broker-secret"] !== sharedSecret) {
-          jsonResponse(res, 403, {
-            ok: false,
-            error: "Forbidden",
-            shared_secret_required: true,
-          });
+        if (!requireSharedSecret(req, res, sharedSecret)) {
           return;
         }
 
         jsonResponse(res, 200, {
           ok: true,
-          shared_secret_required: Boolean(sharedSecret),
+          shared_secret_required: true,
         });
         return;
       }
 
       if (req.method === "POST" && /^\/api\/gateways\/\d+\/commands$/.test(url.pathname)) {
-        if (sharedSecret && req.headers["x-edge-broker-secret"] !== sharedSecret) {
-          jsonResponse(res, 403, { error: "Forbidden" });
+        if (!requireSharedSecret(req, res, sharedSecret)) {
           return;
         }
 
@@ -521,8 +541,7 @@ export function createBrokerServer(options = {}) {
       }
 
       if (req.method === "POST" && /^\/api\/gateways\/\d+\/sync$/.test(url.pathname)) {
-        if (sharedSecret && req.headers["x-edge-broker-secret"] !== sharedSecret) {
-          jsonResponse(res, 403, { error: "Forbidden" });
+        if (!requireSharedSecret(req, res, sharedSecret)) {
           return;
         }
 

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

@@ -116,6 +116,53 @@ test("broker defaults to strict auth and fails closed when manager URL is missin
   }
 });
 
+test("broker rejects protected HTTP endpoints when shared secret is missing", async () => {
+  const broker = createBrokerServer({ authMode: "stub", sharedSecret: "", commandTimeoutMs: 2000 });
+  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 new Promise((resolve) => agent.once("open", resolve));
+  const agentMessages = collectMessages(agent);
+
+  const commandResponse = await fetch(`http://127.0.0.1:${port}/api/gateways/701/commands`, {
+    method: "POST",
+    headers: {
+      "content-type": "application/json",
+    },
+    body: JSON.stringify({
+      commandType: "SET_RELAY_STATE",
+      payload: { relayId: "M-7", on: true },
+    }),
+  });
+  const commandJson = await commandResponse.json();
+
+  assert.equal(commandResponse.status, 503);
+  assert.equal(commandJson.ok, false);
+  assert.equal(commandJson.shared_secret_required, true);
+  assert.match(commandJson.error, /shared secret is not configured/);
+  assert.equal(agentMessages.some((message) => message.type === "COMMAND"), false);
+
+  const diagnosticsResponse = await fetch(`http://127.0.0.1:${port}/api/diagnostics/shared-secret`, {
+    method: "POST",
+  });
+  const diagnosticsJson = await diagnosticsResponse.json();
+
+  assert.equal(diagnosticsResponse.status, 503);
+  assert.equal(diagnosticsJson.shared_secret_required, true);
+
+  const syncResponse = await fetch(`http://127.0.0.1:${port}/api/gateways/701/sync`, {
+    method: "POST",
+  });
+  const syncJson = await syncResponse.json();
+
+  assert.equal(syncResponse.status, 503);
+  assert.equal(syncJson.shared_secret_required, true);
+
+  agent.terminate();
+  await broker.close();
+});
+
 test("broker dispatches commands to connected agents", async () => {
   const broker = createBrokerServer({ authMode: "stub", sharedSecret: "secret", commandTimeoutMs: 2000 });
   const address = await broker.listen(0);

services/nginx/app/classes/edge_broker_client.php

@@ -112,14 +112,19 @@ class edge_broker_client
             throw new Exception('Edge broker URL is not configured');
         }
 
+        $sharedSecret = $this->resolveSharedSecret();
+        if ($sharedSecret === '') {
+            throw new Exception('Edge broker shared secret is not configured');
+        }
+
         $ch = curl_init($url);
         curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
         curl_setopt($ch, CURLOPT_CUSTOMREQUEST, $method);
         curl_setopt($ch, CURLOPT_TIMEOUT, $this->timeoutSeconds);
-        curl_setopt($ch, CURLOPT_HTTPHEADER, array_values(array_filter([
+        curl_setopt($ch, CURLOPT_HTTPHEADER, [
             'Content-Type: application/json',
-            $this->resolveSharedSecret() !== '' ? 'X-Edge-Broker-Secret: ' . $this->resolveSharedSecret() : null,
-        ])));
+            'X-Edge-Broker-Secret: ' . $sharedSecret,
+        ]);
         curl_setopt($ch, CURLOPT_POSTFIELDS, json_encode($payload, JSON_UNESCAPED_UNICODE));
 
         $rawResponse = curl_exec($ch);

services/nginx/app/tests/Unit/Selfserve/EdgeBrokerClientConfigTest.php

@@ -11,3 +11,25 @@ it('uses default broker url but requires explicit shared secret configuration',
     expect($source)->toContain("getenv('EDGE_INTERNAL_SECRET')");
     expect($source)->not->toContain('DEFAULT_SHARED_SECRET');
 });
+
+
+it('fails closed when no broker shared secret is configured', function (): void {
+    $previousBrokerSecret = getenv('EDGE_BROKER_SHARED_SECRET');
+    $previousInternalSecret = getenv('EDGE_INTERNAL_SECRET');
+    putenv('EDGE_BROKER_SHARED_SECRET');
+    putenv('EDGE_INTERNAL_SECRET');
+
+    try {
+        $client = new \classes\edge_broker_client('http://127.0.0.1:9', null, 1);
+
+        expect(fn() => $client->dispatchCommand(1, 'SET_RELAY_STATE', ['on' => true]))
+            ->toThrow(Exception::class, 'Edge broker shared secret is not configured');
+    } finally {
+        $previousBrokerSecret === false
+            ? putenv('EDGE_BROKER_SHARED_SECRET')
+            : putenv('EDGE_BROKER_SHARED_SECRET=' . $previousBrokerSecret);
+        $previousInternalSecret === false
+            ? putenv('EDGE_INTERNAL_SECRET')
+            : putenv('EDGE_INTERNAL_SECRET=' . $previousInternalSecret);
+    }
+});