From af86b7a499af4e1cb28f578af33ab4c8528c8b90 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 8 Dec 2022 19:04:35 -0500 Subject: [PATCH] Support caching preprocesses in FROST (#190) * Remove the explicit included participants from FROST Now, whoever submits preprocesses becomes the signing set. Better separates preprocess from sign, at the cost of slightly more annoying integrations (Monero needs to now independently lagrange/offset its key images). * Support caching preprocesses Closes https://github.com/serai-dex/serai/issues/40. I *could* have added a serialization trait to Algorithm and written a ton of data to disk, while requiring Algorithm implementors also accept such work. Instead, I moved preprocess to a seeded RNG (Chacha20) which should be as secure as the regular RNG. Rebuilding from cache simply loads the previously used Chacha seed, making the Algorithm oblivious to the fact it's being rebuilt from a cache. This removes any requirements for it to be modified while guaranteeing equivalency. This builds on the last commit which delayed determining the signing set till post-preprocess acquisition. Unfortunately, that commit did force preprocess from ThresholdView to ThresholdKeys which had visible effects on Monero. Serai will actually need delayed set determination for #163, and overall, it remains better, hence it's inclusion. * Document FROST preprocess caching * Update ethereum to new FROST * Fix bug in Monero offset calculation and update processor --- Cargo.lock | 1 + coins/ethereum/tests/contract.rs | 5 +- coins/ethereum/tests/crypto.rs | 8 +- coins/monero/src/ringct/clsag/mod.rs | 2 + coins/monero/src/ringct/clsag/multisig.rs | 36 +++- coins/monero/src/tests/clsag.rs | 40 ++--- coins/monero/src/wallet/send/multisig.rs | 78 ++++++--- coins/monero/tests/runner.rs | 3 +- crypto/dkg/src/lib.rs | 14 +- crypto/frost/Cargo.toml | 1 + crypto/frost/src/algorithm.rs | 6 +- crypto/frost/src/lib.rs | 2 +- crypto/frost/src/sign.rs | 194 +++++++++++++--------- crypto/frost/src/tests/mod.rs | 55 +++++- crypto/frost/src/tests/vectors.rs | 23 +-- docs/cryptography/FROST.md | 19 +++ processor/src/coin/mod.rs | 1 - processor/src/coin/monero.rs | 3 - processor/src/tests/mod.rs | 6 +- processor/src/wallet.rs | 4 +- 20 files changed, 339 insertions(+), 162 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a4e7f8a..2ab0a408 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4631,6 +4631,7 @@ dependencies = [ "hkdf", "minimal-ed448", "multiexp", + "rand_chacha 0.3.1", "rand_core 0.6.4", "schnorr-signatures", "serde_json", diff --git a/coins/ethereum/tests/contract.rs b/coins/ethereum/tests/contract.rs index 852fa402..91cf6e3c 100644 --- a/coins/ethereum/tests/contract.rs +++ b/coins/ethereum/tests/contract.rs @@ -51,9 +51,12 @@ async fn test_ecrecover_hack() { let full_message = &[chain_id.to_be_byte_array().as_slice(), &hashed_message].concat(); + let algo = Algo::::new(); let sig = sign( &mut OsRng, - algorithm_machines(&mut OsRng, Algo::::new(), &keys), + algo.clone(), + keys.clone(), + algorithm_machines(&mut OsRng, algo, &keys), full_message, ); let mut processed_sig = diff --git a/coins/ethereum/tests/crypto.rs b/coins/ethereum/tests/crypto.rs index b17857d2..713e2e87 100644 --- a/coins/ethereum/tests/crypto.rs +++ b/coins/ethereum/tests/crypto.rs @@ -40,8 +40,11 @@ fn test_signing() { const MESSAGE: &'static [u8] = b"Hello, World!"; + let algo = Schnorr::::new(); let _sig = sign( &mut OsRng, + algo.clone(), + keys.clone(), algorithm_machines(&mut OsRng, Schnorr::::new(), &keys), MESSAGE, ); @@ -67,9 +70,12 @@ fn test_ecrecover_hack() { let full_message = &[chain_id.to_be_byte_array().as_slice(), &hashed_message].concat(); + let algo = Schnorr::::new(); let sig = sign( &mut OsRng, - algorithm_machines(&mut OsRng, Schnorr::::new(), &keys), + algo.clone(), + keys.clone(), + algorithm_machines(&mut OsRng, algo, &keys), full_message, ); diff --git a/coins/monero/src/ringct/clsag/mod.rs b/coins/monero/src/ringct/clsag/mod.rs index 1ba57d02..f0b7f2b4 100644 --- a/coins/monero/src/ringct/clsag/mod.rs +++ b/coins/monero/src/ringct/clsag/mod.rs @@ -25,6 +25,8 @@ use crate::{ mod multisig; #[cfg(feature = "multisig")] pub use multisig::{ClsagDetails, ClsagAddendum, ClsagMultisig}; +#[cfg(feature = "multisig")] +pub(crate) use multisig::add_key_image_share; lazy_static! { static ref INV_EIGHT: Scalar = Scalar::from(8u8).invert(); diff --git a/coins/monero/src/ringct/clsag/multisig.rs b/coins/monero/src/ringct/clsag/multisig.rs index 8881e418..10734bf3 100644 --- a/coins/monero/src/ringct/clsag/multisig.rs +++ b/coins/monero/src/ringct/clsag/multisig.rs @@ -22,8 +22,9 @@ use transcript::{Transcript, RecommendedTranscript}; use dalek_ff_group as dfg; use dleq::DLEqProof; use frost::{ + dkg::lagrange, curve::Ed25519, - FrostError, ThresholdView, + FrostError, ThresholdKeys, ThresholdView, algorithm::{WriteAddendum, Algorithm}, }; @@ -103,7 +104,7 @@ struct Interim { pub struct ClsagMultisig { transcript: RecommendedTranscript, - H: EdwardsPoint, + pub(crate) H: EdwardsPoint, // Merged here as CLSAG needs it, passing it would be a mess, yet having it beforehand requires // an extra round image: EdwardsPoint, @@ -142,6 +143,20 @@ impl ClsagMultisig { } } +pub(crate) fn add_key_image_share( + image: &mut EdwardsPoint, + generator: EdwardsPoint, + offset: Scalar, + included: &[u16], + participant: u16, + share: EdwardsPoint, +) { + if image.is_identity() { + *image = generator * offset; + } + *image += share * lagrange::(participant, included).0; +} + impl Algorithm for ClsagMultisig { type Transcript = RecommendedTranscript; type Addendum = ClsagAddendum; @@ -154,10 +169,10 @@ impl Algorithm for ClsagMultisig { fn preprocess_addendum( &mut self, rng: &mut R, - view: &ThresholdView, + keys: &ThresholdKeys, ) -> ClsagAddendum { ClsagAddendum { - key_image: dfg::EdwardsPoint(self.H) * view.secret_share().deref(), + key_image: dfg::EdwardsPoint(self.H) * keys.secret_share().deref(), dleq: DLEqProof::prove( rng, // Doesn't take in a larger transcript object due to the usage of this @@ -167,7 +182,7 @@ impl Algorithm for ClsagMultisig { // try to merge later in some form, when it should instead just merge xH (as it does) &mut dleq_transcript(), &[dfg::EdwardsPoint::generator(), dfg::EdwardsPoint(self.H)], - view.secret_share(), + keys.secret_share(), ), } } @@ -205,12 +220,19 @@ impl Algorithm for ClsagMultisig { .verify( &mut dleq_transcript(), &[dfg::EdwardsPoint::generator(), dfg::EdwardsPoint(self.H)], - &[view.verification_share(l), addendum.key_image], + &[view.original_verification_share(l), addendum.key_image], ) .map_err(|_| FrostError::InvalidPreprocess(l))?; self.transcript.append_message(b"key_image_share", addendum.key_image.compress().to_bytes()); - self.image += addendum.key_image.0; + add_key_image_share( + &mut self.image, + self.H, + view.offset().0, + &view.included(), + l, + addendum.key_image.0, + ); Ok(()) } diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/src/tests/clsag.rs index 90138d9c..d0e458cc 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/src/tests/clsag.rs @@ -101,28 +101,28 @@ fn clsag_multisig() { } let mask_sum = random_scalar(&mut OsRng); + let algorithm = ClsagMultisig::new( + RecommendedTranscript::new(b"Monero Serai CLSAG Test"), + keys[&1].group_key().0, + Arc::new(RwLock::new(Some(ClsagDetails::new( + ClsagInput::new( + Commitment::new(randomness, AMOUNT), + Decoys { + i: RING_INDEX, + offsets: (1 ..= RING_LEN).into_iter().collect(), + ring: ring.clone(), + }, + ) + .unwrap(), + mask_sum, + )))), + ); + sign( &mut OsRng, - algorithm_machines( - &mut OsRng, - ClsagMultisig::new( - RecommendedTranscript::new(b"Monero Serai CLSAG Test"), - keys[&1].group_key().0, - Arc::new(RwLock::new(Some(ClsagDetails::new( - ClsagInput::new( - Commitment::new(randomness, AMOUNT), - Decoys { - i: RING_INDEX, - offsets: (1 ..= RING_LEN).into_iter().collect(), - ring: ring.clone(), - }, - ) - .unwrap(), - mask_sum, - )))), - ), - &keys, - ), + algorithm.clone(), + keys.clone(), + algorithm_machines(&mut OsRng, algorithm, &keys), &[1; 32], ); } diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index a324fb3f..2e8d8030 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -4,25 +4,28 @@ use std::{ collections::HashMap, }; +use zeroize::Zeroizing; use rand_core::{RngCore, CryptoRng, SeedableRng}; use rand_chacha::ChaCha20Rng; +use group::ff::Field; use curve25519_dalek::{traits::Identity, scalar::Scalar, edwards::EdwardsPoint}; +use dalek_ff_group as dfg; use transcript::{Transcript, RecommendedTranscript}; use frost::{ curve::Ed25519, FrostError, ThresholdKeys, sign::{ - Writable, Preprocess, SignatureShare, PreprocessMachine, SignMachine, SignatureMachine, - AlgorithmMachine, AlgorithmSignMachine, AlgorithmSignatureMachine, + Writable, Preprocess, CachedPreprocess, SignatureShare, PreprocessMachine, SignMachine, + SignatureMachine, AlgorithmMachine, AlgorithmSignMachine, AlgorithmSignatureMachine, }, }; use crate::{ random_scalar, ringct::{ - clsag::{ClsagInput, ClsagDetails, ClsagAddendum, ClsagMultisig}, + clsag::{ClsagInput, ClsagDetails, ClsagAddendum, ClsagMultisig, add_key_image_share}, RctPrunable, }, transaction::{Input, Transaction}, @@ -34,11 +37,12 @@ use crate::{ pub struct TransactionMachine { signable: SignableTransaction, i: u16, - included: Vec, transcript: RecommendedTranscript, decoys: Vec, + // Hashed key and scalar offset + key_images: Vec<(EdwardsPoint, Scalar)>, inputs: Vec>>>, clsags: Vec>, } @@ -46,11 +50,11 @@ pub struct TransactionMachine { pub struct TransactionSignMachine { signable: SignableTransaction, i: u16, - included: Vec, transcript: RecommendedTranscript, decoys: Vec, + key_images: Vec<(EdwardsPoint, Scalar)>, inputs: Vec>>>, clsags: Vec>, @@ -71,7 +75,6 @@ impl SignableTransaction { keys: ThresholdKeys, mut transcript: RecommendedTranscript, height: usize, - mut included: Vec, ) -> Result { let mut inputs = vec![]; for _ in 0 .. self.inputs.len() { @@ -110,24 +113,20 @@ impl SignableTransaction { transcript.append_message(b"payment_amount", payment.1.to_le_bytes()); } - // Sort included before cloning it around - included.sort_unstable(); - + let mut key_images = vec![]; for (i, input) in self.inputs.iter().enumerate() { // Check this the right set of keys - let offset = keys.offset(dalek_ff_group::Scalar(input.key_offset())); + let offset = keys.offset(dfg::Scalar(input.key_offset())); if offset.group_key().0 != input.key() { Err(TransactionError::WrongPrivateKey)?; } - clsags.push( - AlgorithmMachine::new( - ClsagMultisig::new(transcript.clone(), input.key(), inputs[i].clone()), - offset, - &included, - ) - .map_err(TransactionError::FrostError)?, - ); + let clsag = ClsagMultisig::new(transcript.clone(), input.key(), inputs[i].clone()); + key_images.push(( + clsag.H, + keys.current_offset().unwrap_or(dfg::Scalar::zero()).0 + self.inputs[i].key_offset(), + )); + clsags.push(AlgorithmMachine::new(clsag, offset).map_err(TransactionError::FrostError)?); } // Select decoys @@ -150,11 +149,11 @@ impl SignableTransaction { Ok(TransactionMachine { signable: self, i: keys.params().i(), - included, transcript, decoys, + key_images, inputs, clsags, }) @@ -196,11 +195,11 @@ impl PreprocessMachine for TransactionMachine { TransactionSignMachine { signable: self.signable, i: self.i, - included: self.included, transcript: self.transcript, decoys: self.decoys, + key_images: self.key_images, inputs: self.inputs, clsags, @@ -212,10 +211,30 @@ impl PreprocessMachine for TransactionMachine { } impl SignMachine for TransactionSignMachine { + type Params = (); + type Keys = ThresholdKeys; type Preprocess = Vec>; type SignatureShare = Vec>; type SignatureMachine = TransactionSignatureMachine; + fn cache(self) -> Zeroizing { + unimplemented!( + "Monero transactions don't support caching their preprocesses due to {}", + "being already bound to a specific transaction" + ); + } + + fn from_cache( + _: (), + _: ThresholdKeys, + _: Zeroizing, + ) -> Result { + unimplemented!( + "Monero transactions don't support caching their preprocesses due to {}", + "being already bound to a specific transaction" + ); + } + fn read_preprocess(&self, reader: &mut R) -> io::Result { self.clsags.iter().map(|clsag| clsag.read_preprocess(reader)).collect() } @@ -231,12 +250,18 @@ impl SignMachine for TransactionSignMachine { ))?; } + // Find out who's included + // This may not be a valid set of signers yet the algorithm machine will error if it's not + commitments.remove(&self.i); // Remove, if it was included for some reason + let mut included = commitments.keys().into_iter().cloned().collect::>(); + included.push(self.i); + included.sort_unstable(); + // Convert the unified commitments to a Vec of the individual commitments let mut images = vec![EdwardsPoint::identity(); self.clsags.len()]; let mut commitments = (0 .. self.clsags.len()) .map(|c| { - self - .included + included .iter() .map(|l| { // Add all commitments to the transcript for their entropy @@ -262,7 +287,14 @@ impl SignMachine for TransactionSignMachine { // provides the easiest API overall, as this is where the TX is (which needs the key // images in its message), along with where the outputs are determined (where our // outputs may need these in order to guarantee uniqueness) - images[c] += preprocess.addendum.key_image.0; + add_key_image_share( + &mut images[c], + self.key_images[c].0, + self.key_images[c].1, + &included, + *l, + preprocess.addendum.key_image.0, + ); Ok((*l, preprocess)) }) diff --git a/coins/monero/tests/runner.rs b/coins/monero/tests/runner.rs index aed55edd..db0970cf 100644 --- a/coins/monero/tests/runner.rs +++ b/coins/monero/tests/runner.rs @@ -216,14 +216,13 @@ macro_rules! test { keys[&i].clone(), RecommendedTranscript::new(b"Monero Serai Test Transaction"), rpc.get_height().await.unwrap() - 10, - (1 ..= THRESHOLD).collect::>(), ) .await .unwrap(), ); } - frost::tests::sign(&mut OsRng, machines, &vec![]) + frost::tests::sign_without_caching(&mut OsRng, machines, &vec![]) } } } diff --git a/crypto/dkg/src/lib.rs b/crypto/dkg/src/lib.rs index 48fb4c56..99d9e0c3 100644 --- a/crypto/dkg/src/lib.rs +++ b/crypto/dkg/src/lib.rs @@ -284,11 +284,13 @@ pub struct ThresholdKeys { /// View of keys passed to algorithm implementations. #[derive(Clone, Zeroize)] pub struct ThresholdView { + offset: C::F, group_key: C::G, - #[zeroize(skip)] included: Vec, secret_share: Zeroizing, #[zeroize(skip)] + original_verification_shares: HashMap, + #[zeroize(skip)] verification_shares: HashMap, } @@ -347,10 +349,12 @@ impl ThresholdKeys { let offset_verification_share = C::generator() * offset_share; Ok(ThresholdView { + offset: self.offset.unwrap_or_else(C::F::zero), group_key: self.group_key(), secret_share: Zeroizing::new( (lagrange::(self.params().i, included) * self.secret_share().deref()) + offset_share, ), + original_verification_shares: self.verification_shares(), verification_shares: self .verification_shares() .iter() @@ -364,6 +368,10 @@ impl ThresholdKeys { } impl ThresholdView { + pub fn offset(&self) -> C::F { + self.offset + } + pub fn group_key(&self) -> C::G { self.group_key } @@ -376,6 +384,10 @@ impl ThresholdView { &self.secret_share } + pub fn original_verification_share(&self, l: u16) -> C::G { + self.original_verification_shares[&l] + } + pub fn verification_share(&self, l: u16) -> C::G { self.verification_shares[&l] } diff --git a/crypto/frost/Cargo.toml b/crypto/frost/Cargo.toml index e594cc99..91eaa91d 100644 --- a/crypto/frost/Cargo.toml +++ b/crypto/frost/Cargo.toml @@ -16,6 +16,7 @@ rustdoc-args = ["--cfg", "docsrs"] thiserror = "1" rand_core = "0.6" +rand_chacha = "0.3" zeroize = { version = "1.5", features = ["zeroize_derive"] } subtle = "2" diff --git a/crypto/frost/src/algorithm.rs b/crypto/frost/src/algorithm.rs index 6a0a2ae1..5f96ae31 100644 --- a/crypto/frost/src/algorithm.rs +++ b/crypto/frost/src/algorithm.rs @@ -6,7 +6,7 @@ use rand_core::{RngCore, CryptoRng}; use transcript::Transcript; -use crate::{Curve, FrostError, ThresholdView}; +use crate::{Curve, FrostError, ThresholdKeys, ThresholdView}; pub use schnorr::SchnorrSignature; /// Write an addendum to a writer. @@ -45,7 +45,7 @@ pub trait Algorithm: Clone { fn preprocess_addendum( &mut self, rng: &mut R, - params: &ThresholdView, + keys: &ThresholdKeys, ) -> Self::Addendum; /// Read an addendum from a reader. @@ -148,7 +148,7 @@ impl> Algorithm for Schnorr { vec![vec![C::generator()]] } - fn preprocess_addendum(&mut self, _: &mut R, _: &ThresholdView) {} + fn preprocess_addendum(&mut self, _: &mut R, _: &ThresholdKeys) {} fn read_addendum(&self, _: &mut R) -> io::Result { Ok(()) diff --git a/crypto/frost/src/lib.rs b/crypto/frost/src/lib.rs index 460fb8cc..81518b24 100644 --- a/crypto/frost/src/lib.rs +++ b/crypto/frost/src/lib.rs @@ -36,7 +36,7 @@ pub mod sign; pub mod tests; // Validate a map of values to have the expected included participants -pub(crate) fn validate_map( +pub fn validate_map( map: &HashMap, included: &[u16], ours: u16, diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 5c70a399..1641f1d8 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -4,9 +4,10 @@ use std::{ collections::HashMap, }; -use rand_core::{RngCore, CryptoRng}; +use rand_core::{RngCore, CryptoRng, SeedableRng}; +use rand_chacha::ChaCha20Rng; -use zeroize::Zeroize; +use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; use transcript::Transcript; @@ -47,54 +48,16 @@ pub struct Params> { #[zeroize(skip)] algorithm: A, keys: ThresholdKeys, - view: ThresholdView, } impl> Params { - pub fn new( - algorithm: A, - keys: ThresholdKeys, - included: &[u16], - ) -> Result, FrostError> { - let params = keys.params(); - - let mut included = included.to_vec(); - included.sort_unstable(); - - // Included < threshold - if included.len() < usize::from(params.t()) { - Err(FrostError::InvalidSigningSet("not enough signers"))?; - } - // Invalid index - if included[0] == 0 { - Err(FrostError::InvalidParticipantIndex(included[0], params.n()))?; - } - // OOB index - if included[included.len() - 1] > params.n() { - Err(FrostError::InvalidParticipantIndex(included[included.len() - 1], params.n()))?; - } - // Same signer included multiple times - for i in 0 .. (included.len() - 1) { - if included[i] == included[i + 1] { - Err(FrostError::DuplicatedIndex(included[i]))?; - } - } - // Not included - if !included.contains(¶ms.i()) { - Err(FrostError::InvalidSigningSet("signing despite not being included"))?; - } - - // Out of order arguments to prevent additional cloning - Ok(Params { algorithm, view: keys.view(&included).unwrap(), keys }) + pub fn new(algorithm: A, keys: ThresholdKeys) -> Result, FrostError> { + Ok(Params { algorithm, keys }) } pub fn multisig_params(&self) -> ThresholdParams { self.keys.params() } - - pub fn view(&self) -> ThresholdView { - self.view.clone() - } } /// Preprocess for an instance of the FROST signing protocol. @@ -111,6 +74,12 @@ impl Writable for Preprocess { } } +/// A cached preprocess. A preprocess MUST only be used once. Reuse will enable third-party +/// recovery of your private key share. Additionally, this MUST be handled with the same security +/// as your private key share, as knowledge of it also enables recovery. +#[derive(Zeroize, ZeroizeOnDrop)] +pub struct CachedPreprocess(pub [u8; 32]); + /// Trait for the initial state machine of a two-round signing protocol. pub trait PreprocessMachine { /// Preprocess message for this machine. @@ -134,12 +103,26 @@ pub struct AlgorithmMachine> { impl> AlgorithmMachine { /// Creates a new machine to generate a signature with the specified keys. - pub fn new( - algorithm: A, - keys: ThresholdKeys, - included: &[u16], - ) -> Result, FrostError> { - Ok(AlgorithmMachine { params: Params::new(algorithm, keys, included)? }) + pub fn new(algorithm: A, keys: ThresholdKeys) -> Result, FrostError> { + Ok(AlgorithmMachine { params: Params::new(algorithm, keys)? }) + } + + fn seeded_preprocess( + self, + seed: Zeroizing, + ) -> (AlgorithmSignMachine, Preprocess) { + let mut params = self.params; + + let mut rng = ChaCha20Rng::from_seed(seed.0); + let (nonces, commitments) = Commitments::new::<_, A::Transcript>( + &mut rng, + params.keys.secret_share(), + ¶ms.algorithm.nonces(), + ); + let addendum = params.algorithm.preprocess_addendum(&mut rng, ¶ms.keys); + + let preprocess = Preprocess { commitments, addendum }; + (AlgorithmSignMachine { params, seed, nonces, preprocess: preprocess.clone() }, preprocess) } #[cfg(any(test, feature = "tests"))] @@ -148,7 +131,12 @@ impl> AlgorithmMachine { nonces: Vec>, preprocess: Preprocess, ) -> AlgorithmSignMachine { - AlgorithmSignMachine { params: self.params, nonces, preprocess } + AlgorithmSignMachine { + params: self.params, + seed: Zeroizing::new(CachedPreprocess([0; 32])), + nonces, + preprocess, + } } } @@ -161,17 +149,9 @@ impl> PreprocessMachine for AlgorithmMachine { self, rng: &mut R, ) -> (Self::SignMachine, Preprocess) { - let mut params = self.params; - - let (nonces, commitments) = Commitments::new::<_, A::Transcript>( - &mut *rng, - params.view().secret_share(), - ¶ms.algorithm.nonces(), - ); - let addendum = params.algorithm.preprocess_addendum(rng, ¶ms.view); - - let preprocess = Preprocess { commitments, addendum }; - (AlgorithmSignMachine { params, nonces, preprocess: preprocess.clone() }, preprocess) + let mut seed = Zeroizing::new(CachedPreprocess([0; 32])); + rng.fill_bytes(seed.0.as_mut()); + self.seeded_preprocess(seed) } } @@ -185,7 +165,11 @@ impl Writable for SignatureShare { } /// Trait for the second machine of a two-round signing protocol. -pub trait SignMachine { +pub trait SignMachine: Sized { + /// Params used to instantiate this machine which can be used to rebuild from a cache. + type Params: Clone; + /// Keys used for signing operations. + type Keys; /// Preprocess message for this machine. type Preprocess: Clone + PartialEq + Writable; /// SignatureShare message for this machine. @@ -193,12 +177,28 @@ pub trait SignMachine { /// SignatureMachine this SignMachine turns into. type SignatureMachine: SignatureMachine; - /// Read a Preprocess message. + /// Cache this preprocess for usage later. This cached preprocess MUST only be used once. Reuse + /// of it enables recovery of your private key share. Third-party recovery of a cached preprocess + /// also enables recovery of your private key share, so this MUST be treated with the same + /// security as your private key share. + fn cache(self) -> Zeroizing; + + /// Create a sign machine from a cached preprocess. After this, the preprocess should be fully + /// deleted, as it must never be reused. It is + fn from_cache( + params: Self::Params, + keys: Self::Keys, + cache: Zeroizing, + ) -> Result; + + /// Read a Preprocess message. Despite taking self, this does not save the preprocess. + /// It must be externally cached and passed into sign. fn read_preprocess(&self, reader: &mut R) -> io::Result; /// Sign a message. /// Takes in the participants' preprocess messages. Returns the signature share to be broadcast - /// to all participants, over an authenticated channel. + /// to all participants, over an authenticated channel. The parties who participate here will + /// become the signing set for this session. fn sign( self, commitments: HashMap, @@ -210,16 +210,33 @@ pub trait SignMachine { #[derive(Zeroize)] pub struct AlgorithmSignMachine> { params: Params, + seed: Zeroizing, + pub(crate) nonces: Vec>, #[zeroize(skip)] pub(crate) preprocess: Preprocess, } impl> SignMachine for AlgorithmSignMachine { + type Params = A; + type Keys = ThresholdKeys; type Preprocess = Preprocess; type SignatureShare = SignatureShare; type SignatureMachine = AlgorithmSignatureMachine; + fn cache(self) -> Zeroizing { + self.seed + } + + fn from_cache( + algorithm: A, + keys: ThresholdKeys, + cache: Zeroizing, + ) -> Result { + let (machine, _) = AlgorithmMachine::new(algorithm, keys)?.seeded_preprocess(cache); + Ok(machine) + } + fn read_preprocess(&self, reader: &mut R) -> io::Result { Ok(Preprocess { commitments: Commitments::read::<_, A::Transcript>(reader, &self.params.algorithm.nonces())?, @@ -233,7 +250,35 @@ impl> SignMachine for AlgorithmSignMachi msg: &[u8], ) -> Result<(Self::SignatureMachine, SignatureShare), FrostError> { let multisig_params = self.params.multisig_params(); - validate_map(&preprocesses, &self.params.view.included(), multisig_params.i())?; + + let mut included = Vec::with_capacity(preprocesses.len() + 1); + included.push(multisig_params.i()); + for l in preprocesses.keys() { + included.push(*l); + } + included.sort_unstable(); + + // Included < threshold + if included.len() < usize::from(multisig_params.t()) { + Err(FrostError::InvalidSigningSet("not enough signers"))?; + } + // Invalid index + if included[0] == 0 { + Err(FrostError::InvalidParticipantIndex(included[0], multisig_params.n()))?; + } + // OOB index + if included[included.len() - 1] > multisig_params.n() { + Err(FrostError::InvalidParticipantIndex(included[included.len() - 1], multisig_params.n()))?; + } + // Same signer included multiple times + for i in 0 .. (included.len() - 1) { + if included[i] == included[i + 1] { + Err(FrostError::DuplicatedIndex(included[i]))?; + } + } + + let view = self.params.keys.view(&included).unwrap(); + validate_map(&preprocesses, &included, multisig_params.i())?; { // Domain separate FROST @@ -242,10 +287,10 @@ impl> SignMachine for AlgorithmSignMachi let nonces = self.params.algorithm.nonces(); #[allow(non_snake_case)] - let mut B = BindingFactor(HashMap::::with_capacity(self.params.view.included().len())); + let mut B = BindingFactor(HashMap::::with_capacity(included.len())); { // Parse the preprocesses - for l in &self.params.view.included() { + for l in &included { { self .params @@ -266,7 +311,7 @@ impl> SignMachine for AlgorithmSignMachi } B.insert(*l, commitments); - self.params.algorithm.process_addendum(&self.params.view, *l, addendum)?; + self.params.algorithm.process_addendum(&view, *l, addendum)?; } else { let preprocess = preprocesses.remove(l).unwrap(); preprocess.commitments.transcript(self.params.algorithm.transcript()); @@ -277,7 +322,7 @@ impl> SignMachine for AlgorithmSignMachi } B.insert(*l, preprocess.commitments); - self.params.algorithm.process_addendum(&self.params.view, *l, preprocess.addendum)?; + self.params.algorithm.process_addendum(&view, *l, preprocess.addendum)?; } } @@ -333,10 +378,10 @@ impl> SignMachine for AlgorithmSignMachi }) .collect::>(); - let share = self.params.algorithm.sign_share(&self.params.view, &Rs, nonces, msg); + let share = self.params.algorithm.sign_share(&view, &Rs, nonces, msg); Ok(( - AlgorithmSignatureMachine { params: self.params.clone(), B, Rs, share }, + AlgorithmSignatureMachine { params: self.params.clone(), view, B, Rs, share }, SignatureShare(share), )) } @@ -359,6 +404,7 @@ pub trait SignatureMachine { #[allow(non_snake_case)] pub struct AlgorithmSignatureMachine> { params: Params, + view: ThresholdView, B: BindingFactor, Rs: Vec>, share: C::F, @@ -376,7 +422,7 @@ impl> SignatureMachine for AlgorithmSign mut shares: HashMap>, ) -> Result { let params = self.params.multisig_params(); - validate_map(&shares, &self.params.view.included(), params.i())?; + validate_map(&shares, &self.view.included(), params.i())?; let mut responses = HashMap::new(); responses.insert(params.i(), self.share); @@ -389,16 +435,16 @@ impl> SignatureMachine for AlgorithmSign // Perform signature validation instead of individual share validation // For the success route, which should be much more frequent, this should be faster // It also acts as an integrity check of this library's signing function - if let Some(sig) = self.params.algorithm.verify(self.params.view.group_key(), &self.Rs, sum) { + if let Some(sig) = self.params.algorithm.verify(self.view.group_key(), &self.Rs, sum) { return Ok(sig); } // 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 // TODO - for l in &self.params.view.included() { + for l in &self.view.included() { if !self.params.algorithm.verify_share( - self.params.view.verification_share(*l), + self.view.verification_share(*l), &self.B.bound(*l), responses[l], ) { diff --git a/crypto/frost/src/tests/mod.rs b/crypto/frost/src/tests/mod.rs index 81bb9f8f..927864fa 100644 --- a/crypto/frost/src/tests/mod.rs +++ b/crypto/frost/src/tests/mod.rs @@ -53,10 +53,7 @@ pub fn algorithm_machines>( .iter() .filter_map(|(i, keys)| { if included.contains(i) { - Some(( - *i, - AlgorithmMachine::new(algorithm.clone(), keys.clone(), &included.clone()).unwrap(), - )) + Some((*i, AlgorithmMachine::new(algorithm.clone(), keys.clone()).unwrap())) } else { None } @@ -64,10 +61,14 @@ pub fn algorithm_machines>( .collect() } -/// Execute the signing protocol. -pub fn sign( +fn sign_internal< + R: RngCore + CryptoRng, + M: PreprocessMachine, + F: FnMut(&mut R, &mut HashMap), +>( rng: &mut R, mut machines: HashMap, + mut cache: F, msg: &[u8], ) -> M::Signature { let mut commitments = HashMap::new(); @@ -84,6 +85,8 @@ pub fn sign( }) .collect::>(); + cache(rng, &mut machines); + let mut shares = HashMap::new(); let mut machines = machines .drain() @@ -108,3 +111,43 @@ pub fn sign( } signature.unwrap() } + +/// Execute the signing protocol, without caching any machines. This isn't as comprehensive at +/// testing as sign, and accordingly isn't preferred, yet is usable for machines not supporting +/// caching. +pub fn sign_without_caching( + rng: &mut R, + machines: HashMap, + msg: &[u8], +) -> M::Signature { + sign_internal(rng, machines, |_, _| {}, msg) +} + +/// Execute the signing protocol, randomly caching various machines to ensure they can cache +/// successfully. +pub fn sign( + rng: &mut R, + params: >::Params, + mut keys: HashMap>::Keys>, + machines: HashMap, + msg: &[u8], +) -> M::Signature { + sign_internal( + rng, + machines, + |rng, machines| { + // Cache and rebuild half of the machines + let mut included = machines.keys().into_iter().cloned().collect::>(); + for i in included.drain(..) { + if (rng.next_u64() % 2) == 0 { + let cache = machines.remove(&i).unwrap().cache(); + machines.insert( + i, + M::SignMachine::from_cache(params.clone(), keys.remove(&i).unwrap(), cache).unwrap(), + ); + } + } + }, + msg, + ) +} diff --git a/crypto/frost/src/tests/vectors.rs b/crypto/frost/src/tests/vectors.rs index f9a6ed23..f81ec7cc 100644 --- a/crypto/frost/src/tests/vectors.rs +++ b/crypto/frost/src/tests/vectors.rs @@ -9,7 +9,7 @@ use rand_core::{RngCore, CryptoRng}; use group::{ff::PrimeField, GroupEncoding}; -use dkg::tests::{test_ciphersuite as test_dkg}; +use dkg::tests::{key_gen, test_ciphersuite as test_dkg}; use crate::{ curve::Curve, @@ -19,7 +19,7 @@ use crate::{ Nonce, GeneratorCommitments, NonceCommitments, Commitments, Writable, Preprocess, SignMachine, SignatureMachine, AlgorithmMachine, }, - tests::{clone_without, recover_key, curve::test_curve}, + tests::{clone_without, recover_key, algorithm_machines, sign, curve::test_curve}, }; pub struct Vectors { @@ -124,6 +124,15 @@ pub fn test_with_vectors>( // Test the DKG test_dkg::<_, C>(&mut *rng); + // Test a basic Schnorr signature + { + let keys = key_gen(&mut *rng); + let machines = algorithm_machines(&mut *rng, Schnorr::::new(), &keys); + const MSG: &[u8] = b"Hello, World!"; + let sig = sign(&mut *rng, Schnorr::::new(), keys.clone(), machines, MSG); + assert!(sig.verify(keys[&1].group_key(), H::hram(&sig.R, &keys[&1].group_key(), MSG))); + } + // Test against the vectors let keys = vectors_to_multisig_keys::(&vectors); let group_key = @@ -135,15 +144,7 @@ pub fn test_with_vectors>( let mut machines = vec![]; for i in &vectors.included { - machines.push(( - i, - AlgorithmMachine::new( - Schnorr::::new(), - keys[i].clone(), - &vectors.included.to_vec().clone(), - ) - .unwrap(), - )); + machines.push((i, AlgorithmMachine::new(Schnorr::::new(), keys[i].clone()).unwrap())); } let mut commitments = HashMap::new(); diff --git a/docs/cryptography/FROST.md b/docs/cryptography/FROST.md index 680e3277..f3f89cee 100644 --- a/docs/cryptography/FROST.md +++ b/docs/cryptography/FROST.md @@ -35,3 +35,22 @@ The public key signed for is also offset by this value. During the signing process, the offset is explicitly transcripted. Then, the offset is divided by `p`, the amount of participating signers, and each signer adds it to their post-interpolation key share. + +# Caching + +modular-frost supports caching a preprocess. This is done by having all +preprocesses use a seeded RNG. Accordingly, the entire preprocess can be derived +from the RNG seed, making the cache just the seed. + +Reusing preprocesses would enable a third-party to recover your private key +share. Accordingly, you MUST not reuse preprocesses. Third-party knowledge of +your preprocess would also enable their recovery of your private key share. +Accordingly, you MUST treat cached preprocesses with the same security as your +private key share. + +Since a reused seed will lead to a reused preprocess, seeded RNGs are generally +frowned upon when doing multisignature operations. This isn't an issue as each +new preprocess obtains a fresh seed from the specified RNG. Assuming the +provided RNG isn't generating the same seed multiple times, the only way for +this seeded RNG to fail is if a preprocess is loaded multiple times, which was +already a failure point. diff --git a/processor/src/coin/mod.rs b/processor/src/coin/mod.rs index bc39bbfd..136df59b 100644 --- a/processor/src/coin/mod.rs +++ b/processor/src/coin/mod.rs @@ -75,7 +75,6 @@ pub trait Coin { async fn attempt_send( &self, transaction: Self::SignableTransaction, - included: &[u16], ) -> Result; async fn publish_transaction( diff --git a/processor/src/coin/monero.rs b/processor/src/coin/monero.rs index ab7120fc..32841179 100644 --- a/processor/src/coin/monero.rs +++ b/processor/src/coin/monero.rs @@ -188,7 +188,6 @@ impl Coin for Monero { async fn attempt_send( &self, transaction: SignableTransaction, - included: &[u16], ) -> Result { transaction .actual @@ -198,7 +197,6 @@ impl Coin for Monero { transaction.keys.clone(), transaction.transcript.clone(), transaction.height, - included.to_vec(), ) .await .map_err(|_| CoinError::ConnectionError) @@ -209,7 +207,6 @@ impl Coin for Monero { tx: &Self::Transaction, ) -> Result<(Vec, Vec<::Id>), CoinError> { self.rpc.publish_transaction(tx).await.map_err(|_| CoinError::ConnectionError)?; - Ok((tx.hash().to_vec(), tx.prefix.outputs.iter().map(|output| output.key.to_bytes()).collect())) } diff --git a/processor/src/tests/mod.rs b/processor/src/tests/mod.rs index 11520a63..8e7fe4a5 100644 --- a/processor/src/tests/mod.rs +++ b/processor/src/tests/mod.rs @@ -98,11 +98,7 @@ async fn test_send(coin: C, fee: C::Fee) { .unwrap() .1 .swap_remove(0); - futures.push(wallet.attempt_send( - network, - signable, - (1 ..= threshold).into_iter().collect::>(), - )); + futures.push(wallet.attempt_send(network, signable)); } println!("{:?}", hex::encode(futures::future::join_all(futures).await.swap_remove(0).unwrap().0)); diff --git a/processor/src/wallet.rs b/processor/src/wallet.rs index ff9687ae..f117e41c 100644 --- a/processor/src/wallet.rs +++ b/processor/src/wallet.rs @@ -339,10 +339,8 @@ impl Wallet { &mut self, network: &mut N, prepared: C::SignableTransaction, - included: Vec, ) -> Result<(Vec, Vec<::Id>), SignError> { - let attempt = - self.coin.attempt_send(prepared, &included).await.map_err(SignError::CoinError)?; + let attempt = self.coin.attempt_send(prepared).await.map_err(SignError::CoinError)?; let (attempt, commitments) = attempt.preprocess(&mut OsRng); let commitments = network