From ec1e7189a34953f30c48170bfeca5be4fc59e8f5 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 6 Sep 2026 00:33:28 +0200 Subject: [PATCH 1/2] mosquitto: seed dynsec with a run-once bootstrap before the broker The Dynamic Security plugin refuses to start the broker unless dynamic-security.json already holds an admin client, and no reconcile loop seeds it (novox/hq ADR 0052). Add a run-once init container, declared before the server container, that runs mosquitto's own bootstrap entrypoint in the runtime image: it seeds the store offline via mosquitto_ctrl and exits, and the host gates the broker on its completion. The bootstrap hands the seeded file to the broker's user (uid 1883, chown + 0600): the broker must read the seed at startup AND persist to it as clients come and go, but the init container runs as root and would otherwise leave a file the broker can neither read nor rewrite. This is the ownership question ADR 0052 left for the lab to settle. It seeds only when the file is absent, so what the running plugin grows is never clobbered (issue 035). Claude-Session: https://claude.ai/code/session_01LrgweAeERJYBg88c5cKDzF --- modules/mosquitto/bootstrap/index.ts | 47 ++++++++++++++++++++++++++++ modules/mosquitto/module.json | 21 +++++++++++++ modules/mosquitto/tsconfig.json | 2 +- 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 modules/mosquitto/bootstrap/index.ts diff --git a/modules/mosquitto/bootstrap/index.ts b/modules/mosquitto/bootstrap/index.ts new file mode 100644 index 0000000..53864fd --- /dev/null +++ b/modules/mosquitto/bootstrap/index.ts @@ -0,0 +1,47 @@ +// mosquitto's run-once bootstrap — mosquitto's own code (novox/hq ADR 0039), run once before the +// broker first starts (ADR 0052). The Dynamic Security plugin refuses to bring the broker up unless +// `dynamic-security.json` already holds an admin client, and nothing in a reconcile loop ever seeds +// that file — a state declaration describes what should exist, not a step that runs. This is that +// step: it writes the seed offline, exactly once, and exits. The host runs it to completion and only +// then starts the broker container the manifest places after it. +// +// It runs in the module's own runtime image, under the module's own account, as `mesh-tools run` +// imports it — no broker connection, because seeding is an offline file operation and there is no +// broker to reach yet. `mosquitto_ctrl dynsec init` writes the file; nothing here talks to a server. +// +// **Two disciplines make the seed safe to live beside a file the plugin then grows:** +// +// - It writes only when the file is absent, and never reconciles it. Once the broker is up, the +// dynsec plugin owns that file and rewrites it on every client it creates; re-seeding would wipe +// every provisioned client (novox/hq 04-ISSUES/035). The host's completion marker keeps the step +// from re-running; this absent-check keeps the one pass it does run from clobbering. +// - It hands the file to the broker's user. The broker runs as uid 1883 and must both READ the +// seed at startup and PERSIST to it as clients come and go; this container runs as root and would +// otherwise leave a root-owned file the broker at 1883 can neither read (if 0600) nor rewrite. +// So after writing, it chowns the file to 1883:1883 and sets 0600 — the broker's to read and to +// grow, and no one else's. This is the ownership question ADR 0052 left for the lab to settle. + +import { chownSync, chmodSync, existsSync } from "node:fs"; +import { MosquittoClient } from "../client.js"; + +// The broker (eclipse-mosquitto) runs as this uid/gid; the seeded store must be its to read and +// rewrite. Overridable for a broker image that runs as a different user. +const BROKER_UID = Number(process.env.MESH_MQTT_BROKER_UID ?? "1883") || 1883; +const BROKER_GID = Number(process.env.MESH_MQTT_BROKER_GID ?? "1883") || 1883; + +// The same path the broker's `plugin_opt_config_file` names, reached through the shared data volume. +const configFile = process.env.MESH_DYNSEC_FILE ?? "/mosquitto/data/dynamic-security.json"; + +if (existsSync(configFile)) { + // Already seeded — and possibly grown by the running plugin since. Leave it exactly as it is. + console.log(`[mosquitto:bootstrap] ${configFile} already exists; leaving it untouched`); +} else { + const mosquitto = MosquittoClient.fromEnv(); + await mosquitto.initBootstrapFile(configFile); + // Hand the store to the broker's user so it can read the seed and persist to it (see the header). + chownSync(configFile, BROKER_UID, BROKER_GID); + chmodSync(configFile, 0o600); + console.log( + `[mosquitto:bootstrap] seeded ${configFile} with the dynsec admin client, owned by ${BROKER_UID}:${BROKER_GID}`, + ); +} diff --git a/modules/mosquitto/module.json b/modules/mosquitto/module.json index b804b82..1d730cf 100644 --- a/modules/mosquitto/module.json +++ b/modules/mosquitto/module.json @@ -84,6 +84,27 @@ "type": "network", "name": "mosquitto" }, + { + "id": "bootstrap", + "type": "container", + "name": "mosquitto-bootstrap", + "image": "mesh-runtime-mosquitto@sha256:0000000000000000000000000000000000000000000000000000000000000000", + "run-once": true, + "volumes": [ + "/services/mosquitto/data:/mosquitto/data", + "/var/lib/mosquitto-module/admin.secret:/run/secrets/admin:ro" + ], + "env": { + "MESH_PROVISION_MQTT": "mosquitto:1883", + "MESH_PROVISION_ADMIN_USER": "mesh-admin", + "MESH_PROVISION_PASSWORD_FILE": "/run/secrets/admin", + "MESH_DYNSEC_FILE": "/mosquitto/data/dynamic-security.json" + }, + "args": [ + "run", + "/app/modules/mosquitto/dist/bootstrap/index.js" + ] + }, { "id": "server", "type": "container", diff --git a/modules/mosquitto/tsconfig.json b/modules/mosquitto/tsconfig.json index 51f4046..95f347f 100644 --- a/modules/mosquitto/tsconfig.json +++ b/modules/mosquitto/tsconfig.json @@ -8,5 +8,5 @@ "skipLibCheck": true, "noEmit": true }, - "include": ["client.ts", "index.ts", "provisioner/index.ts", "tools/index.ts"] + "include": ["client.ts", "index.ts", "provisioner/index.ts", "tools/index.ts", "bootstrap/index.ts"] } -- 2.54.0 From e58cbc1c46db1bf3db99e5f76f6bdf94e63c88e2 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 6 Sep 2026 00:57:45 +0200 Subject: [PATCH 2/2] mosquitto: don't read a rejected dynsec command as success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mosquitto_ctrl's dynsec subcommands exit 0 even when they fail — a "Client not found", an "already exists", a "Connection error: Not authorized", an "Unable to connect" all return status 0 and report the failure only as a line of text (verified live against 2.0.11). ctl() trusted the exit code, so clientExists()'s getClient probe never threw and always returned true; createScopedClient therefore took the setClientPassword branch, never ran createClient, and the consumer's client never landed in the dynsec store — while the provisioner logged it as provisioned. That is the assigned-catalogue-mqtt failure. ctl() now scans the combined stdout/stderr for the tool's error markers and raises a match as the failure it is. Surfacing those errors exposed two calls that only "worked" by being swallowed: addRoleACL re-run reports "already exists" (now ignored like createRole), and addClientRole re-run reports a bare "Internal error" that cannot be told from a real fault — so the binding is checked with clientHasRole and only added when absent. Fresh provision, idempotent re-provision, password rotation, bad admin auth and unreachable broker all verified against a live broker. Claude-Session: https://claude.ai/code/session_01LrgweAeERJYBg88c5cKDzF --- modules/mosquitto/client.ts | 85 +++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/modules/mosquitto/client.ts b/modules/mosquitto/client.ts index 3d63c42..f1a7877 100644 --- a/modules/mosquitto/client.ts +++ b/modules/mosquitto/client.ts @@ -63,8 +63,16 @@ export class MosquittoClient { /** * Run one `mosquitto_ctrl dynsec ` command against the broker as the admin client and return - * its stdout. Connects over MQTT with the verified connect flags `-h`/`-p`/`-u`/`-P`. A non-zero - * exit rejects — a failed command is an error here, not a success with a warning. + * its stdout. Connects over MQTT with the verified connect flags `-h`/`-p`/`-u`/`-P`. A failed + * command rejects — a failure here is an error, not a success with a warning. + * + * The exit code cannot carry that verdict: `mosquitto_ctrl` 2.0.x exits 0 from its dynsec + * subcommands **even when they fail** — a "Client not found", an "already exists", a rejected + * "Connection error: Not authorized", an "Unable to connect" all return status 0 and report the + * failure only as a line of text, on stdout or stderr (verified live against 2.0.11). Trusting the + * exit code is exactly how a `createClient` the broker refused reads back as a provisioned + * consumer. So the combined output is scanned for the tool's error markers and a match is raised as + * the failure it is. * * The admin password rides on argv (`-P`): mosquitto_ctrl 2.x exposes no password env var and no * password file for a broker connection — its only non-interactive mechanism is `-P`, its only @@ -79,9 +87,13 @@ export class MosquittoClient { "-u", this.conn.adminUser, "-P", this.conn.adminPassword, ]; - const { stdout } = await run("mosquitto_ctrl", [...base, "dynsec", ...args], { + const { stdout, stderr } = await run("mosquitto_ctrl", [...base, "dynsec", ...args], { maxBuffer: 16 << 20, }); + const failure = ctlError(`${stdout}\n${stderr}`); + if (failure) { + throw new Error(`mosquitto_ctrl dynsec ${args[0] ?? ""} failed: ${failure}`); + } return stdout; } @@ -90,11 +102,32 @@ export class MosquittoClient { try { await this.ctl("getClient", username); return true; - } catch { - return false; + } catch (err) { + // Only "not found" means the client is genuinely absent. Any other failure — an auth + // rejection, an unreachable broker — must NOT be read as "absent": that would send us down + // the createClient path and bury the real error. Re-raise anything that is not a clean miss. + if (/not\s*found|does not exist|no such/i.test(String(err))) return false; + throw err; } } + /** + * Whether this client already carries this role. mosquitto_ctrl has no idempotent addClientRole: + * re-binding a role the client already has does not report "already exists" — it reports a bare + * "Internal error" (verified against 2.0.11), indistinguishable from a genuine fault, so it cannot + * be swallowed by message. Instead the binding is checked first. `getClient` lists each assigned + * role under its "Roles:" heading as ` (priority: N)`; the role is matched as a whole token. + */ + async clientHasRole(username: string, role: string): Promise { + let out: string; + try { + out = await this.ctl("getClient", username); + } catch { + return false; // no such client (or unreadable) — it certainly has no role + } + return new RegExp(`(^|\\s)${escapeRegExp(role)}\\s+\\(priority`, "m").test(out); + } + /** * Create (or reset to a known state) a client scoped to one topic namespace, idempotently. The * client is confined to `/#` by a same-named role: it may publish to, subscribe to and @@ -111,14 +144,23 @@ export class MosquittoClient { await this.ctl("createClient", username, "-p", password); } - // A role carrying exactly this client's topic ACLs. createRole fails if it already exists; that - // is fine — the setRoleACL calls below assert the intended state either way. + // A role carrying exactly this client's topic ACLs. createRole, addRoleACL and addClientRole are + // all one-shot: each rejects with an "already exists" when re-run against a role/ACL/binding it + // created on a previous reconcile. That rejection is the intended terminal state — the ACL is + // deterministic (`/#`, allow), so re-adding the identical entry is a no-op — so it is + // swallowed. (Until the exit code was fixed this was invisible: the tool returned 0 and the + // rejection was lost; now it surfaces, and each of these adds must tolerate its own idempotent + // re-run explicitly.) await ignoreExisting(this.ctl("createRole", role)); for (const acl of ["publishClientSend", "publishClientReceive", "subscribePattern"]) { // allow (1) this client to send to, receive on, and subscribe under its own subtree. - await this.ctl("addRoleACL", role, acl, pattern, "allow"); + await ignoreExisting(this.ctl("addRoleACL", role, acl, pattern, "allow")); + } + // Bind the role only when it is not already bound — addClientRole is the one call whose + // idempotent re-run cannot be recognised by message (see clientHasRole). + if (!(await this.clientHasRole(username, role))) { + await this.ctl("addClientRole", username, role); } - await ignoreExisting(this.ctl("addClientRole", username, role)); } /** Remove a client and the per-client role created for it, idempotently. */ @@ -156,6 +198,31 @@ export function generatePassword(): string { return randomBytes(24).toString("base64url"); } +/** Escape a string for literal use inside a RegExp. */ +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** + * Find the failure `mosquitto_ctrl` reported in text while still exiting 0. Its dynsec subcommands + * surface errors in three shapes, on stdout or stderr: + * - `: Error: ` e.g. "createClient: Error: Client already exists" + * - `Connection error: ` e.g. "Connection error: Not authorized" + * - `Unable to connect ()` e.g. "Unable to connect (Lookup error.)." + * The only other line it prints unprompted is the "running without encryption" warning, which + * carries none of these markers. Returns the offending line, or undefined when the output is clean. + */ +function ctlError(output: string): string | undefined { + for (const raw of output.split(/\r?\n/)) { + const line = raw.trim(); + if (!line) continue; + if (/(^|:\s)Error:/i.test(line) || /^Connection error:/i.test(line) || /^Unable to connect/i.test(line)) { + return line; + } + } + return undefined; +} + /** Swallow a "already exists" failure so create paths are idempotent; rethrow anything else. */ async function ignoreExisting(p: Promise): Promise { try { -- 2.54.0