From cc9c2e0d405ed2d809a0835bdfc1b8aa4a88cf54 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 6 May 2022 01:35:23 -0400 Subject: [PATCH] Use dom-sep tags in the transcripts Also simplifies form in some places --- coins/monero/src/clsag/multisig.rs | 7 +++++-- coins/monero/src/transaction/multisig.rs | 24 ++++++++++-------------- crypto/frost/src/sign.rs | 17 ++++++++++++++--- crypto/transcript/src/lib.rs | 2 ++ 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/coins/monero/src/clsag/multisig.rs b/coins/monero/src/clsag/multisig.rs index bda12ced..0b127539 100644 --- a/coins/monero/src/clsag/multisig.rs +++ b/coins/monero/src/clsag/multisig.rs @@ -26,7 +26,9 @@ use crate::{ }; impl Input { - pub fn transcript(&self, transcript: &mut T) { + fn transcript(&self, transcript: &mut T) { + // Doesn't dom-sep as this is considered part of the larger input signing proof + // Ring index transcript.append_message(b"ring_index", &[self.i]); @@ -170,8 +172,9 @@ impl Algorithm for Multisig { } fn transcript(&self) -> Option { - let mut transcript = Self::Transcript::new(b"CLSAG"); + let mut transcript = Self::Transcript::new(b"Monero Multisig"); self.input.transcript(&mut transcript); + transcript.append_message(b"dom-sep", b"CLSAG"); // 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's be double committed to thanks to the message // It doesn't hurt to have though and ensures security boundaries are well formed diff --git a/coins/monero/src/transaction/multisig.rs b/coins/monero/src/transaction/multisig.rs index 0aa379f8..723c40c4 100644 --- a/coins/monero/src/transaction/multisig.rs +++ b/coins/monero/src/transaction/multisig.rs @@ -52,25 +52,20 @@ impl SignableTransaction { // 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 + // can withstand the burning attack (and has a static spend key? TODO visit bounds) // 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 the above TODO, also consider FROST's TODO of a global transcript instance + let mut transcript = Transcript::new(b"Input Mixins"); + // Does dom-sep despite not being a proof because it's a unique section (and we have no dom-sep yet) + transcript.append_message("dom-sep", "inputs_outputs"); for input in &self.inputs { - shared_keys.extend(&input.key_offset.to_bytes()); + transcript.append_message(b"input_shared_key", &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"payment_address", &payment.0.as_bytes()); + transcript.append_message(b"payment_amount", &payment.1.to_le_bytes()); } - transcript.append_message(b"payments", &payments); // Select mixins let mixins = mixins::select( @@ -129,11 +124,12 @@ impl SignableTransaction { // Seeded RNG so multisig participants agree on one time keys to use, preventing burning attacks fn outputs_rng(tx: &SignableTransaction, entropy: [u8; 32]) -> ::SeededRng { - let mut transcript = Transcript::new(b"StealthAddress"); + let mut transcript = Transcript::new(b"Stealth Addresses"); // This output can only be spent once. Therefore, it forces all one time keys used here to be // unique, even if the entropy is reused. While another transaction could use a different input // ordering to swap which 0 is, that input set can't contain this input without being a double // spend + transcript.append_message(b"dom-sep", b"input_0"); transcript.append_message(b"hash", &tx.inputs[0].tx.0); transcript.append_message(b"index", &u64::try_from(tx.inputs[0].o).unwrap().to_le_bytes()); transcript.seeded_rng(b"tx_keys", Some(entropy)) diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 83d6b676..864079be 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -218,15 +218,26 @@ fn sign_with_share>( // While Merlin, which may or may not be the transcript used here, wants application level // transcripts passed around to proof systems, this maintains a desired level of abstraction and // works without issue + // Not to mention, mandating a global transcript would conflict with the IETF draft UNLESS an + // IetfTranscript was declared which ignores field names and solely does their values, with a + // fresh instantiation per sign round. That could likely be made to align without issue + // TODO: Consider Option? let mut transcript = params.algorithm.transcript(); + if params.keys.offset.is_some() && transcript.is_none() { + transcript = Some(A::Transcript::new(b"FROST Offset")); + } + if transcript.is_some() { + // https://github.com/rust-lang/rust/issues/91345 + transcript = transcript.map(|mut t| { t.append_message(b"dom-sep", b"FROST"); t }); + } // If the offset functionality provided by this library is in use, include it in the transcript. // Not compliant with the IETF spec which doesn't have a concept of offsets, nor does it use // transcripts if params.keys.offset.is_some() { - let mut offset_transcript = transcript.unwrap_or(A::Transcript::new(b"FROST_offset")); - offset_transcript.append_message(b"offset", &C::F_to_le_bytes(¶ms.keys.offset.unwrap())); - transcript = Some(offset_transcript); + transcript = transcript.map( + |mut t| { t.append_message(b"offset", &C::F_to_le_bytes(¶ms.keys.offset.unwrap())); t } + ); } // If a transcript was defined, move the commitments used for the binding factor into it diff --git a/crypto/transcript/src/lib.rs b/crypto/transcript/src/lib.rs index a82cd58e..6b372509 100644 --- a/crypto/transcript/src/lib.rs +++ b/crypto/transcript/src/lib.rs @@ -21,6 +21,8 @@ pub trait Transcript { label: &'static [u8], additional_entropy: Option<[u8; 32]> ) -> Self::SeededRng; + + // TODO: Consider a domain_separate function } #[derive(Clone, Debug)]