diff --git a/src/common.rs b/src/common.rs index d3a1be82..20713022 100644 --- a/src/common.rs +++ b/src/common.rs @@ -37,6 +37,16 @@ impl PolyCommitment { pub fn verify(&self, ctx: &[u8]) -> bool { self.id.verify(&self.poly[0], ctx) } + + /// Zero out the schnorr::ID + pub fn zeroize(&mut self) { + self.id.zeroize(); + } + + /// Check if schnorr proof is zeroed out + pub fn is_zero(&self) -> bool { + self.id.is_zero() + } } impl Display for PolyCommitment { diff --git a/src/schnorr.rs b/src/schnorr.rs index a2444ad2..41cff7c4 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -1,3 +1,4 @@ +use num_traits::Zero; use rand_core::{CryptoRng, RngCore}; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; @@ -12,55 +13,75 @@ use crate::{ #[allow(non_snake_case)] #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] -/// ID type which encapsulates the ID and a schnorr proof of ownership of the polynomial +/// Encapsulatetion of the ID and a zero knowledge proof of ownership of a private key bound to ID pub struct ID { - /// The ID + /// ID pub id: Scalar, - /// The public schnorr response - pub kG: Point, - /// The aggregate of the schnorr committed values - pub kca: Scalar, + /// Commitment to the proof random value + pub random_commitment: Point, + /// Sigma protocol response + pub sigma_response: Scalar, } #[allow(non_snake_case)] impl ID { - /// Construct a new schnorr ID which binds the passed `Scalar` `id` and `Scalar` `a`, with a - /// zero-knowledge proof of ownership of `a`. The `ctx` is a common reference string used to + /// Construct a new schnorr ID which binds the passed `Scalar` `id` and `Scalar` `x`, with a + /// zero-knowledge proof of ownership of `x`. The `ctx` is a common reference string used to /// prevent replay attacks; it can be any length, but will typically be a `u64` value in /// big endian format. pub fn new( id: &Scalar, - a: &Scalar, + x: &Scalar, ctx: &[u8], rng: &mut RNG, ) -> Self { - let k = Scalar::random(rng); - let c = Self::challenge(id, &(&k * &G), &(a * &G), ctx); + let r = Scalar::random(rng); + let random_commitment = r * G; + let public_key = x * G; + let c = Self::challenge(id, &random_commitment, &public_key, ctx); + let sigma_response = r + c * x; Self { id: *id, - kG: &k * G, - kca: &k + c * a, + random_commitment, + sigma_response, } } /// Compute the schnorr challenge - pub fn challenge(id: &Scalar, K: &Point, A: &Point, ctx: &[u8]) -> Scalar { + pub fn challenge( + id: &Scalar, + random_commitment: &Point, + public_key: &Point, + ctx: &[u8], + ) -> Scalar { let mut hasher = Sha256::new(); let tag = "WSTS/polynomial-constant"; hasher.update(tag.as_bytes()); hasher.update(id.to_bytes()); - hasher.update(K.compress().as_bytes()); - hasher.update(A.compress().as_bytes()); + + hasher.update(random_commitment.compress().as_bytes()); + hasher.update(public_key.compress().as_bytes()); hasher.update(ctx); hash_to_scalar(&mut hasher) } /// Verify the proof - pub fn verify(&self, A: &Point, ctx: &[u8]) -> bool { - let c = Self::challenge(&self.id, &self.kG, A, ctx); - &self.kca * &G == &self.kG + c * A + pub fn verify(&self, public_key: &Point, ctx: &[u8]) -> bool { + let c = Self::challenge(&self.id, &self.random_commitment, public_key, ctx); + &self.sigma_response * &G == &self.random_commitment + c * public_key + } + + /// Zero out the schnorr proof + pub fn zeroize(&mut self) { + self.random_commitment = Point::new(); + self.sigma_response = Scalar::zero(); + } + + /// Check if schnorr proof is zeroed out + pub fn is_zero(&self) -> bool { + self.random_commitment == Point::new() && self.sigma_response == Scalar::zero() } } diff --git a/src/state_machine/coordinator/fire.rs b/src/state_machine/coordinator/fire.rs index f89f132e..0aa57b12 100644 --- a/src/state_machine/coordinator/fire.rs +++ b/src/state_machine/coordinator/fire.rs @@ -742,7 +742,9 @@ impl Coordinator { // Cache the polynomials used in DKG for the aggregator for signer_id in self.dkg_private_shares.keys() { for (party_id, comm) in &self.dkg_public_shares[signer_id].comms { - self.party_polynomials.insert(*party_id, comm.clone()); + let mut comm = comm.clone(); + comm.zeroize(); + self.party_polynomials.insert(*party_id, comm); } } @@ -754,6 +756,10 @@ impl Coordinator { .fold(Point::default(), |s, (_, comm)| s + comm.poly[0]); info!("Aggregate public key: {key}"); + + self.dkg_public_shares.clear(); + self.dkg_private_shares.clear(); + self.aggregate_public_key = Some(key); self.move_to(State::Idle) } @@ -1092,6 +1098,7 @@ impl Coordinator { self.move_to(State::Idle)?; } + Ok(()) } @@ -1400,7 +1407,8 @@ pub mod test { bad_signature_share_request, check_signature_shares, coordinator_state_machine, empty_private_shares, empty_public_shares, equal_after_save_load, feedback_messages, feedback_mutated_messages, gen_nonces, invalid_nonce, - new_coordinator, run_dkg_sign, setup, setup_with_timeouts, start_dkg_round, + new_coordinator, run_dkg_sign, sensitive_data, setup, setup_with_timeouts, + start_dkg_round, }, Config, Coordinator as CoordinatorTrait, State, }, @@ -3191,6 +3199,16 @@ pub mod test { gen_nonces::, v2::Signer>(5, 1); } + #[test] + fn sensitive_data_v1() { + sensitive_data::, v1::Signer>(5, 1); + } + + #[test] + fn sensitive_data_v2() { + sensitive_data::, v2::Signer>(5, 1); + } + #[test] fn bad_signature_share_request_v1() { bad_signature_share_request::, v1::Signer>(5, 2); diff --git a/src/state_machine/coordinator/frost.rs b/src/state_machine/coordinator/frost.rs index 1fe86b11..4929dc72 100644 --- a/src/state_machine/coordinator/frost.rs +++ b/src/state_machine/coordinator/frost.rs @@ -414,7 +414,9 @@ impl Coordinator { return Err(Error::BadStateChange(format!("Should not have transitioned to DkgEndGather since we were missing DkgPublicShares from signer {signer_id}"))); }; for (party_id, comm) in &dkg_public_shares.comms { - self.party_polynomials.insert(*party_id, comm.clone()); + let mut comm = comm.clone(); + comm.zeroize(); + self.party_polynomials.insert(*party_id, comm); } } @@ -428,6 +430,9 @@ impl Coordinator { %key, "Aggregate public key" ); + self.dkg_public_shares.clear(); + self.dkg_private_shares.clear(); + self.aggregate_public_key = Some(key); self.move_to(State::Idle) } @@ -975,8 +980,9 @@ pub mod test { frost::Coordinator as FrostCoordinator, test::{ bad_signature_share_request, check_signature_shares, coordinator_state_machine, - empty_private_shares, empty_public_shares, equal_after_save_load, invalid_nonce, - new_coordinator, run_dkg_sign, setup, start_dkg_round, + empty_private_shares, empty_public_shares, equal_after_save_load, gen_nonces, + invalid_nonce, new_coordinator, run_dkg_sign, sensitive_data, setup, + start_dkg_round, }, Config, Coordinator as CoordinatorTrait, State, }, @@ -1397,6 +1403,26 @@ pub mod test { ); } + #[test] + fn gen_nonces_v1() { + gen_nonces::, v1::Signer>(5, 1); + } + + #[test] + fn gen_nonces_v2() { + gen_nonces::, v2::Signer>(5, 1); + } + + #[test] + fn sensitive_data_v1() { + sensitive_data::, v1::Signer>(5, 1); + } + + #[test] + fn sensitive_data_v2() { + sensitive_data::, v2::Signer>(5, 1); + } + #[test] fn bad_signature_share_request_v1() { bad_signature_share_request::, v1::Signer>(5, 2); diff --git a/src/state_machine/coordinator/mod.rs b/src/state_machine/coordinator/mod.rs index 17d8847e..6979bc9b 100644 --- a/src/state_machine/coordinator/mod.rs +++ b/src/state_machine/coordinator/mod.rs @@ -1116,6 +1116,40 @@ pub mod test { } } + pub fn sensitive_data( + num_signers: u32, + keys_per_signer: u32, + ) { + let (coordinators, signers) = + run_dkg::(num_signers, keys_per_signer); + + // check the coordinators to see if they deleted public and private shares + for coordinator in &coordinators { + let state = coordinator.save(); + + assert!(state.dkg_public_shares.is_empty()); + assert!(state.dkg_private_shares.is_empty()); + + for (_party_id, comm) in &state.party_polynomials { + assert!(comm.is_zero()); + } + } + + // check the signers to see if they deleted private shares and zeroed out poly commitments + for signer in &signers { + assert!(signer.dkg_private_shares.is_empty()); + + for (_party_id, comm) in &signer.commitments { + assert!(comm.is_zero()); + } + + for dkg_public_shares in signer.dkg_public_shares.values() { + for (_id, comm) in &dkg_public_shares.comms { + assert!(comm.is_zero()); + } + } + } + } pub fn bad_signature_share_request( num_signers: u32, keys_per_signer: u32, diff --git a/src/state_machine/signer/mod.rs b/src/state_machine/signer/mod.rs index c0ecaa76..c3f74d99 100644 --- a/src/state_machine/signer/mod.rs +++ b/src/state_machine/signer/mod.rs @@ -381,6 +381,7 @@ impl Signer { self.invalid_private_shares.clear(); self.public_nonces.clear(); self.signer.reset_polys(rng); + self.signer.gen_nonces(&self.network_private_key, rng); self.dkg_public_shares.clear(); self.dkg_private_shares.clear(); self.dkg_private_begin_msg = None; @@ -452,6 +453,7 @@ impl Signer { /// DKG is done so compute secrets pub fn dkg_ended(&mut self, rng: &mut R) -> Result { if !self.can_dkg_end() { + self.reset(self.dkg_id, rng); return Ok(Message::DkgEnd(DkgEnd { dkg_id: self.dkg_id, signer_id: self.signer_id, @@ -467,6 +469,7 @@ impl Signer { let Some(dkg_end_begin) = &self.dkg_end_begin_msg else { // no cached DkgEndBegin message + self.reset(self.dkg_id, rng); return Ok(Message::DkgEnd(DkgEnd { dkg_id: self.dkg_id, signer_id: self.signer_id, @@ -490,6 +493,7 @@ impl Signer { } if num_dkg_keys < self.dkg_threshold { + self.reset(self.dkg_id, rng); return Ok(Message::DkgEnd(DkgEnd { dkg_id: self.dkg_id, signer_id: self.signer_id, @@ -532,6 +536,7 @@ impl Signer { } if !missing_public_shares.is_empty() { + self.reset(self.dkg_id, rng); return Ok(Message::DkgEnd(DkgEnd { dkg_id: self.dkg_id, signer_id: self.signer_id, @@ -540,6 +545,7 @@ impl Signer { } if !bad_public_shares.is_empty() { + self.reset(self.dkg_id, rng); return Ok(Message::DkgEnd(DkgEnd { dkg_id: self.dkg_id, signer_id: self.signer_id, @@ -548,6 +554,7 @@ impl Signer { } if !missing_private_shares.is_empty() { + self.reset(self.dkg_id, rng); return Ok(Message::DkgEnd(DkgEnd { dkg_id: self.dkg_id, signer_id: self.signer_id, @@ -615,6 +622,14 @@ impl Signer { "sending DkgEnd" ); + for (_, dps) in self.dkg_public_shares.iter_mut() { + for (_, comm) in dps.comms.iter_mut() { + comm.zeroize(); + } + } + let dkg_public_shares = self.dkg_public_shares.clone(); + self.reset(self.dkg_id, rng); + self.dkg_public_shares = dkg_public_shares; let dkg_end = Message::DkgEnd(dkg_end); Ok(dkg_end) } @@ -766,7 +781,8 @@ impl Signer { SignatureType::Frost => self.signer.sign(msg, &signer_ids, &key_ids, &nonces), }; - self.signer.gen_nonces(&self.network_private_key, rng); + // delete sensitive values to prevent protocol attacks + self.reset(self.dkg_id, rng); let response = SignatureShareResponse { dkg_id: sign_request.dkg_id, diff --git a/src/traits.rs b/src/traits.rs index 92d48293..ba90372c 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -375,7 +375,7 @@ pub mod test_helpers { if party_id == bad_party_id { // alter the schnorr proof so it will fail verification let mut bad_comm = comm.clone(); - bad_comm.id.kca += Scalar::from(1); + bad_comm.id.sigma_response += Scalar::from(1); (party_id, bad_comm) } else { (party_id, comm)