diff --git a/coins/monero/src/bin/reserialize_chain.rs b/coins/monero/src/bin/reserialize_chain.rs index c850b4ed..9f55073a 100644 --- a/coins/monero/src/bin/reserialize_chain.rs +++ b/coins/monero/src/bin/reserialize_chain.rs @@ -129,7 +129,9 @@ mod binaries { // Accordingly, making sure our signature_hash algorithm is correct is great, and further // making sure the verification functions are valid is appreciated match tx.rct_signatures.prunable { - RctPrunable::Null | RctPrunable::MlsagBorromean { .. } => {} + RctPrunable::Null | + RctPrunable::AggregateMlsagBorromean { .. } | + RctPrunable::MlsagBorromean { .. } => {} RctPrunable::MlsagBulletproofs { bulletproofs, .. } => { assert!(bulletproofs.batch_verify( &mut rand_core::OsRng, diff --git a/coins/monero/src/block.rs b/coins/monero/src/block.rs index f923f5f4..2bced983 100644 --- a/coins/monero/src/block.rs +++ b/coins/monero/src/block.rs @@ -17,8 +17,8 @@ const EXISTING_BLOCK_HASH_202612: [u8; 32] = #[derive(Clone, PartialEq, Eq, Debug)] pub struct BlockHeader { - pub major_version: u64, - pub minor_version: u64, + pub major_version: u8, + pub minor_version: u8, pub timestamp: u64, pub previous: [u8; 32], pub nonce: u32, @@ -68,7 +68,7 @@ impl Block { pub fn write(&self, w: &mut W) -> io::Result<()> { self.header.write(w)?; self.miner_tx.write(w)?; - write_varint(&self.txs.len().try_into().unwrap(), w)?; + write_varint(&self.txs.len(), w)?; for tx in &self.txs { w.write_all(tx)?; } @@ -79,20 +79,27 @@ impl Block { merkle_root(self.miner_tx.hash(), &self.txs) } - fn serialize_hashable(&self) -> Vec { + /// Serialize the block as required for the proof of work hash. + /// + /// This is distinct from the serialization required for the block hash. To get the block hash, + /// use the [`Block::hash`] function. + pub fn serialize_hashable(&self) -> Vec { let mut blob = self.header.serialize(); blob.extend_from_slice(&self.tx_merkle_root()); write_varint(&(1 + u64::try_from(self.txs.len()).unwrap()), &mut blob).unwrap(); - let mut out = Vec::with_capacity(8 + blob.len()); - write_varint(&u64::try_from(blob.len()).unwrap(), &mut out).unwrap(); - out.append(&mut blob); - - out + blob } pub fn hash(&self) -> [u8; 32] { - let hash = hash(&self.serialize_hashable()); + let mut hashable = self.serialize_hashable(); + // Monero pre-appends a VarInt of the block hashing blobs length before getting the block hash + // but doesn't do this when getting the proof of work hash :) + let mut hashing_blob = Vec::with_capacity(8 + hashable.len()); + write_varint(&u64::try_from(hashable.len()).unwrap(), &mut hashing_blob).unwrap(); + hashing_blob.append(&mut hashable); + + let hash = hash(&hashing_blob); if hash == CORRECT_BLOCK_HASH_202612 { return EXISTING_BLOCK_HASH_202612; }; @@ -110,7 +117,7 @@ impl Block { Ok(Block { header: BlockHeader::read(r)?, miner_tx: Transaction::read(r)?, - txs: (0 .. read_varint(r)?).map(|_| read_bytes(r)).collect::>()?, + txs: (0_usize .. read_varint(r)?).map(|_| read_bytes(r)).collect::>()?, }) } } diff --git a/coins/monero/src/lib.rs b/coins/monero/src/lib.rs index db020547..a33e4049 100644 --- a/coins/monero/src/lib.rs +++ b/coins/monero/src/lib.rs @@ -23,6 +23,12 @@ mod merkle; mod serialize; use serialize::{read_byte, read_u16}; +/// UnreducedScalar struct with functionality for recovering incorrectly reduced scalars. +mod unreduced_scalar; + +/// Ring Signature structs and functionality. +pub mod ring_signatures; + /// RingCT structs and functionality. pub mod ringct; use ringct::RctType; diff --git a/coins/monero/src/ring_signatures.rs b/coins/monero/src/ring_signatures.rs new file mode 100644 index 00000000..72d30b0e --- /dev/null +++ b/coins/monero/src/ring_signatures.rs @@ -0,0 +1,72 @@ +use std_shims::{ + io::{self, *}, + vec::Vec, +}; + +use zeroize::Zeroize; + +use curve25519_dalek::{EdwardsPoint, Scalar}; + +use monero_generators::hash_to_point; + +use crate::{serialize::*, hash_to_scalar}; + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize)] +pub struct Signature { + c: Scalar, + r: Scalar, +} + +impl Signature { + pub fn write(&self, w: &mut W) -> io::Result<()> { + write_scalar(&self.c, w)?; + write_scalar(&self.r, w)?; + Ok(()) + } + + pub fn read(r: &mut R) -> io::Result { + Ok(Signature { c: read_scalar(r)?, r: read_scalar(r)? }) + } +} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize)] +pub struct RingSignature { + sigs: Vec, +} + +impl RingSignature { + pub fn write(&self, w: &mut W) -> io::Result<()> { + for sig in &self.sigs { + sig.write(w)?; + } + Ok(()) + } + + pub fn read(members: usize, r: &mut R) -> io::Result { + Ok(RingSignature { sigs: read_raw_vec(Signature::read, members, r)? }) + } + + pub fn verify(&self, msg: &[u8; 32], ring: &[EdwardsPoint], key_image: &EdwardsPoint) -> bool { + if ring.len() != self.sigs.len() { + return false; + } + + let mut buf = Vec::with_capacity(32 + (32 * 2 * ring.len())); + buf.extend_from_slice(msg); + + let mut sum = Scalar::ZERO; + + for (ring_member, sig) in ring.iter().zip(&self.sigs) { + #[allow(non_snake_case)] + let Li = EdwardsPoint::vartime_double_scalar_mul_basepoint(&sig.c, ring_member, &sig.r); + buf.extend_from_slice(Li.compress().as_bytes()); + #[allow(non_snake_case)] + let Ri = (sig.r * hash_to_point(ring_member.compress().to_bytes())) + (sig.c * key_image); + buf.extend_from_slice(Ri.compress().as_bytes()); + + sum += sig.c; + } + + sum == hash_to_scalar(&buf) + } +} diff --git a/coins/monero/src/ringct/borromean.rs b/coins/monero/src/ringct/borromean.rs index 3a5aa35f..215b3394 100644 --- a/coins/monero/src/ringct/borromean.rs +++ b/coins/monero/src/ringct/borromean.rs @@ -1,73 +1,63 @@ use core::fmt::Debug; use std_shims::io::{self, Read, Write}; -use curve25519_dalek::edwards::EdwardsPoint; -#[cfg(feature = "experimental")] -use curve25519_dalek::{traits::Identity, scalar::Scalar}; +use curve25519_dalek::{traits::Identity, Scalar, EdwardsPoint}; -#[cfg(feature = "experimental")] use monero_generators::H_pow_2; -#[cfg(feature = "experimental")] -use crate::hash_to_scalar; -use crate::serialize::*; + +use crate::{hash_to_scalar, unreduced_scalar::UnreducedScalar, serialize::*}; /// 64 Borromean ring signatures. /// -/// This type keeps the data as raw bytes as Monero has some transactions with unreduced scalars in -/// this field. While we could use `from_bytes_mod_order`, we'd then not be able to encode this -/// back into it's original form. -/// -/// Those scalars also have a custom reduction algorithm... +/// s0 and s1 are stored as `UnreducedScalar`s due to Monero not requiring they were reduced. +/// `UnreducedScalar` preserves their original byte encoding and implements a custom reduction +/// algorithm which was in use. #[derive(Clone, PartialEq, Eq, Debug)] pub struct BorromeanSignatures { - pub s0: [[u8; 32]; 64], - pub s1: [[u8; 32]; 64], - pub ee: [u8; 32], + pub s0: [UnreducedScalar; 64], + pub s1: [UnreducedScalar; 64], + pub ee: Scalar, } impl BorromeanSignatures { pub fn read(r: &mut R) -> io::Result { Ok(BorromeanSignatures { - s0: read_array(read_bytes, r)?, - s1: read_array(read_bytes, r)?, - ee: read_bytes(r)?, + s0: read_array(UnreducedScalar::read, r)?, + s1: read_array(UnreducedScalar::read, r)?, + ee: read_scalar(r)?, }) } pub fn write(&self, w: &mut W) -> io::Result<()> { for s0 in &self.s0 { - w.write_all(s0)?; + s0.write(w)?; } for s1 in &self.s1 { - w.write_all(s1)?; + s1.write(w)?; } - w.write_all(&self.ee) + write_scalar(&self.ee, w) } - #[cfg(feature = "experimental")] fn verify(&self, keys_a: &[EdwardsPoint], keys_b: &[EdwardsPoint]) -> bool { let mut transcript = [0; 2048]; + for i in 0 .. 64 { - // TODO: These aren't the correct reduction - // TODO: Can either of these be tightened? #[allow(non_snake_case)] let LL = EdwardsPoint::vartime_double_scalar_mul_basepoint( - &Scalar::from_bytes_mod_order(self.ee), + &self.ee, &keys_a[i], - &Scalar::from_bytes_mod_order(self.s0[i]), + &self.s0[i].recover_monero_slide_scalar(), ); #[allow(non_snake_case)] let LV = EdwardsPoint::vartime_double_scalar_mul_basepoint( &hash_to_scalar(LL.compress().as_bytes()), &keys_b[i], - &Scalar::from_bytes_mod_order(self.s1[i]), + &self.s1[i].recover_monero_slide_scalar(), ); - transcript[i .. ((i + 1) * 32)].copy_from_slice(LV.compress().as_bytes()); + transcript[(i * 32) .. ((i + 1) * 32)].copy_from_slice(LV.compress().as_bytes()); } - // TODO: This isn't the correct reduction - // TODO: Can this be tightened to from_canonical_bytes? - hash_to_scalar(&transcript) == Scalar::from_bytes_mod_order(self.ee) + hash_to_scalar(&transcript) == self.ee } } @@ -90,7 +80,6 @@ impl BorromeanRange { write_raw_vec(write_point, &self.bit_commitments, w) } - #[cfg(feature = "experimental")] pub fn verify(&self, commitment: &EdwardsPoint) -> bool { if &self.bit_commitments.iter().sum::() != commitment { return false; diff --git a/coins/monero/src/ringct/clsag/mod.rs b/coins/monero/src/ringct/clsag/mod.rs index b03b0df3..0a6141b2 100644 --- a/coins/monero/src/ringct/clsag/mod.rs +++ b/coins/monero/src/ringct/clsag/mod.rs @@ -180,7 +180,7 @@ fn core( let c_c = mu_C * c; let L = (&s[i] * ED25519_BASEPOINT_TABLE) + (c_p * P[i]) + (c_c * C[i]); - let PH = hash_to_point(P[i]); + let PH = hash_to_point(&P[i]); // Shouldn't be an issue as all of the variables in this vartime statement are public let R = (s[i] * PH) + images_precomp.vartime_multiscalar_mul([c_p, c_c]); @@ -219,7 +219,7 @@ impl Clsag { let pseudo_out = Commitment::new(mask, input.commitment.amount).calculate(); let z = input.commitment.mask - mask; - let H = hash_to_point(input.decoys.ring[r][0]); + let H = hash_to_point(&input.decoys.ring[r][0]); let D = H * z; let mut s = Vec::with_capacity(input.decoys.ring.len()); for _ in 0 .. input.decoys.ring.len() { @@ -259,7 +259,7 @@ impl Clsag { &msg, nonce.deref() * ED25519_BASEPOINT_TABLE, nonce.deref() * - hash_to_point(inputs[i].2.decoys.ring[usize::from(inputs[i].2.decoys.i)][0]), + 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)] = (-((p * inputs[i].0.deref()) + c)) + nonce.deref(); diff --git a/coins/monero/src/ringct/clsag/multisig.rs b/coins/monero/src/ringct/clsag/multisig.rs index 3574a5af..d1f70333 100644 --- a/coins/monero/src/ringct/clsag/multisig.rs +++ b/coins/monero/src/ringct/clsag/multisig.rs @@ -116,7 +116,7 @@ impl ClsagMultisig { ClsagMultisig { transcript, - H: hash_to_point(output_key), + H: hash_to_point(&output_key), image: EdwardsPoint::identity(), details, diff --git a/coins/monero/src/ringct/hash_to_point.rs b/coins/monero/src/ringct/hash_to_point.rs index 65bea9db..a36b6ee7 100644 --- a/coins/monero/src/ringct/hash_to_point.rs +++ b/coins/monero/src/ringct/hash_to_point.rs @@ -3,6 +3,6 @@ use curve25519_dalek::edwards::EdwardsPoint; pub use monero_generators::{hash_to_point as raw_hash_to_point}; /// Monero's hash to point function, as named `ge_fromfe_frombytes_vartime`. -pub fn hash_to_point(key: EdwardsPoint) -> EdwardsPoint { +pub fn hash_to_point(key: &EdwardsPoint) -> EdwardsPoint { raw_hash_to_point(key.compress().to_bytes()) } diff --git a/coins/monero/src/ringct/mlsag.rs b/coins/monero/src/ringct/mlsag.rs index d2deaf50..d3b4080e 100644 --- a/coins/monero/src/ringct/mlsag.rs +++ b/coins/monero/src/ringct/mlsag.rs @@ -3,17 +3,82 @@ use std_shims::{ io::{self, Read, Write}, }; -use curve25519_dalek::scalar::Scalar; -#[cfg(feature = "experimental")] -use curve25519_dalek::edwards::EdwardsPoint; +use zeroize::Zeroize; -use crate::serialize::*; -#[cfg(feature = "experimental")] -use crate::{hash_to_scalar, ringct::hash_to_point}; +use curve25519_dalek::{traits::IsIdentity, Scalar, EdwardsPoint}; -#[derive(Clone, PartialEq, Eq, Debug)] +use monero_generators::H; + +use crate::{hash_to_scalar, ringct::hash_to_point, serialize::*}; + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "std", derive(thiserror::Error))] +pub enum MlsagError { + #[cfg_attr(feature = "std", error("invalid ring"))] + InvalidRing, + #[cfg_attr(feature = "std", error("invalid amount of key images"))] + InvalidAmountOfKeyImages, + #[cfg_attr(feature = "std", error("invalid ss"))] + InvalidSs, + #[cfg_attr(feature = "std", error("key image was identity"))] + IdentityKeyImage, + #[cfg_attr(feature = "std", error("invalid ci"))] + InvalidCi, +} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize)] +pub struct RingMatrix { + matrix: Vec>, +} + +impl RingMatrix { + pub fn new(matrix: Vec>) -> Result { + if matrix.is_empty() { + Err(MlsagError::InvalidRing)?; + } + for member in &matrix { + if member.is_empty() || (member.len() != matrix[0].len()) { + Err(MlsagError::InvalidRing)?; + } + } + + Ok(RingMatrix { matrix }) + } + + /// Construct a ring matrix for an individual output. + pub fn individual( + ring: &[[EdwardsPoint; 2]], + pseudo_out: EdwardsPoint, + ) -> Result { + let mut matrix = Vec::with_capacity(ring.len()); + for ring_member in ring { + matrix.push(vec![ring_member[0], ring_member[1] - pseudo_out]); + } + RingMatrix::new(matrix) + } + + pub fn iter(&self) -> impl Iterator { + self.matrix.iter().map(AsRef::as_ref) + } + + /// Return the amount of members in the ring. + pub fn members(&self) -> usize { + self.matrix.len() + } + + /// Returns the length of a ring member. + /// + /// A ring member is a vector of points for which the signer knows all of the discrete logarithms + /// of. + pub fn member_len(&self) -> usize { + // this is safe to do as the constructors don't allow empty rings + self.matrix[0].len() + } +} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize)] pub struct Mlsag { - pub ss: Vec<[Scalar; 2]>, + pub ss: Vec>, pub cc: Scalar, } @@ -25,47 +90,124 @@ impl Mlsag { write_scalar(&self.cc, w) } - pub fn read(mixins: usize, r: &mut R) -> io::Result { + pub fn read(mixins: usize, ss_2_elements: usize, r: &mut R) -> io::Result { Ok(Mlsag { - ss: (0 .. mixins).map(|_| read_array(read_scalar, r)).collect::>()?, + ss: (0 .. mixins) + .map(|_| read_raw_vec(read_scalar, ss_2_elements, r)) + .collect::>()?, cc: read_scalar(r)?, }) } - #[cfg(feature = "experimental")] pub fn verify( &self, msg: &[u8; 32], - ring: &[[EdwardsPoint; 2]], - key_image: &EdwardsPoint, - ) -> bool { - if ring.is_empty() { - return false; + ring: &RingMatrix, + key_images: &[EdwardsPoint], + ) -> Result<(), MlsagError> { + // Mlsag allows for layers to not need linkability, hence they don't need key images + // Monero requires that there is always only 1 non-linkable layer - the amount commitments. + if ring.member_len() != (key_images.len() + 1) { + Err(MlsagError::InvalidAmountOfKeyImages)?; } let mut buf = Vec::with_capacity(6 * 32); + buf.extend_from_slice(msg); + let mut ci = self.cc; - for (i, ring_member) in ring.iter().enumerate() { - buf.extend_from_slice(msg); - #[allow(non_snake_case)] - let L = - |r| EdwardsPoint::vartime_double_scalar_mul_basepoint(&ci, &ring_member[r], &self.ss[i][r]); + // This is an iterator over the key images as options with an added entry of `None` at the + // end for the non-linkable layer + let key_images_iter = key_images.iter().map(|ki| Some(*ki)).chain(core::iter::once(None)); - buf.extend_from_slice(ring_member[0].compress().as_bytes()); - buf.extend_from_slice(L(0).compress().as_bytes()); - - #[allow(non_snake_case)] - let R = (self.ss[i][0] * hash_to_point(ring_member[0])) + (ci * key_image); - buf.extend_from_slice(R.compress().as_bytes()); - - buf.extend_from_slice(ring_member[1].compress().as_bytes()); - buf.extend_from_slice(L(1).compress().as_bytes()); - - ci = hash_to_scalar(&buf); - buf.clear(); + if ring.matrix.len() != self.ss.len() { + Err(MlsagError::InvalidSs)?; } - ci == self.cc + for (ring_member, ss) in ring.iter().zip(&self.ss) { + if ring_member.len() != ss.len() { + Err(MlsagError::InvalidSs)?; + } + + for ((ring_member_entry, s), ki) in ring_member.iter().zip(ss).zip(key_images_iter.clone()) { + #[allow(non_snake_case)] + let L = EdwardsPoint::vartime_double_scalar_mul_basepoint(&ci, ring_member_entry, s); + + buf.extend_from_slice(ring_member_entry.compress().as_bytes()); + buf.extend_from_slice(L.compress().as_bytes()); + + // Not all dimensions need to be linkable, e.g. commitments, and only linkable layers need + // to have key images. + if let Some(ki) = ki { + if ki.is_identity() { + Err(MlsagError::IdentityKeyImage)?; + } + + #[allow(non_snake_case)] + let R = (s * hash_to_point(ring_member_entry)) + (ci * ki); + buf.extend_from_slice(R.compress().as_bytes()); + } + } + + ci = hash_to_scalar(&buf); + // keep the msg in the buffer. + buf.drain(msg.len() ..); + } + + if ci != self.cc { + Err(MlsagError::InvalidCi)? + } + Ok(()) + } +} + +/// An aggregate ring matrix builder, usable to set up the ring matrix to prove/verify an aggregate +/// MLSAG signature. +#[derive(Clone, PartialEq, Eq, Debug, Zeroize)] +pub struct AggregateRingMatrixBuilder { + key_ring: Vec>, + amounts_ring: Vec, + sum_out: EdwardsPoint, +} + +impl AggregateRingMatrixBuilder { + /// Create a new AggregateRingMatrixBuilder. + /// + /// Takes in the transaction's outputs; commitments and fee. + pub fn new(commitments: &[EdwardsPoint], fee: u64) -> Self { + AggregateRingMatrixBuilder { + key_ring: vec![], + amounts_ring: vec![], + sum_out: commitments.iter().sum::() + (H() * Scalar::from(fee)), + } + } + + /// Push a ring of [output key, commitment] to the matrix. + pub fn push_ring(&mut self, ring: &[[EdwardsPoint; 2]]) -> Result<(), MlsagError> { + if self.key_ring.is_empty() { + self.key_ring = vec![vec![]; ring.len()]; + // Now that we know the length of the ring, fill the `amounts_ring`. + self.amounts_ring = vec![-self.sum_out; ring.len()]; + } + + if (self.amounts_ring.len() != ring.len()) || ring.is_empty() { + // All the rings in an aggregate matrix must be the same length. + return Err(MlsagError::InvalidRing); + } + + for (i, ring_member) in ring.iter().enumerate() { + self.key_ring[i].push(ring_member[0]); + self.amounts_ring[i] += ring_member[1] + } + + Ok(()) + } + + /// Build and return the [`RingMatrix`] + pub fn build(mut self) -> Result { + for (i, amount_commitment) in self.amounts_ring.drain(..).enumerate() { + self.key_ring[i].push(amount_commitment); + } + RingMatrix::new(self.key_ring) } } diff --git a/coins/monero/src/ringct/mod.rs b/coins/monero/src/ringct/mod.rs index fc5c7528..608e129c 100644 --- a/coins/monero/src/ringct/mod.rs +++ b/coins/monero/src/ringct/mod.rs @@ -28,7 +28,7 @@ use crate::{ /// Generate a key image for a given key. Defined as `x * hash_to_point(xG)`. pub fn generate_key_image(secret: &Zeroizing) -> EdwardsPoint { - hash_to_point(ED25519_BASEPOINT_TABLE * secret.deref()) * secret.deref() + hash_to_point(&(ED25519_BASEPOINT_TABLE * secret.deref())) * secret.deref() } #[derive(Clone, PartialEq, Eq, Debug)] @@ -61,7 +61,7 @@ impl EncryptedAmount { pub enum RctType { /// No RCT proofs. Null, - /// One MLSAG for a single input and a Borromean range proof (RCTTypeFull). + /// One MLSAG for multiple inputs and Borromean range proofs (RCTTypeFull). MlsagAggregate, // One MLSAG for each input and a Borromean range proof (RCTTypeSimple). MlsagIndividual, @@ -194,6 +194,10 @@ impl RctBase { #[derive(Clone, PartialEq, Eq, Debug)] pub enum RctPrunable { Null, + AggregateMlsagBorromean { + borromean: Vec, + mlsag: Mlsag, + }, MlsagBorromean { borromean: Vec, mlsags: Vec, @@ -220,6 +224,10 @@ impl RctPrunable { pub fn write(&self, w: &mut W, rct_type: RctType) -> io::Result<()> { match self { RctPrunable::Null => Ok(()), + RctPrunable::AggregateMlsagBorromean { borromean, mlsag } => { + write_raw_vec(BorromeanRange::write, borromean, w)?; + mlsag.write(w) + } RctPrunable::MlsagBorromean { borromean, mlsags } => { write_raw_vec(BorromeanRange::write, borromean, w)?; write_raw_vec(Mlsag::write, mlsags, w) @@ -270,9 +278,13 @@ impl RctPrunable { Ok(match rct_type { RctType::Null => RctPrunable::Null, - RctType::MlsagAggregate | RctType::MlsagIndividual => RctPrunable::MlsagBorromean { + RctType::MlsagAggregate => RctPrunable::AggregateMlsagBorromean { borromean: read_raw_vec(BorromeanRange::read, outputs, r)?, - mlsags: decoys.iter().map(|d| Mlsag::read(*d, r)).collect::>()?, + mlsag: Mlsag::read(decoys[0], decoys.len() + 1, r)?, + }, + RctType::MlsagIndividual => RctPrunable::MlsagBorromean { + borromean: read_raw_vec(BorromeanRange::read, outputs, r)?, + mlsags: decoys.iter().map(|d| Mlsag::read(*d, 2, r)).collect::>()?, }, RctType::Bulletproofs | RctType::BulletproofsCompactAmount => { RctPrunable::MlsagBulletproofs { @@ -287,13 +299,13 @@ impl RctPrunable { } Bulletproofs::read(r)? }, - mlsags: decoys.iter().map(|d| Mlsag::read(*d, r)).collect::>()?, + mlsags: decoys.iter().map(|d| Mlsag::read(*d, 2, r)).collect::>()?, pseudo_outs: read_raw_vec(read_point, decoys.len(), r)?, } } RctType::Clsag | RctType::BulletproofsPlus => RctPrunable::Clsag { bulletproofs: { - if read_varint(r)? != 1 { + if read_varint::<_, u64>(r)? != 1 { Err(io::Error::new(io::ErrorKind::Other, "n bulletproofs instead of one"))?; } (if rct_type == RctType::Clsag { Bulletproofs::read } else { Bulletproofs::read_plus })( @@ -309,6 +321,7 @@ impl RctPrunable { pub(crate) fn signature_write(&self, w: &mut W) -> io::Result<()> { match self { RctPrunable::Null => panic!("Serializing RctPrunable::Null for a signature"), + RctPrunable::AggregateMlsagBorromean { borromean, .. } | RctPrunable::MlsagBorromean { borromean, .. } => { borromean.iter().try_for_each(|rs| rs.write(w)) } @@ -329,30 +342,8 @@ impl RctSignatures { pub fn rct_type(&self) -> RctType { match &self.prunable { RctPrunable::Null => RctType::Null, - RctPrunable::MlsagBorromean { .. } => { - /* - This type of RctPrunable may have no outputs, yet pseudo_outs are per input - This will only be a valid RctSignatures if it's for a TX with inputs - That makes this valid for any valid RctSignatures - - While it will be invalid for any invalid RctSignatures, potentially letting an invalid - MlsagAggregate be interpreted as a valid MlsagIndividual (or vice versa), they have - incompatible deserializations - - This means it's impossible to receive a MlsagAggregate over the wire and interpret it - as a MlsagIndividual (or vice versa) - - That only makes manual manipulation unsafe, which will always be true since these fields - are all pub - - TODO: Consider making them private with read-only accessors? - */ - if self.base.pseudo_outs.is_empty() { - RctType::MlsagAggregate - } else { - RctType::MlsagIndividual - } - } + RctPrunable::AggregateMlsagBorromean { .. } => RctType::MlsagAggregate, + RctPrunable::MlsagBorromean { .. } => RctType::MlsagIndividual, // RctBase ensures there's at least one output, making the following // inferences guaranteed/expects impossible on any valid RctSignatures RctPrunable::MlsagBulletproofs { .. } => { diff --git a/coins/monero/src/rpc/mod.rs b/coins/monero/src/rpc/mod.rs index c4002a99..df1dfb6f 100644 --- a/coins/monero/src/rpc/mod.rs +++ b/coins/monero/src/rpc/mod.rs @@ -305,23 +305,27 @@ impl Rpc { } pub async fn get_block_by_number(&self, number: usize) -> Result { - match self.get_block(self.get_block_hash(number).await?).await { - Ok(block) => { - // Make sure this is actually the block for this number - match block.miner_tx.prefix.inputs.first() { - Some(Input::Gen(actual)) => { - if usize::try_from(*actual).unwrap() == number { - Ok(block) - } else { - Err(RpcError::InvalidNode("different block than requested (number)")) - } - } - _ => { - Err(RpcError::InvalidNode("block's miner_tx didn't have an input of kind Input::Gen")) - } + #[derive(Deserialize, Debug)] + struct BlockResponse { + blob: String, + } + + let res: BlockResponse = + self.json_rpc_call("get_block", Some(json!({ "height": number }))).await?; + + let block = Block::read::<&[u8]>(&mut rpc_hex(&res.blob)?.as_ref()) + .map_err(|_| RpcError::InvalidNode("invalid block"))?; + + // Make sure this is actually the block for this number + match block.miner_tx.prefix.inputs.first() { + Some(Input::Gen(actual)) => { + if usize::try_from(*actual).unwrap() == number { + Ok(block) + } else { + Err(RpcError::InvalidNode("different block than requested (number)")) } } - e => e, + _ => Err(RpcError::InvalidNode("block's miner_tx didn't have an input of kind Input::Gen")), } } diff --git a/coins/monero/src/serialize.rs b/coins/monero/src/serialize.rs index 584efb0e..971d9f71 100644 --- a/coins/monero/src/serialize.rs +++ b/coins/monero/src/serialize.rs @@ -12,7 +12,7 @@ use curve25519_dalek::{ const VARINT_CONTINUATION_MASK: u8 = 0b1000_0000; mod sealed { - pub trait VarInt: TryInto {} + pub trait VarInt: TryInto + TryFrom + Copy {} impl VarInt for u8 {} impl VarInt for u32 {} impl VarInt for u64 {} @@ -29,8 +29,9 @@ pub(crate) fn write_byte(byte: &u8, w: &mut W) -> io::Result<()> { w.write_all(&[*byte]) } -pub(crate) fn write_varint(varint: &u64, w: &mut W) -> io::Result<()> { - let mut varint = *varint; +// This will panic if the VarInt exceeds u64::MAX +pub(crate) fn write_varint(varint: &U, w: &mut W) -> io::Result<()> { + let mut varint: u64 = (*varint).try_into().map_err(|_| "varint exceeded u64").unwrap(); while { let mut b = u8::try_from(varint & u64::from(!VARINT_CONTINUATION_MASK)).unwrap(); varint >>= 7; @@ -67,7 +68,7 @@ pub(crate) fn write_vec io::Result<()>>( values: &[T], w: &mut W, ) -> io::Result<()> { - write_varint(&values.len().try_into().unwrap(), w)?; + write_varint(&values.len(), w)?; write_raw_vec(f, values, w) } @@ -93,7 +94,7 @@ pub(crate) fn read_u64(r: &mut R) -> io::Result { read_bytes(r).map(u64::from_le_bytes) } -pub(crate) fn read_varint(r: &mut R) -> io::Result { +pub(crate) fn read_varint(r: &mut R) -> io::Result { let mut bits = 0; let mut res = 0; while { @@ -109,7 +110,9 @@ pub(crate) fn read_varint(r: &mut R) -> io::Result { bits += 7; b & VARINT_CONTINUATION_MASK == VARINT_CONTINUATION_MASK } {} - Ok(res) + res + .try_into() + .map_err(|_| io::Error::new(io::ErrorKind::Other, "VarInt does not fit into integer type")) } // All scalar fields supported by monero-serai are checked to be canonical for valid transactions @@ -162,5 +165,5 @@ pub(crate) fn read_vec io::Result>( f: F, r: &mut R, ) -> io::Result> { - read_raw_vec(f, read_varint(r)?.try_into().unwrap(), r) + read_raw_vec(f, read_varint(r)?, r) } diff --git a/coins/monero/src/tests/mod.rs b/coins/monero/src/tests/mod.rs index 835b77ca..64e72500 100644 --- a/coins/monero/src/tests/mod.rs +++ b/coins/monero/src/tests/mod.rs @@ -1,3 +1,4 @@ +mod unreduced_scalar; mod clsag; mod bulletproofs; mod address; diff --git a/coins/monero/src/tests/unreduced_scalar.rs b/coins/monero/src/tests/unreduced_scalar.rs new file mode 100644 index 00000000..1816991d --- /dev/null +++ b/coins/monero/src/tests/unreduced_scalar.rs @@ -0,0 +1,32 @@ +use curve25519_dalek::scalar::Scalar; + +use crate::unreduced_scalar::*; + +#[test] +fn recover_scalars() { + let test_recover = |stored: &str, recovered: &str| { + let stored = UnreducedScalar(hex::decode(stored).unwrap().try_into().unwrap()); + let recovered = + Scalar::from_canonical_bytes(hex::decode(recovered).unwrap().try_into().unwrap()).unwrap(); + assert_eq!(stored.recover_monero_slide_scalar(), recovered); + }; + + // https://www.moneroinflation.com/static/data_py/report_scalars_df.pdf + // Table 4. + test_recover( + "cb2be144948166d0a9edb831ea586da0c376efa217871505ad77f6ff80f203f8", + "b8ffd6a1aee47828808ab0d4c8524cb5c376efa217871505ad77f6ff80f20308", + ); + test_recover( + "343d3df8a1051c15a400649c423dc4ed58bef49c50caef6ca4a618b80dee22f4", + "21113355bc682e6d7a9d5b3f2137a30259bef49c50caef6ca4a618b80dee2204", + ); + test_recover( + "c14f75d612800ca2c1dcfa387a42c9cc086c005bc94b18d204dd61342418eba7", + "4f473804b1d27ab2c789c80ab21d034a096c005bc94b18d204dd61342418eb07", + ); + test_recover( + "000102030405060708090a0b0c0d0e0f826c4f6e2329a31bc5bc320af0b2bcbb", + "a124cfd387f461bf3719e03965ee6877826c4f6e2329a31bc5bc320af0b2bc0b", + ); +} diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index 5902bc2f..23f9dd7f 100644 --- a/coins/monero/src/transaction.rs +++ b/coins/monero/src/transaction.rs @@ -6,14 +6,12 @@ use std_shims::{ use zeroize::Zeroize; -use curve25519_dalek::{ - scalar::Scalar, - edwards::{EdwardsPoint, CompressedEdwardsY}, -}; +use curve25519_dalek::edwards::{EdwardsPoint, CompressedEdwardsY}; use crate::{ Protocol, hash, serialize::*, + ring_signatures::RingSignature, ringct::{bulletproofs::Bulletproofs, RctType, RctBase, RctPrunable, RctSignatures}, }; @@ -208,7 +206,7 @@ impl TransactionPrefix { self.timelock.write(w)?; write_vec(Input::write, &self.inputs, w)?; write_vec(Output::write, &self.outputs, w)?; - write_varint(&self.extra.len().try_into().unwrap(), w)?; + write_varint(&self.extra.len(), w)?; w.write_all(&self.extra) } @@ -253,7 +251,7 @@ impl TransactionPrefix { #[derive(Clone, PartialEq, Eq, Debug)] pub struct Transaction { pub prefix: TransactionPrefix, - pub signatures: Vec>, + pub signatures: Vec, pub rct_signatures: RctSignatures, } @@ -272,11 +270,8 @@ impl Transaction { pub fn write(&self, w: &mut W) -> io::Result<()> { self.prefix.write(w)?; if self.prefix.version == 1 { - for sigs in &self.signatures { - for sig in sigs { - write_scalar(&sig.0, w)?; - write_scalar(&sig.1, w)?; - } + for ring_sig in &self.signatures { + ring_sig.write(w)?; } Ok(()) } else if self.prefix.version == 2 { @@ -305,12 +300,7 @@ impl Transaction { .inputs .iter() .filter_map(|input| match input { - Input::ToKey { key_offsets, .. } => Some( - key_offsets - .iter() - .map(|_| Ok((read_scalar(r)?, read_scalar(r)?))) - .collect::>(), - ), + Input::ToKey { key_offsets, .. } => Some(RingSignature::read(key_offsets.len(), r)), _ => None, }) .collect::>()?; @@ -397,6 +387,10 @@ impl Transaction { /// Calculate the hash of this transaction as needed for signing it. pub fn signature_hash(&self) -> [u8; 32] { + if self.prefix.version == 1 { + return self.prefix.hash(); + } + let mut buf = Vec::with_capacity(2048); let mut sig_hash = Vec::with_capacity(96); diff --git a/coins/monero/src/unreduced_scalar.rs b/coins/monero/src/unreduced_scalar.rs new file mode 100644 index 00000000..500c23f1 --- /dev/null +++ b/coins/monero/src/unreduced_scalar.rs @@ -0,0 +1,137 @@ +use core::cmp::Ordering; + +use std_shims::{ + sync::OnceLock, + io::{self, *}, +}; + +use curve25519_dalek::scalar::Scalar; + +use crate::serialize::*; + +static PRECOMPUTED_SCALARS_CELL: OnceLock<[Scalar; 8]> = OnceLock::new(); +/// Precomputed scalars used to recover an incorrectly reduced scalar. +#[allow(non_snake_case)] +pub(crate) fn PRECOMPUTED_SCALARS() -> [Scalar; 8] { + *PRECOMPUTED_SCALARS_CELL.get_or_init(|| { + let mut precomputed_scalars = [Scalar::ONE; 8]; + for (i, scalar) in precomputed_scalars.iter_mut().enumerate().skip(1) { + *scalar = Scalar::from(((i * 2) + 1) as u8); + } + precomputed_scalars + }) +} + +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct UnreducedScalar(pub [u8; 32]); + +impl UnreducedScalar { + pub fn write(&self, w: &mut W) -> io::Result<()> { + w.write_all(&self.0) + } + + pub fn read(r: &mut R) -> io::Result { + Ok(UnreducedScalar(read_bytes(r)?)) + } + + pub fn as_bytes(&self) -> &[u8; 32] { + &self.0 + } + + fn as_bits(&self) -> [u8; 256] { + let mut bits = [0; 256]; + for (i, bit) in bits.iter_mut().enumerate() { + *bit = core::hint::black_box(1 & (self.0[i / 8] >> (i % 8))) + } + + bits + } + + /// Computes the non-adjacent form of this scalar with width 5. + /// + /// This matches Monero's `slide` function and intentionally gives incorrect outputs under + /// certain conditions in order to match Monero. + /// + /// This function does not execute in constant time. + fn non_adjacent_form(&self) -> [i8; 256] { + let bits = self.as_bits(); + let mut naf = [0i8; 256]; + for (b, bit) in bits.into_iter().enumerate() { + naf[b] = bit as i8; + } + + for i in 0 .. 256 { + if naf[i] != 0 { + // if the bit is a one, work our way up through the window + // combining the bits with this bit. + for b in 1 .. 6 { + if (i + b) >= 256 { + // if we are at the length of the array then break out + // the loop. + break; + } + // potential_carry - the value of the bit at i+b compared to the bit at i + let potential_carry = naf[i + b] << b; + + if potential_carry != 0 { + if (naf[i] + potential_carry) <= 15 { + // if our current "bit" plus the potential carry is less than 16 + // add it to our current "bit" and set the potential carry bit to 0. + naf[i] += potential_carry; + naf[i + b] = 0; + } else if (naf[i] - potential_carry) >= -15 { + // else if our current "bit" minus the potential carry is more than -16 + // take it away from our current "bit". + // we then work our way up through the bits setting ones to zero, when + // we hit the first zero we change it to one then stop, this is to factor + // in the minus. + naf[i] -= potential_carry; + #[allow(clippy::needless_range_loop)] + for k in (i + b) .. 256 { + if naf[k] == 0 { + naf[k] = 1; + break; + } + naf[k] = 0; + } + } else { + break; + } + } + } + } + } + + naf + } + + /// Recover the scalar that an array of bytes was incorrectly interpreted as by Monero's `slide` + /// function. + /// + /// In Borromean range proofs Monero was not checking that the scalars used were + /// reduced. This lead to the scalar stored being interpreted as a different scalar, + /// this function recovers that scalar. + /// + /// See: https://github.com/monero-project/monero/issues/8438 + pub fn recover_monero_slide_scalar(&self) -> Scalar { + if self.0[31] & 128 == 0 { + // Computing the w-NAF of a number can only give an output with 1 more bit than + // the number, so even if the number isn't reduced, the `slide` function will be + // correct when the last bit isn't set. + return Scalar::from_bytes_mod_order(self.0); + } + + let precomputed_scalars = PRECOMPUTED_SCALARS(); + + let mut recovered = Scalar::ZERO; + for &numb in self.non_adjacent_form().iter().rev() { + recovered += recovered; + match numb.cmp(&0) { + Ordering::Greater => recovered += precomputed_scalars[(numb as usize) / 2], + Ordering::Less => recovered -= precomputed_scalars[((-numb) as usize) / 2], + Ordering::Equal => (), + } + } + recovered + } +} diff --git a/coins/monero/src/wallet/extra.rs b/coins/monero/src/wallet/extra.rs index 2a694b6f..b8de7016 100644 --- a/coins/monero/src/wallet/extra.rs +++ b/coins/monero/src/wallet/extra.rs @@ -110,11 +110,7 @@ impl ExtraField { } nonce }), - 3 => ExtraField::MergeMining( - usize::try_from(read_varint(r)?) - .map_err(|_| io::Error::new(io::ErrorKind::Other, "varint for height exceeds usize"))?, - read_bytes(r)?, - ), + 3 => ExtraField::MergeMining(read_varint(r)?, read_bytes(r)?), 4 => ExtraField::PublicKeys(read_vec(read_point, r)?), _ => Err(io::Error::new(io::ErrorKind::Other, "unknown extra field"))?, }) diff --git a/coins/monero/src/wallet/mod.rs b/coins/monero/src/wallet/mod.rs index b782925f..b405c3ee 100644 --- a/coins/monero/src/wallet/mod.rs +++ b/coins/monero/src/wallet/mod.rs @@ -74,7 +74,7 @@ pub(crate) fn shared_key( .copy_from_slice(&hash(&[output_derivation.as_ref(), [0x8d].as_ref()].concat())[.. 8]); // || o - write_varint(&o.try_into().unwrap(), &mut output_derivation).unwrap(); + write_varint(&o, &mut output_derivation).unwrap(); let view_tag = hash(&[b"view_tag".as_ref(), &output_derivation].concat())[0]; diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index c137e3b7..39e79437 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -406,7 +406,9 @@ impl SignatureMachine for TransactionSignatureMachine { pseudo_outs.push(pseudo_out); } } - RctPrunable::MlsagBorromean { .. } | RctPrunable::MlsagBulletproofs { .. } => { + RctPrunable::AggregateMlsagBorromean { .. } | + RctPrunable::MlsagBorromean { .. } | + RctPrunable::MlsagBulletproofs { .. } => { unreachable!("attempted to sign a multisig TX which wasn't CLSAG") } }