Frost signature coordinator randomization (anti-exfil)

schnorr_fun/src/binonce.rs

@@ -61,12 +61,26 @@ impl<Z> HashInto for Nonce<Z> {
 }
 
 impl Nonce<Zero> {
-    /// Adds a bunch of binonces together (one for each party signing usually).
+    /// Sums binonces together (one for each party signing usually).
     pub fn aggregate(nonces: impl IntoIterator<Item = Nonce>) -> Self {
-        let agg = nonces.into_iter().fold([Point::zero(); 2], |acc, nonce| {
+        Self::aggregate_and_add(nonces, Point::<Normal, Public, _>::zero())
+    }
+
+    /// Like [`Self::aggregate`] but adds a point to the result. This is used internally by
+    /// [`randomized_coordinator_sign_session`] to randomize the aggregate nonce.
+    ///
+    /// [`randomized_coordinator_sign_session`]: crate::frost::Frost::randomized_coordinator_sign_session
+    /// [Dark Skippy]: https://darkskippy.com
+    pub fn aggregate_and_add(
+        nonces: impl IntoIterator<Item = Nonce>,
+        addition: Point<impl PointType, impl Secrecy, impl ZeroChoice>,
+    ) -> Self {
+        let mut agg = nonces.into_iter().fold([Point::zero(); 2], |acc, nonce| {
             [g!(acc[0] + nonce.0[0]), g!(acc[1] + nonce.0[1])]
         });
 
+        agg[0] += addition;
+
         Self([agg[0].normalize(), agg[1].normalize()])
     }
 }

schnorr_fun/src/frost/mod.rs

@@ -106,6 +106,7 @@ use secp256kfun::{
     poly,
     prelude::*,
     rand_core::{RngCore, SeedableRng},
+    KeyPair,
 };
 
 /// The index of a party's secret share.
@@ -287,16 +288,49 @@ impl<H: Hash32, NG> Frost<H, NG> {
     ///
     /// If the number of nonces is less than the threshold.
     pub fn coordinator_sign_session(
+        &self,
+        shared_key: &SharedKey<EvenY>,
+        nonces: BTreeMap<PartyIndex, Nonce>,
+        message: Message,
+    ) -> CoordinatorSignSession {
+        self.coordinator_sign_session_(shared_key, nonces, message, KeyPair::zero())
+    }
+
+    /// Start a FROST session as a coordiantor with randomization.
+    ///
+    /// Like [`Self::coordinator_sign_session`] except it randomizes the session such that even if
+    /// you knew all the other arguments the final nonce (the nonce that appears in the signature)
+    /// will be indistinguishable from random. This prevents attacks like [Dark
+    /// Skippy](https://darkskippy.com/) (if the coordinator is honest of course).
+    pub fn randomized_coordinator_sign_session(
+        &self,
+        shared_key: &SharedKey<EvenY>,
+        nonces: BTreeMap<PartyIndex, Nonce>,
+        message: Message,
+        rng: &mut impl RngCore,
+    ) -> CoordinatorSignSession {
+        self.coordinator_sign_session_(
+            shared_key,
+            nonces,
+            message,
+            KeyPair::new(Scalar::random(rng)),
+        )
+    }
+
+    fn coordinator_sign_session_(
         &self,
         shared_key: &SharedKey<EvenY>,
         mut nonces: BTreeMap<PartyIndex, Nonce>,
         message: Message,
+        randomization: KeyPair<impl PointType, impl ZeroChoice>,
     ) -> CoordinatorSignSession {
         if nonces.len() < shared_key.threshold() {
             panic!("nonces' length was less than the threshold");
         }
+        let (mut randomization_scalar, randomization_point) = randomization.as_tuple();
 
-        let agg_binonce = binonce::Nonce::aggregate(nonces.values().cloned());
+        let agg_binonce =
+            binonce::Nonce::aggregate_and_add(nonces.values().cloned(), randomization_point);
 
         let binding_coeff = self.binding_coefficient(
             shared_key.public_key(),
@@ -314,7 +348,10 @@ impl<H: Hash32, NG> Frost<H, NG> {
             nonce.conditional_negate(binonce_needs_negation);
         }
 
+        randomization_scalar.conditional_negate(binonce_needs_negation);
+
         CoordinatorSignSession {
+            randomization: randomization_scalar.mark_zero(),
             binding_coeff,
             agg_binonce,
             final_nonce,

schnorr_fun/src/frost/session.rs

@@ -31,14 +31,11 @@ pub struct CoordinatorSignSession {
 
     pub(crate) agg_binonce: binonce::Nonce<Zero>,
     pub(crate) nonces: BTreeMap<PartyIndex, binonce::Nonce>,
+    pub(crate) randomization: Scalar<Secret, Zero>,
 }
 
 impl CoordinatorSignSession {
     /// Fetch the participant indices for this signing session.
-    ///
-    /// ## Return value
-    ///
-    /// An iterator of participant indices
     pub fn parties(&self) -> BTreeSet<PartyIndex> {
         self.nonces.keys().cloned().collect()
     }
@@ -62,10 +59,6 @@ impl CoordinatorSignSession {
     ///
     /// The `verification_share` is usually derived from either [`SharedKey::verification_share`] or
     /// [`PairedSecretShare::verification_share`].
-    ///
-    /// ## Return Value
-    ///
-    /// Returns `true` if signature share is valid.
     pub fn verify_signature_share(
         &self,
         verification_share: VerificationShare<impl PointType>,
@@ -85,10 +78,6 @@ impl CoordinatorSignSession {
             poly::eval_basis_poly_at_0(verification_share.index, self.nonces.keys().cloned());
         let c = &self.challenge;
         let b = &self.binding_coeff;
-        debug_assert!(
-            self.parties().contains(&index),
-            "the party is not part of the session"
-        );
         let [R1, R2] = self
             .nonces
             .get(&index)
@@ -104,7 +93,8 @@ impl CoordinatorSignSession {
 
     /// Combines signature shares from each party into the final signature.
     ///
-    /// You can use this instead of calling [`verify_signature_share`] on each share.
+    /// You can use this instead of calling [`verify_signature_share`] on each share (but you won't
+    /// know when signature share was invalid).
     ///
     /// [`verify_signature_share`]: Self::verify_signature_share
     pub fn verify_and_combine_signature_shares(
@@ -125,42 +115,35 @@ impl CoordinatorSignSession {
             .map_err(VerifySignatureSharesError::Invalid)?;
         }
 
-        let signature =
-            self.combine_signature_shares(self.final_nonce(), signature_shares.values().cloned());
+        let signature = self.combine_signature_shares(signature_shares.values().cloned());
 
         Ok(signature)
     }
 
-    /// Combine a vector of signatures shares into an aggregate signature given the final nonce.
-    ///
-    /// You can get `final_nonce` from either of the [`CoordinatorSignSession`] or the [`PartySignSession`].
+    /// Combine signatures shares into an aggregate signature.
     ///
-    /// This method does not check the validity of the `signature_shares`
-    /// but if you have verified each signature share
-    /// individually the output will be a valid siganture under the `frost_key` and message provided
-    /// when starting the session.
+    /// This method does not check the validity of the `signature_shares` but if you have verified
+    /// each signature share individually the output will be a valid siganture under the `frost_key`
+    /// and message provided when starting the session.
     ///
     /// Alternatively you can use [`verify_and_combine_signature_shares`] which checks and combines
     /// the signature shares.
     ///
-    /// ## Return value
-    ///
-    /// Returns a schnorr [`Signature`] on the message
-    ///
     /// [`CoordinatorSignSession`]: CoordinatorSignSession::final_nonce
-    /// [`PartySignSession`]: PartySignSession::final_nonce
     /// [`verify_and_combine_signature_shares`]: Self::verify_and_combine_signature_shares
     pub fn combine_signature_shares(
         &self,
-        final_nonce: Point<EvenY>,
         signature_shares: impl IntoIterator<Item = SignatureShare>,
     ) -> Signature {
-        let sum_s = signature_shares
+        let mut sum_s = signature_shares
             .into_iter()
             .reduce(|acc, partial_sig| s!(acc + partial_sig).public())
             .unwrap_or(Scalar::zero());
+
+        sum_s += self.randomization;
+
         Signature {
-            R: final_nonce,
+            R: self.final_nonce,
             s: sum_s,
         }
     }
@@ -207,10 +190,6 @@ impl PartySignSession {
     ///
     /// The `secret_share` is taken as a `PairedSecretShare<EvenY>` to guarantee that the secret is aligned with an `EvenY` point.
     ///
-    /// ## Return value
-    ///
-    /// Returns a signature share. It will be valid if the right `secret_nonce` and `secret_share` was used.
-    ///
     /// ## Panics
     ///
     /// - If `secret_share` was not part of the signing session

schnorr_fun/src/musig.rs

@@ -526,8 +526,7 @@ impl<H: Hash32, NG> MuSig<H, NG> {
         Point<EvenY>,
         bool,
     ) {
-        let mut agg_binonce = binonce::Nonce::aggregate(nonces.iter().cloned());
-        agg_binonce.0[0] = g!(agg_binonce.0[0] + encryption_key).normalize();
+        let agg_binonce = binonce::Nonce::aggregate_and_add(nonces.iter().cloned(), encryption_key);
 
         let binding_coeff = {
             let H = self.nonce_coeff_hash.clone();
@@ -545,7 +544,7 @@ impl<H: Hash32, NG> MuSig<H, NG> {
             .schnorr
             .challenge(&R, &agg_key.agg_public_key(), message);
 
-        // we may as well eagerly do
+        // we may as well eagerly do the negation
         for nonce in &mut nonces {
             nonce.conditional_negate(nonces_need_negation);
         }

schnorr_fun/tests/frost_prop.rs

@@ -74,14 +74,17 @@ proptest! {
         // shuffle the mask for random signers
         signer_mask.shuffle(&mut rng);
 
-        let secret_shares_of_signers = signer_mask.into_iter().zip(xonly_secret_shares.into_iter()).filter(|(is_signer, _)| *is_signer)
+        let secret_shares_of_signers = signer_mask
+            .into_iter()
+            .zip(xonly_secret_shares.into_iter())
+            .filter(|(is_signer, _)| *is_signer)
             .map(|(_, secret_share)| secret_share).collect::<Vec<_>>();
 
 
         let sid = b"frost-prop-test".as_slice();
         let message = Message::plain("test", b"test");
 
-        let mut secret_nonces: BTreeMap<_, _> = secret_shares_of_signers.iter().map(|paired_secret_share| {
+        let secret_nonces: BTreeMap<_, _> = secret_shares_of_signers.iter().map(|paired_secret_share| {
             (paired_secret_share.secret_share().index, frost.gen_nonce::<ChaCha20Rng>(
                 &mut frost.seed_nonce_rng(
                     *paired_secret_share,
@@ -94,40 +97,49 @@ proptest! {
 
         let coord_signing_session = frost.coordinator_sign_session(
             &xonly_shared_key,
-            public_nonces,
-            message
+            public_nonces.clone(),
+            message,
         );
 
-        let party_signing_session = frost.party_sign_session(
-            xonly_shared_key.public_key(),
-            coord_signing_session.parties(),
-            coord_signing_session.agg_binonce(),
+        let randomized_coord_signing_session = frost.randomized_coordinator_sign_session(
+            &xonly_shared_key,
+            public_nonces,
             message,
+            &mut rng
         );
 
-        let mut signatures = BTreeMap::default();
-        for secret_share in secret_shares_of_signers  {
-            let sig = party_signing_session.sign(
-                &secret_share,
-                secret_nonces.remove(&secret_share.index()).unwrap()
+        for sign_session in [coord_signing_session, randomized_coord_signing_session] {
+            let party_signing_session = frost.party_sign_session(
+                xonly_shared_key.public_key(),
+                sign_session.parties(),
+                sign_session.agg_binonce(),
+                message,
             );
-            assert_eq!(coord_signing_session.verify_signature_share(
-                secret_share.verification_share(),
-                sig), Ok(())
+
+            let mut signatures = BTreeMap::default();
+            for secret_share in &secret_shares_of_signers  {
+                let sig = party_signing_session.sign(
+                    secret_share,
+                    secret_nonces.get(&secret_share.index()).unwrap().clone(),
+                );
+                assert_eq!(sign_session.verify_signature_share(
+                    secret_share.verification_share(),
+                    sig), Ok(())
+                );
+                signatures.insert(secret_share.index(), sig);
+            }
+            let combined_sig = sign_session.combine_signature_shares(
+                signatures.values().cloned()
             );
-            signatures.insert(secret_share.index(), sig);
-        }
-        let combined_sig = coord_signing_session.combine_signature_shares(
-            coord_signing_session.final_nonce(),
-            signatures.values().cloned()
-        );
 
-        assert_eq!(coord_signing_session.verify_and_combine_signature_shares(&xonly_shared_key, signatures), Ok(combined_sig));
-        assert!(frost.schnorr.verify(
-            &xonly_shared_key.public_key(),
-            message,
-            &combined_sig
-        ));
+            assert_eq!(sign_session.verify_and_combine_signature_shares(&xonly_shared_key, signatures), Ok(combined_sig));
+            assert!(frost.schnorr.verify(
+                &xonly_shared_key.public_key(),
+                message,
+                &combined_sig
+            ));
+
+        }
 
     }
 }