Move verify_share to return batch-verifiable statements

While the previous construction achieved n/2 average detection,
this will run in log2(n). Unfortunately, the need to keep entropy
around (or take in an RNG here) remains.

Cargo.lock

@@ -4631,8 +4631,8 @@ dependencies = [
  "hkdf",
  "minimal-ed448",
  "multiexp",
- "rand 0.8.5",
  "rand_chacha 0.3.1",
+ "rand_core 0.6.4",
  "schnorr-signatures",
  "serde_json",
  "subtle",

coins/monero/src/ringct/clsag/multisig.rs

@@ -10,13 +10,12 @@ use rand_chacha::ChaCha20Rng;
 use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing};
 
 use curve25519_dalek::{
-  constants::ED25519_BASEPOINT_TABLE,
   traits::{Identity, IsIdentity},
   scalar::Scalar,
   edwards::EdwardsPoint,
 };
 
-use group::{Group, GroupEncoding};
+use group::{ff::Field, Group, GroupEncoding};
 
 use transcript::{Transcript, RecommendedTranscript};
 use dalek_ff_group as dfg;
@@ -296,14 +295,17 @@ impl Algorithm<Ed25519> for ClsagMultisig {
     None
   }
 
-  #[must_use]
   fn verify_share(
     &self,
     verification_share: dfg::EdwardsPoint,
     nonces: &[Vec<dfg::EdwardsPoint>],
     share: dfg::Scalar,
-  ) -> bool {
+  ) -> Result<Vec<(dfg::Scalar, dfg::EdwardsPoint)>, ()> {
     let interim = self.interim.as_ref().unwrap();
-    (&share.0 * &ED25519_BASEPOINT_TABLE) == (nonces[0][0].0 - (interim.p * verification_share.0))
+    Ok(vec![
+      (share, dfg::EdwardsPoint::generator()),
+      (dfg::Scalar(interim.p), verification_share),
+      (-dfg::Scalar::one(), nonces[0][0]),
+    ])
   }
 }

crypto/frost/Cargo.toml

@@ -15,8 +15,8 @@ rustdoc-args = ["--cfg", "docsrs"]
 [dependencies]
 thiserror = "1"
 
+rand_core = "0.6"
 rand_chacha = "0.3"
-rand = "0.8"
 
 zeroize = { version = "1.5", features = ["zeroize_derive"] }
 subtle = "2"

crypto/frost/src/algorithm.rs

@@ -2,7 +2,7 @@ use core::{marker::PhantomData, fmt::Debug};
 use std::io::{self, Read, Write};
 
 use zeroize::Zeroizing;
-use rand::{RngCore, CryptoRng};
+use rand_core::{RngCore, CryptoRng};
 
 use transcript::Transcript;
 
@@ -75,10 +75,16 @@ pub trait Algorithm<C: Curve>: Clone {
   #[must_use]
   fn verify(&self, group_key: C::G, nonces: &[Vec<C::G>], sum: C::F) -> Option<Self::Signature>;
 
-  /// Verify a specific share given as a response. Used to determine blame if signature
-  /// verification fails.
-  #[must_use]
-  fn verify_share(&self, verification_share: C::G, nonces: &[Vec<C::G>], share: C::F) -> bool;
+  /// Verify a specific share given as a response.
+  /// This function should return a series of pairs whose products should sum to zero for a valid
+  /// share. Any error raised is treated as the share being invalid.
+  #[allow(clippy::type_complexity, clippy::result_unit_err)]
+  fn verify_share(
+    &self,
+    verification_share: C::G,
+    nonces: &[Vec<C::G>],
+    share: C::F,
+  ) -> Result<Vec<(C::F, C::G)>, ()>;
 }
 
 /// IETF-compliant transcript. This is incredibly naive and should not be used within larger
@@ -176,8 +182,16 @@ impl<C: Curve, H: Hram<C>> Algorithm<C> for Schnorr<C, H> {
     Some(sig).filter(|sig| sig.verify(group_key, self.c.unwrap()))
   }
 
-  #[must_use]
-  fn verify_share(&self, verification_share: C::G, nonces: &[Vec<C::G>], share: C::F) -> bool {
-    SchnorrSignature::<C> { R: nonces[0][0], s: share }.verify(verification_share, self.c.unwrap())
+  fn verify_share(
+    &self,
+    verification_share: C::G,
+    nonces: &[Vec<C::G>],
+    share: C::F,
+  ) -> Result<Vec<(C::F, C::G)>, ()> {
+    Ok(
+      SchnorrSignature::<C> { R: nonces[0][0], s: share }
+        .batch_statements(verification_share, self.c.unwrap())
+        .to_vec(),
+    )
   }
 }

crypto/frost/src/curve/mod.rs

@@ -1,7 +1,7 @@
 use core::ops::Deref;
 use std::io::{self, Read};
 
-use rand::{RngCore, CryptoRng};
+use rand_core::{RngCore, CryptoRng};
 
 use zeroize::{Zeroize, Zeroizing};
 use subtle::ConstantTimeEq;

crypto/frost/src/nonce.rs

@@ -14,7 +14,7 @@ use std::{
   collections::HashMap,
 };
 
-use rand::{RngCore, CryptoRng};
+use rand_core::{RngCore, CryptoRng};
 
 use zeroize::{Zeroize, Zeroizing};
 

crypto/frost/src/sign.rs

@@ -4,15 +4,15 @@ use std::{
   collections::HashMap,
 };
 
-use rand::{RngCore, CryptoRng, SeedableRng};
-use rand_chacha::{ChaCha8Rng, ChaCha20Rng};
-use rand::seq::SliceRandom;
+use rand_core::{RngCore, CryptoRng, SeedableRng};
+use rand_chacha::ChaCha20Rng;
 
 use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing};
 
 use transcript::Transcript;
 
 use group::{ff::PrimeField, GroupEncoding};
