From 26ec75e0a2bef435137144e9d793e64a376a82f2 Mon Sep 17 00:00:00 2001 From: "William K. Santiago" Date: Fri, 21 Aug 2026 16:42:19 -0400 Subject: [PATCH 1/4] Move DKG roster assembly, validation and fingerprint into keep-mobile --- keep-mobile/src/dkg.rs | 141 ++++++++++++++++++++++++++++++++++++++- keep-mobile/src/lib.rs | 34 +++++++++- keep-mobile/src/types.rs | 10 +++ 3 files changed, 182 insertions(+), 3 deletions(-) diff --git a/keep-mobile/src/dkg.rs b/keep-mobile/src/dkg.rs index ad4c50a7..0f93c25f 100644 --- a/keep-mobile/src/dkg.rs +++ b/keep-mobile/src/dkg.rs @@ -32,7 +32,7 @@ use keep_frost_net::dkg::{ use crate::error::KeepMobileError; use crate::network::validate_relay_url; -use crate::types::{DkgConfig, DkgProgressUpdate}; +use crate::types::{DkgConfig, DkgParticipant, DkgProgressUpdate, RosterVerification}; /// Reports DKG progress to the native layer. Implemented on the foreign side /// (Kotlin) so setup UI can render live state without polling. Called from the @@ -157,6 +157,91 @@ fn build_roster(config: &DkgConfig) -> Result { }) } +/// Render a `frost_group_id` as a short, human-comparable fingerprint: the first +/// eight bytes as uppercase hex in four space-separated pairs of bytes. Every +/// device that derives the same `frost_group_id` (same name, threshold, and +/// index-ordered members) renders the same string, so participants can read it +/// aloud out of band; a coordinator who slips an extra key into one device's +/// roster yields a different id there and the mismatch is visible. +fn group_id_fingerprint(group_id: &[u8; 32]) -> String { + let mut out = String::with_capacity(19); + for (i, b) in group_id[..8].iter().enumerate() { + if i > 0 && i % 2 == 0 { + out.push(' '); + } + out.push_str(&format!("{b:02X}")); + } + out +} + +/// Assemble the coordinator's roster: index 1 is the coordinator, each collected +/// joiner subkey takes index i+2 in scan order. This is the roster-assembly +/// policy (§4) — kept in Rust rather than the UI so there is one place that +/// decides how indices map to keys. Every key is parsed and de-duplicated here so +/// a malformed or repeated subkey is refused before it reaches the wire; the +/// assembled roster is still validated by [`build_roster`] before the run. +pub(crate) fn assemble_roster( + coordinator_pubkey: &str, + joiner_pubkeys: &[String], +) -> Result, KeepMobileError> { + let mut entries = Vec::with_capacity(joiner_pubkeys.len() + 1); + let mut seen: Vec = Vec::new(); + for (i, pk) in std::iter::once(coordinator_pubkey) + .chain(joiner_pubkeys.iter().map(String::as_str)) + .enumerate() + { + let parsed = PublicKey::parse(pk) + .map_err(|e| frost_err(format!("roster pubkey {pk:?} is invalid: {e}")))?; + if seen.contains(&parsed) { + return Err(frost_err( + "roster repeats one pubkey; every participant must hold a distinct key", + )); + } + seen.push(parsed); + entries.push(DkgParticipant { + index: (i + 1) as u16, + pubkey: pk.to_string(), + }); + } + Ok(entries) +} + +/// Validate a finalized roster the same way [`run_dkg`] does — index range and +/// uniqueness, duplicate-pubkey rejection, threshold/participant bounds — resolve +/// the verifying device's index by matching `our_pubkey`, and return a +/// human-comparable fingerprint of the canonical `frost_group_id`. This is the +/// single authenticated identity path for the setup UI: the fingerprint the user +/// reads aloud and the `d`-tag channel the run lands on are the same digest, so +/// the two cannot drift the way a UI-side recomputation did. +pub(crate) fn verify_roster( + group_name: &str, + threshold: u16, + participants: u16, + roster: &[DkgParticipant], + our_pubkey: &str, +) -> Result { + let our = PublicKey::parse(our_pubkey) + .map_err(|e| frost_err(format!("this device's pubkey is invalid: {e}")))?; + let our_index = roster + .iter() + .find(|p| PublicKey::parse(&p.pubkey).ok().as_ref() == Some(&our)) + .map(|p| p.index) + .ok_or_else(|| frost_err("this device is not in the roster"))?; + let config = DkgConfig { + group_name: group_name.to_string(), + threshold, + participants, + our_index, + relays: Vec::new(), + roster: roster.to_vec(), + }; + let built = build_roster(&config)?; + Ok(RosterVerification { + fingerprint: group_id_fingerprint(&built.group_id), + our_index, + }) +} + /// Encode the finalized share as an encrypted bech32 export, mirroring what the /// hardware/import paths persist (the caller stores it through `import_share`). fn export_share( @@ -386,4 +471,58 @@ mod tests { bad[1].pubkey = "not-a-key".into(); assert!(build_roster(&config_with(2, 3, 1, bad)).is_err()); } + + #[test] + fn assemble_roster_numbers_coordinator_first_then_joiners() { + let ks = [subkey(1), subkey(2), subkey(3)]; + let joiners = vec![ks[1].public_key().to_hex(), ks[2].public_key().to_hex()]; + let roster = assemble_roster(&ks[0].public_key().to_hex(), &joiners).unwrap(); + assert_eq!(roster, roster_of(&ks)); + } + + #[test] + fn assemble_roster_rejects_duplicate_and_invalid_keys() { + let ks = [subkey(1), subkey(2)]; + // coordinator repeated as a joiner + assert!(assemble_roster( + &ks[0].public_key().to_hex(), + &[ks[0].public_key().to_hex()] + ) + .is_err()); + // unparseable joiner key + assert!(assemble_roster(&ks[0].public_key().to_hex(), &["nope".into()]).is_err()); + } + + #[test] + fn verify_roster_resolves_our_index_and_matches_group_id() { + let ks = [subkey(1), subkey(2), subkey(3)]; + let roster = roster_of(&ks); + let v = verify_roster("test", 2, 3, &roster, &ks[2].public_key().to_hex()).unwrap(); + assert_eq!(v.our_index, 3); + let group_id = + build_roster(&config_with(2, 3, 1, roster.clone())).unwrap().group_id; + assert_eq!(v.fingerprint, group_id_fingerprint(&group_id)); + // bech32 pubkey must resolve to the same index as the hex form + let bech = ks[1].public_key().to_bech32().unwrap(); + assert_eq!(verify_roster("test", 2, 3, &roster, &bech).unwrap().our_index, 2); + } + + #[test] + fn verify_roster_rejects_outsider_and_bad_roster() { + let ks = [subkey(1), subkey(2), subkey(3)]; + let roster = roster_of(&ks); + // a device whose key is not in the roster + assert!(verify_roster("test", 2, 3, &roster, &subkey(9).public_key().to_hex()).is_err()); + // duplicate pubkey across two indices is rejected by build_roster + let mut dup = roster.clone(); + dup[2].pubkey = ks[0].public_key().to_hex(); + assert!(verify_roster("test", 2, 3, &dup, &ks[0].public_key().to_hex()).is_err()); + } + + #[test] + fn group_id_fingerprint_is_four_uppercase_hex_pairs() { + let mut id = [0u8; 32]; + id[..8].copy_from_slice(&[0xAB, 0x12, 0xEF, 0x00, 0x9C, 0x34, 0x7D, 0x5E]); + assert_eq!(group_id_fingerprint(&id), "AB12 EF00 9C34 7D5E"); + } } diff --git a/keep-mobile/src/lib.rs b/keep-mobile/src/lib.rs index f9ae0cef..1e1b58f1 100644 --- a/keep-mobile/src/lib.rs +++ b/keep-mobile/src/lib.rs @@ -51,8 +51,8 @@ pub use storage::{PendingShareInfo, SecureStorage, ShareInfo, ShareMetadataInfo, pub use types::{ AnnouncedXpubInfo, BackupInfo, ConnectionStatus, DescriptorProposal, DeviceRegistrationInfo, DkgConfig, DkgParticipant, DkgProgressUpdate, FrostGenerationResult, GeneratedShareInfo, - KeepLiveState, KeyHealthStatusInfo, PeerInfo, PeerStatus, RecoveryTierConfig, SignRequest, - SignRequestMetadata, ThresholdConfig, WalletDescriptorInfo, + KeepLiveState, KeyHealthStatusInfo, PeerInfo, PeerStatus, RecoveryTierConfig, + RosterVerification, SignRequest, SignRequestMetadata, ThresholdConfig, WalletDescriptorInfo, }; #[uniffi::export] @@ -75,6 +75,36 @@ pub fn is_hex_64(value: String) -> bool { value.len() == 64 && value.bytes().all(|b| b.is_ascii_hexdigit()) } +/// Assemble the coordinator's DKG roster in Rust: index 1 is the coordinator, +/// each collected joiner subkey takes index i+2 in scan order (§4). Rejects a +/// malformed or duplicated subkey before it reaches the wire. The roster-assembly +/// policy lives here, not in the setup UI, so there is one authority on how +/// indices map to keys. +#[uniffi::export] +pub fn frost_assemble_roster( + coordinator_pubkey: String, + joiner_pubkeys: Vec, +) -> Result, KeepMobileError> { + dkg::assemble_roster(&coordinator_pubkey, &joiner_pubkeys) +} + +/// Validate a finalized roster with the same checks `frost_run_dkg` applies +/// (index range/uniqueness, duplicate-pubkey rejection, threshold/participant +/// bounds), resolve this device's index by matching `our_pubkey`, and return a +/// human-comparable fingerprint of the canonical `frost_group_id`. The setup UI +/// renders this fingerprint instead of recomputing its own digest, so the value +/// read aloud out of band cannot drift from the id the run actually uses. +#[uniffi::export] +pub fn frost_verify_roster( + group_name: String, + threshold: u16, + participants: u16, + roster: Vec, + our_pubkey: String, +) -> Result { + dkg::verify_roster(&group_name, threshold, participants, &roster, &our_pubkey) +} + #[uniffi::export] pub fn hex_to_npub(hex: String) -> Option { use nostr_sdk::prelude::{PublicKey, ToBech32}; diff --git a/keep-mobile/src/types.rs b/keep-mobile/src/types.rs index a4ef55bb..f80bafcc 100644 --- a/keep-mobile/src/types.rs +++ b/keep-mobile/src/types.rs @@ -77,6 +77,16 @@ pub struct DkgParticipant { pub pubkey: String, } +/// Result of validating a finalized roster in Rust (`frost_verify_roster`): the +/// human-comparable fingerprint of the canonical `frost_group_id` that every +/// participant reads aloud out of band, and the verifying device's index within +/// the roster (resolved by matching its own subkey pubkey). +#[derive(uniffi::Record, Clone, Debug, PartialEq)] +pub struct RosterVerification { + pub fingerprint: String, + pub our_index: u16, +} + #[derive(uniffi::Record, Clone)] pub struct DkgConfig { pub group_name: String, From 8a1e8af6be74947ac93576e0242f46300949061e Mon Sep 17 00:00:00 2001 From: "William K. Santiago" Date: Fri, 21 Aug 2026 17:11:05 -0400 Subject: [PATCH 2/4] Cap assemble_roster participants at u8 to prevent index wrap --- keep-mobile/src/dkg.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/keep-mobile/src/dkg.rs b/keep-mobile/src/dkg.rs index 0f93c25f..3ca9b2ee 100644 --- a/keep-mobile/src/dkg.rs +++ b/keep-mobile/src/dkg.rs @@ -184,6 +184,15 @@ pub(crate) fn assemble_roster( coordinator_pubkey: &str, joiner_pubkeys: &[String], ) -> Result, KeepMobileError> { + // FROST indices are u8, so cap the count before the u16 cast below can wrap + // (65536 -> index 0). Fail closed here rather than leak a bad index downstream. + if joiner_pubkeys.len() + 1 > u8::MAX as usize { + return Err(frost_err(format!( + "roster carries {} participants but at most {} are allowed", + joiner_pubkeys.len() + 1, + u8::MAX + ))); + } let mut entries = Vec::with_capacity(joiner_pubkeys.len() + 1); let mut seen: Vec = Vec::new(); for (i, pk) in std::iter::once(coordinator_pubkey) @@ -493,6 +502,13 @@ mod tests { assert!(assemble_roster(&ks[0].public_key().to_hex(), &["nope".into()]).is_err()); } + #[test] + fn assemble_roster_rejects_more_than_u8_participants() { + // 255 joiners + coordinator = 256 participants overflows the u8 index. + let joiners = vec![subkey(1).public_key().to_hex(); u8::MAX as usize]; + assert!(assemble_roster(&subkey(2).public_key().to_hex(), &joiners).is_err()); + } + #[test] fn verify_roster_resolves_our_index_and_matches_group_id() { let ks = [subkey(1), subkey(2), subkey(3)]; From d155f728d4011206b5281cdfb70b62ef96af8b3e Mon Sep 17 00:00:00 2001 From: "William K. Santiago" Date: Fri, 21 Aug 2026 17:11:53 -0400 Subject: [PATCH 3/4] Apply rustfmt to keep-mobile dkg tests --- keep-mobile/src/dkg.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/keep-mobile/src/dkg.rs b/keep-mobile/src/dkg.rs index 3ca9b2ee..6730d065 100644 --- a/keep-mobile/src/dkg.rs +++ b/keep-mobile/src/dkg.rs @@ -493,11 +493,9 @@ mod tests { fn assemble_roster_rejects_duplicate_and_invalid_keys() { let ks = [subkey(1), subkey(2)]; // coordinator repeated as a joiner - assert!(assemble_roster( - &ks[0].public_key().to_hex(), - &[ks[0].public_key().to_hex()] - ) - .is_err()); + assert!( + assemble_roster(&ks[0].public_key().to_hex(), &[ks[0].public_key().to_hex()]).is_err() + ); // unparseable joiner key assert!(assemble_roster(&ks[0].public_key().to_hex(), &["nope".into()]).is_err()); } @@ -515,12 +513,18 @@ mod tests { let roster = roster_of(&ks); let v = verify_roster("test", 2, 3, &roster, &ks[2].public_key().to_hex()).unwrap(); assert_eq!(v.our_index, 3); - let group_id = - build_roster(&config_with(2, 3, 1, roster.clone())).unwrap().group_id; + let group_id = build_roster(&config_with(2, 3, 1, roster.clone())) + .unwrap() + .group_id; assert_eq!(v.fingerprint, group_id_fingerprint(&group_id)); // bech32 pubkey must resolve to the same index as the hex form let bech = ks[1].public_key().to_bech32().unwrap(); - assert_eq!(verify_roster("test", 2, 3, &roster, &bech).unwrap().our_index, 2); + assert_eq!( + verify_roster("test", 2, 3, &roster, &bech) + .unwrap() + .our_index, + 2 + ); } #[test] From d3fa247a7a3a0d437533df9fd7f0f8d54991dfa5 Mon Sep 17 00:00:00 2001 From: "William K. Santiago" Date: Fri, 21 Aug 2026 18:25:39 -0400 Subject: [PATCH 4/4] Make u8-cap test use distinct keys so it actually guards the cap --- keep-mobile/src/dkg.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/keep-mobile/src/dkg.rs b/keep-mobile/src/dkg.rs index 6730d065..b14ac42f 100644 --- a/keep-mobile/src/dkg.rs +++ b/keep-mobile/src/dkg.rs @@ -370,6 +370,18 @@ mod tests { Keys::new(sk.into()) } + /// Distinct key for any `n`, unlike `subkey` whose u8 seed tops out at 255 + /// usable values (seed 0 is an invalid secret key). Needed to build a roster + /// of 256 all-distinct participants, the only way to reach the u8 cap without + /// duplicate rejection firing first. + fn subkey_n(n: u16) -> Keys { + let mut b = [1u8; 32]; + b[0] = (n & 0xff) as u8; + b[1] = (n >> 8) as u8; + let sk = nostr_sdk::secp256k1::SecretKey::from_slice(&b).unwrap(); + Keys::new(sk.into()) + } + fn config_with( threshold: u16, participants: u16, @@ -502,9 +514,14 @@ mod tests { #[test] fn assemble_roster_rejects_more_than_u8_participants() { - // 255 joiners + coordinator = 256 participants overflows the u8 index. - let joiners = vec![subkey(1).public_key().to_hex(); u8::MAX as usize]; - assert!(assemble_roster(&subkey(2).public_key().to_hex(), &joiners).is_err()); + // 255 distinct joiners + coordinator = 256 participants overflows the u8 + // index. The keys must be distinct so the cap is what rejects the roster, + // not duplicate detection firing first. + let joiners: Vec = (0..u8::MAX as u16) + .map(|i| subkey_n(i).public_key().to_hex()) + .collect(); + assert_eq!(joiners.len(), u8::MAX as usize); + assert!(assemble_roster(&subkey_n(1000).public_key().to_hex(), &joiners).is_err()); } #[test]