docs(1234): defer the keystore wiring — the design was forbidden by its own requirement - #1407
Merged
Merged
Conversation
…ts own requirement A review artifact and a ledger entry. No code. #1234's wiring half stays unbuilt and `inert_psk_key_id_warning` stays as the honest surface. I proposed option 1 of the blocker comment: a CLI writer plus a daemon reader, so `psk_key_id` stops being inert. It went to review before implementation and did not survive. 1. MY PREFERRED MASTER SOURCE IS FORBIDDEN BY REQ-CTL-04. I proposed an owner-only master-password file. The requirement's bullet ends "The master password must never be written to disk in plaintext" — ONE LINE BELOW the text I quoted in the design's own Prior art. 2. THE ENV-VAR VARIANT IS A LATERAL MOVE, AND THE THREAD ALREADY SAID SO: "env->env buys nothing … the keystore earns its place only via REQ-CTL-03 or multi-secret storage." Neither holds here. Third occurrence of designing from the issue body rather than the thread. 3. THE THREAT MODEL DOES NOT SUPPORT IT. "Environment" is already backed by a 0600 file; /proc/<pid>/environ is 0400 and ptrace-scoped. And the Ed25519 station seed — signing all 13 registered domains — already sits in plaintext at 0600. AEAD-wrapping the PSK beside it protects nothing. COSTS I HAD NOT LISTED: it un-deflates audit finding B4 (FileKeystore::save truncates with no temp+rename or fsync; deflated *because* nothing consumes the keystore — with a consumer, an interrupted write on a Pi SD card leaves a file `open` rejects and the daemon refuses to start with the PSK gone); B3 likewise; a C library (keyring -> dbus-secret-service -> libdbus-sys) enters the release binaries, which the same design doc rejected OpenSSL for; Argon2 at 19 MiB on start; and no deployment has ever set the PSK — every on-air rig binds loopback. TWO DEFECTS IN MY OWN DESIGN: no activation rule at all (psk_key_id defaults to "control-psk" so it is always set, and FileStore::open creates an empty store when the path is absent — fail-open); and it re-opened a decision already resolved at control-channel-security.md:93. Verification-objective: a secret-storage change must be judged against the governing requirement's full text and this product's actual threat model, so that indirection is not mistaken for protection; this records why the proposed wiring is neither, and names the ~20-line alternative that would be. Review: docs/dev/reviews/review-1234-keystore-writer.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A review artifact and a ledger entry. No code. #1234's wiring half stays unbuilt and
inert_psk_key_id_warningstays as the honest surface.I proposed option 1 of that issue's blocker comment — a CLI writer plus a daemon reader, so
psk_key_idstops being inert. The design went to adversarial review before implementation and did not survive it.Three reasons, two of them documents already in this repo
1. My preferred master-password source is forbidden by the requirement it would implement. I proposed an owner-only master-password file. REQ-CTL-04's bullet ends:
That sentence is one line below the text I quoted in the design's own Prior-art section.
control-channel-security.md:59agrees — the master is prompted, or a one-shot env var.2. The env-var variant is a lateral move, and this issue's thread had already said so. From its second comment: "env→env buys nothing … the keystore earns its place only via REQ-CTL-03 (an OS keychain) or multi-secret storage." Neither precondition holds. Third occurrence of designing from the issue body rather than the thread.
3. The threat model does not support it. "Environment" is a delivery mechanism already backed by a 0600 file;
/proc/<pid>/environis 0400 and ptrace-scoped — the same tier as a 0600 master file. And the Ed25519 station seed, which signs all 13 registered domains, already sits in plaintext at 0600 (config/src/lib.rs:910-955). AEAD-wrapping the PSK beside a plaintext identity key protects nothing.Costs the design had not listed
FileKeystore::savetruncates without temp+rename or fsync, and B4 was deflated because nothing consumes the keystore. With a consumer, an interruptedkeystore seton a Pi's SD card — this product's primary deployment — leaves a fileopenrejects, and the daemon then refuses to start with the PSK gone. An env var cannot be half-written. B3 (no zeroize) un-deflates identically.keyring→dbus-secret-service→libdbus-sys. The same design doc rejected OpenSSL for exactly that.Two defects in my own design
psk_key_iddefaults to"control-psk", so it is always set and liveness cannot be inferred from it; andFileStore::opencreates an empty in-memory store when the path is absent, so a mistyped path yields "key missing" rather than an error. Fail-open by construction.control-channel-security.md:93already settles the panel's path as OS-keychain-first.What would reopen this — and it is not a keystore
REQ-CTL-05's text already lists a "PSK file" among secret files.
[control_security] psk_file = "<path>", read once at start throughvalidate_owner_only, is ~20 lines at the same security tier with the moving parts removed, and composes with systemdLoadCredential=/LoadCredentialEncrypted=. A new proposal needing its own review, named so this deferral is not read as "nothing can improve this".Correction owed to #1405
A daemon→keystore dependency would have helped CTL-01/02 only, not CTL-05 (bound in
openpulse-config), and even then those 57 mutants would go from "unreachable by construction" to "reachable and MISSED", sincecontrol_auth.rsnever calls the keystore. Smaller than I stated, and never the justification.Verification
trace check ok · ledger order ok · doc frontmatter ok · doc stamps ok · trailer lint ok · re-homed docs ok. Docs only.
Verification-objective: a secret-storage change must be judged against the governing requirement's full text and this product's actual threat model, so that indirection is not mistaken for protection.
Review: docs/dev/reviews/review-1234-keystore-writer.md
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6