+use multiexp::BatchVerifier;
 
 use crate::{
   curve::Curve,
@@ -478,24 +478,29 @@ impl<C: Curve, A: Algorithm<C>> SignatureMachine<A::Signature> for AlgorithmSign
       return Ok(sig);
     }
 
-    // Find out who misbehaved
-    // Randomly sorts the included participants to discover the answer on average within n/2 tries
-    // If we didn't randomly sort them, it would be gameable to n by a malicious participant
-    let mut rand_included = self.view.included().to_vec();
-    // It is unfortunate we have to construct a ChaCha RNG here, yet it's due to the lack of a
-    // provided RNG. Its hashing is cheaper than abused ECC ops
-    rand_included.shuffle(&mut ChaCha8Rng::from_seed(self.blame_entropy));
-    for l in rand_included {
-      if !self.params.algorithm.verify_share(
-        self.view.verification_share(l),
-        &self.B.bound(l),
-        responses[&l],
+    // We could remove blame_entropy by taking in an RNG here
+    // Considering we don't need any RNG for a valid signature, and we only use the RNG here for
+    // performance reasons, it doesn't feel worthwhile to include as an argument to every
+    // implementor of the trait
+    let mut rng = ChaCha20Rng::from_seed(self.blame_entropy);
+    let mut batch = BatchVerifier::new(self.view.included().len());
+    for l in self.view.included() {
+      if let Ok(statements) = self.params.algorithm.verify_share(
+        self.view.verification_share(*l),
+        &self.B.bound(*l),
+        responses[l],
       ) {
-        Err(FrostError::InvalidShare(l))?;
+        batch.queue(&mut rng, *l, statements);
+      } else {
+        Err(FrostError::InvalidShare(*l))?;
       }
     }
 
-    // If everyone has a valid share and there were enough participants, this should've worked
+    if let Err(l) = batch.verify_vartime_with_vartime_blame() {
+      Err(FrostError::InvalidShare(l))?;
+    }
+
+    // If everyone has a valid share, and there were enough participants, this should've worked
     Err(FrostError::InternalError("everyone had a valid share yet the signature was still invalid"))
   }
 }

crypto/frost/src/tests/curve.rs

@@ -1,4 +1,4 @@
-use rand::{RngCore, CryptoRng};
+use rand_core::{RngCore, CryptoRng};
 
 use group::Group;
 

crypto/frost/src/tests/literal/dalek.rs

@@ -1,4 +1,4 @@
-use rand::rngs::OsRng;
+use rand_core::OsRng;
 
 use crate::{
   curve,

crypto/frost/src/tests/literal/ed448.rs

@@ -1,4 +1,4 @@
-use rand::rngs::OsRng;
+use rand_core::OsRng;
 
 use ciphersuite::Ciphersuite;
 

crypto/frost/src/tests/literal/kp256.rs

@@ -1,4 +1,4 @@
-use rand::rngs::OsRng;
+use rand_core::OsRng;
 
 use crate::tests::vectors::{Vectors, test_with_vectors};
 

crypto/frost/src/tests/mod.rs

@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 
-use rand::{RngCore, CryptoRng};
+use rand_core::{RngCore, CryptoRng};
 
 pub use dkg::tests::{key_gen, recover_key};
 

crypto/frost/src/tests/vectors.rs

@@ -5,7 +5,7 @@ use std::collections::HashMap;
 use std::str::FromStr;
 
 use zeroize::Zeroizing;
-use rand::{RngCore, CryptoRng};
+use rand_core::{RngCore, CryptoRng};
 
 use group::{ff::PrimeField, GroupEncoding};
 

crypto/schnorr/src/lib.rs

@@ -7,10 +7,10 @@ use zeroize::{Zeroize, Zeroizing};
 
 use group::{
   ff::{Field, PrimeField},
-  GroupEncoding,
+  Group, GroupEncoding,
 };
 
-use multiexp::BatchVerifier;
+use multiexp::{multiexp_vartime, BatchVerifier};
 
 use ciphersuite::Ciphersuite;
 
@@ -59,10 +59,26 @@ impl<C: Ciphersuite> SchnorrSignature<C> {
     }
   }
 
+  /// Return the series of pairs whose products sum to zero for a valid signature.
+  /// This is inteded to be used with a multiexp.
+  pub fn batch_statements(&self, public_key: C::G, challenge: C::F) -> [(C::F, C::G); 3] {
+    // s = r + ca
+    // sG == R + cA
+    // R + cA - sG == 0
+    [
+      // R
+      (C::F::one(), self.R),
+      // cA
+      (challenge, public_key),
+      // -sG
+      (-self.s, C::generator()),
+    ]
+  }
+
   /// Verify a Schnorr signature for the given key with the specified challenge.
   #[must_use]
   pub fn verify(&self, public_key: C::G, challenge: C::F) -> bool {
-    (C::generator() * self.s) == (self.R + (public_key * challenge))
+    multiexp_vartime(&self.batch_statements(public_key, challenge)).is_identity().into()
   }
 
   /// Queue a signature for batch verification.
@@ -74,21 +90,6 @@ impl<C: Ciphersuite> SchnorrSignature<C> {
     public_key: C::G,
     challenge: C::F,
   ) {
-    // s = r + ca
-    // sG == R + cA
-    // R + cA - sG == 0
-
-    batch.queue(
-      rng,
-      id,
-      [
-        // R
-        (C::F::one(), self.R),
-        // cA
-        (challenge, public_key),
-        // -sG
-        (-self.s, C::generator()),
-      ],
-    );
+    batch.queue(rng, id, self.batch_statements(public_key, challenge));
   }
 }