Dingtian: close password-less string-protocol relay-fire hole

The string protocol (UDP 60001) has no password field but can fire relays
("11" = relay 1 on), bypassing relay_pw entirely. Proven on hardware: an
unauthenticated packet opened a relay. harden() had left it enabled "for
status reads".

- #status() now reads via the authenticated binary command (relay cmd 0x00)
  instead of the string protocol, so the string protocol is no longer needed.
- harden() disables the string protocol (udp2.p=255). BEST-EFFORT: firmware
  V3.6J's config API silently refuses to disable udp2 (the device web UI can),
  so it's not part of the blocking verify -- harden() re-checks and returns a
  warning instead of throwing. After a web-UI disable, the attack is dead and
  binary control/status still work (verified on hardware).
- HardenResult gains an optional `warnings[]`; the assign route surfaces them
  to the admin and logs them.
- Corrected the false comment claiming relay_pw stops an attacker (it is
  defence-in-depth on plaintext UDP, not a boundary).
- Thread localAddress through the driver's UDP/HTTP calls so a multi-homed
  host sources device traffic from the device-facing NIC.
- Device web login (webUser/webPassword) is no longer redacted from setup
  state -- it's an operational credential for the admin-only device area;
  pushPassword/relayPassword stay machine-only.

Wiki: document the vuln + fix, the firmware caveat, and the out-of-band
actuation gap (the log captures host actions only; reconciliation vs. an
independent witness is the real control and is not yet built).

packages/devices/src/drivers/access-dingtian.ts

@@ -34,59 +34,37 @@ import { hostField, portField, stubLog } from "./common.js";
 // (input_link_relay). That must be DISABLED on the device for ticket-first
 // entry, else the button opens the barrier before the host can act.
 
