Delete sensitive values after use

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 {

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<RNG: RngCore + CryptoRng>(
         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()
     }
 }

src/state_machine/coordinator/fire.rs

@@ -742,7 +742,9 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
         // 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<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
             .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<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
 
             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::<FireCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
     }
 
+    #[test]
+    fn sensitive_data_v1() {
+        sensitive_data::<FireCoordinator<v1::Aggregator>, v1::Signer>(5, 1);
+    }
+
+    #[test]
+    fn sensitive_data_v2() {
+        sensitive_data::<FireCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
+    }
+
     #[test]
     fn bad_signature_share_request_v1() {
         bad_signature_share_request::<FireCoordinator<v1::Aggregator>, v1::Signer>(5, 2);

src/state_machine/coordinator/frost.rs

@@ -414,7 +414,9 @@ impl<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
                 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<Aggregator: AggregatorTrait> Coordinator<Aggregator> {
             %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::<FrostCoordinator<v1::Aggregator>, v1::Signer>(5, 1);
+    }
+
+    #[test]
+    fn gen_nonces_v2() {
+        gen_nonces::<FrostCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
+    }
+
+    #[test]
+    fn sensitive_data_v1() {
+        sensitive_data::<FrostCoordinator<v1::Aggregator>, v1::Signer>(5, 1);
+    }
+
+    #[test]
+    fn sensitive_data_v2() {
+        sensitive_data::<FrostCoordinator<v2::Aggregator>, v2::Signer>(5, 1);
+    }
+
     #[test]
     fn bad_signature_share_request_v1() {
         bad_signature_share_request::<FrostCoordinator<v1::Aggregator>, v1::Signer>(5, 2);

src/state_machine/coordinator/mod.rs

@@ -1116,6 +1116,40 @@ pub mod test {
         }
     }
 
+    pub fn sensitive_data<Coordinator: CoordinatorTrait, SignerType: SignerTrait>(
+        num_signers: u32,
+        keys_per_signer: u32,
+    ) {
+        let (coordinators, signers) =
+            run_dkg::<Coordinator, SignerType>(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<Coordinator: CoordinatorTrait, SignerType: SignerTrait>(
         num_signers: u32,
         keys_per_signer: u32,

src/state_machine/signer/mod.rs

@@ -381,6 +381,7 @@ impl<SignerType: SignerTrait> Signer<SignerType> {
         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<SignerType: SignerTrait> Signer<SignerType> {
     /// DKG is done so compute secrets
     pub fn dkg_ended<R: RngCore + CryptoRng>(&mut self, rng: &mut R) -> Result<Message, Error> {
         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<SignerType: SignerTrait> Signer<SignerType> {
 
         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<SignerType: SignerTrait> Signer<SignerType> {
         }
 
         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<SignerType: SignerTrait> Signer<SignerType> {
         }
 
         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<SignerType: SignerTrait> Signer<SignerType> {
         }
 
         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<SignerType: SignerTrait> Signer<SignerType> {
         }
 
         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<SignerType: SignerTrait> Signer<SignerType> {
             "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<SignerType: SignerTrait> Signer<SignerType> {
                 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,

src/traits.rs

@@ -168,7 +168,7 @@
     ) -> Result<Signature, AggregatorError>;
 
     /// Check and aggregate the signature shares into a BIP-340 `SchnorrProof`.
     /// https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
     fn sign_schnorr(
         &mut self,
         msg: &[u8],
@@ -178,8 +178,8 @@
     ) -> Result<SchnorrProof, AggregatorError>;
 
     /// Check and aggregate the signature shares into a BIP-340 `SchnorrProof` with BIP-341 key tweaks
     /// https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
     /// https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki
     fn sign_taproot(
         &mut self,
         msg: &[u8],
@@ -375,7 +375,7 @@
                 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)