diff --git a/coins/monero/src/clsag/multisig.rs b/coins/monero/src/clsag/multisig.rs index 5b0b8cc8..220a8688 100644 --- a/coins/monero/src/clsag/multisig.rs +++ b/coins/monero/src/clsag/multisig.rs @@ -16,13 +16,12 @@ use monero::util::ringct::{Key, Clsag}; use group::Group; use transcript::Transcript as TranscriptTrait; -use frost::{Curve, FrostError, algorithm::Algorithm, MultisigView}; +use frost::{FrostError, algorithm::Algorithm, MultisigView}; use dalek_ff_group as dfg; use crate::{ hash_to_point, - frost::{Transcript, MultisigError, Ed25519, DLEqProof}, - key_image, + frost::{Transcript, MultisigError, Ed25519, DLEqProof, read_dleq}, clsag::{Input, sign_core, verify} }; @@ -80,8 +79,9 @@ struct Interim { pub struct Multisig { transcript: Transcript, + H: EdwardsPoint, + // Merged here as CLSAG needs it, passing it would be a mess, yet having it beforehand requires a round image: EdwardsPoint, - commitments_H: Vec, AH: (dfg::EdwardsPoint, dfg::EdwardsPoint), details: Rc>>, @@ -100,8 +100,8 @@ impl Multisig { Multisig { transcript, + H: EdwardsPoint::identity(), image: EdwardsPoint::identity(), - commitments_H: vec![], AH: (dfg::EdwardsPoint::identity(), dfg::EdwardsPoint::identity()), details, @@ -134,24 +134,21 @@ impl Algorithm for Multisig { type Signature = (Clsag, EdwardsPoint); fn preprocess_addendum( + &mut self, rng: &mut R, view: &MultisigView, nonces: &[dfg::Scalar; 2] ) -> Vec { - let (share, proof) = key_image::generate_share(rng, view); - - #[allow(non_snake_case)] - let H = hash_to_point(&view.group_key().0); - #[allow(non_snake_case)] - let nH = (nonces[0].0 * H, nonces[1].0 * H); + self.H = hash_to_point(&view.group_key().0); let mut serialized = Vec::with_capacity(Multisig::serialized_len()); - serialized.extend(share.compress().to_bytes()); - serialized.extend(nH.0.compress().to_bytes()); - serialized.extend(nH.1.compress().to_bytes()); - serialized.extend(&DLEqProof::prove(rng, &nonces[0].0, &H, &nH.0).serialize()); - serialized.extend(&DLEqProof::prove(rng, &nonces[1].0, &H, &nH.1).serialize()); - serialized.extend(proof); + serialized.extend((view.secret_share().0 * self.H).compress().to_bytes()); + serialized.extend(DLEqProof::prove(rng, &view.secret_share().0, &self.H).serialize()); + + serialized.extend((nonces[0].0 * self.H).compress().to_bytes()); + serialized.extend(&DLEqProof::prove(rng, &nonces[0].0, &self.H).serialize()); + serialized.extend((nonces[1].0 * self.H).compress().to_bytes()); + serialized.extend(&DLEqProof::prove(rng, &nonces[1].0, &self.H).serialize()); serialized } @@ -167,49 +164,36 @@ impl Algorithm for Multisig { Err(FrostError::InvalidCommitmentQuantity(l, 9, serialized.len() / 32))?; } - if self.commitments_H.len() == 0 { + if self.AH.0.is_identity().into() { self.transcript.domain_separate(b"CLSAG"); self.input().transcript(&mut self.transcript); self.transcript.append_message(b"mask", &self.mask().to_bytes()); self.transcript.append_message(b"message", &self.msg()); } - let (share, serialized) = key_image::verify_share(view, l, serialized).map_err(|_| FrostError::InvalidShare(l))?; + let share = read_dleq( + serialized, + 0, + &self.H, + l, + &view.verification_share(l).0 + ).map_err(|_| FrostError::InvalidCommitment(l))?.0; // Given the fact there's only ever one possible value for this, this may technically not need // to be committed to. If signing a TX, it'll be double committed to thanks to the message // It doesn't hurt to have though and ensures security boundaries are well formed self.transcript.append_message(b"image_share", &share.compress().to_bytes()); self.image += share; - let alt = &hash_to_point(&view.group_key().0); - // Uses the same format FROST does for the expected commitments (nonce * G where this is nonce * H) // Given this is guaranteed to match commitments, which FROST commits to, this also technically // doesn't need to be committed to if a canonical serialization is guaranteed // It, again, doesn't hurt to include and ensures security boundaries are well formed self.transcript.append_message(b"participant", &u16::try_from(l).unwrap().to_be_bytes()); - self.transcript.append_message(b"commitments_H", &serialized[0 .. 64]); - #[allow(non_snake_case)] - let H = ( - ::G_from_slice(&serialized[0 .. 32]).map_err(|_| FrostError::InvalidCommitment(l))?, - ::G_from_slice(&serialized[32 .. 64]).map_err(|_| FrostError::InvalidCommitment(l))? - ); - - DLEqProof::deserialize(&serialized[64 .. 128]).ok_or(FrostError::InvalidCommitment(l))?.verify( - &alt, - &commitments[0], - &H.0 - ).map_err(|_| FrostError::InvalidCommitment(l))?; - - DLEqProof::deserialize(&serialized[128 .. 192]).ok_or(FrostError::InvalidCommitment(l))?.verify( - &alt, - &commitments[1], - &H.1 - ).map_err(|_| FrostError::InvalidCommitment(l))?; - - self.AH.0 += H.0; - self.AH.1 += H.1; + self.transcript.append_message(b"commitment_D_H", &serialized[0 .. 32]); + self.AH.0 += read_dleq(serialized, 96, &self.H, l, &commitments[0]).map_err(|_| FrostError::InvalidCommitment(l))?; + self.transcript.append_message(b"commitment_E_H", &serialized[0 .. 32]); + self.AH.1 += read_dleq(serialized, 192, &self.H, l, &commitments[1]).map_err(|_| FrostError::InvalidCommitment(l))?; Ok(()) } diff --git a/coins/monero/src/frost.rs b/coins/monero/src/frost.rs index bdf2f336..e96a4dd8 100644 --- a/coins/monero/src/frost.rs +++ b/coins/monero/src/frost.rs @@ -12,7 +12,6 @@ use curve25519_dalek::{ edwards::EdwardsPoint as DPoint }; - use ff::PrimeField; use group::Group; @@ -29,7 +28,7 @@ pub enum MultisigError { #[error("internal error ({0})")] InternalError(String), #[error("invalid discrete log equality proof")] - InvalidDLEqProof, + InvalidDLEqProof(usize), #[error("invalid key image {0}")] InvalidKeyImage(usize) } @@ -88,15 +87,17 @@ impl Curve for Ed25519 { } fn G_from_slice(slice: &[u8]) -> Result { - let point = dfg::CompressedEdwardsY::new( - slice.try_into().map_err(|_| CurveError::InvalidLength(32, slice.len()))? - ).decompress(); + let bytes = slice.try_into().map_err(|_| CurveError::InvalidLength(32, slice.len()))?; + let point = dfg::CompressedEdwardsY::new(bytes).decompress(); - if point.is_some() { - let point = point.unwrap(); + if let Some(point) = point { // Ban torsioned points if !point.is_torsion_free() { - Err(CurveError::InvalidPoint)? + Err(CurveError::InvalidPoint)?; + } + // Ban point which weren't canonically encoded + if point.compress().to_bytes() != bytes { + Err(CurveError::InvalidPoint)?; } Ok(point) } else { @@ -113,7 +114,7 @@ impl Curve for Ed25519 { } } -// Used to prove legitimacy in several locations +// Used to prove legitimacy of key images and nonces which both involve other basepoints #[derive(Clone)] pub struct DLEqProof { s: DScalar, @@ -125,19 +126,23 @@ impl DLEqProof { pub fn prove( rng: &mut R, secret: &DScalar, - H: &DPoint, - alt: &DPoint + H: &DPoint ) -> DLEqProof { let r = random_scalar(rng); - let R1 = &DTable * &r; - let R2 = r * H; + let rG = &DTable * &r; + let rH = r * H; + // TODO: Should this use a transcript? let c = dfg::Scalar::from_hash( Blake2b512::new() - .chain(R1.compress().to_bytes()) - .chain(R2.compress().to_bytes()) + // Doesn't include G which is constant, does include H which isn't + .chain(H.compress().to_bytes()) .chain((secret * &DTable).compress().to_bytes()) - .chain(alt.compress().to_bytes()) + // We can frequently save a scalar mul if we accept this as an arg, yet it opens room for + // ambiguity not worth having + .chain((secret * H).compress().to_bytes()) + .chain(rG.compress().to_bytes()) + .chain(rH.compress().to_bytes()) ).0; let s = r + (c * secret); @@ -147,26 +152,28 @@ impl DLEqProof { pub fn verify( &self, H: &DPoint, - primary: &DPoint, - alt: &DPoint -) -> Result<(), MultisigError> { + l: usize, + sG: &DPoint, + sH: &DPoint + ) -> Result<(), MultisigError> { let s = self.s; let c = self.c; - let R1 = (&s * &DTable) - (c * primary); - let R2 = (s * H) - (c * alt); + let rG = (&s * &DTable) - (c * sG); + let rH = (s * H) - (c * sH); let expected_c = dfg::Scalar::from_hash( Blake2b512::new() - .chain(R1.compress().to_bytes()) - .chain(R2.compress().to_bytes()) - .chain(primary.compress().to_bytes()) - .chain(alt.compress().to_bytes()) + .chain(H.compress().to_bytes()) + .chain(sG.compress().to_bytes()) + .chain(sH.compress().to_bytes()) + .chain(rG.compress().to_bytes()) + .chain(rH.compress().to_bytes()) ).0; - // Take the opportunity to ensure a lack of torsion in key images/randomness commitments - if (!primary.is_torsion_free()) || (!alt.is_torsion_free()) || (c != expected_c) { - Err(MultisigError::InvalidDLEqProof)?; + // Take the opportunity to ensure a lack of torsion in key images/nonce commitments + if c != expected_c { + Err(MultisigError::InvalidDLEqProof(l))?; } Ok(()) @@ -196,3 +203,29 @@ impl DLEqProof { ) } } + +#[allow(non_snake_case)] +pub fn read_dleq( + serialized: &[u8], + start: usize, + H: &DPoint, + l: usize, + sG: &DPoint +) -> Result { + // Not using G_from_slice here would enable non-canonical points and break blame + let other = ::G_from_slice( + &serialized[(start + 0) .. (start + 32)] + ).map_err(|_| MultisigError::InvalidDLEqProof(l))?; + + let proof = DLEqProof::deserialize( + &serialized[(start + 32) .. (start + 96)] + ).ok_or(MultisigError::InvalidDLEqProof(l))?; + proof.verify( + H, + l, + sG, + &other + ).map_err(|_| MultisigError::InvalidDLEqProof(l))?; + + Ok(other) +} diff --git a/coins/monero/src/key_image/mod.rs b/coins/monero/src/key_image/mod.rs deleted file mode 100644 index 6140772c..00000000 --- a/coins/monero/src/key_image/mod.rs +++ /dev/null @@ -1,16 +0,0 @@ -use curve25519_dalek::{ - constants::ED25519_BASEPOINT_TABLE, - scalar::Scalar, - edwards::EdwardsPoint -}; - -use crate::hash_to_point; - -#[cfg(feature = "multisig")] -mod multisig; -#[cfg(feature = "multisig")] -pub use crate::key_image::multisig::{generate_share, verify_share}; - -pub fn generate(secret: &Scalar) -> EdwardsPoint { - secret * hash_to_point(&(secret * &ED25519_BASEPOINT_TABLE)) -} diff --git a/coins/monero/src/key_image/multisig.rs b/coins/monero/src/key_image/multisig.rs deleted file mode 100644 index abb1d77e..00000000 --- a/coins/monero/src/key_image/multisig.rs +++ /dev/null @@ -1,48 +0,0 @@ -use rand_core::{RngCore, CryptoRng}; - -use curve25519_dalek::edwards::{EdwardsPoint, CompressedEdwardsY}; - -use frost::MultisigView; - -use crate::{hash_to_point, frost::{MultisigError, Ed25519, DLEqProof}}; - -#[allow(non_snake_case)] -pub fn generate_share( - rng: &mut R, - view: &MultisigView -) -> (EdwardsPoint, Vec) { - let H = hash_to_point(&view.group_key().0); - let image = view.secret_share().0 * H; - // Includes a proof. Since: - // sum(lagranged_secrets) = group_private - // group_private * G = output_key - // group_private * H = key_image - // Then sum(lagranged_secrets * H) = key_image - // lagranged_secret * G is known. lagranged_secret * H is being sent - // Any discrete log equality proof confirms the same secret was used, - // forming a valid key_image share - (image, DLEqProof::prove(rng, &view.secret_share().0, &H, &image).serialize()) -} - -pub fn verify_share( - view: &MultisigView, - l: usize, - share: &[u8] -) -> Result<(EdwardsPoint, Vec), MultisigError> { - if share.len() < 96 { - Err(MultisigError::InvalidDLEqProof)?; - } - let image = CompressedEdwardsY( - share[0 .. 32].try_into().unwrap() - ).decompress().ok_or(MultisigError::InvalidKeyImage(l))?; - let proof = DLEqProof::deserialize( - &share[(share.len() - 64) .. share.len()] - ).ok_or(MultisigError::InvalidDLEqProof)?; - proof.verify( - &hash_to_point(&view.group_key().0), - &view.verification_share(l), - &image - ).map_err(|_| MultisigError::InvalidKeyImage(l))?; - - Ok((image, share[32 .. (share.len() - 64)].to_vec())) -} diff --git a/coins/monero/src/lib.rs b/coins/monero/src/lib.rs index d85b5f36..7dd4a606 100644 --- a/coins/monero/src/lib.rs +++ b/coins/monero/src/lib.rs @@ -15,7 +15,6 @@ use monero::util::key::H; #[cfg(feature = "multisig")] pub mod frost; -pub mod key_image; pub mod bulletproofs; pub mod clsag; @@ -76,3 +75,7 @@ pub fn hash_to_point(point: &EdwardsPoint) -> EdwardsPoint { unsafe { c_hash_to_point(bytes.as_mut_ptr()); } CompressedEdwardsY::from_slice(&bytes).decompress().unwrap() } + +pub fn generate_key_image(secret: &Scalar) -> EdwardsPoint { + secret * hash_to_point(&(secret * &ED25519_BASEPOINT_TABLE)) +} diff --git a/coins/monero/src/transaction/mod.rs b/coins/monero/src/transaction/mod.rs index 49ed537b..6af82896 100644 --- a/coins/monero/src/transaction/mod.rs +++ b/coins/monero/src/transaction/mod.rs @@ -31,7 +31,7 @@ use crate::{ Commitment, random_scalar, hash, hash_to_scalar, - key_image, bulletproofs, clsag, + generate_key_image, bulletproofs, clsag, rpc::{Rpc, RpcError} }; #[cfg(feature = "multisig")] @@ -215,7 +215,7 @@ async fn prepare_inputs( for (i, input) in inputs.iter().enumerate() { signable.push(( spend + input.key_offset, - key_image::generate(&(spend + input.key_offset)), + generate_key_image(&(spend + input.key_offset)), clsag::Input::new( input.commitment, decoys[i].clone() diff --git a/coins/monero/src/transaction/multisig.rs b/coins/monero/src/transaction/multisig.rs index e1b577e2..bfac2c8d 100644 --- a/coins/monero/src/transaction/multisig.rs +++ b/coins/monero/src/transaction/multisig.rs @@ -3,7 +3,7 @@ use std::{rc::Rc, cell::RefCell}; use rand_core::{RngCore, CryptoRng, SeedableRng}; use rand_chacha::ChaCha12Rng; -use curve25519_dalek::{scalar::Scalar, edwards::{EdwardsPoint, CompressedEdwardsY}}; +use curve25519_dalek::{traits::Identity, scalar::Scalar, edwards::{EdwardsPoint, CompressedEdwardsY}}; use monero::{ Hash, VarInt, @@ -17,7 +17,7 @@ use frost::{FrostError, MultisigKeys, MultisigParams, sign::{State, StateMachine use crate::{ frost::{Transcript, Ed25519}, - random_scalar, key_image, bulletproofs, clsag, + random_scalar, bulletproofs, clsag, rpc::Rpc, transaction::{TransactionError, SignableTransaction, decoys::{self, Decoys}} }; @@ -49,6 +49,7 @@ impl SignableTransaction { included: &[usize] ) -> Result { let mut our_images = vec![]; + our_images.resize(self.inputs.len(), EdwardsPoint::identity()); let mut inputs = vec![]; inputs.resize(self.inputs.len(), Rc::new(RefCell::new(None))); let msg = Rc::new(RefCell::new(None)); @@ -91,13 +92,6 @@ impl SignableTransaction { ).await.map_err(|e| TransactionError::RpcError(e))?; for (i, input) in self.inputs.iter().enumerate() { - let keys = keys.offset(dalek_ff_group::Scalar(input.key_offset)); - let (image, _) = key_image::generate_share( - rng, - &keys.view(included).map_err(|e| TransactionError::FrostError(e))? - ); - our_images.push(image); - clsags.push( AlgorithmMachine::new( clsag::Multisig::new( @@ -105,7 +99,7 @@ impl SignableTransaction { inputs[i].clone(), msg.clone() ).map_err(|e| TransactionError::MultisigError(e))?, - Rc::new(keys), + Rc::new(keys.offset(dalek_ff_group::Scalar(input.key_offset))), included ).map_err(|e| TransactionError::FrostError(e))? ); @@ -145,8 +139,10 @@ impl StateMachine for TransactionMachine { // Iterate over each CLSAG calling preprocess let mut serialized = vec![]; - for clsag in self.clsags.iter_mut() { - serialized.extend(&clsag.preprocess(rng)?); + for (i, clsag) in self.clsags.iter_mut().enumerate() { + let preprocess = clsag.preprocess(rng)?; + self.our_images[i] += CompressedEdwardsY(preprocess[0 .. 32].try_into().unwrap()).decompress().unwrap(); + serialized.extend(&preprocess); } if self.leader { diff --git a/coins/monero/tests/clsag.rs b/coins/monero/tests/clsag.rs index 75d5a324..f60c49ed 100644 --- a/coins/monero/tests/clsag.rs +++ b/coins/monero/tests/clsag.rs @@ -7,7 +7,7 @@ use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar}; use monero::VarInt; -use monero_serai::{random_scalar, Commitment, transaction::decoys::Decoys, key_image, clsag}; +use monero_serai::{Commitment, random_scalar, generate_key_image, transaction::decoys::Decoys, clsag}; #[cfg(feature = "multisig")] use monero_serai::frost::{MultisigError, Transcript}; @@ -42,7 +42,7 @@ fn clsag() { ring.push([&dest * &ED25519_BASEPOINT_TABLE, Commitment::new(mask, amount).calculate()]); } - let image = key_image::generate(&secrets[0]); + let image = generate_key_image(&secrets[0]); let (clsag, pseudo_out) = clsag::sign( &mut OsRng, &vec![( diff --git a/coins/monero/tests/key_image.rs b/coins/monero/tests/key_image.rs deleted file mode 100644 index 0fa1c8d6..00000000 --- a/coins/monero/tests/key_image.rs +++ /dev/null @@ -1,51 +0,0 @@ -#![cfg(feature = "multisig")] - -use rand::{RngCore, rngs::OsRng}; - -use curve25519_dalek::{traits::Identity, edwards::EdwardsPoint}; - -use monero_serai::key_image; - -mod frost; -use crate::frost::{THRESHOLD, PARTICIPANTS, generate_keys}; - -#[test] -fn key_image() { - let (keys, group_private) = generate_keys(); - let image = key_image::generate(&group_private); - - let mut included = (1 ..= PARTICIPANTS).into_iter().collect::>(); - while included.len() > THRESHOLD { - included.swap_remove((OsRng.next_u64() as usize) % included.len()); - } - included.sort(); - - let mut views = vec![]; - let mut shares = vec![]; - for i in 1 ..= PARTICIPANTS { - if included.contains(&i) { - // If they were included, include their view - views.push(keys[i - 1].view(&included).unwrap()); - let share = key_image::generate_share(&mut OsRng, &views[i - 1]); - let mut serialized = share.0.compress().to_bytes().to_vec(); - serialized.extend(b"abc"); - serialized.extend(&share.1); - shares.push(serialized); - } else { - // If they weren't included, include dummy data to fill the Vec - // Uses the view of someone actually included as Params::new verifies inclusion - views.push(keys[included[0] - 1].view(&included).unwrap()); - shares.push(vec![]); - } - } - - for i in &included { - let mut multi_image = EdwardsPoint::identity(); - for l in &included { - let share = key_image::verify_share(&views[i - 1], *l, &shares[l - 1]).unwrap(); - assert_eq!(share.1, b"abc"); - multi_image += share.0; - } - assert_eq!(image, multi_image); - } -} diff --git a/crypto/dalek-ff-group/src/lib.rs b/crypto/dalek-ff-group/src/lib.rs index f2d5588b..ff0b2bc5 100644 --- a/crypto/dalek-ff-group/src/lib.rs +++ b/crypto/dalek-ff-group/src/lib.rs @@ -13,7 +13,7 @@ pub use curve25519_dalek as dalek; use dalek::{ constants, - traits::Identity, + traits::{Identity, IsIdentity}, scalar::Scalar as DScalar, edwards::{ EdwardsPoint as DPoint, @@ -248,7 +248,7 @@ impl Group for EdwardsPoint { fn random(mut _rng: impl RngCore) -> Self { unimplemented!() } fn identity() -> Self { Self(DPoint::identity()) } fn generator() -> Self { ED25519_BASEPOINT_POINT } - fn is_identity(&self) -> Choice { unimplemented!() } + fn is_identity(&self) -> Choice { (self.0.is_identity() as u8).into() } fn double(&self) -> Self { *self + self } } diff --git a/crypto/frost/src/algorithm.rs b/crypto/frost/src/algorithm.rs index f64e075c..6c5f2249 100644 --- a/crypto/frost/src/algorithm.rs +++ b/crypto/frost/src/algorithm.rs @@ -18,6 +18,7 @@ pub trait Algorithm: Clone { /// Generate an addendum to FROST"s preprocessing stage fn preprocess_addendum( + &mut self, rng: &mut R, params: &MultisigView, nonces: &[C::F; 2], @@ -119,6 +120,7 @@ impl> Algorithm for Schnorr { } fn preprocess_addendum( + &mut self, _: &mut R, _: &MultisigView, _: &[C::F; 2], diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 91faebd1..e64d3e06 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -104,7 +104,7 @@ fn preprocess>( serialized.extend(&C::G_to_bytes(&commitments[1])); serialized.extend( - &A::preprocess_addendum( + ¶ms.algorithm.preprocess_addendum( rng, ¶ms.view, &nonces