docs(quick-260731-upz): entropy-audit remediation backlog + tracker items
- Adds F-13 (High) to the audit: the BIP-84 account PRIVATE key is imported into Bitcoin Core (bitcoin.rs:203 disable_private_keys=false, :229-231 wpkh(xprv/...)), so the spending key is persisted outside the Argon2 envelope in a wallet with an empty passphrase; the descriptors also carry no [fingerprint/derivation] key origin, so no hardware signer could ever use them. Found by tracing secret class (1) end-to-end - Fills the Remediation Backlog: R-00..R-15, prioritised severity x effort, each with the finding it closes, files, effort, and hardware gating; plus an explicit "not implemented here, and why" section - Records ARCHY-1 as APPLIED with the exact test evidence and an honest note that making the source explicit removes a future failure mode rather than repairing a past one - Wires the resulting open items into docs/UNIFIED-TASK-TRACKER.md in its existing tier/checkbox format: Tier 0 (cargo audit/deny CI, ceremony mnemonic input, a five-item hygiene batch, secrets.rs OsRng), Tier 1 (ISO fail-open first-boot secrets, Argon2 vs ADR-005, the on-node checklist), Tier 2 (the Critical unauthenticated seed RPCs, PSBT Phase 1, PSBT phases 2-7, seed-RPC transport confinement) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
archipelago
co-authored by
Claude Opus 5
Claude Opus 5
parent
5f366f7589
commit
5ba80e49b7
@@ -159,6 +159,7 @@ action of the "your seed may be predictable, migrate now" kind is warranted —
|
||||
| F-10 | **Low** | Container `generated_secrets` use `thread_rng()` rather than an explicit `OsRng` (same T1 shape as F-02, smaller blast radius) | `core/archipelago/src/container/secrets.rs:92`, `:101` |
|
||||
| F-11 | **Informational** | `Math.random()` inside a seed-handling view (benign — UX challenge selection only) | `neode-ui/src/views/OnboardingSeedVerify.vue:159` |
|
||||
| F-12 | **Informational** | Identical default OS credentials on every flashed node | `image-recipe/archipelago-scripts/install-to-disk.sh:205` |
|
||||
| F-13 | **High** | BIP-84 account **private** key is imported into Bitcoin Core's wallet, duplicating the spending key outside the encrypted envelope | `core/archipelago/src/api/rpc/bitcoin.rs:203`, `:229-231` |
|
||||
|
||||
---
|
||||
|
||||
@@ -524,6 +525,55 @@ in scope to name.
|
||||
|
||||
---
|
||||
|
||||
### F-13 — BIP-84 account **private** key is imported into Bitcoin Core — **High**
|
||||
|
||||
**Evidence.** `core/archipelago/src/api/rpc/bitcoin.rs:161-294`
|
||||
(`handle_bitcoin_init_wallet_from_seed`):
|
||||
|
||||
- `:203` passes `disable_private_keys = false` to `createwallet`.
|
||||
- `:188-189` derives the BIP-84 account **xprv** (`crate::seed::derive_bitcoin_xprv`,
|
||||
`core/archipelago/src/seed.rs:207-224`) and stringifies it.
|
||||
- `:229-231` builds `wpkh({xprv}/0/*)` and `wpkh({xprv}/1/*)`.
|
||||
- `:278-281` imports those descriptors via `importdescriptors`.
|
||||
|
||||
**Assessment.** The node's Bitcoin spending key is therefore persisted **twice**: once in the
|
||||
daemon's Argon2 + ChaCha20-Poly1305 envelope (`core/archipelago/src/seed.rs:238-269`, `0600` via
|
||||
`:318-324`), and once in Bitcoin Core's `wallet.dat`, which has neither the Argon2 passphrase
|
||||
protection nor the same ownership story — it lives in the Bitcoin Core container's data volume.
|
||||
The wallet is created with an **empty** encryption passphrase (`bitcoin.rs:205`), so Core's own
|
||||
wallet encryption is not engaged either.
|
||||
|
||||
**Not an entropy defect**, and reported here because Step B required tracing secret class (1),
|
||||
the user Bitcoin/LND wallet seed, from generation to consumer — and this is where that trace
|
||||
ends up.
|
||||
|
||||
**Credit where due:** the in-memory handling of the xprv string is careful — it is zeroized on
|
||||
both the error path (`bitcoin.rs:222`) and the success path (`:284`) — and the wallet is a
|
||||
*descriptor* wallet (`:207`), which is the correct foundation. The defect is which key goes
|
||||
into it.
|
||||
|
||||
**Secondary defect, same lines.** The descriptors at `:230-231` carry **no key-origin
|
||||
annotation** (`[fingerprint/derivation]`). Without it, no hardware signer can locate its key in
|
||||
a PSBT — so the current wallet could not be converted to an external-signer setup even if the
|
||||
private key were removed.
|
||||
|
||||
**Exploitability.** Requires read access to the Bitcoin Core data volume. That is a lower bar
|
||||
than the encrypted envelope: a container escape, a backup of the Bitcoin volume, or a
|
||||
misconfigured bind mount exposes it, whereas `master_seed.enc` additionally requires the user's
|
||||
password.
|
||||
|
||||
**Blast radius.** The node's entire on-chain Bitcoin balance at `m/84'/0'/0'`. It does **not**
|
||||
extend to the other key classes — the release-root key, node identity and FIPS keys are HKDF
|
||||
siblings, not children of the BIP-84 branch, so an attacker with the account xprv cannot climb
|
||||
back to the master seed.
|
||||
|
||||
**Remediation.** Pass `disable_private_keys = true`; import the account **xpub** with a
|
||||
key-origin annotation instead of the xprv; sign via the daemon (or an external signer) rather
|
||||
than via Core. This is Phase 1 of `docs/security/PSBT-SIGNING-ARCHITECTURE.md` §8, including
|
||||
the migration that verifies balance/UTXO parity before removing the private-key-bearing wallet.
|
||||
|
||||
---
|
||||
|
||||
## 4. [ARCHY-1] … [ARCHY-4] adjudication
|
||||
|
||||
### [ARCHY-1] — **CONFIRMED**
|
||||
@@ -785,9 +835,41 @@ every install.
|
||||
|
||||
---
|
||||
|
||||
## 7. `ARCHY-1` remediation status
|
||||
## 7. `ARCHY-1` remediation status — **APPLIED**
|
||||
|
||||
<!-- filled by Task 3 -->
|
||||
The injectable-RNG-seam refactor described in F-02 was applied to
|
||||
`core/archipelago/src/seed.rs`. Scope, precisely:
|
||||
|
||||
- A new private helper `generate_mnemonic_with<R: rand::CryptoRng + rand::RngCore>(rng: &mut R)`
|
||||
calls `bip39::Mnemonic::generate_in_with(rng, Language::English, 24)` — the **injectable**
|
||||
bip39 entry point — instead of the defaulting `Mnemonic::generate(24)`.
|
||||
- `MasterSeed::generate()` passes `&mut rand::rngs::OsRng` explicitly.
|
||||
- A doc comment at the helper pins the rationale to this audit and to T1, so a future
|
||||
`rand`/`bip39` bump cannot rebind the entropy source without someone reading why it matters.
|
||||
- Two tests added to the existing module.
|
||||
|
||||
**Nothing else changed.** The derivation paths, the 24-word count, the empty-BIP-39-passphrase
|
||||
decision, the at-rest encryption envelope, and every existing test are untouched.
|
||||
|
||||
**Tests.** `cd core && CARGO_INCREMENTAL=0 cargo test -p archipelago seed::` →
|
||||
**25 passed, 0 failed.**
|
||||
|
||||
- `mnemonic_generation_uses_injected_rng` — drives generation from a deterministic
|
||||
`CryptoRng + RngCore` test RNG and asserts (a) the result equals
|
||||
`bip39::Mnemonic::from_entropy(<the exact bytes that RNG emitted>)`, which is the direct proof
|
||||
that the **injected** RNG — not bip39's transitive `rand::thread_rng()` default — is the one
|
||||
actually consumed; (b) a known-answer word list; (c) determinism across two identical RNG
|
||||
states. **This test is impossible to write against the pre-change code**, because
|
||||
`Mnemonic::generate(24)` exposes no seam through which the RNG can be observed or substituted.
|
||||
- `mnemonic_generation_is_256_bit` — the production `OsRng` path yields 24 words and two
|
||||
successive productions differ.
|
||||
|
||||
**The residual risk this does not close.** Making the source explicit does not make the *fix*
|
||||
retroactive: mnemonics generated before this change came from `rand::thread_rng()`. That was
|
||||
and remains a genuine CSPRNG (F-02, "Exploitability: none today"), so no existing seed is
|
||||
weakened — but the guarantee for those seeds rests on `rand 0.8.5`'s behaviour, not on this
|
||||
call site. Reviewers should read this as *removing a future failure mode*, not as repairing a
|
||||
past one.
|
||||
|
||||
Everything else in this document is queued in §8, not implemented.
|
||||
|
||||
@@ -795,7 +877,45 @@ Everything else in this document is queued in §8, not implemented.
|
||||
|
||||
## 8. Remediation Backlog
|
||||
|
||||
<!-- filled by Task 3 -->
|
||||
Prioritised by severity × effort, most valuable per unit of work first. **Only R-00 was
|
||||
implemented by this audit.** Everything else is queued here and mirrored into
|
||||
`docs/UNIFIED-TASK-TRACKER.md` so it is not stranded in a document nobody re-reads.
|
||||
|
||||
**Effort scale:** S = under an hour; M = half a day; L = a day or more; **PHASE** = needs its
|
||||
own `/gsd-plan-phase`, not an opportunistic edit.
|
||||
|
||||
| # | Closes | Change | Files | Effort | Hardware? |
|
||||
|---|---|---|---|---|---|
|
||||
| **R-00** | F-02 / [ARCHY-1] | **DONE in this audit.** Route mnemonic generation through an injectable-RNG helper; production passes `OsRng`; known-answer test proves the injected RNG is consumed | `core/archipelago/src/seed.rs` | S | no |
|
||||
| **R-01** | **F-01 (Critical)** | Gate `seed.generate` / `seed.restore` on onboarding being incomplete; add rate limits; narrow the mesh peer path filter | `core/archipelago/src/api/rpc/seed_rpc.rs`, `.../middleware.rs`, `.../rate_limit.rs`, `server.rs` | M | yes (re-onboard + federation re-verify) |
|
||||
| **R-02** | F-03 (High) | Move `touch "$MARKER"` inside a both-succeeded branch so a failed regeneration retries next boot; surface the failure beyond a log file | `image-recipe/_archived/build-auto-installer-iso.sh` | S | **yes** (ISO rebuild + fresh flash) |
|
||||
| **R-03** | F-03 (High) | Strip baked SSH host keys and the TLS keypair from the rootfs tar at build time, so a regeneration failure degrades to "no key" not "shared key" | `image-recipe/_archived/build-auto-installer-iso.sh` | M | **yes** |
|
||||
| **R-04** | **F-13 (High)** | `disable_private_keys=true`; import the **xpub** with a `[fingerprint/derivation]` key origin; migrate with balance/UTXO parity verification | `core/archipelago/src/api/rpc/bitcoin.rs` | **PHASE** | **yes** (node with real UTXO history) |
|
||||
| **R-05** | F-07 (Medium) | Add `cargo audit` / `cargo deny check advisories bans` to CI, with a `bans` rule failing on duplicate `rand` majors | CI config | S | no |
|
||||
| **R-06** | F-05 (Medium) | Reconcile `Argon2::default()` (19 MiB / t=2) with ADR-005 (64 MB / 3) — either raise the params behind a versioned envelope with a migration, or amend the ADR | `core/archipelago/src/seed.rs`, `backup/full.rs`, `backup/identity.rs`, `docs/adr/005-...` | M | no |
|
||||
| **R-07** | F-04 (Medium) | Confine seed-bearing RPCs to loopback/TLS; shrink `MNEMONIC_TTL`; clear on acknowledged verify with a short grace window | `core/archipelago/src/api/rpc/seed_rpc.rs`, `.../mod.rs`, nginx config | **PHASE** | yes |
|
||||
| **R-08** | F-06 (Medium) | Make stdin/TTY the only mnemonic input for `ceremony sign`/`pubkey`; stop printing the mnemonic to stdout in `ceremony gen` | `core/archipelago/src/ceremony.rs` | S | no (but schedule deliberately — it is the signing ceremony) |
|
||||
| **R-09** | [ARCHY-2] hardening | Persist the CSPRNG-readiness verdict as a durable structured event, so any node can answer post-hoc "was the pool ready when this seed was born?" | `core/archipelago/src/seed.rs` | S | no |
|
||||
| **R-10** | [ARCHY-2] hardening | Add a test asserting the `getrandom` crate uses the **blocking** syscall, so the invariant is mechanical rather than a comment | `core/archipelago/src/seed.rs` | S | no |
|
||||
| **R-11** | F-08 (Low) | Clear `_seed_words` on route-leave from onboarding, not only on successful verify; add a wall-clock expiry mirroring `MNEMONIC_TTL` | `neode-ui/src/views/OnboardingSeedGenerate.vue`, `OnboardingSeedVerify.vue` | S | no |
|
||||
| **R-12** | F-09 (Low) | Replace `% charset.len()` with `SliceRandom::choose(&mut OsRng)` and pin the uniformity property with a test | `core/archipelago/src/totp.rs` | S | no |
|
||||
| **R-13** | F-10 (Low) | Swap `random_hex` / `random_base64` from `thread_rng()` to explicit `OsRng` | `core/archipelago/src/container/secrets.rs` | S | no — **blocked**: file has another agent's uncommitted work |
|
||||
| **R-14** | F-11 (Informational) | One-line comment at `pickRandomIndices` recording that the `Math.random()` is a UX challenge selector, not key material | `neode-ui/src/views/OnboardingSeedVerify.vue` | S | no |
|
||||
| **R-15** | §6 checklist | Run the on-node verification checklist — especially C-3 (per-node SSH/TLS keys) and C-5 (cross-node collision test) | — | M | **yes** (2+ nodes from one ISO) |
|
||||
|
||||
### Explicitly NOT implemented in this task, and why
|
||||
|
||||
- **R-01, R-04, R-07** need their own phase. R-01 changes an authentication boundary on a live
|
||||
fleet; R-04 moves the spending key out of a wallet holding real funds; R-07 changes the
|
||||
onboarding transport. Each needs a migration story and real-node verification that an
|
||||
audit-and-spec task cannot provide.
|
||||
- **R-02, R-03** require rebuilding the ISO and flashing at least two machines to verify. Not
|
||||
reachable from this environment.
|
||||
- **R-13** is blocked purely by tree hygiene: `core/archipelago/src/container/secrets.rs` had
|
||||
another agent's uncommitted changes at audit time and this plan's invariant is that no commit
|
||||
it authors touches their files. Trivial once that work lands.
|
||||
- **The whole of `docs/security/PSBT-SIGNING-ARCHITECTURE.md`** is a rollout, not a fix. It is
|
||||
queued as a spec for `/gsd-plan-phase`, not implemented anywhere.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user