From 614badfef78ee7388f16023d5eab6b11745242e8 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Mon, 30 May 2022 01:46:30 -0400 Subject: [PATCH] Lint FROST key gen and optimize sign for the success path --- crypto/frost/src/key_gen.rs | 70 ++++++++++++++++++++++--------------- crypto/frost/src/sign.rs | 24 +++++-------- 2 files changed, 51 insertions(+), 43 deletions(-) diff --git a/crypto/frost/src/key_gen.rs b/crypto/frost/src/key_gen.rs index 354082bb..b2ce6a16 100644 --- a/crypto/frost/src/key_gen.rs +++ b/crypto/frost/src/key_gen.rs @@ -186,7 +186,7 @@ fn generate_key_r2( fn complete_r2( rng: &mut R, params: MultisigParams, - share: C::F, + mut secret_share: C::F, commitments: HashMap>, // Vec to preserve ownership mut serialized: HashMap>, @@ -194,7 +194,7 @@ fn complete_r2( validate_map( &mut serialized, &(1 ..= params.n()).into_iter().collect::>(), - (params.i(), C::F_to_bytes(&share)) + (params.i(), C::F_to_bytes(&secret_share)) )?; // Step 2. Verify each share @@ -203,52 +203,66 @@ fn complete_r2( shares.insert(l, C::F_from_slice(&share).map_err(|_| FrostError::InvalidShare(params.i()))?); } + // Calculate the exponent for a given participant and apply it to a series of commitments + // Initially used with the actual commitments to verify the secret share, later used with stripes + // to generate the verification shares + let exponential = |i: u16, values: &[_]| { + let i = C::F::from(i.into()); + let mut res = Vec::with_capacity(params.t().into()); + (0 .. usize::from(params.t())).into_iter().fold( + C::F::one(), + |exp, l| { + res.push((exp, values[l])); + exp * i + } + ); + res + }; + let mut batch = BatchVerifier::new(shares.len(), C::little_endian()); for (l, share) in &shares { if *l == params.i() { continue; } - let i_scalar = C::F::from(params.i.into()); - let mut exp = C::F::one(); - 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())); + secret_share += share; + // This can be insecurely linearized from n * t to just n using the below sums for a given + // stripe. Doing so uses naive addition which is subject to malleability. The only way to + // ensure that malleability isn't present is to use this n * t algorithm, which runs + // per sender and not as an aggregate of all senders, which also enables blame + let mut values = exponential(params.i, &commitments[l]); + values.push((-*share, C::generator())); batch.queue(rng, *l, values); } batch.verify_with_vartime_blame().map_err(|l| FrostError::InvalidCommitment(l))?; - // TODO: Clear the original share - - let mut secret_share = C::F::zero(); - for (_, share) in shares { - secret_share += share; + // Stripe commitments per t and sum them in advance. Calculating verification shares relies on + // these sums so preprocessing them is a massive speedup + // If these weren't just sums, yet the tables used in multiexp, this would be further optimized + let mut stripes = Vec::with_capacity(usize::from(params.t())); + for t in 0 .. usize::from(params.t()) { + stripes.push(commitments.values().map(|commitments| commitments[t]).sum()); } + // Calculate each user's verification share let mut verification_shares = HashMap::new(); for i in 1 ..= params.n() { - let i_scalar = C::F::from(i.into()); - let mut values = vec![]; - (0 .. params.t()).into_iter().fold(C::F::one(), |exp, j| { - values.push(( - exp, - (1 ..= params.n()).into_iter().map(|l| commitments[&l][usize::from(j)]).sum() - )); - exp * i_scalar - }); - verification_shares.insert(i, multiexp_vartime(values, C::little_endian())); + verification_shares.insert(i, multiexp_vartime(exponential(i, &stripes), C::little_endian())); } debug_assert_eq!(C::generator_table() * secret_share, verification_shares[¶ms.i()]); - let group_key = commitments.iter().map(|(_, commitments)| commitments[0]).sum(); - // TODO: Clear serialized and shares - Ok(MultisigKeys { params, secret_share, group_key, verification_shares, offset: None } ) + Ok( + MultisigKeys { + params, + secret_share, + group_key: stripes[0], + verification_shares, + offset: None + } + ) } /// State of a Key Generation machine diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 9b35b935..1c652b12 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -4,7 +4,6 @@ use std::{rc::Rc, collections::HashMap}; use rand_core::{RngCore, CryptoRng}; use ff::Field; -use group::Group; use transcript::Transcript; @@ -99,7 +98,8 @@ fn preprocess>( #[allow(non_snake_case)] struct Package { - Ris: HashMap, + B: HashMap, + binding: C::F, R: C::G, share: Vec } @@ -170,16 +170,9 @@ fn sign_with_share>( } #[allow(non_snake_case)] - let mut Ris = HashMap::with_capacity(params.view.included.len()); - #[allow(non_snake_case)] - let mut R = C::G::identity(); - for l in ¶ms.view.included { - #[allow(non_snake_case)] - let this_R = B[l][0] + (B[l][1] * binding); - Ris.insert(*l, this_R); - R += this_R; - } - + let R = { + B.values().map(|B| B[0]).sum::() + (B.values().map(|B| B[1]).sum::() * binding) + }; let share = C::F_to_bytes( ¶ms.algorithm.sign_share( ¶ms.view, @@ -190,7 +183,7 @@ fn sign_with_share>( ) ); - Ok((Package { Ris, R, share: share.clone() }, share)) + Ok((Package { B, binding, R, share: share.clone() }, share)) } // This doesn't check the signing set is as expected and unexpected changes can cause false blames @@ -220,11 +213,12 @@ fn complete>( return Ok(res); } - // Find out who misbehaved + // Find out who misbehaved. It may be beneficial to randomly sort this to have detection be + // within n / 2 on average, and not gameable to n, though that should be minor for l in &sign_params.view.included { if !sign_params.algorithm.verify_share( sign_params.view.verification_share(*l), - sign.Ris[l], + sign.B[l][0] + (sign.B[l][1] * sign.binding), responses[l] ) { Err(FrostError::InvalidShare(*l))?;