fix(vault): apply Shamir custody review feedback

server/Cargo.toml

@@ -78,7 +78,7 @@ pub_underscore_fields = "allow"
 redundant_pub_crate = "allow" 
 uninhabited_references = "allow" # safe with unsafe_code = "forbid" and standard uninhabited pattern (match *self {})
 too-many-lines = "allow" # this is a very common pattern in server code, and it's not always possible to break it down into smaller modules without hurting readability
-unused_async_trait_impl = "allow" # to pedantic
+unused_async_trait_impl = "allow" # too pedantic
 
 # restriction lints
 alloc_instead_of_core = "warn"

server/crates/arbiter-server/src/actors/bootstrap.rs

@@ -76,6 +76,9 @@ pub enum Error {
 /// declared, so the token stays valid across several registrations and is
 /// retired by the `Bootstrapped` event rather than by first use, whichever
 /// bootstrap path fired it.
+///
+/// Every daemon start mints a fresh token and overwrites the file: a token
+/// handed out by an earlier run is dead.
 pub struct Bootstrapper {
     token: Option<SafeCell<[u8; TOKEN_LENGTH]>>,
     token_path: Option<PathBuf>,
@@ -116,41 +119,13 @@ impl Bootstrapper {
             });
         }
 
-        let registered = diesel::select(diesel::dsl::exists(
-            schema::operator_identity::table.select(schema::operator_identity::id),
-        ))
-        .get_result::<bool>(&mut conn)
-        .await?;
-
-        let token = if registered {
-            match tokio::fs::read_to_string(&path).await {
-                Ok(existing)
-                    if existing.len() == TOKEN_LENGTH
-                        && existing.chars().all(|c| c.is_ascii_alphanumeric()) =>
-                {
-                    let mut cell = SafeCell::new([0u8; TOKEN_LENGTH]);
-                    cell.write().copy_from_slice(existing.as_bytes());
-                    cell
-                }
-                Ok(_) | Err(_) => generate_token(&path).await?,
-            }
-        } else {
-            generate_token(&path).await?
-        };
-
         Ok(Self {
-            token: Some(token),
+            token: Some(generate_token(&path).await?),
             token_path: Some(path),
             events,
         })
     }
 