-/** Send one UDP datagram and (optionally) await a single reply. */
-function udpRequest(
-  host: string,
-  port: number,
-  payload: string,
-  timeoutMs: number,
-  expectReply: boolean,
-): Promise<string | null> {
-  return new Promise((resolve, reject) => {
-    const sock = createSocket("udp4");
-    let settled = false;
-    const done = (err: Error | null, val: string | null) => {
-      if (settled) return;
-      settled = true;
-      clearTimeout(timer);
-      sock.close();
-      err ? reject(err) : resolve(val);
-    };
-    const timer = setTimeout(
-      () => done(expectReply ? new Error("timeout") : null, null),
-      timeoutMs,
-    );
-    sock.on("error", (e) => done(e, null));
-    sock.on("message", (m) => done(null, m.toString()));
-    sock.bind(() => {
-      sock.send(Buffer.from(payload), port, host, (e) => {
-        if (e) done(e, null);
-        else if (!expectReply) done(null, null);
-      });
-    });
-  });
-}
+// (The string-protocol UDP helper was removed: harden() now disables the
+//  password-less string protocol entirely, and status reads use the
+//  authenticated binary read — see #status() / readStatusFrame.)
 
 /**
  * Send a Dingtian *binary* protocol frame (UDP, default port 60000) and await
- * the reply. Used for relay control because — unlike the string protocol — the
- * binary protocol supports a password (`relay_pw`), so an attacker on a flat
- * network can't fire a relay without it. Frame verified on hardware:
+ * the reply. Used for ALL relay traffic — control AND status read — because,
+ * unlike the string protocol, the binary protocol carries a password (`relay_pw`).
+ * harden() disables the string protocol precisely because it has NO password and
+ * can fire relays (an unauthenticated `"11"` opens relay 1). With the string path
+ * closed, relay_pw actually gates control. Frame verified on hardware:
  *
  *   FF AA <session> <relayCmd> <pwLo> <pwHi> <data...>
  *
  *   FF        = command "set relay"
  *   AA        = result xor (0x00 ^ 0xAA, pc→device)
  *   session   = echoed back
- *   relayCmd  = 1 write, 3 jogging, …
+ *   relayCmd  = 0 read status, 1 write, 3 jogging, …
  *   pwLo,pwHi = relay password, 16-bit LSB-first (0 = none)
  *   data      = command-specific
+ *
+ * NOTE: relay_pw + plaintext UDP is defence-in-depth, NOT a boundary. An attacker
+ * who sniffs the VLAN can replay the password. The real guarantee is the signed
+ * event log (relay open with no signed command = fraud) + VLAN isolation.
  */
 function binaryUdp(
   host: string,
   port: number,
   frame: Buffer,
   timeoutMs: number,
+  localAddress?: string,
 ): Promise<Buffer> {
   return new Promise((resolve, reject) => {
     const sock = createSocket("udp4");
@@ -101,15 +79,33 @@ function binaryUdp(
     const timer = setTimeout(() => done(new Error("timeout"), null), timeoutMs);
     sock.on("error", (e) => done(e, null));
     sock.on("message", (m) => done(null, m));
-    sock.bind(() => {
+    // Bind to a specific local address (the device-facing NIC) on multi-homed
+    // hosts, so the device replies to the right source IP. See net.ts.
+    const onBound = () => {
       sock.send(frame, port, host, (e) => {
         if (e) done(e, null);
       });
-    });
+    };
+    if (localAddress) sock.bind({ address: localAddress }, onBound);
+    else sock.bind(onBound);
   });
 }
 
 let binarySession = 0;
+
+/**
+ * Build a binary "read relay status" frame (relay command 0x00). The device
+ * replies `FF AA <session> 00 <relayBytes> <inputBytes>` (status widths scale
+ * with channel count). This is the *authenticated* status read — unlike the
+ * string protocol's `00`, it carries the relay password, so we can disable the
+ * password-less string protocol entirely. Frame: `FF AA <session> 00 <pwLo> <pwHi>`.
+ * Verified on hardware (4ch): reply `ff aa 00 00 01 0f` = relay1 on, inputs 1111.
+ */
+function readStatusFrame(password: number): Buffer {
+  const session = binarySession++ & 0xff;
+  return Buffer.from([0xff, 0xaa, session, 0x00, password & 0xff, (password >> 8) & 0xff]);
+}
+
 /** Build a binary "write relay with jogging" frame (relay on, auto-off). */
 function jogFrame(channel: number, password: number, jogMs: number): Buffer {
   const session = binarySession++ & 0xff;
@@ -151,9 +147,9 @@ function writeRelayFrame(channel: number, on: boolean, password: number, channel
 const rand16 = () => randomBytes(2).readUInt16BE(0);
 
 /** GET a CGI path on the device's HTTP server and return the raw response text. */
-function cgiGet(host: string, httpPort: number, path: string, timeoutMs: number): Promise<string> {
+function cgiGet(host: string, httpPort: number, path: string, timeoutMs: number, localAddress?: string): Promise<string> {
   return new Promise((resolve, reject) => {
-    const req = httpRequest({ host, port: httpPort, path, method: "GET", timeout: timeoutMs }, (res) => {
+    const req = httpRequest({ host, port: httpPort, path, method: "GET", timeout: timeoutMs, localAddress }, (res) => {
       let data = "";
       res.on("data", (c) => (data += c));
       res.on("end", () => resolve(data));
@@ -186,6 +182,7 @@ function configApi(
   body: string | null,
   timeoutMs: number,
   sessionId?: number, // device session check: sent as Cookie: session=<id>
+  localAddress?: string, // bind outbound to the device-facing NIC (multi-homed hosts)
 ): Promise<string> {
   return new Promise((resolve, reject) => {
     // The device's embedded HTTP server does NOT support chunked request bodies.
@@ -207,6 +204,7 @@ function configApi(
         path,
         method,
         timeout: timeoutMs,
+        localAddress,
         headers: Object.keys(headers).length ? headers : undefined,
       },
       (res) => {
@@ -232,12 +230,15 @@ class DingtianController
 {
   readonly driverId = "dingtian";
   readonly #host: string;
-  readonly #port: number; // string protocol (status read) — UDP 60001
+  readonly #port: number; // legacy string-protocol port (60001) — protocol now disabled by harden(); kept for config compat
   readonly #binaryPort: number; // binary protocol (relay control) — UDP 60000
   readonly #relayPassword: number; // relay_pw (0 = none)
   readonly #sessionId: number; // device CGI session id (0 = session check off)
   readonly #httpPort: number;
   readonly #timeout: number;
+  // Local IP to source outbound device traffic from (the device-facing NIC on a
+  // multi-homed host). undefined = let the OS choose. See net.ts / device-facing-ip.
+  readonly #localAddress: string | undefined;
   readonly #channels: number;
   /** Input level at rest; an input is "active" when it differs from this. */
   readonly #restingHigh: boolean;
@@ -257,6 +258,7 @@ class DingtianController
     this.#relayPassword = config.relayPassword ? Number(config.relayPassword) : 0;
     this.#sessionId = config.sessionId ? Number(config.sessionId) : 0;
     this.#httpPort = config.httpPort ? Number(config.httpPort) : 80;
+    this.#localAddress = config.localAddress ? String(config.localAddress) : undefined;
     this.#timeout = config.timeoutMs ? Number(config.timeoutMs) : 2000;
     this.#channels = config.channels ? Number(config.channels) : 4;
     // This unit idles with inputs HIGH (status "1111"); a press pulls LOW.
@@ -297,14 +299,14 @@ class DingtianController
   async pulseOpen(doorId: number): Promise<void> {
     this.#assertChannel(doorId);
     const frame = jogFrame(doorId, this.#relayPassword, this.#pulseMs);
-    await binaryUdp(this.#host, this.#binaryPort, frame, this.#timeout);
+    await binaryUdp(this.#host, this.#binaryPort, frame, this.#timeout, this.#localAddress);
   }
 
   /** Latch a relay on/off (e.g. for a held-open mode). Channel is 1-based. */
   async setRelay(doorId: number, on: boolean): Promise<void> {
     this.#assertChannel(doorId);
     const frame = writeRelayFrame(doorId, on, this.#relayPassword, this.#channels);
-    await binaryUdp(this.#host, this.#binaryPort, frame, this.#timeout);
+    await binaryUdp(this.#host, this.#binaryPort, frame, this.#timeout, this.#localAddress);
   }
 
   async getDoorStatus(doorId: number): Promise<"open" | "closed"> {
@@ -421,19 +423,30 @@ class DingtianController
   /**
    * Lock the device down for a flat (no-VLAN) network:
    *  - set a random relay password (`relay_pw`) so binary relay commands need it,
-   *  - disable unused protocol channels (rs485/can/tcp×2/mqtt) — keep only UDP1
-   *    binary (relay control) and UDP2 string (status read).
+   *  - keep ONLY UDP1 binary (password-protected relay control + status read),
+   *  - disable every other protocol channel: string, rs485, can, tcp×2, mqtt.
    * Returns the relay password for the backend to persist (required to keep
    * commanding the device afterwards).
    *
+   * SECURITY — why the string protocol (UDP2) is now DISABLED (was a real hole):
+   * the Dingtian string protocol has NO password field and can *fire* relays
+   * (`"11"` = relay 1 on, `"21"` = off, `"11*"` = jog). Leaving it enabled — even
+   * "just for status reads" — let anyone on the network open any barrier with one
+   * unauthenticated UDP packet, completely bypassing relay_pw. Confirmed by
+   * sending `"11"` to port 60001 with no credentials and watching relay 1 close.
+   * So harden() sets udp2.p=255 and status reads move to the authenticated binary
+   * read (relay command 0x00 — see #status()).
+   *
    * NOTE: deliberately does NOT touch the device's HTTP CGI session check
    * (`session_en`). On this firmware enabling it makes the config-read API drop
    * connections, locking us out of the very API we depend on (verified the hard
    * way — required a factory reset). So we leave the config API as-is and rely on
    * relay_pw + fewer open channels + the signed event log.
    *
-   * All are plaintext over HTTP/UDP on a flat network → defence-in-depth, not a
-   * boundary; the signed event log is the real guarantee. See device-input-flow.
+   * Even with the string hole closed, all of this is plaintext over UDP/HTTP →
+   * defence-in-depth, NOT a boundary. The real guarantee is the signed event log
+   * (a relay open with no matching signed command is the fraud signal) plus VLAN
+   * isolation. See device-input-flow / network-isolation.
    */
   async harden(): Promise<HardenResult> {
     const cfg = await this.#readConfig();
@@ -442,17 +455,24 @@ class DingtianController
     const relayPassword = 1 + (rand16() % 9999); // 1..9999 (0 = none)
 
     rc.relay_pw = relayPassword;
-    // Keep UDP1=Binary (p:1) for relay control, UDP2=String (p:0) for status.
-    // Disable everything else (p:255 = None).
+    // Keep ONLY UDP1=Binary (p:1) — it carries relay_pw for both control AND the
+    // status read. Disable everything else (p:255 = None), INCLUDING the string
+    // protocol (udp2), which is password-less and can fire relays.
     (rc.udp1 as Record<string, unknown>).p = 1;
-    (rc.udp2 as Record<string, unknown>).p = 0;
+    (rc.udp2 as Record<string, unknown>).p = 255;
     (rc.rs485 as Record<string, unknown>).p = 255;
     (rc.can as Record<string, unknown>).p = 255;
     (rc.tcpc as Record<string, unknown>).p = 255;
     (rc.tcps as Record<string, unknown>).p = 255;
     (rc.mqtt as Record<string, unknown>).p = 255;
 
-    await this.#writeConfig(cfg, (after) => {
+    // NOTE: udp2 (string protocol) is set to 255 here, but it is NOT part of the
+    // blocking verify. On some firmware (e.g. V3.6J) the CONFIG API silently
+    // refuses to disable udp2 — it accepts the write, reboots, and clamps it back
+    // to enabled — even though every other channel applies and the device's own
+    // web UI CAN disable it. We don't want assign to hard-fail over a firmware
+    // quirk, so we attempt it, then re-check below and warn if it didn't stick.
+    const afterCfg = await this.#writeConfig(cfg, (after) => {
       const a = after.relay_connect as Record<string, unknown> | undefined;
       return (
         a?.relay_pw === relayPassword &&
@@ -463,8 +483,20 @@ class DingtianController
 
     const applied = [
       "set relay password",
-      "disabled rs485/can/tcp/mqtt channels (kept UDP binary + string)",
+      "disabled rs485/can/tcp/mqtt channels (kept password-protected UDP binary)",
     ];
+    const warnings: string[] = [];
+    const stringDisabled =
+      ((afterCfg.relay_connect as Record<string, unknown>)?.udp2 as Record<string, unknown> | undefined)?.p === 255;
+    if (stringDisabled) {
+      applied.push("disabled the password-less string protocol (udp2)");
+    } else {
+      warnings.push(
+        "could not disable the string protocol (udp2) via the config API — this firmware ignores it. " +
+          "An unauthenticated UDP packet to the string port can still fire relays. " +
+          "Disable UDP2 in the device web UI, and rely on VLAN isolation + the signed event log. See dingtian-relay.md.",
+      );
+    }
     const secrets: Record<string, string | number> = { relayPassword };
 
     // Rotate the default admin/admin web login. NOTE: cosmetic — this device's
@@ -483,7 +515,7 @@ class DingtianController
       stubLog(this.driverId, `web-login rotate skipped: ${(err as Error).message}`);
     }
 
-    return { secrets, applied };
+    return { secrets, applied, warnings: warnings.length ? warnings : undefined };
   }
 
   /**
@@ -499,7 +531,7 @@ class DingtianController
     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);
+    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 code = res.split("&")[1];
     if (code !== "0") {
@@ -511,7 +543,7 @@ class DingtianController
   // --- config api internals ----------------------------------------------
 
   async #readConfig(): Promise<Record<string, unknown>> {
-    const raw = await configApi(this.#host, this.#httpPort, "/api/v2/config.cgi", "GET", null, this.#timeout, this.#sessionId);
+    const raw = await configApi(this.#host, this.#httpPort, "/api/v2/config.cgi", "GET", null, this.#timeout, this.#sessionId, this.#localAddress);
     return JSON.parse(raw) as Record<string, unknown>;
   }
 
@@ -527,7 +559,7 @@ class DingtianController
   async #writeConfig(
     cfg: Record<string, unknown>,
     verify: (after: Record<string, unknown>) => boolean,
-  ): Promise<void> {
+  ): Promise<Record<string, unknown>> {
     // The set endpoint requires `"command":"setconfig"` injected after `status`
     // (the GET payload omits it). Rebuild preserving node order, command second.
     const out: Record<string, unknown> = {};
@@ -543,7 +575,7 @@ class DingtianController
       // POST. The device resets on apply, so the connection may drop — that's
       // expected, not failure.
       try {
-        await configApi(this.#host, this.#httpPort, "/api/v2/config_set.cgi", "POST", payload, this.#timeout, this.#sessionId);
+        await configApi(this.#host, this.#httpPort, "/api/v2/config_set.cgi", "POST", payload, this.#timeout, this.#sessionId, this.#localAddress);
       } catch {
         // device likely reset on apply
       }
@@ -552,7 +584,8 @@ class DingtianController
       for (let i = 0; i < 12; i++) {
         await sleep(2000);
         try {
-          if (verify(await this.#readConfig())) return; // applied
+          const after = await this.#readConfig();
+          if (verify(after)) return after; // applied — return the landed config
         } catch {
           // still rebooting / unreachable — keep polling
         }
@@ -581,21 +614,35 @@ class DingtianController
     }
   }
 
-  /** Query "00" → parse "0000:1111:4" into relays/inputs/channels. */
+  /**
+   * Read relay + input status via the AUTHENTICATED binary protocol (relay
+   * command 0x00). Reply: `FF AA <session> 00 <relayBytes...> <inputBytes...>`,
+   * each field `ceil(channels/8)` bytes, LSB-first (bit0 → relay/input 1).
+   *
+   * SECURITY: deliberately NOT the string protocol's `00` — that query has no
+   * password field AND the string protocol can also *fire* relays, so leaving it
+   * enabled defeats relay_pw entirely (an attacker sends `"11"` to open relay 1
+   * with no auth). harden() disables the string protocol; status reads come here.
+   */
   async #status(): Promise<DingtianStatus> {
-    const reply = await udpRequest(this.#host, this.#port, "00", this.#timeout, true);
-    if (!reply) throw new Error("dingtian: empty status reply");
-    const [relayStr, inputStr, countStr] = reply.trim().split(":");
-    if (relayStr === undefined || inputStr === undefined) {
-      throw new Error(`dingtian: bad status reply "${reply}"`);
+    const frame = readStatusFrame(this.#relayPassword);
+    const reply = await binaryUdp(this.#host, this.#binaryPort, frame, this.#timeout, this.#localAddress);
+    const width = Math.max(1, Math.ceil(this.#channels / 8));
+    // header: FF AA session 00  (4 bytes) + relay field + input field
+    if (reply.length < 4 + width * 2) {
+      throw new Error(`dingtian: short binary status reply (${reply.length} bytes)`);
     }
-    const bit = (c: string) => c === "1";
-    return {
-      relays: [...relayStr].map(bit),
-      // active = differs from the resting level (press pulls the line).
-      inputs: [...inputStr].map((c) => bit(c) !== this.#restingHigh),
-      channels: countStr ? Number(countStr) : this.#channels,
-    };
+    const relayVal = reply.readUIntLE(4, width);
+    const inputVal = reply.readUIntLE(4 + width, width);
+    const relays: boolean[] = [];
+    const inputs: boolean[] = [];
+    for (let i = 0; i < this.#channels; i++) {
+      const high = (inputVal & (1 << i)) !== 0;
+      relays.push((relayVal & (1 << i)) !== 0);
+      // active = differs from the resting level (a press pulls the line).
+      inputs.push(high !== this.#restingHigh);
+    }
+    return { relays, inputs, channels: this.#channels };
   }
 
   #startPolling(): void {

packages/devices/src/interfaces.ts

@@ -142,6 +142,10 @@ export interface HardenResult {
   readonly secrets: Record<string, string | number>;
   /** Human-readable summary of what was changed (for logging/UI). */
   readonly applied: string[];
+  /** Hardening steps that could NOT be applied (e.g. a firmware quirk), so the
+   *  admin knows a residual risk remains. Best-effort steps report here instead
+   *  of failing the whole harden. */
+  readonly warnings?: string[];
 }
 
 export function isHardenable(device: Device): device is Device & HardenableDevice {