From d3dd3fc113265c66156b346dfed1324a94aa91fd Mon Sep 17 00:00:00 2001 From: jwinterm Date: Thu, 2 Jul 2026 03:19:32 -0400 Subject: [PATCH] fix(admin): create-admin-wallet fully bootstraps a fresh instance (latch + compose) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create-admin-wallet registered a pending admin key but never latched setup, so on a fresh instance it left the bootstrap half-open and a later `setup` collided with it (setup's guard only checked for an ACTIVE admin; the cli key was pending). - New atomic Database::create_admin_key_bootstrapping: under the admin-keys advisory lock, insert the pending generated key AND latch `locked` (unless already validly locked), append audit, commit — one transaction. Fails CLOSED on a tampered latch (defers to reset-setup) instead of silently healing it. - bootstrap_admin (setup) now refuses on ANY live admin key (active OR pending), not just active, so setup composes with create-admin-wallet. The adopt-existing- deployment path (users but no admin keys) is unaffected: no live key -> proceeds. - reset_setup now takes the same advisory lock (all latch writers serialize) and documents the live-key recovery path. - CLI prints bootstrapped-vs-added and a terminating lost-secret recovery hint. - Docs (README + OPERATOR_SETUP section 5): both mutually-exclusive bootstrap paths. Adversarially reviewed (atomicity / lockout / regression — all ship). L1 test extended: fresh-latch, no-relatch-when-locked, pending semantics, composition guard, and tamper-refuse (fails closed + rolls back the key insert). --- README.md | 6 +- docs/operations/OPERATOR_SETUP.md | 22 ++++- src/bin/smirk-admin.rs | 33 +++++-- src/infra/db/admin_keys.rs | 4 +- src/infra/db/server_config.rs | 121 +++++++++++++++++++++-- tests/server_config.rs | 156 +++++++++++++++++++++++++++++- 6 files changed, 316 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 963beee..ee1fb8e 100644 --- a/README.md +++ b/README.md @@ -83,10 +83,12 @@ inconsistent feature configuration rather than booting with a control silently defeated. Generate secrets with `openssl rand -hex 32`. See [`.env.example`](.env.example) for the full, documented configuration surface. -First admin (headless bootstrap): +First admin (headless bootstrap) — run **one** of these; both latch, so the +other is refused afterward by design: ```sh -smirk-admin setup --pubkey # seed the first admin + latch the bootstrap +smirk-admin setup --pubkey # seed YOUR pubkey (active immediately) +smirk-admin create-admin-wallet --out key.hex # generate a key (pending) + latch, no setup needed ``` `` is the admin's Sign-in-with-Smirk (Nostr) public key; the admin diff --git a/docs/operations/OPERATOR_SETUP.md b/docs/operations/OPERATOR_SETUP.md index 48fc37c..b095d78 100644 --- a/docs/operations/OPERATOR_SETUP.md +++ b/docs/operations/OPERATOR_SETUP.md @@ -70,17 +70,31 @@ curl -s localhost:8080/api/v1/capabilities # enabled chains, features, restor Admin functions — key management, erasure, tamper-evident audit — run on a separate loopback listener; the public API needs no admin. To enable it, set -`ADMIN_ENABLED=true` and the two admin secrets, then bootstrap: +`ADMIN_ENABLED=true` and the two admin secrets, then bootstrap with **one** of +two mutually-exclusive first-run commands: ```sh +# A) You already have a Sign-in-with-Smirk (Nostr) key — seed its pubkey (active now): smirk-admin setup --pubkey # seed the first admin + latch the bootstrap + +# B) You want the CLI to generate a key — writes the secret 0600, registers it +# PENDING, and latches in one step (no `setup` needed): +smirk-admin create-admin-wallet --out admin-key.hex + smirk-admin doctor # database, live keys, audit chain ``` `` is the admin's Sign-in-with-Smirk (Nostr) public key; the admin -authenticates with the matching key over NIP-98. The bootstrap latch is -MAC-protected and one-way — a live deployment is adopted as already-bootstrapped, -and tampering fails closed. +authenticates with the matching key over NIP-98. Both commands latch the +bootstrap, so run **only one** — after either, the other is refused by design (a +live admin key already exists). With (B) the generated key is *pending* and +activates on the holder's first NIP-98 login; import `admin-key.hex` into your +signer and log in. If you lose that secret before the first login, re-run +`create-admin-wallet` (adds a fresh key) and `revoke-key` the stale one. + +The bootstrap latch is MAC-protected and one-way — a live deployment is adopted +as already-bootstrapped, and tampering fails closed (the CLI and the boot path +both refuse a tampered latch; run `reset-setup --i-understand` to re-open setup). Expose the admin plane over a Tor onion or an SSH tunnel; never bind it to a public interface. `ADMIN_BIND` defaults to loopback, and a non-loopback bind diff --git a/src/bin/smirk-admin.rs b/src/bin/smirk-admin.rs index c95b870..638999b 100644 --- a/src/bin/smirk-admin.rs +++ b/src/bin/smirk-admin.rs @@ -12,7 +12,10 @@ //! revoke-key --id (may revoke the LAST key) //! replace-key --old --pubkey <64hex> [--revoke-all] //! create-admin-wallet --out (generates a key; writes the -//! secret 0600; registers the pubkey) +//! secret 0600; registers the pubkey +//! as pending; latches the bootstrap +//! on a fresh instance — a complete +//! first-run, no `setup` needed) //! doctor //! //! Deferred (documented): remac-keys (integrity-secret rotation must also @@ -43,7 +46,7 @@ COMMANDS: add-key --pubkey <64hex> [--label ] revoke-key --id replace-key --old --pubkey <64hex> [--revoke-all] - create-admin-wallet --out + create-admin-wallet --out (generate a key + bootstrap a fresh instance) mint-invite [--count ] [--label ] (registration invite codes) doctor "; @@ -343,9 +346,11 @@ async fn create_admin_wallet(db: &Database, secret: &str, args: &[String]) -> Re secret_hex.zeroize(); write_res.map_err(|e| format!("write secret to {out}: {e}"))?; - // Register the PUBLIC key only. - match db - .create_admin_key_audited( + // Register the PUBLIC key only (pending), and latch the bootstrap if this is a + // fresh instance — so `create-admin-wallet` alone fully bootstraps, atomically, + // rather than leaving setup half-open for a later `setup` to collide with. + let latched = match db + .create_admin_key_bootstrapping( NewAdminKey { pubkey: pubkey.clone(), label: Some("cli-generated".into()), @@ -360,13 +365,23 @@ async fn create_admin_wallet(db: &Database, secret: &str, args: &[String]) -> Re .await .map_err(|e| e.to_string())? { - AddKeyOutcome::Created(_) => {} - AddKeyOutcome::CapReached => return Err("unexpected: cap reached".into()), - } + (AddKeyOutcome::Created(_), latched) => latched, + (AddKeyOutcome::CapReached, _) => return Err("unexpected: cap reached".into()), + }; println!("generated admin key; secret written to {out} (mode 0600)"); println!("pubkey: {pubkey}"); - println!("import the secret into your NIP-98 signer; it activates on first login"); + if latched { + println!("fresh instance bootstrapped: setup latched (locked) — no `setup` needed"); + println!("import the secret into your NIP-98 signer and log in to activate."); + println!( + "if you lose this secret before that first login, recover by re-running \ + `create-admin-wallet` (adds a fresh key), then `revoke-key` the stale one" + ); + } else { + println!("instance already bootstrapped: added a pending admin key"); + println!("import the secret into your NIP-98 signer; it activates on first login"); + } Ok(()) } diff --git a/src/infra/db/admin_keys.rs b/src/infra/db/admin_keys.rs index 4345530..caf0296 100644 --- a/src/infra/db/admin_keys.rs +++ b/src/infra/db/admin_keys.rs @@ -355,7 +355,7 @@ impl Database { /// Insert a pending allowlist entry on `conn` (so it can share a caller's /// transaction). Computes the integrity MAC over the exact stored values. 409 if /// an active key with this pubkey already exists. -async fn insert_admin_key( +pub(crate) async fn insert_admin_key( conn: &mut PgConnection, input: &NewAdminKey, secret: &str, @@ -406,7 +406,7 @@ pub(crate) async fn admin_keys_mutate_lock(conn: &mut PgConnection) -> Result<() } /// Count live (non-revoked) keys on `conn` (so it reflects the locked tx). -async fn live_key_count(conn: &mut PgConnection) -> Result { +pub(crate) async fn live_key_count(conn: &mut PgConnection) -> Result { let n = sqlx::query_scalar::<_, i64>("SELECT COUNT(*) FROM admin_keys WHERE revoked_at IS NULL") .fetch_one(&mut *conn) diff --git a/src/infra/db/server_config.rs b/src/infra/db/server_config.rs index 7251d82..4507303 100644 --- a/src/infra/db/server_config.rs +++ b/src/infra/db/server_config.rs @@ -13,8 +13,9 @@ use tracing::instrument; use crate::core::crypto::pepper::peppered_hex; use crate::error::AppError; -use crate::models::db::{NewAdminAudit, ServerConfig}; +use crate::models::db::{NewAdminAudit, NewAdminKey, ServerConfig}; +use super::admin_keys::AddKeyOutcome; use super::Database; /// Resolved bootstrap state. @@ -121,8 +122,12 @@ impl Database { } /// Headless bootstrap (CLI): insert the FIRST admin (active) and latch - /// `locked`, in ONE transaction + a hash-chained audit row. Refuses if an - /// active admin already exists (use `add-key` for more admins). + /// `locked`, in ONE transaction + a hash-chained audit row. Refuses if ANY + /// live admin key already exists — active OR pending — so `setup` composes + /// cleanly with `create-admin-wallet` (which registers a pending key + latches) + /// instead of seeding a confusing second admin. The adopt-existing-deployment + /// path is unaffected: a deployment with users but no admin keys has no live + /// key, so bootstrap still proceeds to seed the first admin. #[instrument(skip(self, secret))] pub async fn bootstrap_admin(&self, pubkey: &str, secret: &str) -> Result<(), AppError> { use crate::core::crypto::admin_mac::{compute_admin_key_mac, AdminKeyMacInput}; @@ -132,14 +137,16 @@ impl Database { super::admin_keys::admin_keys_mutate_lock(&mut tx).await?; let has_admin = sqlx::query_scalar::<_, bool>( - "SELECT EXISTS (SELECT 1 FROM admin_keys WHERE revoked_at IS NULL AND activated_at IS NOT NULL)", + "SELECT EXISTS (SELECT 1 FROM admin_keys WHERE revoked_at IS NULL)", ) .fetch_one(&mut *tx) .await?; if has_admin { tx.rollback().await?; return Err(AppError::Conflict( - "an active admin already exists; use add-key".into(), + "an admin key already exists (active or pending); use add-key, or \ + create-admin-wallet to add another generated key" + .into(), )); } @@ -203,11 +210,110 @@ impl Database { Ok(()) } + /// Register a generated PENDING admin key and, if the instance is not yet + /// bootstrapped, latch `locked` in the SAME transaction — so `create-admin- + /// wallet` on a fresh instance is a COMPLETE first-run bootstrap (key + latch), + /// not a key without a latch that leaves setup half-open (the bug this fixes). + /// Keeps pending semantics: the generated key activates on its holder's first + /// login (unlike `bootstrap_admin`, which seeds an already-active key from an + /// operator's own pubkey). On an already-locked instance this is a plain pending + /// add (no re-latch). FAILS CLOSED on a tampered latch (defers to `reset-setup`, + /// mirroring the boot path) rather than silently healing it. Serialized with all + /// admin-key mutations via the advisory lock, so it cannot race `bootstrap_admin`. + /// + /// Returns the add outcome and whether it latched (true = this call bootstrapped + /// a fresh instance). NOTE: `CapReached` returns `latched = false` WITHOUT + /// inserting or latching, so a caller passing a finite cap must treat that as + /// "instance still un-bootstrapped". + #[instrument(skip(self, input, audit, secret))] + pub async fn create_admin_key_bootstrapping( + &self, + input: NewAdminKey, + audit: &NewAdminAudit, + secret: &str, + max_live: i64, + ) -> Result<(AddKeyOutcome, bool), AppError> { + let mut tx = self.pool().begin().await?; + super::admin_keys::admin_keys_mutate_lock(&mut tx).await?; + + // Resolve the latch state up front (under the lock). A missing row (Fresh) + // or a valid `uninitialized` row means "bootstrap now"; a valid `locked` row + // means "already bootstrapped, just add the key". A MAC-INVALID row is a + // tamper / restore-to-pre-bootstrap: FAIL CLOSED here exactly like the boot + // path, instead of silently re-stamping a valid `locked` latch over it and + // erasing the tamper evidence. The operator must acknowledge the recovery + // with `reset-setup --i-understand` first. + let cfg = sqlx::query_as::<_, ServerConfig>( + "SELECT id, setup_state, bootstrap_completed_at, locked_at, updated_at, integrity_mac \ + FROM server_config WHERE id = 1", + ) + .fetch_optional(&mut *tx) + .await?; + if matches!(&cfg, Some(c) if !verify_latch(secret, c)) { + tx.rollback().await?; + return Err(AppError::Conflict( + "server_config bootstrap latch MAC invalid (tamper or restore-to-pre-bootstrap); \ + run `reset-setup --i-understand` first if this is intentional" + .into(), + )); + } + // Past the tamper gate, a present row is MAC-valid, so this is genuinely + // "already locked" only for a real `locked` state. + let already_locked = matches!(&cfg, Some(c) if c.setup_state == "locked"); + + // Cap check under the lock (the CLI passes i64::MAX — effectively no cap). + let live = super::admin_keys::live_key_count(&mut tx).await?; + if live >= max_live { + tx.rollback().await?; + return Ok((AddKeyOutcome::CapReached, false)); + } + + let key = super::admin_keys::insert_admin_key(&mut tx, &input, secret).await?; + + // Latch `locked` unless already validly locked (so additional generated keys + // do not re-stamp bootstrap_completed_at). INVARIANT: latching with only a + // PENDING key (zero active admins) is safe ONLY because no runtime path gates + // on `setup_state` — admin login authorizes on a pending key and activates it + // on first use (see api/admin.rs). Do NOT add a "reject unless Locked && >=1 + // active admin" gate: it would make this state an unrecoverable lockout (the + // login that would activate the key is the one such a gate would block). + let latched = if already_locked { + false + } else { + let now = Utc::now().trunc_subsecs(6); + let latch = latch_mac(secret, "locked", Some(now.timestamp_micros())); + sqlx::query( + "INSERT INTO server_config (id, setup_state, bootstrap_completed_at, locked_at, integrity_mac) \ + VALUES (1, 'locked', $1, $1, $2) \ + ON CONFLICT (id) DO UPDATE SET setup_state = 'locked', \ + bootstrap_completed_at = $1, locked_at = $1, updated_at = NOW(), integrity_mac = $2", + ) + .bind(now) + .bind(&latch) + .execute(&mut *tx) + .await?; + true + }; + + self.append_admin_audit(&mut tx, audit, secret).await?; + tx.commit().await?; + Ok((AddKeyOutcome::Created(key), latched)) + } + /// Reset the latch to `uninitialized` (shell `reset-setup` only). Recomputes /// the MAC so it verifies again; existing admin keys are left untouched, so - /// recovery cannot worsen a lockout. + /// recovery cannot worsen a lockout. Takes the admin-keys advisory lock so all + /// latch writers (`bootstrap_admin`, `create_admin_key_bootstrapping`) serialize + /// on one lock — a concurrent reset can't tear a check-then-latch. + /// + /// Recovery note: keys are preserved, so if a LIVE key still exists after the + /// reset, `setup` stays refused (its guard rejects any live key). Recover by + /// re-running `create-admin-wallet` (adds a fresh key + re-latches), or by + /// `revoke-key`-ing the stale key(s) and then running `setup`. #[instrument(skip(self, secret))] pub async fn reset_setup(&self, secret: &str) -> Result<(), AppError> { + let mut tx = self.pool().begin().await?; + super::admin_keys::admin_keys_mutate_lock(&mut tx).await?; let mac = latch_mac(secret, "uninitialized", None); sqlx::query( "INSERT INTO server_config (id, setup_state, bootstrap_completed_at, locked_at, integrity_mac) \ @@ -216,8 +322,9 @@ impl Database { bootstrap_completed_at = NULL, locked_at = NULL, updated_at = NOW(), integrity_mac = $1", ) .bind(&mac) - .execute(self.pool()) + .execute(&mut *tx) .await?; + tx.commit().await?; Ok(()) } } diff --git a/tests/server_config.rs b/tests/server_config.rs index 5351c71..5d6de80 100644 --- a/tests/server_config.rs +++ b/tests/server_config.rs @@ -9,8 +9,8 @@ mod common; -use smirk_backend_core::infra::db::SetupState; -use smirk_backend_core::models::db::NewAdminKey; +use smirk_backend_core::infra::db::{AddKeyOutcome, SetupState}; +use smirk_backend_core::models::db::{NewAdminAudit, NewAdminKey}; use uuid::Uuid; const SECRET: &str = "server-config-integrity-secret-0123456789ab"; @@ -84,4 +84,156 @@ async fn latch_state_machine_and_tamper_detection() { "bootstrap refused while an active admin exists" ); let _ = db.revoke_admin_key(k.id, SECRET).await; + + // ── create_admin_key_bootstrapping: a generated-key first-run (fix B) ────── + // `create-admin-wallet` on a FRESH instance must be a COMPLETE bootstrap: + // register the (pending) generated key AND latch `locked`, atomically. + sqlx::query("DELETE FROM server_config WHERE id = 1") + .execute(db.pool()) + .await + .unwrap(); + assert_eq!( + db.read_setup_state(SECRET).await.unwrap(), + SetupState::Fresh + ); + + let genpk = format!("{:0>64}", Uuid::new_v4().simple()); + let audit = NewAdminAudit { + action: "admin_wallet_created".into(), + actor_kind: "cli".into(), + actor_pubkey_prefix: None, + target: Some(genpk.clone()), + details: None, + ip_address: None, + }; + let (outcome, latched) = db + .create_admin_key_bootstrapping( + NewAdminKey { + pubkey: genpk.clone(), + label: Some("cli-generated".into()), + scope: "admin".into(), + created_by_kind: "cli".into(), + activation_deadline: None, + }, + &audit, + SECRET, + i64::MAX, + ) + .await + .unwrap(); + assert!( + latched, + "a fresh instance is latched by create-admin-wallet" + ); + assert_eq!( + db.read_setup_state(SECRET).await.unwrap(), + SetupState::Locked, + "the generated-key bootstrap latches locked" + ); + let key1 = match outcome { + AddKeyOutcome::Created(k) => k, + AddKeyOutcome::CapReached => panic!("unexpected cap"), + }; + assert!( + key1.activated_at.is_none(), + "the generated key stays pending (activates on first login)" + ); + + // A SECOND generated key on the now-locked instance is a plain pending add — + // it must NOT re-latch (no re-stamping bootstrap_completed_at). + let genpk2 = format!("{:0>64}", Uuid::new_v4().simple()); + let audit2 = NewAdminAudit { + target: Some(genpk2.clone()), + ..audit.clone() + }; + let (outcome2, latched2) = db + .create_admin_key_bootstrapping( + NewAdminKey { + pubkey: genpk2.clone(), + label: Some("cli-generated".into()), + scope: "admin".into(), + created_by_kind: "cli".into(), + activation_deadline: None, + }, + &audit2, + SECRET, + i64::MAX, + ) + .await + .unwrap(); + assert!(!latched2, "an already-locked instance is not re-latched"); + assert_eq!( + db.read_setup_state(SECRET).await.unwrap(), + SetupState::Locked + ); + let key2 = match outcome2 { + AddKeyOutcome::Created(k) => k, + AddKeyOutcome::CapReached => panic!("unexpected cap"), + }; + + // Composition guard: with a live (even PENDING) key present, `setup` refuses + // rather than seeding a confusing second admin. NOTE: on the shared test DB this + // assertion is non-discriminating — sibling test binaries leave ACTIVE keys, so + // it would pass under the old active-only guard too. The any-live widen is proven + // hermetically at the query level in the db unit tests; here it documents intent. + assert!( + db.bootstrap_admin(&"22".repeat(32), SECRET).await.is_err(), + "setup refuses once any live admin key exists (even pending)" + ); + + // create-admin-wallet FAILS CLOSED on a tampered latch (defers to reset-setup), + // rather than silently re-stamping a valid `locked` latch over the tamper. We own + // the singleton, so this is isolable: raw-flip the state without recomputing the + // MAC (the restore-to-pre-bootstrap shape), then assert the call errors, the latch + // is NOT healed, and the key insert was rolled back. + sqlx::query( + "UPDATE server_config SET setup_state = 'uninitialized', bootstrap_completed_at = NULL WHERE id = 1", + ) + .execute(db.pool()) + .await + .unwrap(); + assert_eq!( + db.read_setup_state(SECRET).await.unwrap(), + SetupState::Tampered + ); + let genpk3 = format!("{:0>64}", Uuid::new_v4().simple()); + let audit3 = NewAdminAudit { + target: Some(genpk3.clone()), + ..audit.clone() + }; + let tampered_res = db + .create_admin_key_bootstrapping( + NewAdminKey { + pubkey: genpk3.clone(), + label: Some("cli-generated".into()), + scope: "admin".into(), + created_by_kind: "cli".into(), + activation_deadline: None, + }, + &audit3, + SECRET, + i64::MAX, + ) + .await; + assert!( + tampered_res.is_err(), + "create-admin-wallet fails closed on a tampered latch" + ); + assert_eq!( + db.read_setup_state(SECRET).await.unwrap(), + SetupState::Tampered, + "the tampered latch is NOT silently healed" + ); + let keys = db.list_admin_keys().await.unwrap(); + assert!( + !keys.iter().any(|k| k.pubkey == genpk3), + "the refused call rolled back its pending key insert" + ); + + let _ = db.revoke_admin_key(key1.id, SECRET).await; + let _ = db.revoke_admin_key(key2.id, SECRET).await; + + // Leave the owned singleton in a clean, MAC-valid state (not raw-tampered) for + // the next run / any reader. + db.reset_setup(SECRET).await.unwrap(); }