From 25f1549c6c997f289a900000149137ceb164adf7 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Tue, 13 Dec 2022 20:25:32 -0500 Subject: [PATCH] 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 | 2 +- coins/monero/src/ringct/clsag/multisig.rs | 12 ++++--- crypto/frost/Cargo.toml | 2 +- crypto/frost/src/algorithm.rs | 30 ++++++++++++----- crypto/frost/src/curve/mod.rs | 2 +- crypto/frost/src/nonce.rs | 2 +- crypto/frost/src/sign.rs | 39 +++++++++++++---------- crypto/frost/src/tests/curve.rs | 2 +- crypto/frost/src/tests/literal/dalek.rs | 2 +- crypto/frost/src/tests/literal/ed448.rs | 2 +- crypto/frost/src/tests/literal/kp256.rs | 2 +- crypto/frost/src/tests/mod.rs | 2 +- crypto/frost/src/tests/vectors.rs | 2 +- crypto/schnorr/src/lib.rs | 39 ++++++++++++----------- 14 files changed, 81 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c963ba6a..2ab0a408 100644 --- a/Cargo.lock +++ b/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", diff --git a/coins/monero/src/ringct/clsag/multisig.rs b/coins/monero/src/ringct/clsag/multisig.rs index 9fcbf5e1..f7b24314 100644 --- a/coins/monero/src/ringct/clsag/multisig.rs +++ b/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 for ClsagMultisig { None } - #[must_use] fn verify_share( &self, verification_share: dfg::EdwardsPoint, nonces: &[Vec], share: dfg::Scalar, - ) -> bool { + ) -> Result, ()> { 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]), + ]) } } diff --git a/crypto/frost/Cargo.toml b/crypto/frost/Cargo.toml index f3a2179d..91eaa91d 100644 --- a/crypto/frost/Cargo.toml +++ b/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" diff --git a/crypto/frost/src/algorithm.rs b/crypto/frost/src/algorithm.rs index 18d259c7..d5cd9835 100644 --- a/crypto/frost/src/algorithm.rs +++ b/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: Clone { #[must_use] fn verify(&self, group_key: C::G, nonces: &[Vec], sum: C::F) -> Option; - /// 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], 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], + share: C::F, + ) -> Result, ()>; } /// IETF-compliant transcript. This is incredibly naive and should not be used within larger @@ -176,8 +182,16 @@ impl> Algorithm for Schnorr { Some(sig).filter(|sig| sig.verify(group_key, self.c.unwrap())) } - #[must_use] - fn verify_share(&self, verification_share: C::G, nonces: &[Vec], share: C::F) -> bool { - SchnorrSignature:: { R: nonces[0][0], s: share }.verify(verification_share, self.c.unwrap()) + fn verify_share( + &self, + verification_share: C::G, + nonces: &[Vec], + share: C::F, + ) -> Result, ()> { + Ok( + SchnorrSignature:: { R: nonces[0][0], s: share } + .batch_statements(verification_share, self.c.unwrap()) + .to_vec(), + ) } } diff --git a/crypto/frost/src/curve/mod.rs b/crypto/frost/src/curve/mod.rs index 5d8cf791..c1d95bb0 100644 --- a/crypto/frost/src/curve/mod.rs +++ b/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; diff --git a/crypto/frost/src/nonce.rs b/crypto/frost/src/nonce.rs index 66b1d500..2c009a46 100644 --- a/crypto/frost/src/nonce.rs +++ b/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}; diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 5f2dac04..1a409c7f 100644 --- a/crypto/frost/src/sign.rs +++ b/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> SignatureMachine 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")) } } diff --git a/crypto/frost/src/tests/curve.rs b/crypto/frost/src/tests/curve.rs index 31d00629..980f2435 100644 --- a/crypto/frost/src/tests/curve.rs +++ b/crypto/frost/src/tests/curve.rs @@ -1,4 +1,4 @@ -use rand::{RngCore, CryptoRng}; +use rand_core::{RngCore, CryptoRng}; use group::Group; diff --git a/crypto/frost/src/tests/literal/dalek.rs b/crypto/frost/src/tests/literal/dalek.rs index 66a74438..9a11c5d2 100644 --- a/crypto/frost/src/tests/literal/dalek.rs +++ b/crypto/frost/src/tests/literal/dalek.rs @@ -1,4 +1,4 @@ -use rand::rngs::OsRng; +use rand_core::OsRng; use crate::{ curve, diff --git a/crypto/frost/src/tests/literal/ed448.rs b/crypto/frost/src/tests/literal/ed448.rs index 0588a7b3..dc5bf3ac 100644 --- a/crypto/frost/src/tests/literal/ed448.rs +++ b/crypto/frost/src/tests/literal/ed448.rs @@ -1,4 +1,4 @@ -use rand::rngs::OsRng; +use rand_core::OsRng; use ciphersuite::Ciphersuite; diff --git a/crypto/frost/src/tests/literal/kp256.rs b/crypto/frost/src/tests/literal/kp256.rs index d203ccd5..175039e4 100644 --- a/crypto/frost/src/tests/literal/kp256.rs +++ b/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}; diff --git a/crypto/frost/src/tests/mod.rs b/crypto/frost/src/tests/mod.rs index 725cdf29..927864fa 100644 --- a/crypto/frost/src/tests/mod.rs +++ b/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}; diff --git a/crypto/frost/src/tests/vectors.rs b/crypto/frost/src/tests/vectors.rs index fbe25159..f81ec7cc 100644 --- a/crypto/frost/src/tests/vectors.rs +++ b/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}; diff --git a/crypto/schnorr/src/lib.rs b/crypto/schnorr/src/lib.rs index 41204688..fed606f4 100644 --- a/crypto/schnorr/src/lib.rs +++ b/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 SchnorrSignature { } } + /// 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 SchnorrSignature { 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)); } }