-    fn is_correct_token(&mut self, token: &[u8]) -> bool {
-        self.token.as_mut().is_some_and(|expected| {
-            expected.read_inline(|bytes| bool::from(bytes.as_ref().ct_eq(token)))
-        })
-    }
-
     async fn forget(&mut self) {
         self.token = None;
         if let Some(path) = self.token_path.take() {
@@ -170,12 +145,14 @@ impl Message<events::Bootstrapped> for Bootstrapper {
         self.forget().await;
     }
 }
-
+    
 #[messages]
 impl Bootstrapper {
     #[message]
     pub fn verify_token(&mut self, token: Vec<u8>) -> bool {
-        self.is_correct_token(&token)
+        self.token.as_mut().is_some_and(|expected| {
+            expected.read_inline(|bytes| bool::from(bytes.as_ref().ct_eq(token.as_slice())))
+        })
     }
 
     #[message]

server/crates/arbiter-server/src/actors/vault_coordinator/mod.rs

@@ -192,12 +192,14 @@ async fn finalize_bootstrap(
     let mut shares = shamir::split_key(threshold, total, &mut seal_key, UnwrapErr(SysRng))
         .map_err(|error| Error::Shamir(error.to_string()))?;
 
+    if shares.len() < total {
+        return Err(Error::Shamir("missing share for operator".to_owned()));
+    }
+
     let mut encrypted = Vec::with_capacity(total);
-    for (index, (operator_id, passphrase)) in contributions.0.iter_mut().enumerate() {
+    for ((operator_id, passphrase), share) in contributions.0.iter_mut().zip(shares.iter_mut()) {
-        let share = shares
+        let share = share.read_inline(|share| encrypt_share(passphrase, share))?;
-            .read_inline(|shares| shares.get(index).cloned())
+        encrypted.push((*operator_id, share));
-            .ok_or_else(|| Error::Shamir("missing share for operator".to_owned()))?;
-        encrypted.push((*operator_id, encrypt_share(passphrase, &share)?));
     }
 
     vault
@@ -227,12 +229,9 @@ async fn finalize_unseal(
             .await?
     };
 
-    let mut plaintext = SafeCell::new(Vec::with_capacity(stored.len()));
+    let mut plaintext = Vec::with_capacity(stored.len());
     for ((_, passphrase), share) in contributions.0.iter_mut().zip(stored) {
-        let mut decrypted = decrypt_share(passphrase, share)?;
+        plaintext.push(decrypt_share(passphrase, share)?);
-        decrypted.read_inline(|share| {
-            plaintext.write_inline(|shares| shares.push(share.clone()));
-        });
     }
 
     let seal_key = shamir::combine_shares(threshold, &mut plaintext)

server/crates/arbiter-server/src/crypto/shamir.rs

@@ -16,10 +16,13 @@ pub enum ShamirError {
     Combine(String),
 }
 
-/// Return the required majority threshold for an ordinary operator committee.
+/// Return the required threshold for a Shamir share pool of `committee_size`.
 ///
-/// Committees of two are rejected: a majority of two is two, which gives each
+/// A pool of two is rejected: a majority of two is two, which gives each holder
-/// member a veto over every unseal without giving either one recovery.
+/// a veto over every unseal without giving either one recovery. That rejects no
+/// supported committee, because a two-operator vault must carry at least one
+/// recovery share and so never splits into a pool of two -- see
+/// `docs/ARCHITECTURE.md` 3.9.
 #[expect(
     clippy::integer_division,
     reason = "majority thresholds use integer arithmetic"
@@ -40,7 +43,7 @@ pub fn split_key(
     total: usize,
     key: &mut KeyCell,
     rng: impl CryptoRng,
-) -> Result<SafeCell<Vec<Vec<u8>>>, ShamirError> {
+) -> Result<Vec<SafeCell<Vec<u8>>>, ShamirError> {
     if total == 0 || threshold == 0 || threshold > total || total == 2 || total > MAX_COMMITTEE_SIZE
     {
         return Err(ShamirError::Split(
@@ -48,20 +51,23 @@ pub fn split_key(
         ));
     }
 
+    // Nothing to interpolate when one share suffices.
+    if threshold == 1 {
+        return Ok(key.0.read_inline(|key| {
+            std::iter::repeat_with(|| SafeCell::new(key.as_slice().to_vec()))
+                .take(total)
+                .collect()
+        }));
+    }
+
     key.0.read_inline(|key| {
         let key: &[u8; 32] = key
             .as_slice()
             .try_into()
             .map_err(|_| ShamirError::Split("unexpected seal key length".to_owned()))?;
 
-        if threshold == 1 {
-            return Ok(SafeCell::new(
-                std::iter::repeat_n(key.to_vec(), total).collect(),
-            ));
-        }
-
         Gf256::split_array(threshold, total, key, rng)
-            .map(SafeCell::new)
+            .map(|shares| shares.into_iter().map(SafeCell::new).collect())
             .map_err(|error| ShamirError::Split(format!("{error:?}")))
     })
 }
@@ -74,30 +80,43 @@ pub fn split_key(
 /// sized.
 pub fn combine_shares(
     threshold: usize,
-    shares: &mut SafeCell<Vec<Vec<u8>>>,
+    shares: &mut [SafeCell<Vec<u8>>],
 ) -> Result<KeyCell, ShamirError> {
     if threshold == 0 {
         return Err(ShamirError::Combine("threshold is zero".to_owned()));
     }
-    if shares.read().len() < threshold {
+    if shares.len() < threshold {
         return Err(ShamirError::Combine(
             "not enough shares supplied".to_owned(),
         ));
     }
 
-    let combined = shares.read_inline(|shares| {
+    // Mirror of the one-of-one case in [`split_key`]: the share is the key.
-        if threshold == 1 {
+    if threshold == 1 {
-            let share = shares
+        let share = shares
-                .first()
+            .first_mut()
-                .ok_or_else(|| ShamirError::Combine("no shares supplied".to_owned()))?;
+            .ok_or_else(|| ShamirError::Combine("no shares supplied".to_owned()))?;
-            return Ok(SafeCell::new(share.clone()));
+        return reconstructed_key(share.read_inline(|share| SafeCell::new(share.clone())));
-        }
+    }
-        Gf256::combine_array(shares)
+
+    let mut gathered = SafeCell::new(Vec::with_capacity(shares.len()));
+    for share in shares.iter_mut() {
+        share.read_inline(|share| {
+            gathered.write_inline(|gathered| gathered.push(share.clone()));
+        });
+    }
+
+    let combined = gathered.read_inline(|gathered| {
+        Gf256::combine_array(gathered.as_slice())
             .map(SafeCell::new)
             .map_err(|error| ShamirError::Combine(format!("{error:?}")))
     })?;
 
-    KeyCell::try_from(combined)
+    reconstructed_key(combined)
+}
+
+fn reconstructed_key(bytes: SafeCell<Vec<u8>>) -> Result<KeyCell, ShamirError> {
+    KeyCell::try_from(bytes)
         .map_err(|()| ShamirError::Combine("unexpected reconstructed key length".to_owned()))
 }
 
@@ -108,6 +127,7 @@ mod tests {
     use arbiter_crypto::safecell::{SafeCell, SafeCellHandle as _};
     use rand::rngs::SysRng;
     use rand_core::UnwrapErr;
+    use rstest::rstest;
 
     fn key_bytes(mut key: KeyCell) -> [u8; 32] {
         key.0.read_inline(|key| {
@@ -117,28 +137,29 @@ mod tests {
         })
     }
 
-    fn select(shares: &mut SafeCell<Vec<Vec<u8>>>, indexes: &[usize]) -> SafeCell<Vec<Vec<u8>>> {
+    fn select(shares: &mut [SafeCell<Vec<u8>>], indexes: &[usize]) -> Vec<SafeCell<Vec<u8>>> {
-        shares.read_inline(|shares| {
+        indexes
-            SafeCell::new(
+            .iter()
-                indexes
+            .filter_map(|index| {
-                    .iter()
+                shares
-                    .filter_map(|index| shares.get(*index).cloned())
+                    .get_mut(*index)
-                    .collect(),
+                    .map(|share| share.read_inline(|share| SafeCell::new(share.clone())))
-            )
+            })
-        })
+            .collect()
     }
 
-    #[test]
+    #[rstest]
-    fn threshold_shares_reconstruct_fixed_key() {
+    #[case(&[0, 1])]
+    #[case(&[0, 2])]
+    #[case(&[1, 2])]
+    fn threshold_shares_reconstruct_fixed_key(#[case] indexes: &[usize]) {
         let expected = [9_u8; 32];
         let mut key = KeyCell::from(expected);
         let rng = UnwrapErr(SysRng);
         let mut shares = split_key(2, 3, &mut key, rng).expect("split should succeed");
-        for indexes in [[0_usize, 1_usize], [0, 2], [1, 2]] {
+        let mut selected = select(&mut shares, indexes);
-            let mut selected = select(&mut shares, &indexes);
+        let combined = combine_shares(2, &mut selected).expect("combine should succeed");
-            let combined = combine_shares(2, &mut selected).expect("combine should succeed");
+        assert_eq!(key_bytes(combined), expected);
-            assert_eq!(key_bytes(combined), expected);
-        }
     }
 
     #[test]
@@ -163,22 +184,23 @@ mod tests {
         );
     }
 
-    #[test]
+    #[rstest]
-    fn empty_committee_has_no_threshold() {
+    #[case(0, None)]
-        assert_eq!(shamir_threshold(0), None);
+    #[case(1, Some(1))]
+    #[case(2, None)]
+    #[case(3, Some(2))]
+    #[case(4, Some(3))]
+    #[case(MAX_COMMITTEE_SIZE, Some(128))]
+    #[case(MAX_COMMITTEE_SIZE + 1, None)]
+    fn committee_threshold_is_a_majority(
+        #[case] committee_size: usize,
+        #[case] expected: Option<usize>,
+    ) {
+        assert_eq!(shamir_threshold(committee_size), expected);
     }
 
     #[test]
-    fn committee_threshold_is_majority_for_three_or_more() {
+    fn oversized_committee_is_rejected_by_split() {
-        assert_eq!(shamir_threshold(1), Some(1));
-        assert_eq!(shamir_threshold(3), Some(2));
-        assert_eq!(shamir_threshold(4), Some(3));
-    }
-
-    #[test]
-    fn oversized_committee_has_no_threshold() {
-        assert_eq!(shamir_threshold(MAX_COMMITTEE_SIZE), Some(128));
-        assert_eq!(shamir_threshold(MAX_COMMITTEE_SIZE + 1), None);
         let mut key = KeyCell::from([1_u8; 32]);
         let rng = UnwrapErr(SysRng);
         assert!(
@@ -189,7 +211,6 @@ mod tests {
 
     #[test]
     fn two_operator_committee_is_explicitly_unsupported() {
-        assert_eq!(shamir_threshold(2), None);
         let mut key = KeyCell::from([7_u8; 32]);
         let rng = UnwrapErr(SysRng);
         assert!(