fix(setup): add Dingtian relay-password field + secure secret re-merge on test
The relay control password (relay_pw) was read by the driver but had NO form field, so Test connection sent it as 0 → the device ignored the probe → a controller showed "offline" even though it pinged. Add a "Relay control password" config field (secret; blank keeps the stored value). Because relayPassword is redacted from the client, the edit form can't resend it — so the test endpoint now re-merges the stored secret by device id (mirroring save). It is re-merged ONLY when the submitted config addresses the SAME device: matching driverId and every connection-identity field it sets (host/port/binaryPort/httpPort/serial). A redirected host/port or mismatched driver yields NO secret, so a probe can't exfiltrate the password to an attacker host (the booth operator is the threat-model adversary). testDevice() now passes the device id; setup-secrets.test.ts covers the identity guard. Claude-Session: https://claude.ai/code/session_01Xcm6ikLgGoCxxHrxtjkk5V
This commit is contained in:
@@ -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({});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -36,6 +36,11 @@ interface AssignBody {
|
|||||||
interface TestBody {
|
interface TestBody {
|
||||||
driverId: string;
|
driverId: string;
|
||||||
config: Record<string, string | number | boolean>;
|
config: Record<string, string | number | boolean>;
|
||||||
|
/** 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.
|
// Config keys that hold MACHINE-ONLY secrets — never sent back to the client.
|
||||||
@@ -54,6 +59,41 @@ function redactSecrets(config: Record<string, unknown>): Record<string, unknown>
|
|||||||
return out;
|
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<string, unknown>,
|
||||||
|
): Record<string, unknown> {
|
||||||
|
const row = db.select().from(devices).where(eq(devices.id, id)).get();
|
||||||
|
if (!row || row.driverId !== driverId) return {};
|
||||||
|
const cfg = row.config as Record<string, unknown>;
|
||||||
|
// 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<string, unknown> = {};
|
||||||
|
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
|
/** 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). */
|
* HTTP error to send back. Shared by assign (create) and patch (edit). */
|
||||||
type ConfigureOutcome =
|
type ConfigureOutcome =
|
||||||
@@ -249,13 +289,30 @@ export async function setupRoutes(
|
|||||||
"/api/setup/test",
|
"/api/setup/test",
|
||||||
{ preHandler: adminGuard },
|
{ preHandler: adminGuard },
|
||||||
async (req, reply) => {
|
async (req, reply) => {
|
||||||
const { driverId, config } = req.body;
|
const { driverId, config, id } = req.body;
|
||||||
const driver = registry.get(driverId);
|
const driver = registry.get(driverId);
|
||||||
if (!driver) return reply.code(400).send({ error: `unknown driver: ${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<string, string | number | boolean | undefined> = { ...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;
|
let device;
|
||||||
try {
|
try {
|
||||||
device = registry.create(driverId, config);
|
device = registry.create(driverId, merged as Record<string, string | number | boolean>);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
return reply.code(400).send({ error: (err as Error).message });
|
return reply.code(400).send({ error: (err as Error).message });
|
||||||
}
|
}
|
||||||
|
|||||||
+5
-3
@@ -312,11 +312,13 @@ export interface TestResult {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Test a device config (reachability + preconditions) without saving. */
|
/** Test a device config (reachability + preconditions) without saving. Pass the
|
||||||
export function testDevice(driverId: string, config: DeviceConfig): Promise<TestResult> {
|
* 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<TestResult> {
|
||||||
return apiFetch<TestResult>("/api/setup/test", {
|
return apiFetch<TestResult>("/api/setup/test", {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
body: JSON.stringify({ driverId, config }),
|
body: JSON.stringify({ driverId, config, ...(id ? { id } : {}) }),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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: "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: "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 },
|
{ 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",
|
key: "pulseMs",
|
||||||
label: "Pulse open (ms)",
|
label: "Pulse open (ms)",
|
||||||
|
|||||||
Reference in New Issue
Block a user