PIN hashing is a three-way mismatch: CLI, FSM, and web arm/disarm all incompatible #2

New Issue

alee · 2026-04-05T14:49:53Z

alee commented

2026-04-05 14:49:53 +00:00

Severity: bug (and a mild security regression in one of the paths)
Files: vigilar/cli/cmd_config.py, vigilar/events/state.py, vigilar/web/blueprints/system.py, vigilar/alerts/pin.py, vigilar/config.py

Three different modules hash the arm/disarm PIN three different ways, using two different config keys. None of them interoperate.

Component	File:line	Hash scheme	Config key read/written
Web arm/disarm/reset	`web/blueprints/system.py:63-64, 74-75, 88` via `alerts/pin.py`	PBKDF2-SHA256, 600k iterations, salted, format `pbkdf2_sha256$salt$dk`	`[security] pin_hash`
Events FSM (`ArmStateFSM`)	`events/state.py:50`	Unsalted SHA-256(pin) — one-shot hash, no salt, no iterations	`[system] arm_pin_hash`
CLI `vigilar config set-pin`	`cli/cmd_config.py:86-95`	HMAC-SHA256(random_secret, pin), format `secret_hex:mac`	`[system] arm_pin_hash`

Consequences:

vigilar config set-pin produces an unusable value. It writes an HMAC-format string to [system] arm_pin_hash. The FSM reads that same key but expects a raw SHA-256 hex digest, so verification always fails.
The web arm endpoint is a stub regardless of PIN. system.py:57-66 checks the PBKDF2 hash at [security] pin_hash but then just returns {"ok": true, "state": mode} without calling the FSM. State never transitions from the web UI even when the PIN is correct.
The FSM's PIN scheme is weak. Unsalted SHA-256 is trivially offline-brute-forceable for 4-digit PINs (full keyspace in microseconds). The alerts/pin.py path already uses PBKDF2-SHA256 with 600k iterations, which is the right choice.
Misleading comment. events/state.py:46 says "Verify a PIN against the stored hash using HMAC comparison" — but hmac.compare_digest is a timing-safe equality check, not an HMAC. No HMAC is computed on the FSM path.

Fix (proposed design, wants sign-off before implementation):

Canonicalize on alerts/pin.hash_pin / verify_pin (PBKDF2-SHA256, 600k iterations).
Canonicalize on one config key — recommend keeping [security] pin_hash (already in use by the web path, lives in a security-scoped section) and removing [system] arm_pin_hash from SystemConfig.
Rewrite ArmStateFSM.verify_pin to call alerts.pin.verify_pin.
Rewrite CLI set-pin to call alerts.pin.hash_pin and emit a [security] pin_hash line.
Wire web/blueprints/system.py:arm_system / disarm_system to actually publish a transition request on MQTT that the FSM subscribes to (or call the FSM directly if they share a process — they don't, so MQTT).
Migration: on config load, if [system] arm_pin_hash is set but [security] pin_hash is empty, log a warning that the old key is ignored and vigilar config set-pin must be re-run.
Tests: add an end-to-end case that sets a PIN via the CLI, then arms/disarms via the web and the FSM using the same PIN.

**Severity:** bug (and a mild security regression in one of the paths) **Files:** `vigilar/cli/cmd_config.py`, `vigilar/events/state.py`, `vigilar/web/blueprints/system.py`, `vigilar/alerts/pin.py`, `vigilar/config.py` Three different modules hash the arm/disarm PIN three different ways, using two different config keys. None of them interoperate. | Component | File:line | Hash scheme | Config key read/written | |---|---|---|---| | Web arm/disarm/reset | `web/blueprints/system.py:63-64, 74-75, 88` via `alerts/pin.py` | **PBKDF2-SHA256**, 600k iterations, salted, format `pbkdf2_sha256$salt$dk` | `[security] pin_hash` | | Events FSM (`ArmStateFSM`) | `events/state.py:50` | **Unsalted SHA-256(pin)** — one-shot hash, no salt, no iterations | `[system] arm_pin_hash` | | CLI `vigilar config set-pin` | `cli/cmd_config.py:86-95` | **HMAC-SHA256(random_secret, pin)**, format `secret_hex:mac` | `[system] arm_pin_hash` | Consequences: 1. **`vigilar config set-pin` produces an unusable value.** It writes an HMAC-format string to `[system] arm_pin_hash`. The FSM reads that same key but expects a raw SHA-256 hex digest, so verification always fails. 2. **The web arm endpoint is a stub regardless of PIN.** `system.py:57-66` checks the PBKDF2 hash at `[security] pin_hash` but then just returns `{"ok": true, "state": mode}` without calling the FSM. State never transitions from the web UI even when the PIN is correct. 3. **The FSM's PIN scheme is weak.** Unsalted SHA-256 is trivially offline-brute-forceable for 4-digit PINs (full keyspace in microseconds). The `alerts/pin.py` path already uses PBKDF2-SHA256 with 600k iterations, which is the right choice. 4. **Misleading comment.** `events/state.py:46` says "Verify a PIN against the stored hash using HMAC comparison" — but `hmac.compare_digest` is a timing-safe equality check, not an HMAC. No HMAC is computed on the FSM path. **Fix (proposed design, wants sign-off before implementation):** - Canonicalize on `alerts/pin.hash_pin` / `verify_pin` (PBKDF2-SHA256, 600k iterations). - Canonicalize on one config key — recommend **keeping `[security] pin_hash`** (already in use by the web path, lives in a security-scoped section) and **removing `[system] arm_pin_hash`** from `SystemConfig`. - Rewrite `ArmStateFSM.verify_pin` to call `alerts.pin.verify_pin`. - Rewrite CLI `set-pin` to call `alerts.pin.hash_pin` and emit a `[security] pin_hash` line. - Wire `web/blueprints/system.py:arm_system` / `disarm_system` to actually publish a transition request on MQTT that the FSM subscribes to (or call the FSM directly if they share a process — they don't, so MQTT). - Migration: on config load, if `[system] arm_pin_hash` is set but `[security] pin_hash` is empty, log a warning that the old key is ignored and `vigilar config set-pin` must be re-run. - Tests: add an end-to-end case that sets a PIN via the CLI, then arms/disarms via the web and the FSM using the same PIN.

alee referenced this issue

2026-04-05 14:51:19 +00:00

fix: correct set-password docstring (scrypt, not bcrypt) #4

alee referenced this issue from a commit

2026-04-05 15:18:19 +00:00

docs(plan): implementation plan for PIN hashing unification (issue #2)

alee referenced this issue from a commit

2026-04-05 16:27:14 +00:00

feat(config): deprecation warning for [system] arm_pin_hash

alee referenced this issue from a commit

2026-04-05 16:27:14 +00:00

feat(events): processor handles SYSTEM_ARM_REQUEST over MQTT