From d916997ef35cfc80caabd32db22efdefbc6c8dfd Mon Sep 17 00:00:00 2001 From: CleverWild Date: Mon, 7 Sep 2026 13:03:57 +0200 Subject: [PATCH] fix(crypto): return None from shamir_threshold for an empty committee --- .../src/actors/proposal_manager.rs | 6 ++- .../src/actors/vault_coordinator/mod.rs | 15 +++++-- .../arbiter-server/src/crypto/shamir.rs | 31 +++++++++++--- .../arbiter-server/tests/vault/lifecycle.rs | 42 +++++++++++++++++++ 4 files changed, 83 insertions(+), 11 deletions(-) diff --git a/server/crates/arbiter-server/src/actors/proposal_manager.rs b/server/crates/arbiter-server/src/actors/proposal_manager.rs index 03a6938..c153d0b 100644 --- a/server/crates/arbiter-server/src/actors/proposal_manager.rs +++ b/server/crates/arbiter-server/src/actors/proposal_manager.rs @@ -277,7 +277,11 @@ impl ProposalManager { let threshold: i64 = if requires_full_quorum { total_eligible } else { - crate::crypto::shamir::shamir_threshold(tally.total_ordinary as usize) as i64 + match crate::crypto::shamir::shamir_threshold(tally.total_ordinary as usize) { + Some(threshold) => threshold as i64, + // No ordinary operators means no electorate: nothing can settle. + None => return VoteOutcome::Pending, + } }; if tally.approve >= threshold { diff --git a/server/crates/arbiter-server/src/actors/vault_coordinator/mod.rs b/server/crates/arbiter-server/src/actors/vault_coordinator/mod.rs index ba403ca..af967fd 100644 --- a/server/crates/arbiter-server/src/actors/vault_coordinator/mod.rs +++ b/server/crates/arbiter-server/src/actors/vault_coordinator/mod.rs @@ -52,6 +52,8 @@ pub enum Error { TwoOperatorsRequireRecovery, #[error("Broken database")] BrokenDatabase, + #[error("A committee must have at least one ordinary operator")] + EmptyCommittee, } // Passphrases stored as plain Vec (not SafeCell) so CoordinatorState is Sync. @@ -153,7 +155,7 @@ async fn finalize_bootstrap( let ordinary_count = ordinary_passphrases.len(); let recovery_count = recovery_passphrases.len(); let total = ordinary_count + recovery_count; - let threshold = shamir_threshold(ordinary_count); + let threshold = shamir_threshold(ordinary_count).ok_or(Error::EmptyCommittee)?; let mut seal_key_bytes = [0u8; 32]; OsRng.fill_bytes(&mut seal_key_bytes); @@ -232,7 +234,8 @@ async fn finalize_unseal( .count() .get_result(&mut conn) .await?; - let threshold = shamir_threshold(ordinary_operator_count as usize); + let threshold = + shamir_threshold(ordinary_operator_count as usize).ok_or(Error::EmptyCommittee)?; let mut shares: Vec> = Vec::new(); @@ -314,7 +317,7 @@ async fn finalize_rekey( let ordinary_count = ordinary_passphrases.len(); let recovery_count = recovery_passphrases.len(); let total = ordinary_count + recovery_count; - let threshold = shamir_threshold(ordinary_count); + let threshold = shamir_threshold(ordinary_count).ok_or(Error::EmptyCommittee)?; let mut new_seal_key_bytes = [0u8; 32]; OsRng.fill_bytes(&mut new_seal_key_bytes); @@ -398,6 +401,9 @@ impl VaultCoordinator { if !matches!(self.state, CoordinatorState::Idle) { return Err(Error::AlreadyBootstrapping); } + if declared_count == 0 { + return Err(Error::EmptyCommittee); + } if declared_count == 2 && recovery_count == 0 { return Err(Error::TwoOperatorsRequireRecovery); } @@ -584,7 +590,8 @@ impl VaultCoordinator { .count() .get_result(&mut conn) .await?; - let threshold = shamir_threshold(usize::try_from(ordinary_count).unwrap_or_default()); + let threshold = shamir_threshold(usize::try_from(ordinary_count).unwrap_or_default()) + .ok_or(Error::EmptyCommittee)?; self.state = CoordinatorState::Unsealing { threshold, ordinary_passphrases: HashMap::new(), diff --git a/server/crates/arbiter-server/src/crypto/shamir.rs b/server/crates/arbiter-server/src/crypto/shamir.rs index 61c59e1..4600f99 100644 --- a/server/crates/arbiter-server/src/crypto/shamir.rs +++ b/server/crates/arbiter-server/src/crypto/shamir.rs @@ -21,14 +21,14 @@ pub fn split_key( } /// Returns the minimum number of shares required to reconstruct the secret -/// for a committee of `n` operators. +/// for a committee of `n` operators, or `None` for an empty committee. #[must_use] -pub const fn shamir_threshold(n: usize) -> usize { +pub const fn shamir_threshold(n: usize) -> Option { match n { - 0 => panic!("No operators"), - 1 => 1, - 2 => 2, - n => n / 2 + 1, + 0 => None, + 1 => Some(1), + 2 => Some(2), + n => Some(n / 2 + 1), } } @@ -39,3 +39,22 @@ pub fn combine_shares(shares: &[Vec]) -> Result<[u8; 32], ShamirError> { <[u8; 32]>::try_from(bytes.as_slice()) .map_err(|_| ShamirError::Combine("unexpected reconstructed key length".to_owned())) } + +#[cfg(test)] +mod tests { + use super::shamir_threshold; + + #[test] + fn empty_committee_has_no_threshold() { + assert_eq!(shamir_threshold(0), None); + } + + #[test] + fn threshold_follows_the_ordinary_quorum() { + // ARCHITECTURE.md §3.1/§3.4: 1 decides alone, 2 need consensus, N needs N/2 + 1. + assert_eq!(shamir_threshold(1), Some(1)); + assert_eq!(shamir_threshold(2), Some(2)); + assert_eq!(shamir_threshold(3), Some(2)); + assert_eq!(shamir_threshold(4), Some(3)); + } +} diff --git a/server/crates/arbiter-server/tests/vault/lifecycle.rs b/server/crates/arbiter-server/tests/vault/lifecycle.rs index e11a27d..012e251 100644 --- a/server/crates/arbiter-server/tests/vault/lifecycle.rs +++ b/server/crates/arbiter-server/tests/vault/lifecycle.rs @@ -268,3 +268,45 @@ async fn recovery_share_stored_and_used_for_unseal() { let state = vault_ref2.ask(GetState {}).await.unwrap(); assert_eq!(state, VaultState::Unsealed); } + +/// A committee of zero ordinary operators used to reach `shamir_threshold(0)` and panic, +/// taking the global coordinator down with it. +#[tokio::test] +#[test_log::test] +async fn empty_committee_is_rejected_without_panicking() { + let db = db::create_test_pool().await; + let bus = GlobalActors::spawn_message_bus(); + let vault_ref = Vault::spawn(Vault::new(db.clone(), bus).await.unwrap()); + let coordinator = VaultCoordinator::spawn(VaultCoordinator::new(db, vault_ref)); + + let err = coordinator + .ask(StartBootstrap { + operator_id: 1, + declared_count: 0, + recovery_count: 1, + }) + .await + .unwrap_err(); + + assert!( + matches!( + err, + kameo::error::SendError::HandlerError(CoordinatorError::EmptyCommittee) + ), + "expected EmptyCommittee, got {err:?}" + ); + + // The actor must still be alive to serve the next caller. + let err = coordinator + .ask(StartBootstrap { + operator_id: 1, + declared_count: 0, + recovery_count: 0, + }) + .await + .unwrap_err(); + assert!(matches!( + err, + kameo::error::SendError::HandlerError(CoordinatorError::EmptyCommittee) + )); +}