Skip to content

sec(key-mgmt): two at-rest reads carry the no-mint rule incidentally — read_legacy's ? swapped for .ok() mints over a locked seed and deletes it #345

Description

@MichaelTaylor3d

Task

Three follow-ups from the security gate on PR #342 (05a480a3, machine-key sealing). None gated that
PR. The first is the one that matters.

N-A — two at-rest reads carry the no-mint rule INCIDENTALLY, not by name

read_legacy:508 and unseal_stored:391 are safe because of a ? and an early return
respectively
, not because of a named arm — and neither is tested.

read_legacy:508 is the one worth a battery row. The gate's stated exploit:

swap its ? for .ok() and it mints over a present-but-locked plaintext seed — and then deletes
it.

That is the same destructive class the PR spent three rounds closing on the device-key read, surviving in
a place nobody looked because the ? happens to do the right thing. A rule enforced by an operator
rather than by an arm is a rule one refactor away from being gone
, and the lane's own retrospective
named this shape: "a rule applied twice is not a rule applied."

Fix: give both reads an explicit named arm, and add a battery row for each — specifically one that
swaps read_legacy's ? for .ok() and proves a named test fails.

N-B — a zero-length device.key stub permanently blocks the first mint

And its message asserts an identity that does not exist. Fails closed and is recoverable, so it is not
urgent — but the message should say what is known and what is undetermined, per dig-keystore
SPEC.md §17.5b's discipline, rather than naming a cause it cannot establish.

N-C — crates/dig-node-core/src/lib.rs:4423 says "tracked separately" with no URL

The gate looked and found no ticket. Either cite this one by URL or delete the phrase. A dangling
"tracked separately" is worse than silence: it tells a reader the work is owned when it is not.

Related but SEPARATE — do not fold in

N1 from the same gate: presence() reports an unreachable Windows path as Absent, contradicting
SPEC.md §18.25's MUST. Measured: //dig-no-such-host/share/… -> presence=Ok(Absent).

It is not a regression and it is not attacker-steerable — every traced path still fails closed,
because the sibling device dir is on the same unreachable tree and the write fails first. Only the
CLAIM is new.
So the honest fix is one clause in §18.25 scoping the MUST to what try_exists can
actually determine, not a code change. Worth its own ticket if it is not folded into N-A's pass.

Evidence

  • Prove the negative for N-A: the .ok() mutation must make a named test fail, and it must fail
    for the right reason rather than as a compile error.
  • Every mutation asserts its patch applied — a replace that matched nothing prints a pass and reads as
    "the guard never fires", the exact inverse of the truth.
  • Parse the test result: line, never stderrcargo test prints error: test failed on a
    legitimately red run.
  • One cargo process at a time in the worktree; concurrent invocations sharing a target/ produce
    non-reproducible failures.
  • Read passed-counts, never ok. Baseline at merge: machine_key 20 passed / 0 failed.
  • Never print a seed, device key, or mnemonic.

Cross-references

From: #342 (eco#2168), merged 05a480a3
Related, still open: #343 (the derived BLS key is at rest unsealed at
peer-net/identity/node.key, which bounds what #342 can claim about copy-resistance) ·
dig_ecosystem#2177 (assert the dig-keystore custody feature never enables — Cargo unifies features
across a workspace, so custody OFF is a fail-closed default, not an invariant)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions