Skip to content

keep-frost-net: hardening & test-coverage backlog (21 items) #790

Description

@kwsantiago

Hardening and test-coverage findings in keep-frost-net.

Read this before picking anything up. The original list was produced by an automated pass whose header claimed each entry was verified still open. A manual pass on 2026-07-30 read the code for all twenty-one and that claim did not hold: four were already fixed, two were duplicates of each other, five described one unbuilt feature rather than five gaps, and three were wrong in a way that would have made the code worse if implemented as written. Those have been removed, with the reasoning kept in the comments below rather than in the list.

Entries here name where the code looks wrong. They are not instructions. Read the code before acting on one, and if it does not say what the entry says, correct the entry.

Open

  • Announced verifying share is not bound for holders with an incomplete package — The announce handler compares an announced verifying share against the group's canonical share for the claimed index and rejects a mismatch, so a self-generated share cannot be announced under a member index. A node whose share arrived through an encrypted export holds only its own verifying share, cannot perform that comparison for an index it does not know, and falls back to the proof-of-secret gate alone. Fix is to make exports carry the full verifying-share set. (Replaces two entries that described the binding as absent; it exists, at the announce path rather than at verify_peer_share_index.)
  • Software and hardware DKG scaffolding duplicatedcmd_frost_network_dkg_hardware and cmd_frost_network_dkg_software remain two large functions with duplicated relay setup, round-1 publish, fetch loops, timeouts and encryption. No shared transport helper extracted.
  • Shutdown does not abort an in-flight announce — Single-flight suppression is covered by a test; nothing asserts the in-flight announce is aborted on shutdown or run-exit.
  • Group pubkey and session id travel in unencrypted event tags — Descriptor coordination events attach group pubkey, session id and message type as plaintext tags. Unverified in the 2026-07-30 pass; confirm against current code before acting.
  • Descriptor lookup fails closed while the vault is lockedfind_by_hash returns no-match when locked and the caller emits a misleading "no finalized descriptor for group" error. Needs either an unlock-independent index or an event distinguishing the two.
  • Mixed-version pools reject descriptor coordination — The recovery descriptor canonical form changed and is compared by exact string equality, with no version negotiation. Pre-0.4 pools and current ones reject each other.
  • TLS port falls back to 443 through an unwrap — Port is derived via port_or_known_default().unwrap_or(443).

Removed as already fixed

The canonical descriptor hash is no longer inlined at the descriptor session; it calls the shared helper. PSBT finalize now marks the session finalized before broadcasting. Announce single-flight has a test (the shutdown half is kept above). Hostname pin lookups are already case-normalized, because the host comes from the URL parser, with a test asserting bare host, case, port, scheme and path all canonicalize.

Removed as wrong

Certificate pinning module missing forbid(unsafe_code). The attribute is a crate-root inner attribute and already applies to every module, which is why no module carries its own copy. Adding one would make this the only module with a local copy and imply the others are unprotected. The crate contains no unsafe at all.

Signature-complete skips participant auth for unknown sessions. Nothing is accepted: the handler logs and returns without writing an audit entry, emitting an event or invoking a hook. Ignoring a completion for a session we already finished and pruned is the reasonable response. More decisively, dispatch is gated on trusted-peer status for every message type except announce, sign request, ECDH request, OPRF eval request and OPRF enroll, so an untrusted sender never reaches the handler.

Certificate pins need expiry or a maximum age. Wrong for both kinds of pin here. Operator-provisioned pins loaded from configuration would either silently stop pinning or start refusing connections at an arbitrary later moment with no operator action. Trust-on-first-use pins are never persisted at all, so they are already bounded by process lifetime, and expiring them would hand an attacker who missed the first connection another opportunity on every renewal. The maximum-age concept comes from a specification governing pins a browser learned from a server header and cached across sessions; neither case here matches it.

No unit test for descriptor reconstruction. It had two, covering an invalid network and a missing contribution. This entry was my own error, from a search that reported absence because a test function's name line contains neither the attribute nor the word "test". Three further cases have since been added for paths that genuinely had none.

Cross-layer xpub prefix inconsistency. Protocol-level validation is network-agnostic because that layer does not know the network; the session layer enforces the network-specific rule and rejects with a clear message. Layered validation, not a defect.

Moved out: distributed refresh

Five entries described the refresh handlers as missing a replay window, a trust-check entry, a participant minimum, authentication, and target-index cross-validation. All five reduce to the same fact: the handlers are a single warn-only stub. There is nothing to harden until the feature exists. They belong on a feature issue for distributed refresh with those five as acceptance criteria, not on a hardening list where they make the backlog read five times deeper than it is.

Metadata

Metadata

Assignees

No one assigned

    Labels

    nostr-frostNostr FROST coordination protocolp2PrioritysecuritySecurity-related issues

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions