diff --git a/apps/server/src/routes/setup-secrets.test.ts b/apps/server/src/routes/setup-secrets.test.ts new file mode 100644 index 0000000..51e9eb5 --- /dev/null +++ b/apps/server/src/routes/setup-secrets.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { randomUUID } from "node:crypto"; +import { devices, type Db } from "@parking/db"; +import { createTestDb } from "@parking/db/testing"; +import { storedSecrets } from "./setup.js"; + +// storedSecrets re-merges a device's machine-only secrets (relayPassword/pushPassword) +// into a test/save — but ONLY when the submitted config addresses the SAME device at the +// SAME host/port. This guards against a redirected probe exfiltrating the secret to an +// attacker host (an admin keeps a real device id but swaps the host). The booth operator +// is the threat-model adversary, so an authenticated-admin redirect must NOT leak. + +let db: Db; +const ID = "ctl-secret"; +const HOST = "10.0.10.5"; + +beforeEach(() => { + ({ db } = createTestDb()); + db.insert(devices).values({ + id: ID, + category: "access", + driverId: "dingtian", + config: { host: HOST, binaryPort: 60000, relayPassword: 1996, pushPassword: "p-secret" }, + enabled: true, + }).run(); +}); + +describe("storedSecrets identity guard", () => { + it("re-merges secrets when host/port/driver match the stored device", () => { + const out = storedSecrets(db, ID, "dingtian", { host: HOST, binaryPort: 60000 }); + expect(out.relayPassword).toBe(1996); + expect(out.pushPassword).toBe("p-secret"); + }); + + it("re-merges when identity fields are OMITTED (fall back to the stored device)", () => { + const out = storedSecrets(db, ID, "dingtian", {}); + expect(out.relayPassword).toBe(1996); + }); + + it("REFUSES secrets when the host is redirected (exfiltration attempt)", () => { + const out = storedSecrets(db, ID, "dingtian", { host: "10.66.66.66", binaryPort: 60000 }); + expect(out).toEqual({}); + }); + + it("REFUSES secrets when a control port is changed", () => { + const out = storedSecrets(db, ID, "dingtian", { host: HOST, binaryPort: 9999 }); + expect(out).toEqual({}); + }); + + it("REFUSES secrets when the driver doesn't match the stored row", () => { + const out = storedSecrets(db, ID, "stub-access", { host: HOST }); + expect(out).toEqual({}); + }); + + it("returns nothing for an unknown device id", () => { + expect(storedSecrets(db, randomUUID(), "dingtian", { host: HOST })).toEqual({}); + }); +}); diff --git a/apps/server/src/routes/setup.ts b/apps/server/src/routes/setup.ts index 9a44b21..f132554 100644 --- a/apps/server/src/routes/setup.ts +++ b/apps/server/src/routes/setup.ts @@ -36,6 +36,11 @@ interface AssignBody { interface TestBody { driverId: string; config: Record; + /** When editing an EXISTING device, its id — so the test re-merges the stored + * machine secrets (relayPassword/pushPassword) the client never received. Without + * this, testing an edited device would send no relay password → the device ignores + * the probe → a false "offline". Omitted when testing a brand-new device. */ + id?: string; } // Config keys that hold MACHINE-ONLY secrets — never sent back to the client. @@ -54,6 +59,41 @@ function redactSecrets(config: Record): Record return out; } +// Connection-identity keys: the fields that decide WHERE a probe is sent. A stored +// secret may only be re-merged when these match the stored row — otherwise an admin +// could point a test at an attacker host while keeping a real device id and have the +// secret sent there (exfiltration). host/port/binaryPort/httpPort cover the Dingtian's +// UDP + CGI targets; serial covers serial-bound readers. +const IDENTITY_KEYS = ["host", "port", "binaryPort", "httpPort", "serial"] as const; + +/** Stored machine-only secrets (relayPassword/pushPassword) for a device `id`, but ONLY + * when the submitted config addresses the SAME device — same driver, and every + * connection-identity field (host/port/…) that the submitted config sets equals the + * stored value. If the admin redirected the probe (different host/port) or the driver + * doesn't match, NO secret is returned: they must re-enter it explicitly. This stops a + * redirected test from exfiltrating the secret to an attacker host. */ +export function storedSecrets( + db: Db, + id: string, + driverId: string, + submitted: Record, +): Record { + const row = db.select().from(devices).where(eq(devices.id, id)).get(); + if (!row || row.driverId !== driverId) return {}; + const cfg = row.config as Record; + // Any identity field the client SENT must equal the stored value. (A field the client + // omits falls back to the stored device, so it can't be used to redirect.) + for (const k of IDENTITY_KEYS) { + const sent = submitted[k]; + if (sent !== undefined && sent !== "" && String(sent) !== String(cfg[k] ?? "")) { + return {}; + } + } + const out: Record = {}; + for (const k of SECRET_CONFIG_KEYS) if (cfg[k] !== undefined) out[k] = cfg[k]; + return out; +} + /** Result of the device configure pipeline: a ready-to-persist config, or an * HTTP error to send back. Shared by assign (create) and patch (edit). */ type ConfigureOutcome = @@ -249,13 +289,30 @@ export async function setupRoutes( "/api/setup/test", { preHandler: adminGuard }, async (req, reply) => { - const { driverId, config } = req.body; + const { driverId, config, id } = req.body; const driver = registry.get(driverId); if (!driver) return reply.code(400).send({ error: `unknown driver: ${driverId}` }); + // When editing an existing device, re-merge its stored machine secrets (e.g. + // relayPassword) — redacted from the client, so the submitted config omits them. + // Submitted values win (an admin can override), but a blank/0 field falls back to + // the stored secret so the probe authenticates. Without this, an edited Dingtian + // tests with no relay password → false "offline". The submitted-value-wins rule: + // only fill a secret from the store when the form didn't send a real one. + // Re-merge stored secrets ONLY when this addresses the same device at the same + // host/port (storedSecrets enforces identity) — so a redirected probe can't leak + // the secret to an attacker host. Submitted values still win. + const merged: Record = { ...config }; + if (id) { + for (const [k, v] of Object.entries(storedSecrets(db, id, driverId, config))) { + const sent = merged[k]; + if (sent === undefined || sent === "" || sent === 0) merged[k] = v as string | number; + } + } + let device; try { - device = registry.create(driverId, config); + device = registry.create(driverId, merged as Record); } catch (err) { return reply.code(400).send({ error: (err as Error).message }); } diff --git a/apps/web/src/api.ts b/apps/web/src/api.ts index fee5ff9..85af531 100644 --- a/apps/web/src/api.ts +++ b/apps/web/src/api.ts @@ -312,11 +312,13 @@ export interface TestResult { }; } -/** Test a device config (reachability + preconditions) without saving. */ -export function testDevice(driverId: string, config: DeviceConfig): Promise { +/** Test a device config (reachability + preconditions) without saving. Pass the + * device `id` when editing an existing one so the server re-merges its stored + * machine secrets (e.g. the relay password redacted from the client). */ +export function testDevice(driverId: string, config: DeviceConfig, id?: string): Promise { return apiFetch("/api/setup/test", { method: "POST", - body: JSON.stringify({ driverId, config }), + body: JSON.stringify({ driverId, config, ...(id ? { id } : {}) }), }); } diff --git a/packages/devices/src/drivers/access-dingtian.ts b/packages/devices/src/drivers/access-dingtian.ts index 328b9d0..417c4cd 100644 --- a/packages/devices/src/drivers/access-dingtian.ts +++ b/packages/devices/src/drivers/access-dingtian.ts @@ -779,6 +779,19 @@ export const dingtianDriver: AccessDriver = { { key: "binaryPort", label: "Binary protocol port", type: "port", required: false, default: 60000, help: "Dingtian binary protocol UDP port — authenticated relay control (default 60000)." }, { key: "httpPort", label: "HTTP config port", type: "port", required: false, default: 80, help: "Device web/config-API port (default 80)." }, { key: "channels", label: "Channels (relays/inputs)", type: "number", required: true, default: 4 }, + { + // relay_pw — the BINARY-protocol control/status password (NOT the web-UI login + // below). Every relay command + the status read embeds it; with the wrong/no + // value the device silently ignores the packet → healthCheck times out → the + // controller shows "offline" even though it pings. Redacted from the client + // (SECRET_CONFIG_KEYS), so it renders as a secret: blank KEEPS the stored value + // (the server re-merges it on test/save); type a value to set/change it. + key: "relayPassword", + label: "Relay control password", + type: "secret", + required: false, + help: "Binary-protocol relay password (relay_pw). Leave blank to keep the current one; a wrong/missing value makes the device ignore commands (Test connection times out).", + }, { key: "pulseMs", label: "Pulse open (ms)",