From f856faa762c69f5a8bbe42c7226b8ce0bad95f2d Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Wed, 4 May 2022 06:24:52 -0400 Subject: [PATCH] Implement simple random mixin selection which passes sanity --- coins/monero/src/rpc.rs | 83 +++++++++++----- coins/monero/src/transaction/mixins.rs | 116 +++++++++++++++++++++-- coins/monero/src/transaction/mod.rs | 30 +++--- coins/monero/src/transaction/multisig.rs | 60 +++++++++--- 4 files changed, 230 insertions(+), 59 deletions(-) diff --git a/coins/monero/src/rpc.rs b/coins/monero/src/rpc.rs index bbca6960..5be28804 100644 --- a/coins/monero/src/rpc.rs +++ b/coins/monero/src/rpc.rs @@ -1,4 +1,4 @@ -use std::fmt::Debug; +use std::{fmt::Debug, str::FromStr}; use thiserror::Error; @@ -8,8 +8,9 @@ use curve25519_dalek::edwards::{EdwardsPoint, CompressedEdwardsY}; use monero::{ Hash, + cryptonote::hash::Hashable, blockdata::{ - transaction::Transaction, + transaction::{TxIn, Transaction}, block::Block }, consensus::encode::{serialize, deserialize} @@ -41,6 +42,8 @@ pub enum RpcError { InvalidTransaction } +pub struct Rpc(String); + fn rpc_hex(value: &str) -> Result, RpcError> { hex::decode(value).map_err(|_| RpcError::InternalError("Monero returned invalid hex".to_string())) } @@ -51,8 +54,6 @@ fn rpc_point(point: &str) -> Result { ).decompress().ok_or(RpcError::InvalidPoint(point.to_string())) } -pub struct Rpc(String); - impl Rpc { pub fn new(daemon: String) -> Rpc { Rpc(daemon) @@ -90,7 +91,7 @@ impl Rpc { Ok( if !method.ends_with(".bin") { serde_json::from_str(&res.text().await.map_err(|_| RpcError::ConnectionError)?) - .map_err(|_| RpcError::InternalError("Failed to parse json response".to_string()))? + .map_err(|_| RpcError::InternalError("Failed to parse JSON response".to_string()))? } else { monero_epee_bin_serde::from_bytes(&res.bytes().await.map_err(|_| RpcError::ConnectionError)?) .map_err(|_| RpcError::InternalError("Failed to parse binary response".to_string()))? @@ -109,7 +110,8 @@ impl Rpc { pub async fn get_transactions(&self, hashes: Vec) -> Result, RpcError> { #[derive(Deserialize, Debug)] struct TransactionResponse { - as_hex: String + as_hex: String, + pruned_as_hex: String } #[derive(Deserialize, Debug)] struct TransactionsResponse { @@ -123,18 +125,26 @@ impl Rpc { Err(RpcError::TransactionsNotFound(txs.txs.len(), hashes.len()))?; } - let mut res = Vec::with_capacity(txs.txs.len()); + let mut res: Vec = Vec::with_capacity(txs.txs.len()); for tx in txs.txs { res.push( deserialize( - &rpc_hex(&tx.as_hex)? - ).expect("Monero returned a transaction we couldn't deserialize") + &rpc_hex(if tx.as_hex.len() != 0 { &tx.as_hex } else { &tx.pruned_as_hex })? + ).map_err(|_| RpcError::InvalidTransaction)? ); + + if tx.as_hex.len() == 0 { + match res[res.len() - 1].prefix.inputs[0] { + TxIn::Gen { .. } => 0, + _ => Err(RpcError::TransactionsNotFound(hashes.len() - 1, hashes.len()))? + }; + } } + Ok(res) } - pub async fn get_block_transactions(&self, height: usize) -> Result, RpcError> { + pub async fn get_block(&self, height: usize) -> Result { #[derive(Deserialize, Debug)] struct BlockResponse { blob: String @@ -147,10 +157,15 @@ impl Rpc { } }))).await?; - let block: Block = deserialize( - &rpc_hex(&block.result.blob)? - ).expect("Monero returned a block we couldn't deserialize"); + Ok( + deserialize( + &rpc_hex(&block.result.blob)? + ).expect("Monero returned a block we couldn't deserialize") + ) + } + pub async fn get_block_transactions(&self, height: usize) -> Result, RpcError> { + let block = self.get_block(height).await?; let mut res = vec![block.miner_tx]; if block.tx_hashes.len() != 0 { res.extend(self.get_transactions(block.tx_hashes).await?); @@ -183,11 +198,16 @@ impl Rpc { Ok(indexes.o_indexes) } - pub async fn get_ring(&self, mixins: &[u64]) -> Result, RpcError> { + pub async fn get_outputs( + &self, + indexes: &[u64], + height: usize + ) -> Result>, RpcError> { #[derive(Deserialize, Debug)] - struct Out { + pub struct Out { key: String, - mask: String + mask: String, + txid: String } #[derive(Deserialize, Debug)] @@ -196,17 +216,34 @@ impl Rpc { } let outs: Outs = self.rpc_call("get_outs", Some(json!({ - "outputs": mixins.iter().map(|m| json!({ + "get_txid": true, + "outputs": indexes.iter().map(|o| json!({ "amount": 0, - "index": m + "index": o })).collect::>() }))).await?; - let mut res = vec![]; - for out in outs.outs { - res.push([rpc_point(&out.key)?, rpc_point(&out.mask)?]); - } - Ok(res) + let txs = self.get_transactions( + outs.outs.iter().map(|out| Hash::from_str(&out.txid).expect("Monero returned an invalid hash")).collect() + ).await?; + // TODO: Support time based lock times. These shouldn't be needed, and it may be painful to + // get the median time for the given height, yet we do need to in order to be complete + outs.outs.iter().enumerate().map( + |(i, out)| Ok( + if txs[i].prefix.unlock_time.0 <= u64::try_from(height).unwrap() { + Some([rpc_point(&out.key)?, rpc_point(&out.mask)?]) + } else { None } + ) + ).collect() + } + + pub async fn get_high_output(&self, height: usize) -> Result { + let block = self.get_block(height).await?; + Ok( + *self.get_o_indexes( + *block.tx_hashes.last().unwrap_or(&block.miner_tx.hash()) + ).await?.last().ok_or(RpcError::InvalidTransaction)? + ) } pub async fn publish_transaction(&self, tx: &Transaction) -> Result<(), RpcError> { diff --git a/coins/monero/src/transaction/mixins.rs b/coins/monero/src/transaction/mixins.rs index ec7cec0d..d092d32e 100644 --- a/coins/monero/src/transaction/mixins.rs +++ b/coins/monero/src/transaction/mixins.rs @@ -1,15 +1,113 @@ -// TODO -pub(crate) fn select(o: u64) -> (u8, Vec) { - let mut mixins: Vec = (o .. o + 11).into_iter().collect(); - mixins.sort(); - (0, mixins) +use std::collections::HashSet; + +use rand_core::{RngCore, CryptoRng}; + +use curve25519_dalek::edwards::EdwardsPoint; + +use monero::VarInt; + +use crate::{transaction::SpendableOutput, rpc::{RpcError, Rpc}}; + +const MIXINS: usize = 11; + +async fn select_single( + rng: &mut R, + rpc: &Rpc, + height: usize, + high: u64, + used: &mut HashSet +) -> Result<(u64, [EdwardsPoint; 2]), RpcError> { + let mut o; + let mut output = None; + while { + o = rng.next_u64() % u64::try_from(high).unwrap(); + used.contains(&o) || { + output = rpc.get_outputs(&[o], height).await?[0]; + output.is_none() + } + } {} + used.insert(o); + Ok((o, output.unwrap())) } -pub(crate) fn offset(mixins: &[u64]) -> Vec { - let mut res = vec![mixins[0]]; - res.resize(11, 0); +// Uses VarInt as this is solely used for key_offsets which is serialized by monero-rs +fn offset(mixins: &[u64]) -> Vec { + let mut res = vec![VarInt(mixins[0])]; + res.resize(mixins.len(), VarInt(0)); for m in (1 .. mixins.len()).rev() { - res[m] = mixins[m] - mixins[m - 1]; + res[m] = VarInt(mixins[m] - mixins[m - 1]); } res } + +pub(crate) async fn select( + rng: &mut R, + rpc: &Rpc, + height: usize, + inputs: &[SpendableOutput] +) -> Result, u8, Vec<[EdwardsPoint; 2]>)>, RpcError> { + // Convert the inputs in question to the raw output data + let mut outputs = Vec::with_capacity(inputs.len()); + for input in inputs { + outputs.push(( + rpc.get_o_indexes(input.tx).await?[input.o], + [input.key, input.commitment.calculate()] + )); + } + + let high = rpc.get_high_output(height - 1).await?; + let high_f = high as f64; + if (high_f as u64) != high { + panic!("Transaction output index exceeds f64"); + } + + let mut used = HashSet::::new(); + for o in &outputs { + used.insert(o.0); + } + + let mut res = Vec::with_capacity(inputs.len()); + for (i, o) in outputs.iter().enumerate() { + let mut mixins = Vec::with_capacity(MIXINS); + for _ in 0 .. MIXINS { + mixins.push(select_single(rng, rpc, height, high, &mut used).await?); + } + mixins.sort_by(|a, b| a.0.cmp(&b.0)); + + // Make sure the TX passes the sanity check that the median output is within the last 40% + // This actually checks the median is within the last third, a slightly more aggressive boundary, + // as the height used in this calculation will be slightly under the height this is sanity + // checked against + while mixins[MIXINS / 2].0 < (high * 2 / 3) { + // If it's not, update the bottom half with new values to ensure the median only moves up + for m in 0 .. MIXINS / 2 { + // We could not remove this, saving CPU time and removing low values as possibilities, yet + // it'd increase the amount of mixins required to create this transaction and some banned + // outputs may be the best options + used.remove(&mixins[m].0); + mixins[m] = select_single(rng, rpc, height, high, &mut used).await?; + } + mixins.sort_by(|a, b| a.0.cmp(&b.0)); + } + + // Replace the closest selected decoy with the actual + let mut replace = 0; + let mut distance = u64::MAX; + for m in 0 .. mixins.len() { + let diff = mixins[m].0.abs_diff(o.0); + if diff < distance { + replace = m; + distance = diff; + } + } + + mixins[replace] = outputs[i]; + res.push(( + offset(&mixins.iter().map(|output| output.0).collect::>()), + u8::try_from(replace).unwrap(), + mixins.iter().map(|output| output.1).collect() + )); + } + + Ok(res) +} diff --git a/coins/monero/src/transaction/mod.rs b/coins/monero/src/transaction/mod.rs index 3987b267..6fccb130 100644 --- a/coins/monero/src/transaction/mod.rs +++ b/coins/monero/src/transaction/mod.rs @@ -71,6 +71,7 @@ pub enum TransactionError { pub struct SpendableOutput { pub tx: Hash, pub o: usize, + pub key: EdwardsPoint, pub key_offset: Scalar, pub commitment: Commitment } @@ -126,7 +127,7 @@ pub fn scan(tx: &Transaction, view: Scalar, spend: EdwardsPoint) -> Vec( + rng: &mut R, rpc: &Rpc, - spend: &Scalar, inputs: &[SpendableOutput], + spend: &Scalar, tx: &mut Transaction ) -> Result, TransactionError> { // TODO sort inputs let mut signable = Vec::with_capacity(inputs.len()); - for (i, input) in inputs.iter().enumerate() { - // Select mixins - let (m, mixins) = mixins::select( - rpc.get_o_indexes(input.tx).await.map_err(|e| TransactionError::RpcError(e))?[input.o] - ); + // Select mixins + let mixins = mixins::select( + rng, + rpc, + rpc.get_height().await.map_err(|e| TransactionError::RpcError(e))?, + inputs + ).await.map_err(|e| TransactionError::RpcError(e))?; + + for (i, input) in inputs.iter().enumerate() { signable.push(( spend + input.key_offset, clsag::Input::new( - rpc.get_ring(&mixins).await.map_err(|e| TransactionError::RpcError(e))?, - m, + mixins[i].2.clone(), + mixins[i].1, input.commitment ).map_err(|e| TransactionError::ClsagError(e))?, key_image::generate(&(spend + input.key_offset)) @@ -218,7 +224,7 @@ async fn prepare_inputs( tx.prefix.inputs.push(TxIn::ToKey { amount: VarInt(0), - key_offsets: mixins::offset(&mixins).iter().map(|x| VarInt(*x)).collect(), + key_offsets: mixins[i].0.clone(), k_image: KeyImage { image: Hash(signable[i].2.compress().to_bytes()) } }); } @@ -360,7 +366,7 @@ impl SignableTransaction { let (commitments, mask_sum) = self.prepare_outputs(rng)?; let mut tx = self.prepare_transaction(&commitments, bulletproofs::generate(&commitments)?); - let signable = prepare_inputs(rpc, spend, &self.inputs, &mut tx).await?; + let signable = prepare_inputs(rng, rpc, &self.inputs, spend, &mut tx).await?; let clsags = clsag::sign( rng, diff --git a/coins/monero/src/transaction/multisig.rs b/coins/monero/src/transaction/multisig.rs index 916b79da..e4c0acbc 100644 --- a/coins/monero/src/transaction/multisig.rs +++ b/coins/monero/src/transaction/multisig.rs @@ -25,11 +25,11 @@ pub struct TransactionMachine { leader: bool, signable: SignableTransaction, our_images: Vec, - inputs: Vec, - tx: Option, mask_sum: Rc>, msg: Rc>, - clsags: Vec> + clsags: Vec>, + inputs: Vec, + tx: Option, } impl SignableTransaction { @@ -41,28 +41,58 @@ impl SignableTransaction { included: &[usize] ) -> Result { let mut our_images = vec![]; - let mut inputs = vec![]; + let mask_sum = Rc::new(RefCell::new(Scalar::zero())); let msg = Rc::new(RefCell::new([0; 32])); let mut clsags = vec![]; - for input in &self.inputs { - // Select mixins - let (m, mixins) = mixins::select( - rpc.get_o_indexes(input.tx).await.map_err(|e| TransactionError::RpcError(e))?[input.o] - ); + let mut inputs = vec![]; + + // Create a RNG out of the input shared keys, which either requires the view key or being every + // sender, and the payments (address and amount), which a passive adversary may be able to know + // The use of input shared keys technically makes this one time given a competent wallet which + // can withstand the burning attack + // The lack of dedicated entropy here is frustrating. We can probably provide entropy inclusion + // if we move CLSAG ring to a Rc RefCell like msg and mask? TODO + let mut transcript = Transcript::new(b"InputMixins"); + let mut shared_keys = Vec::with_capacity(self.inputs.len() * 32); + for input in &self.inputs { + shared_keys.extend(&input.key_offset.to_bytes()); + } + transcript.append_message(b"input_shared_keys", &shared_keys); + let mut payments = Vec::with_capacity(self.payments.len() * ((2 * 32) + 8)); + for payment in &self.payments { + // Network byte and spend/view key + // Doesn't use the full address as monero-rs may provide a payment ID which adds bytes + // By simply cutting this short, we get the relevant data without length differences nor the + // need to prefix + payments.extend(&payment.0.as_bytes()[0 .. 65]); + payments.extend(payment.1.to_le_bytes()); + } + transcript.append_message(b"payments", &payments); + + // Select mixins + let mixins = mixins::select( + &mut transcript.seeded_rng(b"mixins", None), + rpc, + rpc.get_height().await.map_err(|e| TransactionError::RpcError(e))?, + &self.inputs + ).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( clsag::Input::new( - rpc.get_ring(&mixins).await.map_err(|e| TransactionError::RpcError(e))?, - m, + mixins[i].2.clone(), + mixins[i].1, input.commitment ).map_err(|e| TransactionError::ClsagError(e))?, msg.clone(), @@ -75,7 +105,7 @@ impl SignableTransaction { inputs.push(TxIn::ToKey { amount: VarInt(0), - key_offsets: mixins::offset(&mixins).iter().map(|x| VarInt(*x)).collect(), + key_offsets: mixins[i].0.clone(), k_image: KeyImage { image: Hash([0; 32]) } }); } @@ -87,11 +117,11 @@ impl SignableTransaction { leader: keys.params().i() == included[0], signable: self, our_images, - inputs, - tx: None, mask_sum, msg, - clsags + clsags, + inputs, + tx: None }) } }