From b6beb9622bc6158f3ae91edcc176e0e730c937cb Mon Sep 17 00:00:00 2001 From: Jeppe B <2jepp9350@gmail.com> Date: Mon, 1 Jun 2026 23:13:04 +0200 Subject: [PATCH] Fail closed when edge broker secret is missing --- services/edge-broker/server.mjs | 43 ++++++++++++----- services/edge-broker/test/broker.test.mjs | 47 +++++++++++++++++++ .../nginx/app/classes/edge_broker_client.php | 11 +++-- .../Selfserve/EdgeBrokerClientConfigTest.php | 22 +++++++++ 4 files changed, 108 insertions(+), 15 deletions(-) diff --git a/services/edge-broker/server.mjs b/services/edge-broker/server.mjs index f600078e..535a2a65 100644 --- a/services/edge-broker/server.mjs +++ b/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; } diff --git a/services/edge-broker/test/broker.test.mjs b/services/edge-broker/test/broker.test.mjs index 1f539b69..f971aeed 100644 --- a/services/edge-broker/test/broker.test.mjs +++ b/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); diff --git a/services/nginx/app/classes/edge_broker_client.php b/services/nginx/app/classes/edge_broker_client.php index ff8d2cba..dc7e8f0e 100644 --- a/services/nginx/app/classes/edge_broker_client.php +++ b/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); diff --git a/services/nginx/app/tests/Unit/Selfserve/EdgeBrokerClientConfigTest.php b/services/nginx/app/tests/Unit/Selfserve/EdgeBrokerClientConfigTest.php index 13d1c1d4..07cea212 100644 --- a/services/nginx/app/tests/Unit/Selfserve/EdgeBrokerClientConfigTest.php +++ b/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); + } +});