diff --git a/apps/server/src/routes/setup.ts b/apps/server/src/routes/setup.ts index 4c9f5d7..5b27379 100644 --- a/apps/server/src/routes/setup.ts +++ b/apps/server/src/routes/setup.ts @@ -162,6 +162,15 @@ export async function setupRoutes(app: FastifyInstance, db: Db): Promise { const id = randomUUID(); const fullConfig: Record = { ...config }; + // The web password the admin typed is a DESIRED value, not a stored fact: + // it's passed to the driver (via create(config) below) as the rotation + // target, but we do NOT persist it from the form. Only harden()'s VERIFIED + // secrets.webPassword gets saved — otherwise a failed rotation would leave + // the DB claiming a password the device never accepted (login stays old). + delete fullConfig.webPassword; + // webPasswordCurrent is an input-only credential (the OLD password used to + // authorize the change) — never persist it as typed. + delete fullConfig.webPasswordCurrent; // Residual-risk warnings from device hardening (shown to the admin; the // save still succeeds — these are "configured, but note X" advisories). const hardenWarnings: string[] = []; diff --git a/apps/web/src/SetupWizard.tsx b/apps/web/src/SetupWizard.tsx index eecfb29..3f9912e 100644 --- a/apps/web/src/SetupWizard.tsx +++ b/apps/web/src/SetupWizard.tsx @@ -108,6 +108,9 @@ function CategorySection({ // Show the add-form automatically when nothing is assigned yet; otherwise it's // collapsed behind "Add another" so the list stays the focus. const [adding, setAdding] = useState(false); + // Warnings from the most recent save (e.g. "string protocol could not be + // disabled — finish in the device web UI"). Persist after the form closes. + const [warnings, setWarnings] = useState([]); const showForm = adding || assignments.length === 0; return ( @@ -116,6 +119,28 @@ function CategorySection({ {title} · lane {lane} + {warnings.length > 0 && ( +
+ ⚠ Saved, but action needed: +
    + {warnings.map((w, i) => ( +
  • {w}
    • + ))} +
+ +
+ )} + {assignments.length > 0 && (
    {assignments.map((a) => ( @@ -130,7 +155,8 @@ function CategorySection({ category={category} entries={entries} discoverableIds={discoverableIds} - onSaved={async () => { + onSaved={async (w) => { + setWarnings(w); await onChanged(); setAdding(false); }} @@ -208,7 +234,7 @@ function DeviceForm({ category: DeviceCategory; entries: CatalogEntry[]; discoverableIds: string[]; - onSaved: () => Promise | void; + onSaved: (warnings: string[]) => Promise | void; onCancel?: () => void; }) { const [selectedId, setSelectedId] = useState(""); @@ -318,15 +344,15 @@ function DeviceForm({ setSaving(true); setSaveError(null); try { - await assignDevice({ + const result = await assignDevice({ lane, category, driverId: selected.id, config: mergedConfig(), ...(backendIp ? { backendIp } : {}), }); - // Parent reloads the list; this form is unmounted or reset by it. - await onSaved(); + // Hand warnings to the parent so they persist after this form unmounts. + await onSaved(result.warnings ?? []); } catch (e) { setSaveError((e as Error).message); } finally { diff --git a/apps/web/src/api.ts b/apps/web/src/api.ts index 71ca6f7..22be382 100644 --- a/apps/web/src/api.ts +++ b/apps/web/src/api.ts @@ -160,11 +160,11 @@ export interface AssignBody { } /** Save + configure the device (preconditions, push setup), then persist. */ -export function assignDevice(body: AssignBody): Promise { +export function assignDevice(body: AssignBody): Promise { return apiFetch("/api/setup/assign", { method: "POST", body: JSON.stringify(body) }); } -/** A persisted device assignment (one per instance; secrets stripped). */ +/** A persisted device assignment (one per instance; machine-only secrets stripped). */ export interface Assignment { id: string; lane: number; @@ -175,6 +175,12 @@ export interface Assignment { createdAt?: string; } +/** Assign response = the saved assignment plus any residual-risk warnings + * (e.g. "string protocol could not be disabled — finish in the device web UI"). */ +export interface AssignResult extends Assignment { + warnings?: string[]; +} + export interface SetupState { completedAt: string | null; assignments: Assignment[]; diff --git a/packages/devices/src/drivers/access-dingtian.ts b/packages/devices/src/drivers/access-dingtian.ts index 3e7af9c..23ee206 100644 --- a/packages/devices/src/drivers/access-dingtian.ts +++ b/packages/devices/src/drivers/access-dingtian.ts @@ -243,9 +243,15 @@ class DingtianController /** Input level at rest; an input is "active" when it differs from this. */ readonly #restingHigh: boolean; readonly #pulseMs: number; - /** Current device web-UI login (gates the browser UI only, not the CGI API). */ + /** Device web-UI login user (gates the browser UI only, not the CGI API). */ readonly #webUser: string; - readonly #webPassword: string; + /** The password the admin WANTS the device to have (the rotation target). If + * blank, harden() generates a random one. */ + readonly #webPassword: string | undefined; + /** The device's CURRENT password, used as the OLD cred for userset.cgi. Defaults + * to "admin" (factory). Distinct from #webPassword (the desired new value) so an + * admin typing a desired password doesn't break rotation. */ + readonly #webPasswordCurrent: string; #poll: ReturnType | null = null; #last: boolean[] | null = null; @@ -264,11 +270,15 @@ class DingtianController // This unit idles with inputs HIGH (status "1111"); a press pulls LOW. this.#restingHigh = config.inputRestingHigh !== false; this.#pulseMs = config.pulseMs ? Number(config.pulseMs) : 500; - // The device ships with admin/admin. After harden() rotates it, the new - // creds are stored back in config so a re-created driver knows the current - // login (needed to rotate again — userset.cgi checks the old credentials). this.#webUser = config.webUser ? String(config.webUser) : "admin"; - this.#webPassword = config.webPassword ? String(config.webPassword) : "admin"; + // webPassword = the DESIRED login (admin's choice; blank → harden generates). + this.#webPassword = config.webPassword ? String(config.webPassword) : undefined; + // webPasswordCurrent = the device's EXISTING password (the old cred userset.cgi + // checks). Defaults to admin (factory). After a successful rotation, assign + // stores the new value back here so a re-run can rotate again. + this.#webPasswordCurrent = config.webPasswordCurrent + ? String(config.webPasswordCurrent) + : "admin"; } async connect(): Promise { @@ -499,43 +509,66 @@ class DingtianController } const secrets: Record = { relayPassword }; - // Rotate the default admin/admin web login. NOTE: cosmetic — this device's - // CGI API needs NO auth (config read/write + relay fire + this very call all - // work unauthenticated), so the login only gates the interactive browser UI, - // not the control plane. We rotate it anyway (defence-in-depth: stops a - // casual browser reaching the settings page), but it is NOT a boundary; the - // signed event log is. See dingtian-relay.md. + // Set the device web login to the admin's chosen password (or a random one). + // NOTE: cosmetic for the control plane — the CGI API needs NO auth (config + // read/write + relay fire all work unauthenticated), so the login only gates + // the interactive browser UI. We set it anyway (defence-in-depth) but it is + // NOT a boundary; the signed event log is. See dingtian-relay.md. + // + // CRITICAL: only persist webPassword if the rotation VERIFIABLY took effect. + // Otherwise the DB would claim a password the device doesn't have (the bug: + // admin types a new pw, rotation fails on the wrong old-cred, DB still saves + // the typed value, login stays admin/admin). On failure we warn instead. try { const newPassword = await this.#rotateWebLogin(); secrets.webUser = this.#webUser; secrets.webPassword = newPassword; - applied.push("rotated the admin/admin web-UI login (cosmetic — CGI API is unauthenticated)"); + // The new password is now the device's CURRENT one — store it so a future + // re-harden uses the right old cred. + secrets.webPasswordCurrent = newPassword; + applied.push("set the device web-UI login (verified on the device)"); } catch (err) { - // Don't fail the whole harden over a cosmetic step — log and continue. - stubLog(this.driverId, `web-login rotate skipped: ${(err as Error).message}`); + warnings.push( + `could not set the device web-UI login: ${(err as Error).message} ` + + `The device login is UNCHANGED (still its previous password). The saved web password was NOT updated.`, + ); } return { secrets, applied, warnings: warnings.length ? warnings : undefined }; } /** - * Rotate the device web-UI login password (keeps the username) via - * `userset.cgi?&&&&`. Returns the new - * password. The device validates the OLD credentials in the query, so we send - * the current ones (admin/admin on first run, the stored pair afterwards). - * Response is `&&&` with code 0 = success. Password is hex - * (URL-safe, no escaping) and ≤31 chars (the device truncates longer). + * Set the device web-UI login to the DESIRED password (the admin's choice, or a + * random one if none was given) via + * `userset.cgi?&&&&`. The device validates the + * OLD credentials, so we send #webPasswordCurrent (admin on a fresh device). + * Response `&&…&`, code 0 = success. + * + * After the rotation we VERIFY by attempting a no-op rotate using the NEW + * password as the old cred — if that succeeds, the device really has the new + * password (this is what catches the "DB says X but device is still admin/admin" + * bug: a wrong old-cred makes the first call fail, and we never claim success). + * Returns the password now live on the device. */ async #rotateWebLogin(): Promise { - const newPassword = randomBytes(12).toString("hex"); // 24 hex chars + const newPassword = this.#webPassword ?? randomBytes(12).toString("hex"); const u = encodeURIComponent(this.#webUser); - const oldP = encodeURIComponent(this.#webPassword); - const path = `/userset.cgi?${u}&${oldP}&${u}&${newPassword}&`; - const res = await cgiGet(this.#host, this.#httpPort, path, this.#timeout, this.#localAddress); - // "&0&/&" = success; anything else (e.g. "&-5&/&" bad params / wrong old pw). + const setPath = (oldP: string, newP: string) => + `/userset.cgi?${u}&${encodeURIComponent(oldP)}&${u}&${encodeURIComponent(newP)}&`; + + const res = await cgiGet(this.#host, this.#httpPort, setPath(this.#webPasswordCurrent, newPassword), this.#timeout, this.#localAddress); const code = res.split("&")[1]; if (code !== "0") { - throw new Error(`userset.cgi rejected (response "${res.trim()}")`); + throw new Error( + `userset.cgi rejected (response "${res.trim()}") — the device's current password is probably not "${this.#webPasswordCurrent}". ` + + `Set the correct current password, or factory-reset the device.`, + ); + } + + // VERIFY: a no-op rotate (new → new) only succeeds if the device truly has it. + const verify = await cgiGet(this.#host, this.#httpPort, setPath(newPassword, newPassword), this.#timeout, this.#localAddress); + if (verify.split("&")[1] !== "0") { + throw new Error(`web-login change did not take effect (verify response "${verify.trim()}")`); } return newPassword; } @@ -711,11 +744,14 @@ export const dingtianDriver: AccessDriver = { help: "This board idles inputs HIGH (status 1111); a press pulls LOW.", }, { key: "timeoutMs", label: "Timeout (ms)", type: "number", required: false, default: 2000 }, - // Current device web-UI login. Defaults to admin/admin; harden() rotates the - // password and stores the new pair back here so a re-run can rotate again. - // (Gates only the browser UI — the CGI control plane is unauthenticated.) + // Device web-UI login. webPassword = the password you WANT (blank → a random + // one is generated). webPasswordCurrent = the device's EXISTING password, used + // as the old credential to change it (defaults to "admin" on a fresh device). + // On a verified change, the new password is stored as both the saved login and + // the current one. (Gates only the browser UI — CGI control plane is open.) { key: "webUser", label: "Device web username", type: "string", required: false, default: "admin", help: "Device web-UI login user (default admin)." }, - { key: "webPassword", label: "Device web password", type: "secret", required: false, help: "Device web-UI login password (default admin; rotated on save)." }, + { key: "webPassword", label: "New device web password", type: "secret", required: false, help: "The password to SET on the device web UI. Leave blank to auto-generate. Applied + verified on save." }, + { key: "webPasswordCurrent", label: "Current device web password", type: "secret", required: false, help: "The device's existing web password (default admin on a fresh device). Needed to change it." }, ], create: (c) => new DingtianController(c), }; diff --git a/wiki/entities/dingtian-relay.md b/wiki/entities/dingtian-relay.md index 2360333..2f1e937 100644 --- a/wiki/entities/dingtian-relay.md +++ b/wiki/entities/dingtian-relay.md @@ -115,6 +115,17 @@ On assign the driver runs `harden()` (the [[device-registry|HardenableDevice]] c > UDP2 off in the device web UI. Verified: after the web-UI disable, the `"11"` attack gets no > reply and the relay stays off, while authenticated binary control/status still work. +> 🔑 **Web-login model (bug fixed).** The login set has TWO distinct config keys: +> `webPassword` = the password the admin WANTS (blank → harden generates a random one), and +> `webPasswordCurrent` = the device's EXISTING password (the old cred `userset.cgi` checks; +> defaults to `admin`). The original code conflated them — an admin typing a *desired* password +> made harden send it as the *old* cred, the rotation failed, yet the DB still saved the typed +> value: **the DB claimed a password the device never accepted (login stayed admin/admin).** +> Fix: harden now rotates `current → desired`, **verifies** by re-authenticating with the new +> password, and only then returns `secrets.webPassword`; assign strips the typed inputs and +> persists only the verified value (else a warning, no save). Verified on hardware: device +> rejects `admin/admin` (`&2&`) and accepts the chosen password (`&0&`) after harden. +> > ⚠️ **The device CGI API is UNAUTHENTICATED.** Verified on hardware: `GET /api/v2/config.cgi`, > `/`, and even `/userset.cgi` all return **200 with no credentials**. The `admin`/`admin` login > gates only the interactive **browser UI** — the CGI control plane (read/write full config, fire diff --git a/wiki/log.md b/wiki/log.md index 767c42a..7ac667a 100644 --- a/wiki/log.md +++ b/wiki/log.md @@ -274,3 +274,17 @@ guarantee. Recorded in [[dingtian-relay]] (new Hardening section). - INCIDENT: probing default.cgi factory-reset the bench device (now at 192.168.1.100, defaults). Re-provisioning is the ADMIN's job via First-run setup (app must not hardcode site IPs). - Updated [[append-only-event-chain]], [[dingtian-relay]]. + +## [2026-06-15] fix | Dingtian web-password: desired-vs-current split + verify + UI warnings +- BUG (found in real assign): admin typed a web password; harden used it as the OLD cred, rotation + failed silently, DB saved the typed value but device login stayed admin/admin. Also UDP2 warning + never reached the admin (frontend discarded the assign response). +- FIX: split config into webPassword (desired; blank→random) and webPasswordCurrent (existing old + cred, default admin). harden() rotates current→desired, VERIFIES by re-auth with the new pw, and + only returns secrets.webPassword on success (else warning, no save). assign strips typed + webPassword/webPasswordCurrent and persists only verified secrets. +- SetupWizard now shows assign-response warnings (amber banner, per category) — closes the + feedback loop for the UDP2-can't-disable case. +- Verified on hardware (192.168.1.100): harden set login to a chosen pw; device then rejects + admin/admin (&2&) and accepts the chosen pw (&0&). UDP2 warning surfaced as designed. +- Updated [[dingtian-relay]].