fix(vault): read the unseal threshold from the recorded split parameters
This commit is contained in:
@@ -37,7 +37,11 @@ create table if not exists tls_history (
|
||||
create table if not exists arbiter_settings (
|
||||
id INTEGER not null PRIMARY KEY CHECK (id = 1), -- singleton row, id must be 1
|
||||
root_key_id integer references root_key_history (id) on delete RESTRICT, -- if null, means wasn't bootstrapped yet
|
||||
tls_id integer references tls_history (id) on delete RESTRICT
|
||||
tls_id integer references tls_history (id) on delete RESTRICT,
|
||||
-- Shamir threshold of the split that produced the stored shares. Null before bootstrap.
|
||||
-- Recorded rather than recomputed: an aborted operator replacement leaves fewer share
|
||||
-- rows than the split has shares, and a recomputed threshold would then be wrong.
|
||||
shamir_threshold integer
|
||||
) STRICT;
|
||||
|
||||
insert into arbiter_settings (id) values (1) on conflict do nothing;
|
||||
|
||||
@@ -142,6 +142,36 @@ fn decrypt_share(
|
||||
Ok(share_buffer.read().clone())
|
||||
}
|
||||
|
||||
/// Records the threshold of the split that produced the stored shares.
|
||||
async fn store_threshold(conn: &mut db::DatabaseConnection, threshold: usize) -> Result<(), Error> {
|
||||
// A threshold that doesn't fit in the column is a bug, not a real empty-committee refusal.
|
||||
let threshold = i32::try_from(threshold).map_err(|_| Error::BrokenDatabase)?;
|
||||
let rows_updated = diesel::update(schema::arbiter_settings::table)
|
||||
.set(schema::arbiter_settings::shamir_threshold.eq(Some(threshold)))
|
||||
.execute(conn)
|
||||
.await?;
|
||||
// The singleton row always exists (up.sql seeds it), so anything else means the update did
|
||||
// not land -- bootstrap would then report success while the threshold stays NULL forever.
|
||||
if rows_updated != 1 {
|
||||
return Err(Error::BrokenDatabase);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Reads back the threshold recorded at bootstrap or re-key time.
|
||||
async fn load_threshold(conn: &mut db::DatabaseConnection) -> Result<usize, Error> {
|
||||
let stored: Option<i32> = schema::arbiter_settings::table
|
||||
.select(schema::arbiter_settings::shamir_threshold)
|
||||
.first(conn)
|
||||
.await?;
|
||||
|
||||
// A missing or out-of-range value here means the recorded threshold is corrupt, not that the
|
||||
// committee is genuinely empty -- `EmptyCommittee` is reserved for the real domain refusal.
|
||||
stored
|
||||
.and_then(|threshold| usize::try_from(threshold).ok())
|
||||
.ok_or(Error::BrokenDatabase)
|
||||
}
|
||||
|
||||
/// §3.4: Split the seal key across ordinary + recovery operators.
|
||||
/// Threshold = `shamir_threshold(ordinary_count)`; total shares = ordinary + recovery.
|
||||
/// When `ordinary_count` == 1 (threshold = 1), vsss-rs does not support a proper split,
|
||||
@@ -212,6 +242,8 @@ async fn finalize_bootstrap(
|
||||
.await?;
|
||||
}
|
||||
|
||||
store_threshold(&mut conn, threshold).await?;
|
||||
|
||||
vault.ask(Bootstrap { seal_key }).await.map_err(|err| {
|
||||
error!(?err, "Vault bootstrap failed");
|
||||
Error::VaultError
|
||||
@@ -230,12 +262,7 @@ async fn finalize_unseal(
|
||||
let mut conn = db.get().await?;
|
||||
|
||||
// Determine whether shares were stored as raw keys (threshold=1) or vsss-rs splits (threshold>=2).
|
||||
let ordinary_operator_count: i64 = schema::operator::table
|
||||
.count()
|
||||
.get_result(&mut conn)
|
||||
.await?;
|
||||
let threshold =
|
||||
shamir_threshold(ordinary_operator_count as usize).ok_or(Error::EmptyCommittee)?;
|
||||
let threshold = load_threshold(&mut conn).await?;
|
||||
|
||||
let mut shares: Vec<Vec<u8>> = Vec::new();
|
||||
|
||||
@@ -372,6 +399,8 @@ async fn finalize_rekey(
|
||||
.await?;
|
||||
}
|
||||
|
||||
store_threshold(&mut conn, threshold).await?;
|
||||
|
||||
drop(conn);
|
||||
|
||||
let new_seal_key = KeyCell::from(new_seal_key_bytes);
|
||||
@@ -582,16 +611,12 @@ impl VaultCoordinator {
|
||||
|
||||
impl VaultCoordinator {
|
||||
/// Initializes `CoordinatorState::Unsealing` on first call if still `Idle`.
|
||||
/// Threshold is based on ordinary operator count only (§3.4).
|
||||
/// Threshold comes from the recorded split parameters (§3.4), not from a live row count.
|
||||
async fn ensure_unsealing_state(&mut self) -> Result<(), Error> {
|
||||
if matches!(self.state, CoordinatorState::Idle) {
|
||||
let mut conn = self.db.get().await?;
|
||||
let ordinary_count: i64 = schema::operator::table
|
||||
.count()
|
||||
.get_result(&mut conn)
|
||||
.await?;
|
||||
let threshold = shamir_threshold(usize::try_from(ordinary_count).unwrap_or_default())
|
||||
.ok_or(Error::EmptyCommittee)?;
|
||||
let threshold = load_threshold(&mut conn).await?;
|
||||
drop(conn);
|
||||
self.state = CoordinatorState::Unsealing {
|
||||
threshold,
|
||||
ordinary_passphrases: HashMap::new(),
|
||||
|
||||
@@ -17,6 +17,7 @@ diesel::table! {
|
||||
id -> Integer,
|
||||
root_key_id -> Nullable<Integer>,
|
||||
tls_id -> Nullable<Integer>,
|
||||
shamir_threshold -> Nullable<Integer>,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ use arbiter_server::{
|
||||
vault::{Error, GetState, Vault, VaultState},
|
||||
vault_coordinator::{
|
||||
ContributeBootstrap, ContributeRecoveryBootstrap, ContributeRecoveryUnseal,
|
||||
Error as CoordinatorError, StartBootstrap, VaultCoordinator,
|
||||
ContributeUnseal, Error as CoordinatorError, StartBootstrap, VaultCoordinator,
|
||||
},
|
||||
},
|
||||
crypto::{KeyCell, encryption::v1::{Nonce, ROOT_KEY_TAG}},
|
||||
@@ -310,3 +310,110 @@ async fn empty_committee_is_rejected_without_panicking() {
|
||||
kameo::error::SendError::HandlerError(CoordinatorError::EmptyCommittee)
|
||||
));
|
||||
}
|
||||
|
||||
/// An approved-but-unfinished operator replacement deletes a share row. The unseal threshold
|
||||
/// must still describe the split that is actually stored, not the surviving row count.
|
||||
///
|
||||
/// Four ordinary operators give a real 3-of-4 `vsss-rs` split (`shamir_threshold(4) == 3`).
|
||||
/// Deleting one share row leaves 3 rows, and `shamir_threshold(3) == 2` -- a *different* number
|
||||
/// from the recorded threshold. A recount-based unseal would therefore finalize one contribution
|
||||
/// early, combine only 2 shares of a 3-of-4 split, and fail to reconstruct the seal key.
|
||||
#[tokio::test]
|
||||
#[test_log::test]
|
||||
async fn unseal_threshold_survives_a_deleted_share_row() {
|
||||
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.clone(), vault_ref));
|
||||
|
||||
// Four ordinary operators: threshold is 3-of-4.
|
||||
let mut ids = Vec::new();
|
||||
for n in 1..=4u8 {
|
||||
let mut conn = db.get().await.unwrap();
|
||||
let id: i32 = insert_into(schema::operator_identity::table)
|
||||
.values(schema::operator_identity::public_key.eq(vec![n; 32]))
|
||||
.returning(schema::operator_identity::id)
|
||||
.get_result(&mut conn)
|
||||
.await
|
||||
.unwrap();
|
||||
ids.push(id);
|
||||
}
|
||||
|
||||
coordinator
|
||||
.ask(StartBootstrap {
|
||||
operator_id: ids[0],
|
||||
declared_count: 4,
|
||||
recovery_count: 0,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
for (n, id) in ids.iter().enumerate() {
|
||||
coordinator
|
||||
.ask(ContributeBootstrap {
|
||||
operator_id: *id,
|
||||
passphrase: SafeCell::new(format!("pass-{n}").into_bytes()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
let stored: Option<i32> = {
|
||||
let mut conn = db.get().await.unwrap();
|
||||
schema::arbiter_settings::table
|
||||
.select(schema::arbiter_settings::shamir_threshold)
|
||||
.first(&mut conn)
|
||||
.await
|
||||
.unwrap()
|
||||
};
|
||||
assert_eq!(stored, Some(3), "bootstrap must record the split threshold");
|
||||
|
||||
// Simulate the aborted replacement: one share row is gone, leaving 3 of the 4 shares.
|
||||
{
|
||||
let mut conn = db.get().await.unwrap();
|
||||
diesel::delete(schema::operator::table)
|
||||
.filter(schema::operator::id.eq(Some(ids[3])))
|
||||
.execute(&mut conn)
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
// Restart and unseal with the three surviving operators' original passphrases.
|
||||
drop(coordinator);
|
||||
let bus2 = GlobalActors::spawn_message_bus();
|
||||
let vault_ref2 = Vault::spawn(Vault::new(db.clone(), bus2).await.unwrap());
|
||||
let coordinator2 = VaultCoordinator::spawn(VaultCoordinator::new(db.clone(), vault_ref2.clone()));
|
||||
|
||||
let done = coordinator2
|
||||
.ask(ContributeUnseal {
|
||||
operator_id: ids[0],
|
||||
passphrase: SafeCell::new(b"pass-0".to_vec()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!done, "one share must not be enough for a 3-of-4 split");
|
||||
|
||||
let done = coordinator2
|
||||
.ask(ContributeUnseal {
|
||||
operator_id: ids[1],
|
||||
passphrase: SafeCell::new(b"pass-1".to_vec()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(
|
||||
!done,
|
||||
"two shares must not be enough for a 3-of-4 split -- a recount would wrongly finalize here"
|
||||
);
|
||||
|
||||
let done = coordinator2
|
||||
.ask(ContributeUnseal {
|
||||
operator_id: ids[2],
|
||||
passphrase: SafeCell::new(b"pass-2".to_vec()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(done, "three shares must reconstruct a 3-of-4 split");
|
||||
assert_eq!(
|
||||
vault_ref2.ask(GetState {}).await.unwrap(),
|
||||
VaultState::Unsealed
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user