From 797be71eb3ff802534fb0caae6d00a4417d00b68 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Wed, 3 Aug 2022 03:25:18 -0500 Subject: [PATCH] Utilize zeroize (#76) * Apply Zeroize to nonces used in Bulletproofs Also makes bit decomposition constant time for a given amount of outputs. * Fix nonce reuse for single-signer CLSAG * Attach Zeroize to most structures in Monero, and ZOnDrop to anything with private data * Zeroize private keys and nonces * Merge prepare_outputs and prepare_transactions * Ensure CLSAG is constant time * Pass by borrow where needed, bug fixes The past few commitments have been one in-progress chunk which I've broken up as best read. * Add Zeroize to FROST structs Still needs to zeroize internally, yet next step. Not quite as aggressive as Monero, partially due to the limitations of HashMaps, partially due to less concern about metadata, yet does still delete a few smaller items of metadata (group key, context string...). * Remove Zeroize from most Monero multisig structs These structs largely didn't have private data, just fields with private data, yet those fields implemented ZeroizeOnDrop making them already covered. While there is still traces of the transaction left in RAM, fully purging that was never the intent. * Use Zeroize within dleq bitvec doesn't offer Zeroize, so a manual zeroing has been implemented. * Use Zeroize for random_nonce It isn't perfect, due to the inability to zeroize the digest, and due to kp256 requiring a few transformations. It does the best it can though. Does move the per-curve random_nonce to a provided one, which is allowed as of https://github.com/cfrg/draft-irtf-cfrg-frost/pull/231. * Use Zeroize on FROST keygen/signing * Zeroize constant time multiexp. * Correct when FROST keygen zeroizes * Move the FROST keys Arc into FrostKeys Reduces amount of instances in memory. * Manually implement Debug for FrostCore to not leak the secret share * Misc bug fixes * clippy + multiexp test bug fixes * Correct FROST key gen share summation It leaked our own share for ourself. * Fix cross-group DLEq tests --- Cargo.lock | 5 + coins/monero/Cargo.toml | 1 + coins/monero/src/frost.rs | 5 +- coins/monero/src/lib.rs | 6 +- coins/monero/src/ringct/bulletproofs/core.rs | 11 +- coins/monero/src/ringct/bulletproofs/mod.rs | 4 +- .../src/ringct/bulletproofs/original.rs | 41 ++- coins/monero/src/ringct/bulletproofs/plus.rs | 24 +- .../src/ringct/bulletproofs/scalar_vector.rs | 4 +- coins/monero/src/ringct/clsag/mod.rs | 25 +- coins/monero/src/ringct/clsag/multisig.rs | 6 +- coins/monero/src/ringct/mod.rs | 8 +- coins/monero/src/tests/bulletproofs.rs | 8 +- coins/monero/src/tests/clsag.rs | 2 +- coins/monero/src/transaction.rs | 5 +- coins/monero/src/wallet/address.rs | 10 +- coins/monero/src/wallet/decoys.rs | 4 +- coins/monero/src/wallet/mod.rs | 6 +- coins/monero/src/wallet/scan.rs | 9 +- coins/monero/src/wallet/send/mod.rs | 103 ++++--- coins/monero/src/wallet/send/multisig.rs | 19 +- coins/monero/tests/send.rs | 6 +- crypto/dalek-ff-group/Cargo.toml | 1 + crypto/dalek-ff-group/src/field.rs | 3 +- crypto/dalek-ff-group/src/lib.rs | 5 +- crypto/dleq/Cargo.toml | 2 + crypto/dleq/src/cross_group/aos.rs | 22 +- crypto/dleq/src/cross_group/bits.rs | 26 +- crypto/dleq/src/cross_group/mod.rs | 36 ++- crypto/dleq/src/cross_group/scalar.rs | 38 ++- crypto/dleq/src/cross_group/schnorr.rs | 19 +- crypto/dleq/src/lib.rs | 14 +- crypto/dleq/src/tests/cross_group/aos.rs | 2 +- crypto/dleq/src/tests/cross_group/schnorr.rs | 6 +- crypto/frost/Cargo.toml | 3 + crypto/frost/src/curve/dalek.rs | 11 +- crypto/frost/src/curve/kp256.rs | 43 ++- crypto/frost/src/curve/mod.rs | 38 ++- crypto/frost/src/key_gen.rs | 67 +++-- crypto/frost/src/lib.rs | 276 +++++++++++------- crypto/frost/src/schnorr.rs | 11 +- crypto/frost/src/sign.rs | 55 ++-- crypto/frost/src/tests/curve.rs | 10 +- crypto/frost/src/tests/mod.rs | 14 +- crypto/frost/src/tests/schnorr.rs | 6 +- crypto/frost/src/tests/vectors.rs | 10 +- crypto/multiexp/Cargo.toml | 2 + crypto/multiexp/src/batch.rs | 34 ++- crypto/multiexp/src/lib.rs | 14 +- crypto/multiexp/src/pippenger.rs | 5 +- crypto/multiexp/src/straus.rs | 8 +- crypto/multiexp/src/tests/mod.rs | 6 +- processor/src/coin/mod.rs | 4 +- processor/src/coin/monero.rs | 12 +- processor/src/tests/mod.rs | 2 +- processor/src/wallet.rs | 6 +- 56 files changed, 698 insertions(+), 425 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7c935411..501aac7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1376,6 +1376,7 @@ dependencies = [ "group", "rand_core 0.6.3", "subtle", + "zeroize", ] [[package]] @@ -1538,6 +1539,7 @@ dependencies = [ "multiexp", "rand_core 0.6.3", "thiserror", + "zeroize", ] [[package]] @@ -4534,6 +4536,7 @@ dependencies = [ "rand_core 0.6.3", "sha2 0.10.2", "thiserror", + "zeroize", ] [[package]] @@ -4592,6 +4595,7 @@ dependencies = [ "thiserror", "tiny-keccak", "tokio", + "zeroize", ] [[package]] @@ -4638,6 +4642,7 @@ dependencies = [ "group", "k256", "rand_core 0.6.3", + "zeroize", ] [[package]] diff --git a/coins/monero/Cargo.toml b/coins/monero/Cargo.toml index e0213b5a..b6ebd028 100644 --- a/coins/monero/Cargo.toml +++ b/coins/monero/Cargo.toml @@ -16,6 +16,7 @@ rand_chacha = { version = "0.3", optional = true } rand = "0.8" rand_distr = "0.4" +zeroize = { version = "1.3", features = ["zeroize_derive"] } subtle = "2.4" tiny-keccak = { version = "2", features = ["keccak"] } diff --git a/coins/monero/src/frost.rs b/coins/monero/src/frost.rs index d2b78576..0453e28c 100644 --- a/coins/monero/src/frost.rs +++ b/coins/monero/src/frost.rs @@ -3,6 +3,8 @@ use std::io::Read; use thiserror::Error; use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use curve25519_dalek::{scalar::Scalar, edwards::EdwardsPoint}; use group::{Group, GroupEncoding}; @@ -29,7 +31,7 @@ fn transcript() -> RecommendedTranscript { pub(crate) fn write_dleq( rng: &mut R, H: EdwardsPoint, - x: Scalar, + mut x: Scalar, ) -> Vec { let mut res = Vec::with_capacity(64); DLEqProof::prove( @@ -45,6 +47,7 @@ pub(crate) fn write_dleq( ) .serialize(&mut res) .unwrap(); + x.zeroize(); res } diff --git a/coins/monero/src/lib.rs b/coins/monero/src/lib.rs index 952eff5d..0b22db0c 100644 --- a/coins/monero/src/lib.rs +++ b/coins/monero/src/lib.rs @@ -1,6 +1,8 @@ use lazy_static::lazy_static; use rand_core::{RngCore, CryptoRng}; +use zeroize::{Zeroize, ZeroizeOnDrop}; + use tiny_keccak::{Hasher, Keccak}; use curve25519_dalek::{ @@ -25,7 +27,7 @@ pub mod wallet; #[cfg(test)] mod tests; -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] #[allow(non_camel_case_types)] pub enum Protocol { Unsupported, @@ -61,7 +63,7 @@ lazy_static! { } #[allow(non_snake_case)] -#[derive(Copy, Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct Commitment { pub mask: Scalar, pub amount: u64, diff --git a/coins/monero/src/ringct/bulletproofs/core.rs b/coins/monero/src/ringct/bulletproofs/core.rs index fc6f9fc3..e1a85d5d 100644 --- a/coins/monero/src/ringct/bulletproofs/core.rs +++ b/coins/monero/src/ringct/bulletproofs/core.rs @@ -4,6 +4,8 @@ use lazy_static::lazy_static; use rand_core::{RngCore, CryptoRng}; +use subtle::{Choice, ConditionallySelectable}; + use curve25519_dalek::edwards::EdwardsPoint as DalekPoint; use group::{ff::Field, Group}; @@ -99,11 +101,12 @@ pub(crate) fn bit_decompose(commitments: &[Commitment]) -> (ScalarVector, Scalar for j in 0 .. M { for i in (0 .. N).rev() { - if (j < sv.len()) && ((sv[j][i / 8] & (1u8 << (i % 8))) != 0) { - aL.0[(j * N) + i] = Scalar::one(); - } else { - aR.0[(j * N) + i] = -Scalar::one(); + let mut bit = Choice::from(0); + if j < sv.len() { + bit = Choice::from((sv[j][i / 8] >> (i % 8)) & 1); } + aL.0[(j * N) + i] = Scalar::conditional_select(&Scalar::zero(), &Scalar::one(), bit); + aR.0[(j * N) + i] = Scalar::conditional_select(&-Scalar::one(), &Scalar::zero(), bit); } } diff --git a/coins/monero/src/ringct/bulletproofs/mod.rs b/coins/monero/src/ringct/bulletproofs/mod.rs index de5b449c..e877175a 100644 --- a/coins/monero/src/ringct/bulletproofs/mod.rs +++ b/coins/monero/src/ringct/bulletproofs/mod.rs @@ -2,6 +2,8 @@ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use curve25519_dalek::edwards::EdwardsPoint; use multiexp::BatchVerifier; @@ -73,7 +75,7 @@ impl Bulletproofs { } #[must_use] - pub fn batch_verify( + pub fn batch_verify( &self, rng: &mut R, verifier: &mut BatchVerifier, diff --git a/coins/monero/src/ringct/bulletproofs/original.rs b/coins/monero/src/ringct/bulletproofs/original.rs index 82a38890..bf235d21 100644 --- a/coins/monero/src/ringct/bulletproofs/original.rs +++ b/coins/monero/src/ringct/bulletproofs/original.rs @@ -1,6 +1,8 @@ use lazy_static::lazy_static; use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use curve25519_dalek::{scalar::Scalar as DalekScalar, edwards::EdwardsPoint as DalekPoint}; use group::{ff::Field, Group}; @@ -47,11 +49,12 @@ impl OriginalStruct { let (aL, aR) = bit_decompose(commitments); let (mut cache, _) = hash_commitments(commitments.iter().map(Commitment::calculate)); - let (alpha, A) = alpha_rho(&mut *rng, &GENERATORS, &aL, &aR); let (sL, sR) = ScalarVector((0 .. (MN * 2)).map(|_| Scalar::random(&mut *rng)).collect::>()).split(); - let (rho, S) = alpha_rho(&mut *rng, &GENERATORS, &sL, &sR); + + let (mut alpha, A) = alpha_rho(&mut *rng, &GENERATORS, &aL, &aR); + let (mut rho, S) = alpha_rho(&mut *rng, &GENERATORS, &sL, &sR); let y = hash_cache(&mut cache, &[A.compress().to_bytes(), S.compress().to_bytes()]); let mut cache = hash_to_scalar(&y.to_bytes()); @@ -72,23 +75,33 @@ impl OriginalStruct { let r0 = (&(aR + z) * &yMN) + ScalarVector(zero_twos); let r1 = yMN * sR; - let t1 = inner_product(&l0, &r1) + inner_product(&l1, &r0); - let t2 = inner_product(&l1, &r1); + let (T1, T2, x, mut taux) = { + let t1 = inner_product(&l0, &r1) + inner_product(&l1, &r0); + let t2 = inner_product(&l1, &r1); - let tau1 = Scalar::random(&mut *rng); - let tau2 = Scalar::random(rng); + let mut tau1 = Scalar::random(&mut *rng); + let mut tau2 = Scalar::random(rng); - let T1 = prove_multiexp(&[(t1, *H), (tau1, EdwardsPoint::generator())]); - let T2 = prove_multiexp(&[(t2, *H), (tau2, EdwardsPoint::generator())]); + let T1 = prove_multiexp(&[(t1, *H), (tau1, EdwardsPoint::generator())]); + let T2 = prove_multiexp(&[(t2, *H), (tau2, EdwardsPoint::generator())]); - let x = - hash_cache(&mut cache, &[z.to_bytes(), T1.compress().to_bytes(), T2.compress().to_bytes()]); + let x = + hash_cache(&mut cache, &[z.to_bytes(), T1.compress().to_bytes(), T2.compress().to_bytes()]); + + let taux = (tau2 * (x * x)) + (tau1 * x); + + tau1.zeroize(); + tau2.zeroize(); + (T1, T2, x, taux) + }; + + let mu = (x * rho) + alpha; + alpha.zeroize(); + rho.zeroize(); - let mut taux = (tau2 * (x * x)) + (tau1 * x); for (i, gamma) in commitments.iter().map(|c| Scalar(c.mask)).enumerate() { taux += zpow[i + 2] * gamma; } - let mu = (x * rho) + alpha; let l = &l0 + &(l1 * x); let r = &r0 + &(r1 * x); @@ -155,7 +168,7 @@ impl OriginalStruct { } #[must_use] - fn verify_core( + fn verify_core( &self, rng: &mut R, verifier: &mut BatchVerifier, @@ -284,7 +297,7 @@ impl OriginalStruct { } #[must_use] - pub(crate) fn batch_verify( + pub(crate) fn batch_verify( &self, rng: &mut R, verifier: &mut BatchVerifier, diff --git a/coins/monero/src/ringct/bulletproofs/plus.rs b/coins/monero/src/ringct/bulletproofs/plus.rs index 551c6f20..4082e169 100644 --- a/coins/monero/src/ringct/bulletproofs/plus.rs +++ b/coins/monero/src/ringct/bulletproofs/plus.rs @@ -1,6 +1,8 @@ use lazy_static::lazy_static; use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use curve25519_dalek::{scalar::Scalar as DalekScalar, edwards::EdwardsPoint as DalekPoint}; use group::ff::Field; @@ -109,7 +111,7 @@ impl PlusStruct { let cL = weighted_inner_product(&aL, &bR, y); let cR = weighted_inner_product(&(&aR * ypow[aR.len()]), &bL, y); - let (dL, dR) = (Scalar::random(&mut *rng), Scalar::random(&mut *rng)); + let (mut dL, mut dR) = (Scalar::random(&mut *rng), Scalar::random(&mut *rng)); let (G_L, G_R) = G_proof.split_at(aL.len()); let (H_L, H_R) = H_proof.split_at(aL.len()); @@ -134,12 +136,15 @@ impl PlusStruct { b = (bL * winv) + (bR * w); alpha1 += (dL * (w * w)) + (dR * (winv * winv)); + + dL.zeroize(); + dR.zeroize(); } - let r = Scalar::random(&mut *rng); - let s = Scalar::random(&mut *rng); - let d = Scalar::random(&mut *rng); - let eta = Scalar::random(rng); + let mut r = Scalar::random(&mut *rng); + let mut s = Scalar::random(&mut *rng); + let mut d = Scalar::random(&mut *rng); + let mut eta = Scalar::random(rng); let A1 = prove_multiexp(&[ (r, G_proof[0]), @@ -151,8 +156,13 @@ impl PlusStruct { let e = hash_cache(&mut cache, &[A1.compress().to_bytes(), B.compress().to_bytes()]); let r1 = (a[0] * e) + r; + r.zeroize(); let s1 = (b[0] * e) + s; + s.zeroize(); let d1 = ((d * e) + eta) + (alpha1 * (e * e)); + d.zeroize(); + eta.zeroize(); + alpha1.zeroize(); PlusStruct { A: *A, @@ -167,7 +177,7 @@ impl PlusStruct { } #[must_use] - fn verify_core( + fn verify_core( &self, rng: &mut R, verifier: &mut BatchVerifier, @@ -293,7 +303,7 @@ impl PlusStruct { } #[must_use] - pub(crate) fn batch_verify( + pub(crate) fn batch_verify( &self, rng: &mut R, verifier: &mut BatchVerifier, diff --git a/coins/monero/src/ringct/bulletproofs/scalar_vector.rs b/coins/monero/src/ringct/bulletproofs/scalar_vector.rs index 094a0f0f..39cde8b2 100644 --- a/coins/monero/src/ringct/bulletproofs/scalar_vector.rs +++ b/coins/monero/src/ringct/bulletproofs/scalar_vector.rs @@ -1,11 +1,13 @@ use core::ops::{Add, Sub, Mul, Index}; +use zeroize::{Zeroize, ZeroizeOnDrop}; + use group::ff::Field; use dalek_ff_group::{Scalar, EdwardsPoint}; use multiexp::multiexp; -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub(crate) struct ScalarVector(pub(crate) Vec); macro_rules! math_op { ($Op: ident, $op: ident, $f: expr) => { diff --git a/coins/monero/src/ringct/clsag/mod.rs b/coins/monero/src/ringct/clsag/mod.rs index 20763b64..3ad1032a 100644 --- a/coins/monero/src/ringct/clsag/mod.rs +++ b/coins/monero/src/ringct/clsag/mod.rs @@ -4,6 +4,9 @@ use lazy_static::lazy_static; use thiserror::Error; use rand_core::{RngCore, CryptoRng}; +use zeroize::{Zeroize, ZeroizeOnDrop}; +use subtle::{ConstantTimeEq, Choice, CtOption}; + use curve25519_dalek::{ constants::ED25519_BASEPOINT_TABLE, scalar::Scalar, @@ -45,7 +48,7 @@ pub enum ClsagError { InvalidC1, } -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct ClsagInput { // The actual commitment for the true spend pub commitment: Commitment, @@ -161,11 +164,12 @@ fn core( } // Perform the core loop - let mut c1 = None; + let mut c1 = CtOption::new(Scalar::zero(), Choice::from(0)); for i in (start .. end).map(|i| i % n) { - if i == 0 { - c1 = Some(c); - } + // This will only execute once and shouldn't need to be constant time. Making it constant time + // removes the risk of branch prediction creating timing differences depending on ring index + // however + c1 = c1.or_else(|| CtOption::new(c, i.ct_eq(&0))); let c_p = mu_P * c; let c_c = mu_C * c; @@ -224,14 +228,10 @@ impl Clsag { // Single signer CLSAG pub fn sign( rng: &mut R, - inputs: &[(Scalar, EdwardsPoint, ClsagInput)], + mut inputs: Vec<(Scalar, EdwardsPoint, ClsagInput)>, sum_outputs: Scalar, msg: [u8; 32], ) -> Vec<(Clsag, EdwardsPoint)> { - let nonce = random_scalar(rng); - let mut rand_source = [0; 64]; - rng.fill_bytes(&mut rand_source); - let mut res = Vec::with_capacity(inputs.len()); let mut sum_pseudo_outs = Scalar::zero(); for i in 0 .. inputs.len() { @@ -242,8 +242,7 @@ impl Clsag { sum_pseudo_outs += mask; } - let mut rand_source = [0; 64]; - rng.fill_bytes(&mut rand_source); + let mut nonce = random_scalar(rng); let (mut clsag, pseudo_out, p, c) = Clsag::sign_core( rng, &inputs[i].1, @@ -254,6 +253,8 @@ impl Clsag { nonce * hash_to_point(inputs[i].2.decoys.ring[usize::from(inputs[i].2.decoys.i)][0]), ); clsag.s[usize::from(inputs[i].2.decoys.i)] = nonce - ((p * inputs[i].0) + c); + inputs[i].0.zeroize(); + nonce.zeroize(); res.push((clsag, pseudo_out)); } diff --git a/coins/monero/src/ringct/clsag/multisig.rs b/coins/monero/src/ringct/clsag/multisig.rs index 25c86aa5..e5e7f9cb 100644 --- a/coins/monero/src/ringct/clsag/multisig.rs +++ b/coins/monero/src/ringct/clsag/multisig.rs @@ -7,6 +7,8 @@ use std::{ use rand_core::{RngCore, CryptoRng, SeedableRng}; use rand_chacha::ChaCha12Rng; +use zeroize::{Zeroize, ZeroizeOnDrop}; + use curve25519_dalek::{ constants::ED25519_BASEPOINT_TABLE, traits::{Identity, IsIdentity}, @@ -52,7 +54,7 @@ impl ClsagInput { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Zeroize, ZeroizeOnDrop)] pub struct ClsagDetails { input: ClsagInput, mask: Scalar, @@ -195,7 +197,7 @@ impl Algorithm for ClsagMultisig { ); self.interim = Some(Interim { p, c, clsag, pseudo_out }); - dfg::Scalar(nonces[0].0 - (p * view.secret_share().0)) + nonces[0] - (dfg::Scalar(p) * view.secret_share()) } #[must_use] diff --git a/coins/monero/src/ringct/mod.rs b/coins/monero/src/ringct/mod.rs index be9293be..5d863936 100644 --- a/coins/monero/src/ringct/mod.rs +++ b/coins/monero/src/ringct/mod.rs @@ -1,3 +1,5 @@ +use zeroize::Zeroize; + use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar, edwards::EdwardsPoint}; pub(crate) mod hash_to_point; @@ -11,8 +13,10 @@ use crate::{ ringct::{clsag::Clsag, bulletproofs::Bulletproofs}, }; -pub fn generate_key_image(secret: Scalar) -> EdwardsPoint { - secret * hash_to_point(&secret * &ED25519_BASEPOINT_TABLE) +pub fn generate_key_image(mut secret: Scalar) -> EdwardsPoint { + let res = secret * hash_to_point(&secret * &ED25519_BASEPOINT_TABLE); + secret.zeroize(); + res } #[derive(Clone, PartialEq, Eq, Debug)] diff --git a/coins/monero/src/tests/bulletproofs.rs b/coins/monero/src/tests/bulletproofs.rs index a7ee9d50..3bb88921 100644 --- a/coins/monero/src/tests/bulletproofs.rs +++ b/coins/monero/src/tests/bulletproofs.rs @@ -84,9 +84,11 @@ macro_rules! bulletproofs_tests { #[test] fn $max() { // Check Bulletproofs errors if we try to prove for too many outputs - assert!( - Bulletproofs::prove(&mut OsRng, &[Commitment::new(Scalar::zero(), 0); 17], $plus).is_err() - ); + let mut commitments = vec![]; + for _ in 0 .. 17 { + commitments.push(Commitment::new(Scalar::zero(), 0)); + } + assert!(Bulletproofs::prove(&mut OsRng, &commitments, $plus).is_err()); } }; } diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/src/tests/clsag.rs index 7f5bd63d..d51f7c91 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/src/tests/clsag.rs @@ -56,7 +56,7 @@ fn clsag() { let image = generate_key_image(secrets[0]); let (clsag, pseudo_out) = Clsag::sign( &mut OsRng, - &vec![( + vec![( secrets[0], image, ClsagInput::new( diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index 174a83d8..7abddce5 100644 --- a/coins/monero/src/transaction.rs +++ b/coins/monero/src/transaction.rs @@ -1,5 +1,7 @@ use core::cmp::Ordering; +use zeroize::Zeroize; + use curve25519_dalek::edwards::EdwardsPoint; use crate::{ @@ -11,7 +13,6 @@ use crate::{ #[derive(Clone, PartialEq, Eq, Debug)] pub enum Input { Gen(u64), - ToKey { amount: u64, key_offsets: Vec, key_image: EdwardsPoint }, } @@ -107,7 +108,7 @@ impl Output { } } -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub enum Timelock { None, Block(usize), diff --git a/coins/monero/src/wallet/address.rs b/coins/monero/src/wallet/address.rs index a82256e9..b625596e 100644 --- a/coins/monero/src/wallet/address.rs +++ b/coins/monero/src/wallet/address.rs @@ -2,6 +2,8 @@ use std::string::ToString; use thiserror::Error; +use zeroize::Zeroize; + use curve25519_dalek::{ constants::ED25519_BASEPOINT_TABLE, edwards::{EdwardsPoint, CompressedEdwardsY}, @@ -11,14 +13,14 @@ use base58_monero::base58::{encode_check, decode_check}; use crate::wallet::ViewPair; -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub enum Network { Mainnet, Testnet, Stagenet, } -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub enum AddressType { Standard, Integrated([u8; 8]), @@ -35,7 +37,7 @@ impl AddressType { } } -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub struct AddressMeta { pub network: Network, pub kind: AddressType, @@ -91,7 +93,7 @@ impl AddressMeta { } } -#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub struct Address { pub meta: AddressMeta, pub spend: EdwardsPoint, diff --git a/coins/monero/src/wallet/decoys.rs b/coins/monero/src/wallet/decoys.rs index 4a102564..ea546a58 100644 --- a/coins/monero/src/wallet/decoys.rs +++ b/coins/monero/src/wallet/decoys.rs @@ -5,6 +5,8 @@ use lazy_static::lazy_static; use rand_core::{RngCore, CryptoRng}; use rand_distr::{Distribution, Gamma}; +use zeroize::{Zeroize, ZeroizeOnDrop}; + use curve25519_dalek::edwards::EdwardsPoint; use crate::{ @@ -91,7 +93,7 @@ fn offset(ring: &[u64]) -> Vec { res } -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct Decoys { pub i: u8, pub offsets: Vec, diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs index e60979cc..dc140640 100644 --- a/coins/monero/src/wallet/mod.rs +++ b/coins/monero/src/wallet/mod.rs @@ -1,3 +1,5 @@ +use zeroize::{Zeroize, ZeroizeOnDrop}; + use curve25519_dalek::{scalar::Scalar, edwards::EdwardsPoint}; use crate::{hash, hash_to_scalar, serialize::write_varint, transaction::Input}; @@ -39,7 +41,7 @@ pub(crate) fn uniqueness(inputs: &[Input]) -> [u8; 32] { #[allow(non_snake_case)] pub(crate) fn shared_key( uniqueness: Option<[u8; 32]>, - s: Scalar, + s: &Scalar, P: &EdwardsPoint, o: usize, ) -> (u8, Scalar) { @@ -76,7 +78,7 @@ pub(crate) fn commitment_mask(shared_key: Scalar) -> Scalar { hash_to_scalar(&mask) } -#[derive(Clone, Copy)] +#[derive(Clone, Zeroize, ZeroizeOnDrop)] pub struct ViewPair { pub spend: EdwardsPoint, pub view: Scalar, diff --git a/coins/monero/src/wallet/scan.rs b/coins/monero/src/wallet/scan.rs index 021c5c6f..bad54c9a 100644 --- a/coins/monero/src/wallet/scan.rs +++ b/coins/monero/src/wallet/scan.rs @@ -1,5 +1,7 @@ use std::convert::TryFrom; +use zeroize::{Zeroize, ZeroizeOnDrop}; + use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar, edwards::EdwardsPoint}; use monero::{consensus::deserialize, blockdata::transaction::ExtraField}; @@ -11,7 +13,7 @@ use crate::{ wallet::{ViewPair, uniqueness, shared_key, amount_decryption, commitment_mask}, }; -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct SpendableOutput { pub tx: [u8; 32], pub o: u8, @@ -20,6 +22,7 @@ pub struct SpendableOutput { pub commitment: Commitment, } +#[derive(Zeroize, ZeroizeOnDrop)] pub struct Timelocked(Timelock, Vec); impl Timelocked { pub fn timelock(&self) -> Timelock { @@ -76,7 +79,7 @@ impl SpendableOutput { } impl Transaction { - pub fn scan(&self, view: ViewPair, guaranteed: bool) -> Timelocked { + pub fn scan(&self, view: &ViewPair, guaranteed: bool) -> Timelocked { let mut extra = vec![]; write_varint(&u64::try_from(self.prefix.extra.len()).unwrap(), &mut extra).unwrap(); extra.extend(&self.prefix.extra); @@ -103,7 +106,7 @@ impl Transaction { for pubkey in &pubkeys { let (view_tag, key_offset) = shared_key( Some(uniqueness(&self.prefix.inputs)).filter(|_| guaranteed), - view.view, + &view.view, pubkey, o, ); diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index f89f777d..7b2d4055 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -3,6 +3,8 @@ use thiserror::Error; use rand_core::{RngCore, CryptoRng}; use rand::seq::SliceRandom; +use zeroize::{Zeroize, ZeroizeOnDrop}; + use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar, edwards::EdwardsPoint}; use monero::{consensus::Encodable, PublicKey, blockdata::transaction::SubField}; @@ -35,7 +37,7 @@ mod multisig; pub use multisig::TransactionMachine; #[allow(non_snake_case)] -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] struct SendOutput { R: EdwardsPoint, view_tag: u8, @@ -53,7 +55,7 @@ impl SendOutput { ) -> SendOutput { let r = random_scalar(rng); let (view_tag, shared_key) = - shared_key(Some(unique).filter(|_| output.0.meta.guaranteed), r, &output.0.view, o); + shared_key(Some(unique).filter(|_| output.0.meta.guaranteed), &r, &output.0.view, o); let spend = output.0.spend; SendOutput { @@ -127,7 +129,8 @@ async fn prepare_inputs( signable.push(( spend + input.key_offset, generate_key_image(spend + input.key_offset), - ClsagInput::new(input.commitment, decoys[i].clone()).map_err(TransactionError::ClsagError)?, + ClsagInput::new(input.commitment.clone(), decoys[i].clone()) + .map_err(TransactionError::ClsagError)?, )); tx.prefix.inputs.push(Input::ToKey { @@ -161,12 +164,11 @@ impl Fee { } } -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct SignableTransaction { protocol: Protocol, inputs: Vec, payments: Vec<(Address, u64)>, - outputs: Vec, fee: u64, } @@ -251,80 +253,78 @@ impl SignableTransaction { Err(TransactionError::TooManyOutputs)?; } - Ok(SignableTransaction { protocol, inputs, payments, outputs: vec![], fee }) + Ok(SignableTransaction { protocol, inputs, payments, fee }) } - fn prepare_outputs( + fn prepare_transaction( &mut self, rng: &mut R, uniqueness: [u8; 32], - ) -> (Vec, Scalar) { + ) -> (Transaction, Scalar) { // Shuffle the payments self.payments.shuffle(rng); // Actually create the outputs - self.outputs = Vec::with_capacity(self.payments.len() + 1); - for (o, output) in self.payments.iter().enumerate() { - self.outputs.push(SendOutput::new(rng, uniqueness, *output, o)); - } + let outputs = self + .payments + .drain(..) + .enumerate() + .map(|(o, output)| SendOutput::new(rng, uniqueness, output, o)) + .collect::>(); - let commitments = self.outputs.iter().map(|output| output.commitment).collect::>(); + let commitments = outputs.iter().map(|output| output.commitment.clone()).collect::>(); let sum = commitments.iter().map(|commitment| commitment.mask).sum(); - (commitments, sum) - } - fn prepare_transaction( - &self, - rng: &mut R, - commitments: &[Commitment], - ) -> Transaction { // Safe due to the constructor checking MAX_OUTPUTS - let bp = Bulletproofs::prove(rng, commitments, self.protocol.bp_plus()).unwrap(); + let bp = Bulletproofs::prove(rng, &commitments, self.protocol.bp_plus()).unwrap(); // Create the TX extra // TODO: Review this for canonicity with Monero let mut extra = vec![]; - SubField::TxPublicKey(PublicKey { point: self.outputs[0].R.compress() }) + SubField::TxPublicKey(PublicKey { point: outputs[0].R.compress() }) .consensus_encode(&mut extra) .unwrap(); SubField::AdditionalPublickKey( - self.outputs[1 ..].iter().map(|output| PublicKey { point: output.R.compress() }).collect(), + outputs[1 ..].iter().map(|output| PublicKey { point: output.R.compress() }).collect(), ) .consensus_encode(&mut extra) .unwrap(); - let mut tx_outputs = Vec::with_capacity(self.outputs.len()); - let mut ecdh_info = Vec::with_capacity(self.outputs.len()); - for o in 0 .. self.outputs.len() { + let mut tx_outputs = Vec::with_capacity(outputs.len()); + let mut ecdh_info = Vec::with_capacity(outputs.len()); + for output in &outputs { tx_outputs.push(Output { amount: 0, - key: self.outputs[o].dest, - view_tag: Some(self.outputs[o].view_tag).filter(|_| matches!(self.protocol, Protocol::v16)), + key: output.dest, + view_tag: Some(output.view_tag).filter(|_| matches!(self.protocol, Protocol::v16)), }); - ecdh_info.push(self.outputs[o].amount); + ecdh_info.push(output.amount); } - Transaction { - prefix: TransactionPrefix { - version: 2, - timelock: Timelock::None, - inputs: vec![], - outputs: tx_outputs, - extra, - }, - rct_signatures: RctSignatures { - base: RctBase { - fee: self.fee, - ecdh_info, - commitments: commitments.iter().map(|commitment| commitment.calculate()).collect(), + ( + Transaction { + prefix: TransactionPrefix { + version: 2, + timelock: Timelock::None, + inputs: vec![], + outputs: tx_outputs, + extra, }, - prunable: RctPrunable::Clsag { - bulletproofs: vec![bp], - clsags: vec![], - pseudo_outs: vec![], + rct_signatures: RctSignatures { + base: RctBase { + fee: self.fee, + ecdh_info, + commitments: commitments.iter().map(|commitment| commitment.calculate()).collect(), + }, + prunable: RctPrunable::Clsag { + bulletproofs: vec![bp], + clsags: vec![], + pseudo_outs: vec![], + }, }, }, - } + sum, + ) } pub async fn sign( @@ -335,16 +335,17 @@ impl SignableTransaction { ) -> Result { let mut images = Vec::with_capacity(self.inputs.len()); for input in &self.inputs { - let offset = spend + input.key_offset; + let mut offset = spend + input.key_offset; if (&offset * &ED25519_BASEPOINT_TABLE) != input.key { Err(TransactionError::WrongPrivateKey)?; } images.push(generate_key_image(offset)); + offset.zeroize(); } images.sort_by(key_image_sort); - let (commitments, mask_sum) = self.prepare_outputs( + let (mut tx, mask_sum) = self.prepare_transaction( rng, uniqueness( &images @@ -354,12 +355,10 @@ impl SignableTransaction { ), ); - let mut tx = self.prepare_transaction(rng, &commitments); - let signable = prepare_inputs(rng, rpc, self.protocol.ring_len(), &self.inputs, spend, &mut tx).await?; - let clsag_pairs = Clsag::sign(rng, &signable, mask_sum, tx.signature_hash()); + let clsag_pairs = Clsag::sign(rng, signable, mask_sum, tx.signature_hash()); match tx.rct_signatures.prunable { RctPrunable::Null => panic!("Signing for RctPrunable::Null"), RctPrunable::Clsag { ref mut clsags, ref mut pseudo_outs, .. } => { diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index 436273a9..c8bd5e70 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -125,7 +125,7 @@ impl SignableTransaction { AlgorithmMachine::new( ClsagMultisig::new(transcript.clone(), input.key, inputs[i].clone()) .map_err(TransactionError::MultisigError)?, - Arc::new(offset), + offset, &included, ) .map_err(TransactionError::FrostError)?, @@ -283,25 +283,18 @@ impl SignMachine for TransactionSignMachine { } // Create the actual transaction - let output_masks; - let mut tx = { + let (mut tx, output_masks) = { let mut sorted_images = images.clone(); sorted_images.sort_by(key_image_sort); - let commitments; - (commitments, output_masks) = self.signable.prepare_outputs( - &mut ChaCha12Rng::from_seed(self.transcript.rng_seed(b"tx_keys")), + self.signable.prepare_transaction( + &mut ChaCha12Rng::from_seed(self.transcript.rng_seed(b"transaction_keys_bulletproofs")), uniqueness( - &images + &sorted_images .iter() .map(|image| Input::ToKey { amount: 0, key_offsets: vec![], key_image: *image }) .collect::>(), ), - ); - - self.signable.prepare_transaction( - &mut ChaCha12Rng::from_seed(self.transcript.rng_seed(b"bulletproofs")), - &commitments, ) }; @@ -338,7 +331,7 @@ impl SignMachine for TransactionSignMachine { }); *value.3.write().unwrap() = Some(ClsagDetails::new( - ClsagInput::new(value.1.commitment, value.2).map_err(|_| { + ClsagInput::new(value.1.commitment.clone(), value.2).map_err(|_| { panic!("Signing an input which isn't present in the ring we created for it") })?, mask, diff --git a/coins/monero/tests/send.rs b/coins/monero/tests/send.rs index 9cbe94d4..98193baf 100644 --- a/coins/monero/tests/send.rs +++ b/coins/monero/tests/send.rs @@ -101,7 +101,7 @@ async fn send_core(test: usize, multisig: bool) { // Grab the largest output available let output = { - let mut outputs = tx.as_ref().unwrap().scan(view_pair, false).ignore_timelock(); + let mut outputs = tx.as_ref().unwrap().scan(&view_pair, false).ignore_timelock(); outputs.sort_by(|x, y| x.commitment.amount.cmp(&y.commitment.amount).reverse()); outputs.swap_remove(0) }; @@ -126,7 +126,7 @@ async fn send_core(test: usize, multisig: bool) { for i in (start + 1) .. (start + 9) { let tx = rpc.get_block_transactions(i).await.unwrap().swap_remove(0); - let output = tx.scan(view_pair, false).ignore_timelock().swap_remove(0); + let output = tx.scan(&view_pair, false).ignore_timelock().swap_remove(0); amount += output.commitment.amount; outputs.push(output); } @@ -154,7 +154,7 @@ async fn send_core(test: usize, multisig: bool) { .clone() .multisig( &rpc, - (*keys[&i]).clone(), + keys[&i].clone(), RecommendedTranscript::new(b"Monero Serai Test Transaction"), rpc.get_height().await.unwrap() - 10, (1 ..= THRESHOLD).collect::>(), diff --git a/crypto/dalek-ff-group/Cargo.toml b/crypto/dalek-ff-group/Cargo.toml index 1e88927a..92b76a22 100644 --- a/crypto/dalek-ff-group/Cargo.toml +++ b/crypto/dalek-ff-group/Cargo.toml @@ -12,6 +12,7 @@ edition = "2021" rand_core = "0.6" digest = "0.10" +zeroize = { version = "1.3", features = ["zeroize_derive"] } subtle = "2.4" ff = "0.12" diff --git a/crypto/dalek-ff-group/src/field.rs b/crypto/dalek-ff-group/src/field.rs index cf09330a..7e0c12d5 100644 --- a/crypto/dalek-ff-group/src/field.rs +++ b/crypto/dalek-ff-group/src/field.rs @@ -3,6 +3,7 @@ use core::ops::{Add, AddAssign, Sub, SubAssign, Neg, Mul, MulAssign}; use rand_core::RngCore; use subtle::{Choice, CtOption, ConstantTimeEq, ConditionallySelectable}; + use crypto_bigint::{Encoding, U256, U512}; use ff::{Field, PrimeField, FieldBits, PrimeFieldBits}; @@ -12,7 +13,7 @@ use crate::{choice, constant_time, math_op, math, from_wrapper, from_uint}; const FIELD_MODULUS: U256 = U256::from_be_hex("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffed"); -#[derive(Clone, Copy, PartialEq, Eq, Debug, Default)] +#[derive(Clone, Copy, PartialEq, Eq, Default, Debug)] pub struct FieldElement(U256); pub const SQRT_M1: FieldElement = FieldElement(U256::from_be_hex( diff --git a/crypto/dalek-ff-group/src/lib.rs b/crypto/dalek-ff-group/src/lib.rs index d7ec7262..6dbf7fba 100644 --- a/crypto/dalek-ff-group/src/lib.rs +++ b/crypto/dalek-ff-group/src/lib.rs @@ -6,6 +6,7 @@ use core::{ iter::{Iterator, Sum}, }; +use zeroize::Zeroize; use subtle::{ConstantTimeEq, ConditionallySelectable}; use rand_core::RngCore; @@ -167,7 +168,7 @@ macro_rules! from_uint { } /// Wrapper around the dalek Scalar type -#[derive(Clone, Copy, PartialEq, Eq, Debug, Default)] +#[derive(Clone, Copy, PartialEq, Eq, Default, Debug, Zeroize)] pub struct Scalar(pub DScalar); deref_borrow!(Scalar, DScalar); constant_time!(Scalar, DScalar); @@ -285,7 +286,7 @@ macro_rules! dalek_group { $BASEPOINT_TABLE: ident ) => { /// Wrapper around the dalek Point type. For Ed25519, this is restricted to the prime subgroup - #[derive(Clone, Copy, PartialEq, Eq, Debug)] + #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub struct $Point(pub $DPoint); deref_borrow!($Point, $DPoint); constant_time!($Point, $DPoint); diff --git a/crypto/dleq/Cargo.toml b/crypto/dleq/Cargo.toml index ddf7fed2..e544b996 100644 --- a/crypto/dleq/Cargo.toml +++ b/crypto/dleq/Cargo.toml @@ -10,6 +10,8 @@ edition = "2021" thiserror = "1" rand_core = "0.6" +zeroize = "1.3" + digest = "0.10" transcript = { package = "flexible-transcript", path = "../transcript", version = "0.1" } diff --git a/crypto/dleq/src/cross_group/aos.rs b/crypto/dleq/src/cross_group/aos.rs index a7611388..0c52d501 100644 --- a/crypto/dleq/src/cross_group/aos.rs +++ b/crypto/dleq/src/cross_group/aos.rs @@ -1,5 +1,7 @@ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use transcript::Transcript; use group::{ @@ -46,15 +48,16 @@ impl Re { #[allow(non_snake_case)] #[derive(Clone, PartialEq, Eq, Debug)] -pub(crate) struct Aos { +pub(crate) struct Aos { Re_0: Re, s: [(G0::Scalar, G1::Scalar); RING_LEN], } -impl Aos +impl + Aos where - G0::Scalar: PrimeFieldBits, - G1::Scalar: PrimeFieldBits, + G0::Scalar: PrimeFieldBits + Zeroize, + G1::Scalar: PrimeFieldBits + Zeroize, { #[allow(non_snake_case)] fn nonces(mut transcript: T, nonces: (G0, G1)) -> (G0::Scalar, G1::Scalar) { @@ -102,8 +105,8 @@ where transcript: T, generators: (Generators, Generators), ring: &[(G0, G1)], - actual: usize, - blinding_key: (G0::Scalar, G1::Scalar), + mut actual: usize, + blinding_key: &mut (G0::Scalar, G1::Scalar), mut Re_0: Re, ) -> Self { // While it is possible to use larger values, it's not efficient to do so @@ -113,7 +116,7 @@ where let mut s = [(G0::Scalar::zero(), G1::Scalar::zero()); RING_LEN]; - let r = (G0::Scalar::random(&mut *rng), G1::Scalar::random(&mut *rng)); + let mut r = (G0::Scalar::random(&mut *rng), G1::Scalar::random(&mut *rng)); #[allow(non_snake_case)] let original_R = (generators.0.alt * r.0, generators.1.alt * r.1); #[allow(non_snake_case)] @@ -135,6 +138,11 @@ where if i == actual { s[i] = (r.0 + (e.0 * blinding_key.0), r.1 + (e.1 * blinding_key.1)); debug_assert_eq!(Self::R(generators, s[i], ring[actual], e), original_R); + actual.zeroize(); + blinding_key.0.zeroize(); + blinding_key.1.zeroize(); + r.0.zeroize(); + r.1.zeroize(); break; // Generate a decoy response } else { diff --git a/crypto/dleq/src/cross_group/bits.rs b/crypto/dleq/src/cross_group/bits.rs index ffd881bc..54774cdd 100644 --- a/crypto/dleq/src/cross_group/bits.rs +++ b/crypto/dleq/src/cross_group/bits.rs @@ -1,5 +1,7 @@ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use transcript::Transcript; use group::{ff::PrimeFieldBits, prime::PrimeGroup}; @@ -67,16 +69,25 @@ impl BitSignature { } #[derive(Clone, PartialEq, Eq, Debug)] -pub(crate) struct Bits { +pub(crate) struct Bits< + G0: PrimeGroup + Zeroize, + G1: PrimeGroup + Zeroize, + const SIGNATURE: u8, + const RING_LEN: usize, +> { pub(crate) commitments: (G0, G1), signature: Aos, } -impl - Bits +impl< + G0: PrimeGroup + Zeroize, + G1: PrimeGroup + Zeroize, + const SIGNATURE: u8, + const RING_LEN: usize, + > Bits where - G0::Scalar: PrimeFieldBits, - G1::Scalar: PrimeFieldBits, + G0::Scalar: PrimeFieldBits + Zeroize, + G1::Scalar: PrimeFieldBits + Zeroize, { fn transcript(transcript: &mut T, i: usize, commitments: (G0, G1)) { transcript.domain_separate(b"bits"); @@ -106,8 +117,8 @@ where generators: (Generators, Generators), i: usize, pow_2: &mut (G0, G1), - bits: u8, - blinding_key: (G0::Scalar, G1::Scalar), + mut bits: u8, + blinding_key: &mut (G0::Scalar, G1::Scalar), ) -> Self { let mut commitments = ((generators.0.alt * blinding_key.0), (generators.1.alt * blinding_key.1)); @@ -125,6 +136,7 @@ where blinding_key, BitSignature::from(SIGNATURE).aos_form(), ); + bits.zeroize(); Self::shift(pow_2); Bits { commitments, signature } diff --git a/crypto/dleq/src/cross_group/mod.rs b/crypto/dleq/src/cross_group/mod.rs index 3e34543e..c6e76b39 100644 --- a/crypto/dleq/src/cross_group/mod.rs +++ b/crypto/dleq/src/cross_group/mod.rs @@ -1,6 +1,9 @@ use thiserror::Error; use rand_core::{RngCore, CryptoRng}; + +use zeroize::Zeroize; + use digest::Digest; use transcript::Transcript; @@ -73,8 +76,8 @@ pub enum DLEqError { // anyone who wants it #[derive(Clone, PartialEq, Eq, Debug)] pub struct __DLEqProof< - G0: PrimeGroup, - G1: PrimeGroup, + G0: PrimeGroup + Zeroize, + G1: PrimeGroup + Zeroize, const SIGNATURE: u8, const RING_LEN: usize, const REMAINDER_RING_LEN: usize, @@ -131,15 +134,15 @@ dleq!(EfficientLinearDLEq, BitSignature::EfficientLinear, false); dleq!(CompromiseLinearDLEq, BitSignature::CompromiseLinear, true); impl< - G0: PrimeGroup, - G1: PrimeGroup, + G0: PrimeGroup + Zeroize, + G1: PrimeGroup + Zeroize, const SIGNATURE: u8, const RING_LEN: usize, const REMAINDER_RING_LEN: usize, > __DLEqProof where - G0::Scalar: PrimeFieldBits, - G1::Scalar: PrimeFieldBits, + G0::Scalar: PrimeFieldBits + Zeroize, + G1::Scalar: PrimeFieldBits + Zeroize, { pub(crate) fn transcript( transcript: &mut T, @@ -213,22 +216,27 @@ where let mut pow_2 = (generators.0.primary, generators.1.primary); - let raw_bits = f.0.to_le_bits(); + let mut raw_bits = f.0.to_le_bits(); let mut bits = Vec::with_capacity(capacity); let mut these_bits: u8 = 0; - for (i, bit) in raw_bits.iter().enumerate() { + // Needed to zero out the bits + #[allow(unused_assignments)] + for (i, mut raw_bit) in raw_bits.iter_mut().enumerate() { if i == capacity { break; } - let bit = *bit as u8; + let mut bit = *raw_bit as u8; debug_assert_eq!(bit | 1, 1); + *raw_bit = false; // Accumulate this bit these_bits |= bit << (i % bits_per_group); + bit = 0; + if (i % bits_per_group) == (bits_per_group - 1) { let last = i == (capacity - 1); - let blinding_key = blinding_key(&mut *rng, last); + let mut blinding_key = blinding_key(&mut *rng, last); bits.push(Bits::prove( &mut *rng, transcript, @@ -236,7 +244,7 @@ where i / bits_per_group, &mut pow_2, these_bits, - blinding_key, + &mut blinding_key, )); these_bits = 0; } @@ -245,7 +253,7 @@ where let mut remainder = None; if capacity != ((capacity / bits_per_group) * bits_per_group) { - let blinding_key = blinding_key(&mut *rng, true); + let mut blinding_key = blinding_key(&mut *rng, true); remainder = Some(Bits::prove( &mut *rng, transcript, @@ -253,10 +261,12 @@ where capacity / bits_per_group, &mut pow_2, these_bits, - blinding_key, + &mut blinding_key, )); } + these_bits.zeroize(); + let proof = __DLEqProof { bits, remainder, poks }; debug_assert_eq!( proof.reconstruct_keys(), diff --git a/crypto/dleq/src/cross_group/scalar.rs b/crypto/dleq/src/cross_group/scalar.rs index 6df5dee7..1cc2e5f9 100644 --- a/crypto/dleq/src/cross_group/scalar.rs +++ b/crypto/dleq/src/cross_group/scalar.rs @@ -1,7 +1,11 @@ use ff::PrimeFieldBits; +use zeroize::Zeroize; + /// Convert a uniform scalar into one usable on both fields, clearing the top bits as needed -pub fn scalar_normalize(scalar: F0) -> (F0, F1) { +pub fn scalar_normalize( + mut scalar: F0, +) -> (F0, F1) { let mutual_capacity = F0::CAPACITY.min(F1::CAPACITY); // The security of a mutual key is the security of the lower field. Accordingly, this bans a @@ -13,30 +17,50 @@ pub fn scalar_normalize(scalar: F0) -> ( let mut res2 = F1::zero(); // Uses the bit view API to ensure a consistent endianess let mut bits = scalar.to_le_bits(); + scalar.zeroize(); // Convert it to big endian bits.reverse(); - for bit in bits.iter().skip(bits.len() - usize::try_from(mutual_capacity).unwrap()) { + + let mut skip = bits.len() - usize::try_from(mutual_capacity).unwrap(); + // Needed to zero out the bits + #[allow(unused_assignments)] + for mut raw_bit in bits.iter_mut() { + if skip > 0 { + *raw_bit = false; + skip -= 1; + continue; + } + res1 = res1.double(); res2 = res2.double(); - let bit = *bit as u8; + let mut bit = *raw_bit as u8; debug_assert_eq!(bit | 1, 1); + *raw_bit = false; res1 += F0::from(bit.into()); res2 += F1::from(bit.into()); + bit = 0; } (res1, res2) } /// Helper to convert a scalar between fields. Returns None if the scalar isn't mutually valid -pub fn scalar_convert(scalar: F0) -> Option { - let (valid, converted) = scalar_normalize(scalar); - Some(converted).filter(|_| scalar == valid) +pub fn scalar_convert( + mut scalar: F0, +) -> Option { + let (mut valid, converted) = scalar_normalize(scalar); + let res = Some(converted).filter(|_| scalar == valid); + scalar.zeroize(); + valid.zeroize(); + res } /// Create a mutually valid scalar from bytes via bit truncation to not introduce bias -pub fn mutual_scalar_from_bytes(bytes: &[u8]) -> (F0, F1) { +pub fn mutual_scalar_from_bytes( + bytes: &[u8], +) -> (F0, F1) { let capacity = usize::try_from(F0::CAPACITY.min(F1::CAPACITY)).unwrap(); debug_assert!((bytes.len() * 8) >= capacity); diff --git a/crypto/dleq/src/cross_group/schnorr.rs b/crypto/dleq/src/cross_group/schnorr.rs index 530b28b1..564d868e 100644 --- a/crypto/dleq/src/cross_group/schnorr.rs +++ b/crypto/dleq/src/cross_group/schnorr.rs @@ -1,5 +1,7 @@ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use transcript::Transcript; use group::{ @@ -19,14 +21,14 @@ use crate::{read_scalar, cross_group::read_point}; #[allow(non_snake_case)] #[derive(Clone, PartialEq, Eq, Debug)] -pub(crate) struct SchnorrPoK { +pub(crate) struct SchnorrPoK { R: G, s: G::Scalar, } -impl SchnorrPoK +impl SchnorrPoK where - G::Scalar: PrimeFieldBits, + G::Scalar: PrimeFieldBits + Zeroize, { // Not hram due to the lack of m #[allow(non_snake_case)] @@ -42,15 +44,18 @@ where rng: &mut R, transcript: &mut T, generator: G, - private_key: G::Scalar, + mut private_key: G::Scalar, ) -> SchnorrPoK { - let nonce = G::Scalar::random(rng); + let mut nonce = G::Scalar::random(rng); #[allow(non_snake_case)] let R = generator * nonce; - SchnorrPoK { + let res = SchnorrPoK { R, s: nonce + (private_key * SchnorrPoK::hra(transcript, generator, R, generator * private_key)), - } + }; + private_key.zeroize(); + nonce.zeroize(); + res } pub(crate) fn verify( diff --git a/crypto/dleq/src/lib.rs b/crypto/dleq/src/lib.rs index 26a6b7e1..ddc09bd5 100644 --- a/crypto/dleq/src/lib.rs +++ b/crypto/dleq/src/lib.rs @@ -2,6 +2,8 @@ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use transcript::Transcript; use ff::{Field, PrimeField}; @@ -76,9 +78,12 @@ impl DLEqProof { rng: &mut R, transcript: &mut T, generators: &[G], - scalar: G::Scalar, - ) -> DLEqProof { - let r = G::Scalar::random(rng); + mut scalar: G::Scalar, + ) -> DLEqProof + where + G::Scalar: Zeroize, + { + let mut r = G::Scalar::random(rng); transcript.domain_separate(b"dleq"); for generator in generators { @@ -88,6 +93,9 @@ impl DLEqProof { let c = challenge(transcript); let s = r + (c * scalar); + scalar.zeroize(); + r.zeroize(); + DLEqProof { c, s } } diff --git a/crypto/dleq/src/tests/cross_group/aos.rs b/crypto/dleq/src/tests/cross_group/aos.rs index 09b8c0b5..4fb43ca1 100644 --- a/crypto/dleq/src/tests/cross_group/aos.rs +++ b/crypto/dleq/src/tests/cross_group/aos.rs @@ -38,7 +38,7 @@ fn test_aos(default: Re) { generators, &ring, actual, - ring_keys[actual], + &mut ring_keys[actual], default.clone(), ); diff --git a/crypto/dleq/src/tests/cross_group/schnorr.rs b/crypto/dleq/src/tests/cross_group/schnorr.rs index d57b57f1..cc6a2cdb 100644 --- a/crypto/dleq/src/tests/cross_group/schnorr.rs +++ b/crypto/dleq/src/tests/cross_group/schnorr.rs @@ -1,5 +1,7 @@ use rand_core::OsRng; +use zeroize::Zeroize; + use group::{ ff::{Field, PrimeFieldBits}, prime::PrimeGroup, @@ -10,9 +12,9 @@ use transcript::{Transcript, RecommendedTranscript}; use crate::cross_group::schnorr::SchnorrPoK; -fn test_schnorr() +fn test_schnorr() where - G::Scalar: PrimeFieldBits, + G::Scalar: PrimeFieldBits + Zeroize, { let private = G::Scalar::random(&mut OsRng); diff --git a/crypto/frost/Cargo.toml b/crypto/frost/Cargo.toml index cf8a928b..dfed181a 100644 --- a/crypto/frost/Cargo.toml +++ b/crypto/frost/Cargo.toml @@ -12,6 +12,9 @@ edition = "2021" thiserror = "1" rand_core = "0.6" + +zeroize = { version = "1.3", features = ["zeroize_derive"] } + hex = "0.4" sha2 = { version = "0.10", optional = true } diff --git a/crypto/frost/src/curve/dalek.rs b/crypto/frost/src/curve/dalek.rs index f0357bf5..8968cbd9 100644 --- a/crypto/frost/src/curve/dalek.rs +++ b/crypto/frost/src/curve/dalek.rs @@ -1,4 +1,4 @@ -use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; use sha2::{Digest, Sha512}; @@ -21,7 +21,7 @@ macro_rules! dalek_curve { ) => { use dalek_ff_group::{$Point, $POINT}; - #[derive(Clone, Copy, PartialEq, Eq, Debug)] + #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub struct $Curve; impl Curve for $Curve { type F = Scalar; @@ -30,13 +30,6 @@ macro_rules! dalek_curve { const ID: &'static [u8] = $ID; const GENERATOR: Self::G = $POINT; - fn random_nonce(secret: Self::F, rng: &mut R) -> Self::F { - let mut seed = vec![0; 32]; - rng.fill_bytes(&mut seed); - seed.extend(&secret.to_bytes()); - Self::hash_to_F(b"nonce", &seed) - } - fn hash_msg(msg: &[u8]) -> Vec { Sha512::new() .chain_update($CONTEXT) diff --git a/crypto/frost/src/curve/kp256.rs b/crypto/frost/src/curve/kp256.rs index f81ed118..a26fc334 100644 --- a/crypto/frost/src/curve/kp256.rs +++ b/crypto/frost/src/curve/kp256.rs @@ -1,12 +1,14 @@ -use std::io::Cursor; - -use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; use sha2::{digest::Update, Digest, Sha256}; -use group::{ff::Field, GroupEncoding}; +use group::{ + ff::{Field, PrimeField}, + GroupEncoding, +}; use elliptic_curve::{ + generic_array::GenericArray, bigint::{Encoding, U384}, hash2curve::{Expander, ExpandMsg, ExpandMsgXmd}, }; @@ -22,7 +24,7 @@ macro_rules! kp_curve { $ID: literal, $CONTEXT: literal ) => { - #[derive(Clone, Copy, PartialEq, Eq, Debug)] + #[derive(Clone, Copy, PartialEq, Eq, Debug, Zeroize)] pub struct $Curve; impl Curve for $Curve { type F = $lib::Scalar; @@ -31,13 +33,6 @@ macro_rules! kp_curve { const ID: &'static [u8] = $ID; const GENERATOR: Self::G = $lib::ProjectivePoint::GENERATOR; - fn random_nonce(secret: Self::F, rng: &mut R) -> Self::F { - let mut seed = vec![0; 32]; - rng.fill_bytes(&mut seed); - seed.extend(secret.to_bytes()); - Self::hash_to_F(&[$CONTEXT as &[u8], b"nonce"].concat(), &seed) - } - fn hash_msg(msg: &[u8]) -> Vec { (&Sha256::new().chain($CONTEXT).chain(b"digest").chain(msg).finalize()).to_vec() } @@ -58,17 +53,21 @@ macro_rules! kp_curve { let mut modulus = vec![0; 16]; modulus.extend((Self::F::zero() - Self::F::one()).to_bytes()); let modulus = U384::from_be_slice(&modulus).wrapping_add(&U384::ONE); - Self::read_F(&mut Cursor::new( - &U384::from_be_slice(&{ - let mut bytes = [0; 48]; - ExpandMsgXmd::::expand_message(&[msg], dst, 48).unwrap().fill_bytes(&mut bytes); - bytes - }) - .reduce(&modulus) - .unwrap() - .to_be_bytes()[16 ..], - )) + + let mut unreduced = U384::from_be_bytes({ + let mut bytes = [0; 48]; + ExpandMsgXmd::::expand_message(&[msg], dst, 48).unwrap().fill_bytes(&mut bytes); + bytes + }) + .reduce(&modulus) .unwrap() + .to_be_bytes(); + + let mut array = *GenericArray::from_slice(&unreduced[16 ..]); + let res = $lib::Scalar::from_repr(array).unwrap(); + unreduced.zeroize(); + array.zeroize(); + res } } diff --git a/crypto/frost/src/curve/mod.rs b/crypto/frost/src/curve/mod.rs index 7e0acb7f..2f0ba65a 100644 --- a/crypto/frost/src/curve/mod.rs +++ b/crypto/frost/src/curve/mod.rs @@ -5,6 +5,8 @@ use thiserror::Error; use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use ff::{PrimeField, PrimeFieldBits}; use group::{Group, GroupOps, GroupEncoding, prime::PrimeGroup}; @@ -39,12 +41,12 @@ pub enum CurveError { // elliptic-curve exists, yet it doesn't really serve the same role, nor does it use &[u8]/Vec // It uses GenericArray which will hopefully be deprecated as Rust evolves and doesn't offer enough // advantages in the modern day to be worth the hassle -- Kayaba -pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { +pub trait Curve: Clone + Copy + PartialEq + Eq + Debug + Zeroize { /// Scalar field element type // This is available via G::Scalar yet `C::G::Scalar` is ambiguous, forcing horrific accesses - type F: PrimeField + PrimeFieldBits; + type F: PrimeField + PrimeFieldBits + Zeroize; /// Group element type - type G: Group + GroupOps + PrimeGroup; + type G: Group + GroupOps + PrimeGroup + Zeroize; /// ID for this curve const ID: &'static [u8]; @@ -53,9 +55,6 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { // While group does provide this in its API, privacy coins may want to use a custom basepoint const GENERATOR: Self::G; - /// Securely generate a random nonce. H4 from the IETF draft - fn random_nonce(secret: Self::F, rng: &mut R) -> Self::F; - /// Hash the message for the binding factor. H3 from the IETF draft // This doesn't actually need to be part of Curve as it does nothing with the curve // This also solely relates to FROST and with a proper Algorithm/HRAM, all projects using @@ -69,8 +68,23 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { /// Hash the commitments and message to calculate the binding factor. H1 from the IETF draft fn hash_binding_factor(binding: &[u8]) -> Self::F; - // The following methods would optimally be F:: and G:: yet developers can't control F/G - // They can control a trait they pass into this library + /// Securely generate a random nonce. H4 from the IETF draft + fn random_nonce(mut secret: Self::F, rng: &mut R) -> Self::F { + let mut seed = vec![0; 32]; + rng.fill_bytes(&mut seed); + + let mut repr = secret.to_repr(); + secret.zeroize(); + + seed.extend(repr.as_ref()); + for i in repr.as_mut() { + *i = 0; + } + + let res = Self::hash_to_F(b"nonce", &seed); + seed.zeroize(); + res + } /// Field element from hash. Used during key gen and by other crates under Serai as a general /// utility @@ -93,8 +107,14 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { fn read_F(r: &mut R) -> Result { let mut encoding = ::Repr::default(); r.read_exact(encoding.as_mut()).map_err(|_| CurveError::InvalidScalar)?; + // ff mandates this is canonical - Option::::from(Self::F::from_repr(encoding)).ok_or(CurveError::InvalidScalar) + let res = + Option::::from(Self::F::from_repr(encoding)).ok_or(CurveError::InvalidScalar); + for b in encoding.as_mut() { + *b = 0; + } + res } #[allow(non_snake_case)] diff --git a/crypto/frost/src/key_gen.rs b/crypto/frost/src/key_gen.rs index 00143d62..c56a5d42 100644 --- a/crypto/frost/src/key_gen.rs +++ b/crypto/frost/src/key_gen.rs @@ -6,6 +6,8 @@ use std::{ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use group::{ ff::{Field, PrimeField}, GroupEncoding, @@ -15,7 +17,7 @@ use multiexp::{multiexp_vartime, BatchVerifier}; use crate::{ curve::Curve, - FrostError, FrostParams, FrostKeys, + FrostError, FrostParams, FrostCore, schnorr::{self, SchnorrSignature}, validate_map, }; @@ -54,7 +56,7 @@ fn generate_key_r1( } // Step 2: Provide a proof of knowledge - let r = C::F::random(rng); + let mut r = C::F::random(rng); serialized.extend( schnorr::sign::( coefficients[0], @@ -67,6 +69,7 @@ fn generate_key_r1( ) .serialize(), ); + r.zeroize(); // Step 4: Broadcast (coefficients, commitments, serialized) @@ -148,7 +151,7 @@ fn generate_key_r2( rng: &mut R, params: &FrostParams, context: &str, - coefficients: Vec, + coefficients: &mut Vec, our_commitments: Vec, commitments: HashMap, ) -> Result<(C::F, HashMap>, HashMap>), FrostError> { @@ -163,19 +166,13 @@ fn generate_key_r2( continue; } - res.insert(l, polynomial(&coefficients, l).to_repr().as_ref().to_vec()); + res.insert(l, polynomial(coefficients, l).to_repr().as_ref().to_vec()); } // Calculate our own share - let share = polynomial(&coefficients, params.i()); + let share = polynomial(coefficients, params.i()); - // The secret shares are discarded here, not cleared. While any system which leaves its memory - // accessible is likely totally lost already, making the distinction meaningless when the key gen - // system acts as the signer system and therefore actively holds the signing key anyways, it - // should be overwritten with /dev/urandom in the name of security (which still doesn't meet - // requirements for secure data deletion yet those requirements expect hardware access which is - // far past what this library can reasonably counter) - // TODO: Zero out the coefficients + coefficients.zeroize(); Ok((share, commitments, res)) } @@ -189,17 +186,17 @@ fn complete_r2( rng: &mut R, params: FrostParams, mut secret_share: C::F, - commitments: HashMap>, + commitments: &mut HashMap>, mut serialized: HashMap, -) -> Result, FrostError> { +) -> Result, FrostError> { validate_map(&mut serialized, &(1 ..= params.n()).collect::>(), params.i())?; // Step 2. Verify each share let mut shares = HashMap::new(); + // TODO: Clear serialized for (l, share) in serialized.iter_mut() { shares.insert(*l, C::read_F(share).map_err(|_| FrostError::InvalidShare(*l))?); } - shares.insert(params.i(), secret_share); // 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 @@ -215,12 +212,12 @@ fn complete_r2( }; let mut batch = BatchVerifier::new(shares.len()); - for (l, share) in &shares { + for (l, share) in shares.iter_mut() { if *l == params.i() { continue; } - secret_share += share; + 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 @@ -228,6 +225,8 @@ fn complete_r2( // 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)); + share.zeroize(); + batch.queue(rng, *l, values); } batch.verify_with_vartime_blame().map_err(FrostError::InvalidCommitment)?; @@ -249,9 +248,7 @@ fn complete_r2( // Removing this check would enable optimizing the above from t + (n * t) to t + ((n - 1) * t) debug_assert_eq!(C::GENERATOR * secret_share, verification_shares[¶ms.i()]); - // TODO: Clear serialized and shares - - Ok(FrostKeys { params, secret_share, group_key: stripes[0], verification_shares, offset: None }) + Ok(FrostCore { params, secret_share, group_key: stripes[0], verification_shares }) } pub struct KeyGenMachine { @@ -260,19 +257,37 @@ pub struct KeyGenMachine { _curve: PhantomData, } +#[derive(Zeroize)] pub struct SecretShareMachine { + #[zeroize(skip)] params: FrostParams, context: String, coefficients: Vec, + #[zeroize(skip)] our_commitments: Vec, } +impl Drop for SecretShareMachine { + fn drop(&mut self) { + self.zeroize() + } +} + +#[derive(Zeroize)] pub struct KeyMachine { + #[zeroize(skip)] params: FrostParams, secret: C::F, + #[zeroize(skip)] commitments: HashMap>, } +impl Drop for KeyMachine { + fn drop(&mut self) { + self.zeroize() + } +} + impl KeyGenMachine { /// Creates a new machine to generate a key for the specified curve in the specified multisig // The context string must be unique among multisigs @@ -309,7 +324,7 @@ impl SecretShareMachine { /// is also expected at index i which is locally handled. Returns a byte vector representing a /// secret share for each other participant which should be encrypted before sending pub fn generate_secret_shares( - self, + mut self, rng: &mut R, commitments: HashMap, ) -> Result<(KeyMachine, HashMap>), FrostError> { @@ -317,8 +332,8 @@ impl SecretShareMachine { rng, &self.params, &self.context, - self.coefficients, - self.our_commitments, + &mut self.coefficients, + self.our_commitments.clone(), commitments, )?; Ok((KeyMachine { params: self.params, secret, commitments }, shares)) @@ -333,10 +348,10 @@ impl KeyMachine { /// 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( - self, + mut self, rng: &mut R, shares: HashMap, - ) -> Result, FrostError> { - complete_r2(rng, self.params, self.secret, self.commitments, shares) + ) -> Result, FrostError> { + complete_r2(rng, self.params, self.secret, &mut self.commitments, shares) } } diff --git a/crypto/frost/src/lib.rs b/crypto/frost/src/lib.rs index e9c9a6d1..30b37bf0 100644 --- a/crypto/frost/src/lib.rs +++ b/crypto/frost/src/lib.rs @@ -1,8 +1,10 @@ -use core::fmt::Debug; -use std::{io::Read, collections::HashMap}; +use core::fmt::{self, Debug}; +use std::{io::Read, sync::Arc, collections::HashMap}; use thiserror::Error; +use zeroize::Zeroize; + use group::{ ff::{Field, PrimeField}, GroupEncoding, @@ -18,6 +20,32 @@ pub mod sign; pub mod tests; +// Validate a map of serialized values to have the expected included participants +pub(crate) fn validate_map( + map: &mut HashMap, + included: &[u16], + ours: u16, +) -> Result<(), FrostError> { + if (map.len() + 1) != included.len() { + Err(FrostError::InvalidParticipantQuantity(included.len(), map.len() + 1))?; + } + + for included in included { + if *included == ours { + if map.contains_key(included) { + Err(FrostError::DuplicatedIndex(*included))?; + } + continue; + } + + if !map.contains_key(included) { + Err(FrostError::MissingParticipant(*included))?; + } + } + + Ok(()) +} + /// Parameters for a multisig // These fields can not be made public as they should be static #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -89,33 +117,6 @@ pub enum FrostError { InternalError(&'static str), } -// View of keys passable to algorithm implementations -#[derive(Clone)] -pub struct FrostView { - group_key: C::G, - included: Vec, - secret_share: C::F, - verification_shares: HashMap, -} - -impl FrostView { - pub fn group_key(&self) -> C::G { - self.group_key - } - - pub fn included(&self) -> Vec { - self.included.clone() - } - - pub fn secret_share(&self) -> C::F { - self.secret_share - } - - pub fn verification_share(&self, l: u16) -> C::G { - self.verification_shares[&l] - } -} - /// Calculate the lagrange coefficient for a signing set pub fn lagrange(i: u16, included: &[u16]) -> F { let mut num = F::one(); @@ -135,9 +136,11 @@ pub fn lagrange(i: u16, included: &[u16]) -> F { num * denom.invert().unwrap() } -#[derive(Clone, PartialEq, Eq, Debug)] -pub struct FrostKeys { +/// Core keys generated by performing a FROST keygen protocol +#[derive(Clone, PartialEq, Eq, Zeroize)] +pub struct FrostCore { /// FROST Parameters + #[zeroize(skip)] params: FrostParams, /// Secret share key @@ -145,32 +148,32 @@ pub struct FrostKeys { /// Group key group_key: C::G, /// Verification shares + #[zeroize(skip)] verification_shares: HashMap, - - /// Offset applied to these keys - offset: Option, } -impl FrostKeys { - /// Offset the keys by a given scalar to allow for account and privacy schemes - /// This offset is ephemeral and will not be included when these keys are serialized - /// Keys offset multiple times will form a new offset of their sum - /// Not IETF compliant - pub fn offset(&self, offset: C::F) -> FrostKeys { - let mut res = self.clone(); - // Carry any existing offset - // Enables schemes like Monero's subaddresses which have a per-subaddress offset and then a - // one-time-key offset - res.offset = Some(offset + res.offset.unwrap_or_else(C::F::zero)); - res.group_key += C::GENERATOR * offset; - res +impl Drop for FrostCore { + fn drop(&mut self) { + self.zeroize() } +} +impl Debug for FrostCore { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("FrostCore") + .field("params", &self.params) + .field("group_key", &self.group_key) + .field("verification_shares", &self.verification_shares) + .finish() + } +} + +impl FrostCore { pub fn params(&self) -> FrostParams { self.params } - fn secret_share(&self) -> C::F { + pub(crate) fn secret_share(&self) -> C::F { self.secret_share } @@ -178,39 +181,16 @@ impl FrostKeys { self.group_key } - fn verification_shares(&self) -> HashMap { + pub(crate) fn verification_shares(&self) -> HashMap { self.verification_shares.clone() } - pub fn view(&self, included: &[u16]) -> Result, FrostError> { - if (included.len() < self.params.t.into()) || (usize::from(self.params.n) < included.len()) { - Err(FrostError::InvalidSigningSet("invalid amount of participants included"))?; - } - - let secret_share = self.secret_share * lagrange::(self.params.i, included); - let offset = self.offset.unwrap_or_else(C::F::zero); - let offset_share = offset * C::F::from(included.len().try_into().unwrap()).invert().unwrap(); - - Ok(FrostView { - group_key: self.group_key, - secret_share: secret_share + offset_share, - verification_shares: self - .verification_shares - .iter() - .map(|(l, share)| { - (*l, (*share * lagrange::(*l, included)) + (C::GENERATOR * offset_share)) - }) - .collect(), - included: included.to_vec(), - }) - } - pub fn serialized_len(n: u16) -> usize { 8 + C::ID.len() + (3 * 2) + C::F_len() + C::G_len() + (usize::from(n) * C::G_len()) } pub fn serialize(&self) -> Vec { - let mut serialized = Vec::with_capacity(FrostKeys::::serialized_len(self.params.n)); + let mut serialized = Vec::with_capacity(FrostCore::::serialized_len(self.params.n)); serialized.extend(u32::try_from(C::ID.len()).unwrap().to_be_bytes()); serialized.extend(C::ID); serialized.extend(&self.params.t.to_be_bytes()); @@ -224,10 +204,10 @@ impl FrostKeys { serialized } - pub fn deserialize(cursor: &mut R) -> Result, FrostError> { + pub fn deserialize(cursor: &mut R) -> Result, FrostError> { { - let missing = FrostError::InternalError("FrostKeys serialization is missing its curve"); - let different = FrostError::InternalError("deserializing FrostKeys for another curve"); + let missing = FrostError::InternalError("FrostCore serialization is missing its curve"); + let different = FrostError::InternalError("deserializing FrostCore for another curve"); let mut id_len = [0; 4]; cursor.read_exact(&mut id_len).map_err(|_| missing)?; @@ -266,39 +246,133 @@ impl FrostKeys { ); } - Ok(FrostKeys { + Ok(FrostCore { params: FrostParams::new(t, n, i) .map_err(|_| FrostError::InternalError("invalid parameters"))?, secret_share, group_key, verification_shares, - offset: None, }) } } -// Validate a map of serialized values to have the expected included participants -pub(crate) fn validate_map( - map: &mut HashMap, - included: &[u16], - ours: u16, -) -> Result<(), FrostError> { - if (map.len() + 1) != included.len() { - Err(FrostError::InvalidParticipantQuantity(included.len(), map.len() + 1))?; - } +/// FROST keys usable for signing +#[derive(Clone, Debug, Zeroize)] +pub struct FrostKeys { + /// Core keys + #[zeroize(skip)] + core: Arc>, - for included in included { - if *included == ours { - if map.contains_key(included) { - Err(FrostError::DuplicatedIndex(*included))?; - } - continue; - } - - if !map.contains_key(included) { - Err(FrostError::MissingParticipant(*included))?; - } - } - - Ok(()) + /// Offset applied to these keys + pub(crate) offset: Option, +} + +// Manually implement Drop due to https://github.com/RustCrypto/utils/issues/786 +impl Drop for FrostKeys { + fn drop(&mut self) { + self.zeroize() + } +} + +// View of keys passable to algorithm implementations +#[derive(Clone, Zeroize)] +pub struct FrostView { + group_key: C::G, + #[zeroize(skip)] + included: Vec, + secret_share: C::F, + #[zeroize(skip)] + verification_shares: HashMap, +} + +impl Drop for FrostView { + fn drop(&mut self) { + self.zeroize() + } +} + +impl FrostKeys { + pub fn new(core: FrostCore) -> FrostKeys { + FrostKeys { core: Arc::new(core), offset: None } + } + + /// Offset the keys by a given scalar to allow for account and privacy schemes + /// This offset is ephemeral and will not be included when these keys are serialized + /// Keys offset multiple times will form a new offset of their sum + /// Not IETF compliant + pub fn offset(&self, offset: C::F) -> FrostKeys { + let mut res = self.clone(); + // Carry any existing offset + // Enables schemes like Monero's subaddresses which have a per-subaddress offset and then a + // one-time-key offset + res.offset = Some(offset + res.offset.unwrap_or_else(C::F::zero)); + res + } + + pub fn params(&self) -> FrostParams { + self.core.params + } + + pub(crate) fn secret_share(&self) -> C::F { + self.core.secret_share + } + + pub fn group_key(&self) -> C::G { + self.core.group_key + (C::GENERATOR * self.offset.unwrap_or_else(C::F::zero)) + } + + pub(crate) fn verification_shares(&self) -> HashMap { + self.core.verification_shares.clone() + } + + pub fn serialized_len(n: u16) -> usize { + FrostCore::::serialized_len(n) + } + + pub fn serialize(&self) -> Vec { + self.core.serialize() + } + + pub fn view(&self, included: &[u16]) -> Result, FrostError> { + if (included.len() < self.params().t.into()) || (usize::from(self.params().n) < included.len()) + { + Err(FrostError::InvalidSigningSet("invalid amount of participants included"))?; + } + + let offset_share = self.offset.unwrap_or_else(C::F::zero) * + C::F::from(included.len().try_into().unwrap()).invert().unwrap(); + let offset_verification_share = C::GENERATOR * offset_share; + + Ok(FrostView { + group_key: self.group_key(), + secret_share: (self.secret_share() * lagrange::(self.params().i, included)) + + offset_share, + verification_shares: self + .verification_shares() + .iter() + .map(|(l, share)| { + (*l, (*share * lagrange::(*l, included)) + offset_verification_share) + }) + .collect(), + included: included.to_vec(), + }) + } +} + +impl FrostView { + pub fn group_key(&self) -> C::G { + self.group_key + } + + pub fn included(&self) -> Vec { + self.included.clone() + } + + pub fn secret_share(&self) -> C::F { + self.secret_share + } + + pub fn verification_share(&self, l: u16) -> C::G { + self.verification_shares[&l] + } } diff --git a/crypto/frost/src/schnorr.rs b/crypto/frost/src/schnorr.rs index 98ec115e..2174a062 100644 --- a/crypto/frost/src/schnorr.rs +++ b/crypto/frost/src/schnorr.rs @@ -1,5 +1,7 @@ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use group::{ ff::{Field, PrimeField}, GroupEncoding, @@ -26,11 +28,14 @@ impl SchnorrSignature { } pub(crate) fn sign( - private_key: C::F, - nonce: C::F, + mut private_key: C::F, + mut nonce: C::F, challenge: C::F, ) -> SchnorrSignature { - SchnorrSignature { R: C::GENERATOR * nonce, s: nonce + (private_key * challenge) } + let res = SchnorrSignature { R: C::GENERATOR * nonce, s: nonce + (private_key * challenge) }; + private_key.zeroize(); + nonce.zeroize(); + res } #[must_use] diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 2311e9d5..1dffa026 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -1,12 +1,13 @@ use core::fmt; use std::{ io::{Read, Cursor}, - sync::Arc, collections::HashMap, }; use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use transcript::Transcript; use group::{ @@ -25,7 +26,7 @@ use crate::{ #[derive(Clone)] pub struct Params> { algorithm: A, - keys: Arc>, + keys: FrostKeys, view: FrostView, } @@ -33,23 +34,25 @@ pub struct Params> { impl> Params { pub fn new( algorithm: A, - keys: Arc>, + keys: FrostKeys, included: &[u16], ) -> Result, FrostError> { + let params = keys.params(); + let mut included = included.to_vec(); included.sort_unstable(); // Included < threshold - if included.len() < usize::from(keys.params.t) { + if included.len() < usize::from(params.t) { Err(FrostError::InvalidSigningSet("not enough signers"))?; } // Invalid index if included[0] == 0 { - Err(FrostError::InvalidParticipantIndex(included[0], keys.params.n))?; + Err(FrostError::InvalidParticipantIndex(included[0], params.n))?; } // OOB index - if included[included.len() - 1] > keys.params.n { - Err(FrostError::InvalidParticipantIndex(included[included.len() - 1], keys.params.n))?; + 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 { @@ -58,7 +61,7 @@ impl> Params { } } // Not included - if !included.contains(&keys.params.i) { + if !included.contains(¶ms.i) { Err(FrostError::InvalidSigningSet("signing despite not being included"))?; } @@ -67,7 +70,7 @@ impl> Params { } pub fn multisig_params(&self) -> FrostParams { - self.keys.params + self.keys.params() } pub fn view(&self) -> FrostView { @@ -79,12 +82,20 @@ fn nonce_transcript() -> T { T::new(b"FROST_nonce_dleq") } +#[derive(Zeroize)] pub(crate) struct PreprocessPackage { pub(crate) nonces: Vec<[C::F; 2]>, + #[zeroize(skip)] pub(crate) commitments: Vec>, pub(crate) addendum: Vec, } +impl Drop for PreprocessPackage { + fn drop(&mut self) { + self.zeroize() + } +} + // This library unifies the preprocessing step with signing due to security concerns and to provide // a simpler UX fn preprocess>( @@ -122,10 +133,11 @@ fn preprocess>( // This could be further optimized with a multi-nonce proof. // See https://github.com/serai-dex/serai/issues/38 - for nonce in nonces { + for mut nonce in nonces { DLEqProof::prove(&mut *rng, &mut transcript, generators, nonce) .serialize(&mut serialized) .unwrap(); + nonce.zeroize(); } } @@ -190,7 +202,7 @@ fn sign_with_share>( t.append_message(b"commitment_E", commitments[1].to_bytes().as_ref()); }; - if *l == params.keys.params.i { + if *l == params.keys.params().i { for nonce_commitments in &our_preprocess.commitments { for commitments in nonce_commitments { transcript(params.algorithm.transcript(), *commitments); @@ -282,16 +294,15 @@ fn sign_with_share>( } } - let share = params.algorithm.sign_share( - ¶ms.view, - &Rs, - &our_preprocess - .nonces - .iter() - .map(|nonces| nonces[0] + (nonces[1] * B[¶ms.keys.params.i()].1)) - .collect::>(), - msg, - ); + let mut nonces = our_preprocess + .nonces + .iter() + .map(|nonces| nonces[0] + (nonces[1] * B[¶ms.keys.params().i()].1)) + .collect::>(); + + let share = params.algorithm.sign_share(¶ms.view, &Rs, &nonces, msg); + nonces.zeroize(); + Ok((Package { B, Rs, share }, share.to_repr().as_ref().to_vec())) } @@ -397,7 +408,7 @@ impl> AlgorithmMachine { /// Creates a new machine to generate a key for the specified curve in the specified multisig pub fn new( algorithm: A, - keys: Arc>, + keys: FrostKeys, included: &[u16], ) -> Result, FrostError> { Ok(AlgorithmMachine { params: Params::new(algorithm, keys, included)? }) diff --git a/crypto/frost/src/tests/curve.rs b/crypto/frost/src/tests/curve.rs index fc825273..ed6a37d1 100644 --- a/crypto/frost/src/tests/curve.rs +++ b/crypto/frost/src/tests/curve.rs @@ -4,18 +4,18 @@ use rand_core::{RngCore, CryptoRng}; use group::{ff::Field, Group}; -use crate::{Curve, FrostKeys, tests::key_gen}; +use crate::{Curve, FrostCore, tests::core_gen}; // Test generation of FROST keys fn key_generation(rng: &mut R) { // This alone verifies the verification shares and group key are agreed upon as expected - key_gen::<_, C>(rng); + core_gen::<_, C>(rng); } // Test serialization of generated keys fn keys_serialization(rng: &mut R) { - for (_, keys) in key_gen::<_, C>(rng) { - assert_eq!(&FrostKeys::::deserialize(&mut Cursor::new(keys.serialize())).unwrap(), &*keys); + for (_, keys) in core_gen::<_, C>(rng) { + assert_eq!(&FrostCore::::deserialize(&mut Cursor::new(keys.serialize())).unwrap(), &keys); } } @@ -38,7 +38,7 @@ pub fn test_curve(rng: &mut R) { } } - // Test FROST key generation and serialization of FrostKeys works as expected + // Test FROST key generation and serialization of FrostCore works as expected key_generation::<_, C>(rng); keys_serialization::<_, C>(rng); } diff --git a/crypto/frost/src/tests/mod.rs b/crypto/frost/src/tests/mod.rs index ba9d736d..b82af65c 100644 --- a/crypto/frost/src/tests/mod.rs +++ b/crypto/frost/src/tests/mod.rs @@ -1,11 +1,11 @@ -use std::{io::Cursor, sync::Arc, collections::HashMap}; +use std::{io::Cursor, collections::HashMap}; use rand_core::{RngCore, CryptoRng}; use group::ff::Field; use crate::{ - Curve, FrostParams, FrostKeys, lagrange, + Curve, FrostParams, FrostCore, FrostKeys, lagrange, key_gen::KeyGenMachine, algorithm::Algorithm, sign::{PreprocessMachine, SignMachine, SignatureMachine, AlgorithmMachine}, @@ -32,7 +32,7 @@ pub fn clone_without( res } -pub fn key_gen(rng: &mut R) -> HashMap>> { +pub fn core_gen(rng: &mut R) -> HashMap> { let mut machines = HashMap::new(); let mut commitments = HashMap::new(); for i in 1 ..= PARTICIPANTS { @@ -82,11 +82,15 @@ pub fn key_gen(rng: &mut R) -> HashMap>() } +pub fn key_gen(rng: &mut R) -> HashMap> { + core_gen(rng).drain().map(|(i, core)| (i, FrostKeys::new(core))).collect() +} + pub fn recover(keys: &HashMap>) -> C::F { let first = keys.values().next().expect("no keys provided"); assert!(keys.len() >= first.params().t().into(), "not enough keys provided"); @@ -102,7 +106,7 @@ pub fn recover(keys: &HashMap>) -> C::F { pub fn algorithm_machines>( rng: &mut R, algorithm: A, - keys: &HashMap>>, + keys: &HashMap>, ) -> HashMap> { let mut included = vec![]; while included.len() < usize::from(keys[&1].params().t()) { diff --git a/crypto/frost/src/tests/schnorr.rs b/crypto/frost/src/tests/schnorr.rs index da59257d..4767621a 100644 --- a/crypto/frost/src/tests/schnorr.rs +++ b/crypto/frost/src/tests/schnorr.rs @@ -1,4 +1,4 @@ -use std::{marker::PhantomData, sync::Arc, collections::HashMap}; +use std::{marker::PhantomData, collections::HashMap}; use rand_core::{RngCore, CryptoRng}; @@ -78,7 +78,7 @@ pub(crate) fn core_batch_verify(rng: &mut R) { fn sign_core( rng: &mut R, group_key: C::G, - keys: &HashMap>>, + keys: &HashMap>, ) { const MESSAGE: &[u8] = b"Hello, World!"; @@ -109,7 +109,7 @@ fn sign_with_offset(rng: &mut R) { let offset = C::hash_to_F(b"FROST Test sign_with_offset", b"offset"); for i in 1 ..= u16::try_from(keys.len()).unwrap() { - keys.insert(i, Arc::new(keys[&i].offset(offset))); + keys.insert(i, keys[&i].offset(offset)); } let offset_key = group_key + (C::GENERATOR * offset); diff --git a/crypto/frost/src/tests/vectors.rs b/crypto/frost/src/tests/vectors.rs index 67e40bb8..a0582503 100644 --- a/crypto/frost/src/tests/vectors.rs +++ b/crypto/frost/src/tests/vectors.rs @@ -1,4 +1,4 @@ -use std::{io::Cursor, sync::Arc, collections::HashMap}; +use std::{io::Cursor, collections::HashMap}; use rand_core::{RngCore, CryptoRng}; @@ -6,7 +6,7 @@ use group::{ff::PrimeField, GroupEncoding}; use crate::{ curve::Curve, - FrostKeys, + FrostCore, FrostKeys, algorithm::{Schnorr, Hram}, sign::{PreprocessPackage, SignMachine, SignatureMachine, AlgorithmMachine}, tests::{clone_without, curve::test_curve, schnorr::test_schnorr, recover}, @@ -48,13 +48,13 @@ fn vectors_to_multisig_keys(vectors: &Vectors) -> HashMap::deserialize(&mut Cursor::new(serialized)).unwrap(); + let these_keys = FrostCore::::deserialize(&mut Cursor::new(serialized)).unwrap(); assert_eq!(these_keys.params().t(), vectors.threshold); assert_eq!(usize::from(these_keys.params().n()), shares.len()); assert_eq!(these_keys.params().i(), i); assert_eq!(these_keys.secret_share(), shares[usize::from(i - 1)]); assert_eq!(&hex::encode(these_keys.group_key().to_bytes().as_ref()), vectors.group_key); - keys.insert(i, these_keys); + keys.insert(i, FrostKeys::new(these_keys)); } keys @@ -86,7 +86,7 @@ pub fn test_with_vectors>( *i, AlgorithmMachine::new( Schnorr::::new(), - Arc::new(keys[i].clone()), + keys[i].clone(), &vectors.included.to_vec().clone(), ) .unwrap(), diff --git a/crypto/multiexp/Cargo.toml b/crypto/multiexp/Cargo.toml index 67d89b92..d12c954b 100644 --- a/crypto/multiexp/Cargo.toml +++ b/crypto/multiexp/Cargo.toml @@ -9,6 +9,8 @@ keywords = ["multiexp", "ff", "group"] edition = "2021" [dependencies] +zeroize = { version = "1.3", features = ["zeroize_derive"] } + ff = "0.12" group = "0.12" diff --git a/crypto/multiexp/src/batch.rs b/crypto/multiexp/src/batch.rs index 04777150..63985fb2 100644 --- a/crypto/multiexp/src/batch.rs +++ b/crypto/multiexp/src/batch.rs @@ -1,17 +1,20 @@ use rand_core::{RngCore, CryptoRng}; +use zeroize::Zeroize; + use ff::{Field, PrimeFieldBits}; use group::Group; use crate::{multiexp, multiexp_vartime}; #[cfg(feature = "batch")] -pub struct BatchVerifier(Vec<(Id, Vec<(G::Scalar, G)>)>); +#[derive(Clone, Zeroize)] +pub struct BatchVerifier(Vec<(Id, Vec<(G::Scalar, G)>)>); #[cfg(feature = "batch")] -impl BatchVerifier +impl BatchVerifier where - ::Scalar: PrimeFieldBits, + ::Scalar: PrimeFieldBits + Zeroize, { pub fn new(capacity: usize) -> BatchVerifier { BatchVerifier(Vec::with_capacity(capacity)) @@ -39,10 +42,17 @@ where } #[must_use] - pub fn verify(&self) -> bool { - multiexp(&self.0.iter().flat_map(|pairs| pairs.1.iter()).cloned().collect::>()) - .is_identity() - .into() + pub fn verify_core(&self) -> bool { + let mut flat = self.0.iter().flat_map(|pairs| pairs.1.iter()).cloned().collect::>(); + let res = multiexp(&flat).is_identity().into(); + flat.zeroize(); + res + } + + pub fn verify(mut self) -> bool { + let res = self.verify_core(); + self.zeroize(); + res } #[must_use] @@ -75,12 +85,10 @@ where .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_with_vartime_blame(mut self) -> Result<(), Id> { + let res = if self.verify_core() { Ok(()) } else { Err(self.blame_vartime().unwrap()) }; + self.zeroize(); + res } pub fn verify_vartime_with_vartime_blame(&self) -> Result<(), Id> { diff --git a/crypto/multiexp/src/lib.rs b/crypto/multiexp/src/lib.rs index 9b92609e..a58b6857 100644 --- a/crypto/multiexp/src/lib.rs +++ b/crypto/multiexp/src/lib.rs @@ -1,3 +1,5 @@ +use zeroize::Zeroize; + use ff::PrimeFieldBits; use group::Group; @@ -24,13 +26,17 @@ where let mut groupings = vec![]; for pair in pairs { let p = groupings.len(); - let bits = pair.0.to_le_bits(); + let mut bits = pair.0.to_le_bits(); groupings.push(vec![0; (bits.len() + (w_usize - 1)) / w_usize]); - for (i, bit) in bits.into_iter().enumerate() { - let bit = bit as u8; + #[allow(unused_assignments)] + for (i, mut raw_bit) in bits.iter_mut().enumerate() { + let mut bit = *raw_bit as u8; debug_assert_eq!(bit | 1, 1); + *raw_bit = false; + groupings[p][i / w_usize] |= bit << (i % w_usize); + bit = 0; } } @@ -156,7 +162,7 @@ fn algorithm(len: usize) -> Algorithm { // Performs a multiexp, automatically selecting the optimal algorithm based on amount of pairs pub fn multiexp(pairs: &[(G::Scalar, G)]) -> G where - G::Scalar: PrimeFieldBits, + G::Scalar: PrimeFieldBits + Zeroize, { match algorithm(pairs.len()) { Algorithm::Null => Group::identity(), diff --git a/crypto/multiexp/src/pippenger.rs b/crypto/multiexp/src/pippenger.rs index e3879524..79d945cb 100644 --- a/crypto/multiexp/src/pippenger.rs +++ b/crypto/multiexp/src/pippenger.rs @@ -1,3 +1,5 @@ +use zeroize::Zeroize; + use ff::PrimeFieldBits; use group::Group; @@ -7,7 +9,7 @@ pub(crate) fn pippenger(pairs: &[(G::Scalar, G)], window: u8) -> G where G::Scalar: PrimeFieldBits, { - let bits = prep_bits(pairs, window); + let mut bits = prep_bits(pairs, window); let mut res = G::identity(); for n in (0 .. bits[0].len()).rev() { @@ -27,6 +29,7 @@ where } } + bits.zeroize(); res } diff --git a/crypto/multiexp/src/straus.rs b/crypto/multiexp/src/straus.rs index f10d5d61..a670cc74 100644 --- a/crypto/multiexp/src/straus.rs +++ b/crypto/multiexp/src/straus.rs @@ -1,3 +1,5 @@ +use zeroize::Zeroize; + use ff::PrimeFieldBits; use group::Group; @@ -5,9 +7,9 @@ use crate::{prep_bits, prep_tables}; pub(crate) fn straus(pairs: &[(G::Scalar, G)], window: u8) -> G where - G::Scalar: PrimeFieldBits, + G::Scalar: PrimeFieldBits + Zeroize, { - let groupings = prep_bits(pairs, window); + let mut groupings = prep_bits(pairs, window); let tables = prep_tables(pairs, window); let mut res = G::identity(); @@ -20,6 +22,8 @@ where res += tables[s][usize::from(groupings[s][b])]; } } + + groupings.zeroize(); res } diff --git a/crypto/multiexp/src/tests/mod.rs b/crypto/multiexp/src/tests/mod.rs index adad5302..b587d356 100644 --- a/crypto/multiexp/src/tests/mod.rs +++ b/crypto/multiexp/src/tests/mod.rs @@ -2,6 +2,8 @@ use std::time::Instant; use rand_core::OsRng; +use zeroize::Zeroize; + use ff::{Field, PrimeFieldBits}; use group::Group; @@ -13,7 +15,7 @@ use crate::{straus, pippenger, multiexp, multiexp_vartime}; #[allow(dead_code)] fn benchmark_internal(straus_bool: bool) where - G::Scalar: PrimeFieldBits, + G::Scalar: PrimeFieldBits + Zeroize, { let runs: usize = 20; @@ -81,7 +83,7 @@ where fn test_multiexp() where - G::Scalar: PrimeFieldBits, + G::Scalar: PrimeFieldBits + Zeroize, { let mut pairs = Vec::with_capacity(1000); let mut sum = G::identity(); diff --git a/processor/src/coin/mod.rs b/processor/src/coin/mod.rs index f40a537e..7a8ce22b 100644 --- a/processor/src/coin/mod.rs +++ b/processor/src/coin/mod.rs @@ -1,4 +1,4 @@ -use std::{marker::Send, sync::Arc}; +use std::marker::Send; use async_trait::async_trait; use thiserror::Error; @@ -57,7 +57,7 @@ pub trait Coin { async fn prepare_send( &self, - keys: Arc>, + keys: FrostKeys, transcript: RecommendedTranscript, height: usize, inputs: Vec, diff --git a/processor/src/coin/monero.rs b/processor/src/coin/monero.rs index 4a56244e..2a40c572 100644 --- a/processor/src/coin/monero.rs +++ b/processor/src/coin/monero.rs @@ -1,5 +1,3 @@ -use std::sync::Arc; - use async_trait::async_trait; use curve25519_dalek::scalar::Scalar; @@ -57,7 +55,7 @@ impl OutputTrait for Output { #[derive(Debug)] pub struct SignableTransaction( - Arc>, + FrostKeys, RecommendedTranscript, usize, MSignableTransaction, @@ -137,14 +135,14 @@ impl Coin for Monero { async fn get_outputs(&self, block: &Self::Block, key: dfg::EdwardsPoint) -> Vec { block .iter() - .flat_map(|tx| tx.scan(self.view_pair(key), true).not_locked()) + .flat_map(|tx| tx.scan(&self.view_pair(key), true).not_locked()) .map(Output::from) .collect() } async fn prepare_send( &self, - keys: Arc>, + keys: FrostKeys, transcript: RecommendedTranscript, height: usize, mut inputs: Vec, @@ -177,7 +175,7 @@ impl Coin for Monero { .clone() .multisig( &self.rpc, - (*transaction.0).clone(), + transaction.0.clone(), transaction.1.clone(), transaction.2, included.to_vec(), @@ -235,7 +233,7 @@ impl Coin for Monero { .await .unwrap() .swap_remove(0) - .scan(self.empty_view_pair(), false) + .scan(&self.empty_view_pair(), false) .ignore_timelock(); let amount = outputs[0].commitment.amount; diff --git a/processor/src/tests/mod.rs b/processor/src/tests/mod.rs index 9d7d559e..c27c373a 100644 --- a/processor/src/tests/mod.rs +++ b/processor/src/tests/mod.rs @@ -71,7 +71,7 @@ async fn test_send(coin: C, fee: C::Fee) { for i in 1 ..= threshold { let mut wallet = Wallet::new(MemCoinDb::new(), coin.clone()); wallet.acknowledge_height(0, height); - wallet.add_keys(&WalletKeys::new(Arc::try_unwrap(keys.remove(&i).take().unwrap()).unwrap(), 0)); + wallet.add_keys(&WalletKeys::new(keys.remove(&i).unwrap(), 0)); wallets.push(wallet); } diff --git a/processor/src/wallet.rs b/processor/src/wallet.rs index 97735fd4..0fdb06c7 100644 --- a/processor/src/wallet.rs +++ b/processor/src/wallet.rs @@ -1,4 +1,4 @@ -use std::{sync::Arc, collections::HashMap}; +use std::collections::HashMap; use rand_core::OsRng; @@ -203,7 +203,7 @@ fn select_inputs_outputs( pub struct Wallet { db: D, coin: C, - keys: Vec<(Arc>, Vec)>, + keys: Vec<(FrostKeys, Vec)>, pending: Vec<(usize, FrostKeys)>, } @@ -249,7 +249,7 @@ impl Wallet { //if b < self.pending[k].0 { //} else if b == self.pending[k].0 { if b <= self.pending[k].0 { - self.keys.push((Arc::new(self.pending.swap_remove(k).1), vec![])); + self.keys.push((self.pending.swap_remove(k).1, vec![])); } else { k += 1; }