diff --git a/coins/monero/src/frost.rs b/coins/monero/src/frost.rs index a5368b4a..a1955451 100644 --- a/coins/monero/src/frost.rs +++ b/coins/monero/src/frost.rs @@ -7,7 +7,6 @@ use blake2::{digest::Update, Digest, Blake2b512}; use curve25519_dalek::{ constants::ED25519_BASEPOINT_TABLE as DTable, - traits::VartimeMultiscalarMul, scalar::Scalar as DScalar, edwards::EdwardsPoint as DPoint }; @@ -56,8 +55,8 @@ impl Curve for Ed25519 { &dfg::ED25519_BASEPOINT_TABLE } - fn multiexp_vartime(scalars: &[Self::F], points: &[Self::G]) -> Self::G { - dfg::EdwardsPoint(DPoint::vartime_multiscalar_mul(scalars, points)) + fn little_endian() -> bool { + true } // This, as used by CLSAG, will already be a keccak256 hash diff --git a/coins/monero/tests/frost.rs b/coins/monero/tests/frost.rs index ff232b6d..46dce9cc 100644 --- a/coins/monero/tests/frost.rs +++ b/coins/monero/tests/frost.rs @@ -62,7 +62,7 @@ pub fn generate_keys() -> (HashMap>, Scalar) { } our_secret_shares.insert(*l, shares[&i].clone()); } - keys.insert(*i, machine.complete(our_secret_shares).unwrap().clone()); + keys.insert(*i, machine.complete(&mut OsRng, our_secret_shares).unwrap().clone()); } let mut group_private = Scalar::zero(); diff --git a/crypto/frost/Cargo.toml b/crypto/frost/Cargo.toml index d444c83c..efe7a5a0 100644 --- a/crypto/frost/Cargo.toml +++ b/crypto/frost/Cargo.toml @@ -17,7 +17,7 @@ group = "0.11" blake2 = "0.10" transcript = { path = "../transcript" } -multiexp = { path = "../multiexp" } +multiexp = { path = "../multiexp", features = ["batch"] } [dev-dependencies] rand = "0.8" diff --git a/crypto/frost/src/key_gen.rs b/crypto/frost/src/key_gen.rs index 6480e8f3..6108f858 100644 --- a/crypto/frost/src/key_gen.rs +++ b/crypto/frost/src/key_gen.rs @@ -5,6 +5,8 @@ use rand_core::{RngCore, CryptoRng}; use ff::{Field, PrimeField}; +use multiexp::{multiexp_vartime, BatchVerifier}; + use crate::{ Curve, MultisigParams, MultisigKeys, FrostError, schnorr::{self, SchnorrSignature}, @@ -122,13 +124,7 @@ fn verify_r1( commitments.insert(l, these_commitments); } - schnorr::batch_verify(rng, &signatures).map_err( - |l| if l == 0 { - FrostError::InternalError("batch validation is broken".to_string()) - } else { - FrostError::InvalidProofOfKnowledge(l) - } - )?; + schnorr::batch_verify(rng, &signatures).map_err(|l| FrostError::InvalidProofOfKnowledge(l))?; Ok(commitments) } @@ -192,7 +188,8 @@ fn generate_key_r2( /// issue, yet simply confirming protocol completion without issue is enough to confirm the same /// key was generated as long as a lack of duplicated commitments was also confirmed when they were /// broadcasted initially -fn complete_r2( +fn complete_r2( + rng: &mut R, params: MultisigParams, share: C::F, commitments: HashMap>, @@ -211,6 +208,7 @@ fn complete_r2( shares.insert(l, C::F_from_slice(&share).map_err(|_| FrostError::InvalidShare(params.i()))?); } + let mut batch = BatchVerifier::new(shares.len(), C::little_endian()); for (l, share) in &shares { if *l == params.i() { continue; @@ -218,16 +216,18 @@ fn complete_r2( let i_scalar = C::F::from(params.i.into()); let mut exp = C::F::one(); - let mut exps = Vec::with_capacity(usize::from(params.t())); - for _ in 0 .. params.t() { - exps.push(exp); + let mut values = Vec::with_capacity(usize::from(params.t()) + 1); + for lt in 0 .. params.t() { + values.push((exp, commitments[&l][usize::from(lt)])); exp *= i_scalar; } + values.push((-*share, C::generator())); - // Doesn't use multiexp_vartime with -shares[l] due to not being able to push to commitments - if C::multiexp_vartime(&exps, &commitments[&l]) != (C::generator_table() * *share) { - Err(FrostError::InvalidCommitment(*l))?; - } + batch.queue(rng, *l, values); + } + + if !batch.verify() { + Err(FrostError::InvalidCommitment(batch.blame_vartime().unwrap()))?; } // TODO: Clear the original share @@ -239,19 +239,18 @@ fn complete_r2( let mut verification_shares = HashMap::new(); for l in 1 ..= params.n() { - let mut exps = vec![]; - let mut cs = vec![]; + let mut values = vec![]; for i in 1 ..= params.n() { for j in 0 .. params.t() { let mut exp = C::F::one(); for _ in 0 .. j { exp *= C::F::from(u64::try_from(l).unwrap()); } - exps.push(exp); - cs.push(commitments[&i][usize::from(j)]); + values.push((exp, commitments[&i][usize::from(j)])); } } - verification_shares.insert(l, C::multiexp_vartime(&exps, &cs)); + // Doesn't do a unified multiexp due to needing individual verification shares + verification_shares.insert(l, multiexp_vartime(values, C::little_endian())); } debug_assert_eq!(C::generator_table() * secret_share, verification_shares[¶ms.i()]); @@ -362,8 +361,9 @@ impl StateMachine { /// group's public key, while setting a valid secret share inside the machine. > t participants /// must report completion without issue before this key can be considered usable, yet you should /// wait for all participants to report as such - pub fn complete( + pub fn complete( &mut self, + rng: &mut R, shares: HashMap>, ) -> Result, FrostError> { if self.state != State::GeneratedSecretShares { @@ -371,6 +371,7 @@ impl StateMachine { } let keys = complete_r2( + rng, self.params, self.secret.take().unwrap(), self.commitments.take().unwrap(), diff --git a/crypto/frost/src/lib.rs b/crypto/frost/src/lib.rs index b9229a09..902d7bc3 100644 --- a/crypto/frost/src/lib.rs +++ b/crypto/frost/src/lib.rs @@ -4,9 +4,7 @@ use std::collections::HashMap; use thiserror::Error; use ff::{Field, PrimeField}; -use group::{Group, GroupOps, ScalarMul}; - -pub use multiexp::multiexp_vartime; +use group::{Group, GroupOps}; mod schnorr; @@ -38,7 +36,7 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { // This is available via G::Scalar yet `C::G::Scalar` is ambiguous, forcing horrific accesses type F: PrimeField; /// Group element type - type G: Group + GroupOps + ScalarMul; + type G: Group + GroupOps; /// Precomputed table type type T: Mul; @@ -57,12 +55,8 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { /// If there isn't a precomputed table available, the generator itself should be used fn generator_table() -> Self::T; - /// Multiexponentation function, presumably Straus or Pippenger - /// This library does forward an implementation of Straus which should increase key generation - /// performance by around 4x, also named multiexp_vartime, with a similar API. However, if a more - /// performant implementation is available, that should be used instead - // This could also be written as -> Option with None for not implemented - fn multiexp_vartime(scalars: &[Self::F], points: &[Self::G]) -> Self::G; + /// If little endian is used for the scalar field's Repr + fn little_endian() -> bool; /// Hash the message as needed to calculate the binding factor /// H3 from the IETF draft diff --git a/crypto/frost/src/schnorr.rs b/crypto/frost/src/schnorr.rs index 10408e2d..8c4ce401 100644 --- a/crypto/frost/src/schnorr.rs +++ b/crypto/frost/src/schnorr.rs @@ -3,6 +3,8 @@ use rand_core::{RngCore, CryptoRng}; use ff::Field; use group::Group; +use multiexp::BatchVerifier; + use crate::Curve; #[allow(non_snake_case)] @@ -44,39 +46,25 @@ pub(crate) fn batch_verify( rng: &mut R, triplets: &[(u16, C::G, C::F, SchnorrSignature)] ) -> Result<(), u16> { - let mut first = true; - let mut scalars = Vec::with_capacity(triplets.len() * 3); - let mut points = Vec::with_capacity(triplets.len() * 3); + let mut values = [(C::F::one(), C::G::generator()); 3]; + let mut batch = BatchVerifier::new(triplets.len() * 3, C::little_endian()); for triple in triplets { - let mut u = C::F::one(); - if !first { - u = C::F::random(&mut *rng); - } + // R + values[0].1 = triple.3.R; + // cA + values[1] = (triple.2, triple.1); + // -sG + values[2].0 = -triple.3.s; - // uR - scalars.push(u); - points.push(triple.3.R); - - // -usG - scalars.push(-triple.3.s * u); - points.push(C::generator()); - - // ucA - scalars.push(if first { first = false; triple.2 } else { triple.2 * u}); - points.push(triple.1); + batch.queue(rng, triple.0, values); } // s = r + ca // sG == R + cA // R + cA - sG == 0 - if C::multiexp_vartime(&scalars, &points) == C::G::identity() { + if batch.verify_vartime() { Ok(()) } else { - for triple in triplets { - if !verify::(triple.1, triple.2, &triple.3) { - Err(triple.0)?; - } - } - Err(0) + Err(batch.blame_vartime().unwrap()) } } diff --git a/crypto/frost/src/tests/literal/secp256k1.rs b/crypto/frost/src/tests/literal/secp256k1.rs index c21de90b..4d40d689 100644 --- a/crypto/frost/src/tests/literal/secp256k1.rs +++ b/crypto/frost/src/tests/literal/secp256k1.rs @@ -13,7 +13,7 @@ use k256::{ ProjectivePoint }; -use crate::{CurveError, Curve, multiexp_vartime, algorithm::Hram, tests::curve::test_curve}; +use crate::{CurveError, Curve, algorithm::Hram, tests::curve::test_curve}; #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct Secp256k1; @@ -38,8 +38,8 @@ impl Curve for Secp256k1 { Self::G::GENERATOR } - fn multiexp_vartime(scalars: &[Self::F], points: &[Self::G]) -> Self::G { - multiexp_vartime(scalars, points, false) + fn little_endian() -> bool { + false } // The IETF draft doesn't specify a secp256k1 ciphersuite diff --git a/crypto/frost/src/tests/mod.rs b/crypto/frost/src/tests/mod.rs index 08e2cd24..4c74034d 100644 --- a/crypto/frost/src/tests/mod.rs +++ b/crypto/frost/src/tests/mod.rs @@ -82,7 +82,7 @@ pub fn key_gen( } our_secret_shares.insert(*l, shares[&i].clone()); } - let these_keys = machine.complete(our_secret_shares).unwrap(); + let these_keys = machine.complete(rng, our_secret_shares).unwrap(); // Verify the verification_shares are agreed upon if verification_shares.is_none() { diff --git a/crypto/multiexp/Cargo.toml b/crypto/multiexp/Cargo.toml index 960ed8ff..facc1aef 100644 --- a/crypto/multiexp/Cargo.toml +++ b/crypto/multiexp/Cargo.toml @@ -7,5 +7,9 @@ authors = ["Luke Parker "] edition = "2021" [dependencies] -ff = "0.11" group = "0.11" + +rand_core = { version = "0.6", optional = true } + +[features] +batch = ["rand_core"] diff --git a/crypto/multiexp/src/lib.rs b/crypto/multiexp/src/lib.rs index 40a2591f..e935bb95 100644 --- a/crypto/multiexp/src/lib.rs +++ b/crypto/multiexp/src/lib.rs @@ -1,43 +1,52 @@ -use ff::PrimeField; -use group::{Group, GroupEncoding, ScalarMul}; +use group::{ff::PrimeField, Group}; -// An implementation of Straus, with a extremely minimal API that lets us add other algorithms in -// the future. Takes in a list of scalars and points with a boolean for if the scalars are little -// endian encoded or not -pub fn multiexp_vartime>( - scalars: &[F], - points: &[G], - little: bool -) -> G { +#[cfg(feature = "batch")] +use group::ff::Field; +#[cfg(feature = "batch")] +use rand_core::{RngCore, CryptoRng}; + +fn prep< + G: Group, + I: IntoIterator +>(pairs: I, little: bool) -> (Vec>, Vec<[G; 16]>) { + let mut nibbles = vec![]; let mut tables = vec![]; - // dalek uses 8 in their impl, along with a carry scheme where values are [-8, 8) - // Moving to a similar system here did save a marginal amount, yet not one significant enough for - // its pain (as some fields do have scalars which can have their top bit set, a scenario dalek - // assumes is never true) - tables.resize(points.len(), [G::identity(); 16]); - for p in 0 .. points.len() { + for pair in pairs.into_iter() { + let p = nibbles.len(); + nibbles.push(vec![]); + { + let mut repr = pair.0.to_repr(); + let bytes = repr.as_mut(); + if !little { + bytes.reverse(); + } + + nibbles[p].resize(bytes.len() * 2, 0); + for i in 0 .. bytes.len() { + nibbles[p][i * 2] = bytes[i] & 0b1111; + nibbles[p][(i * 2) + 1] = (bytes[i] >> 4) & 0b1111; + } + } + + tables.push([G::identity(); 16]); let mut accum = G::identity(); for i in 1 .. 16 { - accum += points[p]; + accum += pair.1; tables[p][i] = accum; } } - let mut nibbles = vec![]; - nibbles.resize(scalars.len(), vec![]); - for s in 0 .. scalars.len() { - let mut repr = scalars[s].to_repr(); - let bytes = repr.as_mut(); - if !little { - bytes.reverse(); - } + (nibbles, tables) +} - nibbles[s].resize(bytes.len() * 2, 0); - for i in 0 .. bytes.len() { - nibbles[s][i * 2] = bytes[i] & 0b1111; - nibbles[s][(i * 2) + 1] = (bytes[i] >> 4) & 0b1111; - } - } +// An implementation of Straus, with a extremely minimal API that lets us add other algorithms in +// the future. Takes in an iterator of scalars and points with a boolean for if the scalars are +// little endian encoded in their Reprs or not +pub fn multiexp< + G: Group, + I: IntoIterator +>(pairs: I, little: bool) -> G { + let (nibbles, tables) = prep(pairs, little); let mut res = G::identity(); for b in (0 .. nibbles[0].len()).rev() { @@ -45,7 +54,26 @@ pub fn multiexp_vartime>( res = res.double(); } - for s in 0 .. scalars.len() { + for s in 0 .. tables.len() { + res += tables[s][nibbles[s][b] as usize]; + } + } + res +} + +pub fn multiexp_vartime< + G: Group, + I: IntoIterator +>(pairs: I, little: bool) -> G { + let (nibbles, tables) = prep(pairs, little); + + let mut res = G::identity(); + for b in (0 .. nibbles[0].len()).rev() { + for _ in 0 .. 4 { + res = res.double(); + } + + for s in 0 .. tables.len() { if nibbles[s][b] != 0 { res += tables[s][nibbles[s][b] as usize]; } @@ -53,3 +81,52 @@ pub fn multiexp_vartime>( } res } + +#[cfg(feature = "batch")] +pub struct BatchVerifier(Vec<(Id, Vec<(G::Scalar, G)>)>, bool); + +#[cfg(feature = "batch")] +impl BatchVerifier { + pub fn new(capacity: usize, endian: bool) -> BatchVerifier { + BatchVerifier(Vec::with_capacity(capacity), endian) + } + + pub fn queue< + R: RngCore + CryptoRng, + I: IntoIterator + >(&mut self, rng: &mut R, id: Id, pairs: I) { + // Define a unique scalar factor for this set of variables so individual items can't overlap + let u = if self.0.len() == 0 { + G::Scalar::one() + } else { + G::Scalar::random(rng) + }; + self.0.push((id, pairs.into_iter().map(|(scalar, point)| (scalar * u, point)).collect())); + } + + pub fn verify(&self) -> bool { + multiexp( + self.0.iter().flat_map(|sets| sets.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.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 + 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); + } + } + None + } +}