fix(10-05): delete the Bitcoin Core wallet path that duplicated the spending key (F-13, D-07b)
`handle_bitcoin_init_wallet_from_seed` derived the BIP-84 account extended *private* key, stringified it, and imported `wpkh(xprv/0/*)` / `wpkh(xprv/1/*)` into a Bitcoin Core descriptor wallet created with `disable_private_keys=false` and an empty passphrase. That put a second copy of the node's spending key in Core's `wallet.dat`, outside the daemon's Argon2 + ChaCha20-Poly1305 envelope. That duplication into weaker protection was audit finding F-13 (High). Deleted rather than rewritten watch-only (D-07b supersedes D-07/D-07a): - No caller anywhere. Repo-wide search leaves exactly one occurrence of the method name (its own dispatcher registration) and two of the symbol in code (definition + dispatch call); every other hit is prose in docs. - LND is the wallet the product drives. Across neode-ui/src every `bitcoin.*` call is read-only status (getinfo/prune-status/onion); the wallet UI sends via `lnd.sendcoins`. - It never ran on archi-dev-box: no wallet named `archipelago` exists there, and the one loaded wallet reports blank=true, keypoolsize=0, txcount=0. - It was authenticated AND password-gated, so F-13 was key-at-rest duplication, not an exposed endpoint. No migration is performed and none is planned. This removes code, not wallets: nothing on disk is touched, no funds move, no wallet.dat is modified. If a node is ever found holding a wallet this handler created, that is a finding to surface and stop on, not a trigger to auto-migrate. `seed::derive_bitcoin_xprv` loses its only non-test caller and is retained deliberately with `#[allow(dead_code)]` and a stated reason: it keeps its existing test coverage and it is the derivation D-07c's deferred BDK cold vault will need. Records the evidence, the D-08/D-09 consequences and the D-07c deferral in docs/security/KEY-03-SIGNING-POSTURE.md. 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
8255b69af2
commit
9622926868
@@ -1,7 +1,6 @@
|
||||
use super::RpcHandler;
|
||||
use anyhow::{Context, Result};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use zeroize::Zeroize;
|
||||
|
||||
/// Retry configuration for [`bitcoin_rpc_post_with_retry`].
|
||||
///
|
||||
@@ -155,144 +154,18 @@ impl RpcHandler {
|
||||
.await
|
||||
}
|
||||
|
||||
/// Initialize a Bitcoin Core descriptor wallet with keys derived from the master seed.
|
||||
/// Creates a blank wallet and imports BIP-84 (native segwit) descriptors.
|
||||
/// Requires: password re-verification, encrypted seed on disk.
|
||||
pub(super) async fn handle_bitcoin_init_wallet_from_seed(
|
||||
&self,
|
||||
params: Option<serde_json::Value>,
|
||||
) -> Result<serde_json::Value> {
|
||||
let params = params.ok_or_else(|| anyhow::anyhow!("Missing params"))?;
|
||||
let password = params
|
||||
.get("password")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("Missing 'password' for seed access"))?;
|
||||
let wallet_name = params
|
||||
.get("wallet_name")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("archipelago");
|
||||
|
||||
// Verify user password.
|
||||
self.auth_manager
|
||||
.verify_password(password)
|
||||
.await
|
||||
.context("Password verification failed")?;
|
||||
|
||||
// Load encrypted seed.
|
||||
let mnemonic = crate::seed::load_seed_encrypted(&self.config.data_dir, password)
|
||||
.await
|
||||
.context("Failed to load encrypted seed")?;
|
||||
let seed = crate::seed::MasterSeed::from_mnemonic(&mnemonic);
|
||||
|
||||
// Derive BIP-84 account xprv.
|
||||
let xprv = crate::seed::derive_bitcoin_xprv(&seed)?;
|
||||
let mut xprv_str = xprv.to_string();
|
||||
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(std::time::Duration::from_secs(30))
|
||||
.build()
|
||||
.context("Failed to create HTTP client")?;
|
||||
|
||||
// Step 1: Create a blank descriptor wallet.
|
||||
let create_result = self
|
||||
.bitcoin_rpc_call::<serde_json::Value>(
|
||||
&client,
|
||||
"createwallet",
|
||||
&[
|
||||
serde_json::json!(wallet_name), // wallet_name
|
||||
serde_json::json!(false), // disable_private_keys
|
||||
serde_json::json!(true), // blank
|
||||
serde_json::json!(""), // passphrase
|
||||
serde_json::json!(false), // avoid_reuse
|
||||
serde_json::json!(true), // descriptors
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
match create_result {
|
||||
Ok(_) => tracing::info!("Created blank descriptor wallet '{}'", wallet_name),
|
||||
Err(e) => {
|
||||
let msg = e.to_string();
|
||||
if msg.contains("already exists") {
|
||||
tracing::info!(
|
||||
"Wallet '{}' already exists, importing descriptors",
|
||||
wallet_name
|
||||
);
|
||||
} else {
|
||||
xprv_str.zeroize();
|
||||
return Err(e.context("Failed to create wallet"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Step 2: Import BIP-84 descriptors (external + internal/change).
|
||||
// Format: wpkh(xprv/0/*) for receive, wpkh(xprv/1/*) for change.
|
||||
let external_desc = format!("wpkh({}/0/*)", xprv_str);
|
||||
let internal_desc = format!("wpkh({}/1/*)", xprv_str);
|
||||
|
||||
// Get checksums from Bitcoin Core.
|
||||
let ext_info: serde_json::Value = self
|
||||
.bitcoin_rpc_call(
|
||||
&client,
|
||||
"getdescriptorinfo",
|
||||
&[serde_json::json!(external_desc)],
|
||||
)
|
||||
.await
|
||||
.context("getdescriptorinfo failed for external descriptor")?;
|
||||
|
||||
let int_info: serde_json::Value = self
|
||||
.bitcoin_rpc_call(
|
||||
&client,
|
||||
"getdescriptorinfo",
|
||||
&[serde_json::json!(internal_desc)],
|
||||
)
|
||||
.await
|
||||
.context("getdescriptorinfo failed for internal descriptor")?;
|
||||
|
||||
let ext_desc_with_checksum = ext_info
|
||||
.get("descriptor")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("No descriptor in getdescriptorinfo response"))?;
|
||||
let int_desc_with_checksum = int_info
|
||||
.get("descriptor")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("No descriptor in getdescriptorinfo response"))?;
|
||||
|
||||
let import_params = serde_json::json!([
|
||||
{
|
||||
"desc": ext_desc_with_checksum,
|
||||
"timestamp": "now",
|
||||
"active": true,
|
||||
"internal": false,
|
||||
"range": [0, 1000],
|
||||
},
|
||||
{
|
||||
"desc": int_desc_with_checksum,
|
||||
"timestamp": "now",
|
||||
"active": true,
|
||||
"internal": true,
|
||||
"range": [0, 1000],
|
||||
}
|
||||
]);
|
||||
|
||||
let _import_result: serde_json::Value = self
|
||||
.bitcoin_rpc_call(&client, "importdescriptors", &[import_params])
|
||||
.await
|
||||
.context("importdescriptors failed")?;
|
||||
|
||||
// Zeroize the xprv string from memory.
|
||||
xprv_str.zeroize();
|
||||
|
||||
tracing::info!(
|
||||
"Bitcoin Core wallet '{}' initialized from master seed (BIP-84)",
|
||||
wallet_name
|
||||
);
|
||||
|
||||
Ok(serde_json::json!({
|
||||
"initialized": true,
|
||||
"wallet_name": wallet_name,
|
||||
}))
|
||||
}
|
||||
// NOTE: the Bitcoin Core wallet-init handler that used to live here was deleted in
|
||||
// Phase 10 (D-07b) to close audit finding F-13. It derived the BIP-84 account
|
||||
// extended *private* key, stringified it, and imported `wpkh(xprv/0/*)` /
|
||||
// `wpkh(xprv/1/*)` into Core's `wallet.dat` — a second copy of the node's spending
|
||||
// key, outside the daemon's Argon2 + ChaCha20-Poly1305 envelope. It had no caller
|
||||
// anywhere in the repo; LND is the wallet the UI drives.
|
||||
//
|
||||
// Do NOT reintroduce a Bitcoin Core wallet path that imports private keys. If a
|
||||
// Core wallet is ever needed again it must be watch-only by construction
|
||||
// (`disable_private_keys = true`, xpub descriptors with a `[fingerprint/derivation]`
|
||||
// key origin). Full rationale, evidence and the deleted symbol's name:
|
||||
// docs/security/KEY-03-SIGNING-POSTURE.md
|
||||
}
|
||||
|
||||
/// Free-function counterpart to `RpcHandler::bitcoin_rpc_call`.
|
||||
|
||||
@@ -119,9 +119,11 @@ impl RpcHandler {
|
||||
"bitcoin.relay-create-tor-service" => {
|
||||
self.handle_bitcoin_relay_create_tor_service().await
|
||||
}
|
||||
"bitcoin.init-wallet-from-seed" => {
|
||||
self.handle_bitcoin_init_wallet_from_seed(params).await
|
||||
}
|
||||
// NOTE: the Bitcoin Core wallet-init arm that used to sit here was deleted in
|
||||
// Phase 10 (D-07b, F-13). Its handler derived the BIP-84 account xprv and
|
||||
// imported it into Core's wallet.dat, duplicating the spending key outside the
|
||||
// Argon2 envelope. It had no caller. The `lnd.` arm below is a different,
|
||||
// still-live endpoint. See docs/security/KEY-03-SIGNING-POSTURE.md.
|
||||
"lnd.getinfo" => self.handle_lnd_getinfo().await,
|
||||
"lnd.listchannels" => self.handle_lnd_listchannels().await,
|
||||
"lnd.closedchannels" => self.handle_lnd_closedchannels().await,
|
||||
|
||||
@@ -226,8 +226,18 @@ pub fn derive_nostr_identity_key(seed: &MasterSeed, index: u32) -> Result<nostr_
|
||||
|
||||
// ─── Bitcoin / LND Derivation ───────────────────────────────────────────
|
||||
|
||||
/// Derive the BIP-84 account-level extended private key for Bitcoin Core.
|
||||
/// Derive the BIP-84 account-level extended private key.
|
||||
/// Path: m/84'/0'/0' (native segwit, mainnet).
|
||||
///
|
||||
/// **Retained deliberately with no production caller (Phase 10, D-07c).** Its only
|
||||
/// non-test caller was the Bitcoin Core wallet-init RPC handler, deleted under D-07b
|
||||
/// because it imported this xprv into Bitcoin Core's `wallet.dat` (audit finding
|
||||
/// F-13; see `docs/security/KEY-03-SIGNING-POSTURE.md`). This function is *not*
|
||||
/// cruft: it is covered by existing tests below, and
|
||||
/// it is the derivation D-07c's deferred BDK cold vault (ElectrumX-backed, daemon
|
||||
/// side) will need. Do not delete it as dead code; if D-07c is abandoned, remove
|
||||
/// the decision and the function together.
|
||||
#[allow(dead_code)]
|
||||
pub fn derive_bitcoin_xprv(seed: &MasterSeed) -> Result<bitcoin::bip32::Xpriv> {
|
||||
use bitcoin::bip32::{ChildNumber, DerivationPath, Xpriv};
|
||||
use bitcoin::Network;
|
||||
|
||||
@@ -0,0 +1,180 @@
|
||||
# KEY-03 — Signing posture after the Bitcoin Core wallet deletion
|
||||
|
||||
> **What this document is.** The evidence-backed record of how Archipelago's Bitcoin signing
|
||||
> posture stands after Phase 10 KEY-03. It supersedes, for the Bitcoin Core wallet specifically,
|
||||
> the target state described in `docs/security/PSBT-SIGNING-ARCHITECTURE.md` §8 Phase 1 — that
|
||||
> phase planned to *convert* Core's wallet to watch-only; **D-07b deleted the path instead.**
|
||||
>
|
||||
> **Governing decisions:** `.planning/phases/10-key-material-hardening/10-CONTEXT.md`
|
||||
> **D-07b** (final KEY-03 scope — delete, do not migrate) and **D-07c** (the deferred BDK cold
|
||||
> vault, recorded so it is not lost with the code). D-07b supersedes D-07 and D-07a's conditional
|
||||
> migration.
|
||||
>
|
||||
> **Audit finding closed:** F-13 (High) —
|
||||
> `docs/security/ENTROPY-SEED-AUDIT-2026-07-31.md:604`, remediation register R-04.
|
||||
|
||||
---
|
||||
|
||||
## Bitcoin Core wallet path — deleted (D-07b)
|
||||
|
||||
### What was deleted
|
||||
|
||||
| Symbol | Kind | Location before deletion |
|
||||
|---|---|---|
|
||||
| `handle_bitcoin_init_wallet_from_seed` | `async fn` | `core/archipelago/src/api/rpc/bitcoin.rs:161-295` |
|
||||
| `"bitcoin.init-wallet-from-seed"` | JSON-RPC dispatch arm | `core/archipelago/src/api/rpc/dispatcher.rs:122-124` |
|
||||
|
||||
### The defect (F-13)
|
||||
|
||||
The handler loaded the encrypted seed, derived the **BIP-84 account extended private key**
|
||||
(`crate::seed::derive_bitcoin_xprv`, `bitcoin.rs:188`), stringified it (`:189`), and imported
|
||||
`wpkh(xprv/0/*)` and `wpkh(xprv/1/*)` (`:230-231`) into a Bitcoin Core descriptor wallet created
|
||||
with `disable_private_keys = false` (`:203`) and an **empty** wallet passphrase (`:205`).
|
||||
|
||||
The result was a **second copy of the node's spending key**, persisted in Core's `wallet.dat`
|
||||
inside the Bitcoin container's data volume, with no Argon2 passphrase — while the first copy sits
|
||||
in the daemon's Argon2 + ChaCha20-Poly1305 envelope written `0600`
|
||||
(`core/archipelago/src/seed.rs:238-269`, `:318-324`). That duplication, into weaker protection,
|
||||
was the entire finding.
|
||||
|
||||
### Evidence that deletion was the right close (re-established for this task, not inherited)
|
||||
|
||||
The four D-07a evidence points, verified again against the tree before anything was removed:
|
||||
|
||||
**1. No caller anywhere.** Repo-wide search across `core/`, `neode-ui/src`, `scripts/`, `web/`,
|
||||
`apps/`, `tests/` and `docs/`, excluding `core/target`, `node_modules` and `.git`:
|
||||
|
||||
```
|
||||
$ grep -rn 'bitcoin\.init-wallet-from-seed' core/ neode-ui/src scripts/ web/ apps/ tests/ docs/
|
||||
core/archipelago/src/api/rpc/dispatcher.rs:122: "bitcoin.init-wallet-from-seed" => {
|
||||
|
||||
$ grep -rn 'handle_bitcoin_init_wallet_from_seed' core/ neode-ui/src scripts/ web/ apps/ tests/ docs/
|
||||
core/archipelago/src/api/rpc/bitcoin.rs:161: pub(super) async fn handle_bitcoin_init_wallet_from_seed(
|
||||
core/archipelago/src/api/rpc/dispatcher.rs:123: self.handle_bitcoin_init_wallet_from_seed(params).await
|
||||
docs/UNIFIED-TASK-TRACKER.md:208: §8 Phase 1). `handle_bitcoin_init_wallet_from_seed` passes
|
||||
docs/security/ENTROPY-SEED-AUDIT-2026-07-31.md:607:(`handle_bitcoin_init_wallet_from_seed`):
|
||||
docs/security/PSBT-SIGNING-ARCHITECTURE.md:147: `handle_bitcoin_init_wallet_from_seed`, `core/archipelago/src/api/rpc/bitcoin.rs:161-294`).
|
||||
```
|
||||
|
||||
Exactly one occurrence of the method name (its own dispatcher registration) and two of the symbol
|
||||
in code (its definition and the dispatcher call). The three remaining symbol hits are prose in
|
||||
documentation — the audit, the task tracker, and the PSBT architecture spec — not callers. No
|
||||
frontend, script, test or other Rust module invoked it.
|
||||
|
||||
**2. LND is the wallet the product actually drives.** Across all of `neode-ui/src`, every
|
||||
`bitcoin.*` RPC call is read-only status: `bitcoin.getinfo` (14 call sites),
|
||||
`bitcoin.prune-status` (3), `bitcoin.onion` (1). There are **no** `bitcoin.*` wallet operations.
|
||||
The wallet UI (`Web5Wallet.vue`, `SendBitcoinModal.vue`) sends via `lnd.sendcoins`, estimates via
|
||||
`lnd.estimatefee`, and reads balance via `lnd.getinfo`.
|
||||
|
||||
**3. The wallet it creates never existed on the reference node.** Verified live on
|
||||
**archi-dev-box, 2026-08-02**, against the running `bitcoin-knots` container (read-only RPCs
|
||||
only — see the census section for the exact commands and the standing ban on
|
||||
`listdescriptors true`):
|
||||
|
||||
```
|
||||
listwalletdir → { "wallets": [ "gatewayd-02004b91…", "gatewayd-03443c0c…", "" ] }
|
||||
listwallets → [ "" ]
|
||||
```
|
||||
|
||||
**There is no wallet named `archipelago`** — the handler's default `wallet_name`
|
||||
(`bitcoin.rs:170-173`). It has never run on this node. `getwalletinfo` on the one loaded wallet
|
||||
(the unnamed default) reports:
|
||||
|
||||
```
|
||||
walletname: "" blank: true keypoolsize: 0
|
||||
txcount: 0 balance: 0.00000000
|
||||
descriptors: true private_keys_enabled: true
|
||||
```
|
||||
|
||||
`blank: true` with `keypoolsize: 0` and `txcount: 0` is Bitcoin Core's own statement that **no
|
||||
key was ever imported into it and no transaction ever touched it**. The two `gatewayd-*` entries
|
||||
are Fedimint gateway wallets, unrelated to the BIP-84 path. The `wallet.dat` at the datadir root
|
||||
is Core's own legacy default-wallet location, not this handler's output.
|
||||
|
||||
**This is one node.** archi-dev-box is verified; the rest of the fleet is **UNVERIFIED** pending
|
||||
the census below.
|
||||
|
||||
**Supporting history evidence:** `git log -S "init-wallet-from-seed"` scoped to
|
||||
`core/archipelago/src/api/rpc/dispatcher.rs` and `neode-ui/src` returns exactly one commit —
|
||||
`19dcfd4f feat: BIP-39 master seed for unified key derivation`, the commit that **added** it. No
|
||||
frontend wrapper was ever written: it was built and never wired up.
|
||||
|
||||
**4. It was never remotely reachable.** The endpoint is absent from `UNAUTHENTICATED_METHODS`
|
||||
(`core/archipelago/src/api/rpc/middleware.rs:5-40`) — so it required an authenticated session —
|
||||
**and** it additionally re-verified the user's password before touching the seed
|
||||
(`self.auth_manager.verify_password(password)`, `bitcoin.rs:176-179`). **F-13 was therefore
|
||||
key-at-rest duplication, not an exposed endpoint.** That is why it was rated High rather than
|
||||
Critical, and why deleting it is a hardening measure rather than an incident response.
|
||||
|
||||
### What was *not* wrong with it
|
||||
|
||||
Worth stating so the record is fair, and so the next reader does not mistake the lesson. The
|
||||
in-memory handling of the xprv string was **careful**: it was zeroized on the error path
|
||||
(`bitcoin.rs:222`) and on the success path (`:284`), matching the standard set elsewhere in
|
||||
`seed.rs`. The wallet type was also correct — `createwallet` already passed `descriptors = true`
|
||||
(`:207`), which is the right foundation.
|
||||
|
||||
**The defect was which key went into the wallet, not how the key was held in memory or what kind
|
||||
of wallet it was.** A watch-only rewrite (xpub + `[fingerprint/derivation]` key origin) would
|
||||
have been a legitimate fix. Deletion was chosen over rewrite because the endpoint had no caller,
|
||||
no consumer, and no product role: rewriting it would have produced a correct implementation of
|
||||
something nothing uses, and left a wallet-creating code path to be maintained and re-audited
|
||||
forever.
|
||||
|
||||
### How F-13 is closed
|
||||
|
||||
**By removal, not by conversion to watch-only.** After this change there is no code path in the
|
||||
daemon that writes the BIP-84 account private key into Bitcoin Core. The only on-node copy of
|
||||
that key is the daemon's Argon2 + ChaCha20-Poly1305 envelope.
|
||||
|
||||
**No migration was performed and none is planned.** D-07's parity-proof migration and its
|
||||
one-way checkpoint are **withdrawn** (D-07b) — there is no wallet to migrate. If a fleet node is
|
||||
ever found holding a descriptor wallet this handler created, that is a **finding to surface and
|
||||
stop on**, not a trigger to auto-migrate: it would mean the endpoint was invoked by hand and that
|
||||
node's spending key is duplicated in Core, which deserves a human decision rather than an
|
||||
automated rewrite of a wallet that may hold funds.
|
||||
|
||||
### This deletion removes code, not wallets
|
||||
|
||||
Stated explicitly so nobody reading the change later has to wonder whether it was destructive:
|
||||
|
||||
> **Nothing on disk is touched.** No `wallet.dat` is modified, unloaded or removed. No funds
|
||||
> move. No LND state, secret, descriptor or seed is altered. The change removes a Rust function
|
||||
> and a `match` arm — the *path* by which a private key could be imported into Bitcoin Core —
|
||||
> and nothing else.
|
||||
|
||||
This holds even on a hypothetical node where the endpoint had been invoked by hand: deleting the
|
||||
handler destroys nothing there either. It closes the door; it does not clean the room. Cleaning
|
||||
up such a wallet, if one is ever found, is a separate human decision (see the census below), and
|
||||
CLAUDE.md's **"migrations never destroy data"** invariant is not engaged by this change because
|
||||
there is no migration.
|
||||
|
||||
### What deletion does to D-08 and D-09
|
||||
|
||||
Neither decision lapses; both are satisfied by a different mechanism.
|
||||
|
||||
- **D-08** asked that the spending key exist in exactly one place, with an opt-in air-gapped
|
||||
path. Deleting the Core import achieves the first half outright. The opt-in path is LND's
|
||||
existing PSBT round trip, not a Core watch-only wallet — see the next section, including the
|
||||
recorded verdict on how far that actually goes today.
|
||||
- **D-09** required a `[fingerprint/derivation]` key origin on emitted descriptors so a hardware
|
||||
signer can locate its key. With Core's descriptors deleted there are **no Archipelago-emitted
|
||||
descriptors left to annotate**, so D-09's actual protection moves to the PSBT itself. That is
|
||||
why `lnd.create-psbt` now inspects and reports the key-origin data its PSBT carries
|
||||
(`psbt_key_origin_report`, `core/archipelago/src/api/rpc/lnd/wallet.rs`).
|
||||
|
||||
### `derive_bitcoin_xprv` is retained deliberately (D-07c)
|
||||
|
||||
`crate::seed::derive_bitcoin_xprv` (`core/archipelago/src/seed.rs:231`) lost its only non-test
|
||||
caller and was **kept**, marked `#[allow(dead_code)]` with the reason in its doc comment. It is
|
||||
covered by existing tests (`seed.rs:601-602`, `:856`) and it is the derivation **D-07c's deferred
|
||||
BDK cold vault** — a descriptor wallet in the daemon using the node's own ElectrumX app
|
||||
(`apps/electrumx`, `electrs_status.rs`) as chain source — will need.
|
||||
|
||||
D-07c was considered and deliberately deferred out of Phase 10 (it needs its own phase: a new
|
||||
dependency and a new UI surface). It is recorded here, and in the function's doc comment, so the
|
||||
option is not quietly lost along with the code that was deleted. The alternative shape — LND
|
||||
watch-only via `importaccount` plus remote signing — was considered and rejected for coupling
|
||||
cold storage to LND's upgrade path.
|
||||
|
||||
Reference in New Issue
Block a user