From c69e1ba7c8116d499ca2a25650529cedf834f8f7 Mon Sep 17 00:00:00 2001 From: "William K. Santiago" Date: Mon, 24 Aug 2026 16:28:41 -0400 Subject: [PATCH 1/2] DKG: clean up abandoned dkg_subkeys entries (#970) --- keep-mobile/src/lib.rs | 53 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/keep-mobile/src/lib.rs b/keep-mobile/src/lib.rs index e551730a..0cf03d0a 100644 --- a/keep-mobile/src/lib.rs +++ b/keep-mobile/src/lib.rs @@ -1401,6 +1401,19 @@ impl KeepMobile { } } + /// Discard the pending DKG subkey minted by `frost_dkg_begin` for + /// `group_name` without running the ceremony. A begin that is never followed + /// by `frost_run_dkg` (the user toggles Start/Join, or the ceremony is + /// abandoned) would otherwise leave its minted subkey secret stashed for the + /// process lifetime; the client calls this on abandonment to zeroize and drop + /// it. Idempotent: a group with no pending subkey is a no-op. + pub fn frost_dkg_discard(&self, group_name: String) { + self.dkg_subkeys + .lock() + .unwrap_or_else(|p| p.into_inner()) + .remove(&group_name); + } + /// Run a full relay-driven DKG to create a new group on this device, then /// persist the resulting share. Every participant calls this concurrently /// with the same invite roster (`config.roster`) and relays but its own @@ -1478,14 +1491,16 @@ impl KeepMobile { // Load this device's pending subkey (from `frost_dkg_begin`) and rebuild // its keypair. Fail before any network I/O if the two-call sequence was - // skipped. + // skipped. Consume it here: the subkey is single-use for this run, so a + // completed or failed run must not leave it stashed for the process + // lifetime (a retry re-mints via `frost_dkg_begin`). let subkey = { - let map = self.dkg_subkeys.lock().unwrap_or_else(|p| p.into_inner()); - let secret = map.get(&config.group_name).cloned().ok_or_else(|| { - KeepMobileError::FrostError { - msg: "call frost_dkg_begin for this group before frost_run_dkg".into(), - } - })?; + let mut map = self.dkg_subkeys.lock().unwrap_or_else(|p| p.into_inner()); + let secret = + map.remove(&config.group_name) + .ok_or_else(|| KeepMobileError::FrostError { + msg: "call frost_dkg_begin for this group before frost_run_dkg".into(), + })?; let sk = nostr_sdk::secp256k1::SecretKey::from_slice(&*secret).map_err(|e| { KeepMobileError::FrostError { msg: format!("stored DKG subkey is invalid: {e}"), @@ -5597,6 +5612,30 @@ mod dkg_pending_share_tests { mobile.frost_cancel_dkg(2); } + // `frost_dkg_begin` stashes a per-group subkey secret; an abandoned begin + // (never followed by `frost_run_dkg`) must be discardable so it can't + // accumulate for the process lifetime (GH #970). + #[test] + fn discard_drops_pending_dkg_subkey() { + let storage = Arc::new(FailingShareStorage::default()); + let mobile = KeepMobile::new(storage as Arc).unwrap(); + + mobile.frost_dkg_begin("group-a".into()).unwrap(); + mobile.frost_dkg_begin("group-b".into()).unwrap(); + assert_eq!(mobile.dkg_subkeys.lock().unwrap().len(), 2); + + // Discarding one abandoned begin drops only that group's subkey. + mobile.frost_dkg_discard("group-a".into()); + let map = mobile.dkg_subkeys.lock().unwrap(); + assert!(!map.contains_key("group-a")); + assert!(map.contains_key("group-b")); + drop(map); + + // Discarding a group with no pending subkey is a harmless no-op. + mobile.frost_dkg_discard("group-a".into()); + assert_eq!(mobile.dkg_subkeys.lock().unwrap().len(), 1); + } + // A wrong passphrase must not clear the stash: the share stays recoverable. #[test] fn failed_recovery_retains_stash() { From 0c2468f17b210f5a87157570d4ea256b403d1373 Mon Sep 17 00:00:00 2001 From: "William K. Santiago" Date: Mon, 24 Aug 2026 16:48:20 -0400 Subject: [PATCH 2/2] DKG: retain pending subkey on frost_run_dkg preflight failure --- keep-mobile/src/lib.rs | 76 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 10 deletions(-) diff --git a/keep-mobile/src/lib.rs b/keep-mobile/src/lib.rs index 0cf03d0a..a3333de0 100644 --- a/keep-mobile/src/lib.rs +++ b/keep-mobile/src/lib.rs @@ -1490,17 +1490,18 @@ impl KeepMobile { } // Load this device's pending subkey (from `frost_dkg_begin`) and rebuild - // its keypair. Fail before any network I/O if the two-call sequence was - // skipped. Consume it here: the subkey is single-use for this run, so a - // completed or failed run must not leave it stashed for the process - // lifetime (a retry re-mints via `frost_dkg_begin`). + // its keypair. Clone (don't consume) here so a preflight failure below + // — relay TLS pinning or proxy setup — leaves the subkey stashed for a + // cheap retry rather than forcing a re-mint (and a fresh roster across + // every participant). Fail before any network I/O if the two-call + // sequence was skipped. let subkey = { - let mut map = self.dkg_subkeys.lock().unwrap_or_else(|p| p.into_inner()); - let secret = - map.remove(&config.group_name) - .ok_or_else(|| KeepMobileError::FrostError { - msg: "call frost_dkg_begin for this group before frost_run_dkg".into(), - })?; + let map = self.dkg_subkeys.lock().unwrap_or_else(|p| p.into_inner()); + let secret = map.get(&config.group_name).cloned().ok_or_else(|| { + KeepMobileError::FrostError { + msg: "call frost_dkg_begin for this group before frost_run_dkg".into(), + } + })?; let sk = nostr_sdk::secp256k1::SecretKey::from_slice(&*secret).map_err(|e| { KeepMobileError::FrostError { msg: format!("stored DKG subkey is invalid: {e}"), @@ -1527,6 +1528,17 @@ impl KeepMobile { None }; + // Preflight has passed and the ceremony is about to start, so consume the + // subkey now: it is single-use for this run — a completed or failed + // *ceremony* must not leave it stashed for the process lifetime (a retry + // re-mints via `frost_dkg_begin`). A concurrent `frost_dkg_discard` that + // raced the preflight above simply makes this a no-op; we proceed with the + // keypair already cloned. + self.dkg_subkeys + .lock() + .unwrap_or_else(|p| p.into_inner()) + .remove(&config.group_name); + // Mint this run's identity and a fresh cancel flag, then register it so // `frost_cancel_dkg(run_id)` targets only this run. The id reaches the UI // via `Started` so it can issue a matching cancel. @@ -5636,6 +5648,50 @@ mod dkg_pending_share_tests { assert_eq!(mobile.dkg_subkeys.lock().unwrap().len(), 1); } + // A preflight failure in `frost_run_dkg` (here: no relay clears TLS pinning, + // failing before the ceremony starts) must leave the pending subkey stashed + // so the run can be retried without re-minting — which would otherwise force + // a fresh roster across every participant. + #[test] + fn run_dkg_preflight_failure_retains_pending_subkey() { + let storage = Arc::new(FailingShareStorage::default()); + let mobile = KeepMobile::new(storage as Arc).unwrap(); + + struct NoopProgress; + impl dkg::DkgProgressCallback for NoopProgress { + fn on_progress(&self, _: DkgProgressUpdate) {} + } + + mobile.frost_dkg_begin("group-a".into()).unwrap(); + + // Empty relay set: nothing passes TLS pinning, so `frost_run_dkg` fails + // in preflight, before any ceremony network I/O. + let config = DkgConfig { + group_name: "group-a".into(), + threshold: 2, + participants: 3, + our_index: 1, + relays: Vec::new(), + roster: Vec::new(), + }; + let err = mobile + .frost_run_dkg( + config, + "group-a-share".into(), + "pass".into(), + 30, + Arc::new(NoopProgress), + ) + .expect_err("a run with no verifiable relay must fail in preflight"); + assert!( + matches!(err, KeepMobileError::NetworkError { .. }), + "expected a network/transport error, got {err:?}" + ); + + // The subkey survives the failed preflight, so a retry needs no re-mint. + assert!(mobile.dkg_subkeys.lock().unwrap().contains_key("group-a")); + } + // A wrong passphrase must not clear the stash: the share stays recoverable. #[test] fn failed_recovery_retains_stash() {