diff --git a/crypto/frost/src/key_gen.rs b/crypto/frost/src/key_gen.rs index 6108f858..b63c5d22 100644 --- a/crypto/frost/src/key_gen.rs +++ b/crypto/frost/src/key_gen.rs @@ -225,10 +225,7 @@ fn complete_r2( batch.queue(rng, *l, values); } - - if !batch.verify() { - Err(FrostError::InvalidCommitment(batch.blame_vartime().unwrap()))?; - } + batch.verify_with_vartime_blame().map_err(|l| FrostError::InvalidCommitment(l))?; // TODO: Clear the original share diff --git a/crypto/frost/src/schnorr.rs b/crypto/frost/src/schnorr.rs index 8c4ce401..31cc6065 100644 --- a/crypto/frost/src/schnorr.rs +++ b/crypto/frost/src/schnorr.rs @@ -47,8 +47,12 @@ pub(crate) fn batch_verify( triplets: &[(u16, C::G, C::F, SchnorrSignature)] ) -> Result<(), u16> { let mut values = [(C::F::one(), C::G::generator()); 3]; - let mut batch = BatchVerifier::new(triplets.len() * 3, C::little_endian()); + let mut batch = BatchVerifier::new(triplets.len(), C::little_endian()); for triple in triplets { + // s = r + ca + // sG == R + cA + // R + cA - sG == 0 + // R values[0].1 = triple.3.R; // cA @@ -59,12 +63,5 @@ pub(crate) fn batch_verify( batch.queue(rng, triple.0, values); } - // s = r + ca - // sG == R + cA - // R + cA - sG == 0 - if batch.verify_vartime() { - Ok(()) - } else { - Err(batch.blame_vartime().unwrap()) - } + batch.verify_vartime_with_vartime_blame() } diff --git a/crypto/frost/src/tests/schnorr.rs b/crypto/frost/src/tests/schnorr.rs index 617cd549..5f64c303 100644 --- a/crypto/frost/src/tests/schnorr.rs +++ b/crypto/frost/src/tests/schnorr.rs @@ -31,18 +31,18 @@ pub(crate) fn verify(rng: &mut R) { } pub(crate) fn batch_verify(rng: &mut R) { - // Create 3 signatures + // Create 5 signatures let mut keys = vec![]; let mut challenges = vec![]; let mut sigs = vec![]; - for i in 0 .. 3 { + for i in 0 .. 5 { keys.push(C::F::random(&mut *rng)); challenges.push(C::F::random(&mut *rng)); sigs.push(schnorr::sign::(keys[i], C::F::random(&mut *rng), challenges[i])); } // Batch verify - let mut triplets = (0 .. 3).map( + let triplets = (0 .. 5).map( |i| (u16::try_from(i + 1).unwrap(), C::generator_table() * keys[i], challenges[i], sigs[i]) ).collect::>(); schnorr::batch_verify(rng, &triplets).unwrap(); @@ -59,14 +59,15 @@ pub(crate) fn batch_verify(rng: &mut R) { assert!(false); } } - // Sanity - schnorr::batch_verify(rng, &triplets).unwrap(); // Make sure a completely invalid signature fails when included - triplets[0].3.s = C::F::random(&mut *rng); - if let Err(blame) = schnorr::batch_verify(rng, &triplets) { - assert_eq!(blame, 1); - } else { - assert!(false); + for i in 0 .. 5 { + let mut triplets = triplets.clone(); + triplets[i].3.s = C::F::random(&mut *rng); + if let Err(blame) = schnorr::batch_verify(rng, &triplets) { + assert_eq!(blame, u16::try_from(i + 1).unwrap()); + } else { + assert!(false); + } } } diff --git a/crypto/multiexp/src/lib.rs b/crypto/multiexp/src/lib.rs index e935bb95..a89a8125 100644 --- a/crypto/multiexp/src/lib.rs +++ b/crypto/multiexp/src/lib.rs @@ -106,27 +106,51 @@ impl BatchVerifier { pub fn verify(&self) -> bool { multiexp( - self.0.iter().flat_map(|sets| sets.1.iter()).cloned(), + self.0.iter().flat_map(|pairs| pairs.1.iter()).cloned(), self.1 ).is_identity().into() } pub fn verify_vartime(&self) -> bool { multiexp_vartime( - self.0.iter().flat_map(|sets| sets.1.iter()).cloned(), + self.0.iter().flat_map(|pairs| pairs.1.iter()).cloned(), self.1 ).is_identity().into() } - // Solely has a vartime variant as there shouldn't be any reason for this to not be vartime, yet - // we should explicitly label vartime software as vartime - // TODO: Binary search, or at least randomly sort + // A constant time variant may be beneficial for robust protocols pub fn blame_vartime(&self) -> Option { - for value in &self.0 { - if !bool::from(multiexp_vartime(value.1.clone(), self.1).is_identity()) { - return Some(value.0); + let mut slice = self.0.as_slice(); + while slice.len() > 1 { + let split = slice.len() / 2; + if multiexp_vartime( + slice[.. split].iter().flat_map(|pairs| pairs.1.iter()).cloned(), + self.1 + ).is_identity().into() { + slice = &slice[split ..]; + } else { + slice = &slice[.. split]; } } - None + + slice.get(0).filter( + |(_, value)| !bool::from(multiexp_vartime(value.clone(), self.1).is_identity()) + ).map(|(id, _)| *id) + } + + pub fn verify_with_vartime_blame(&self) -> Result<(), Id> { + if self.verify() { + Ok(()) + } else { + Err(self.blame_vartime().unwrap()) + } + } + + pub fn verify_vartime_with_vartime_blame(&self) -> Result<(), Id> { + if self.verify_vartime() { + Ok(()) + } else { + Err(self.blame_vartime().unwrap()) + } } }