From e4fc469e585b4849a3a52acab30800e5ddc9e086 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 3 Jun 2022 01:37:12 -0400 Subject: [PATCH] Use a transcript when generating the per-chain binding for a given set of keys While it was fine as-is, as it only had one variable length property, this is a bit more robust. Also binds the Curve ID, which should declare differently even for just different basepoints, and therefore adds two variable length properties (justifying the transcript). --- coins/monero/src/frost.rs | 2 +- coins/monero/src/tests/clsag.rs | 2 +- coins/monero/src/wallet/send/multisig.rs | 5 +++-- coins/monero/tests/send.rs | 4 ++-- crypto/transcript/src/lib.rs | 9 +++------ crypto/transcript/src/merlin.rs | 2 +- processor/Cargo.toml | 1 + processor/src/wallet.rs | 18 ++++++++---------- 8 files changed, 20 insertions(+), 23 deletions(-) diff --git a/coins/monero/src/frost.rs b/coins/monero/src/frost.rs index 0da52dc0..e0cef7c4 100644 --- a/coins/monero/src/frost.rs +++ b/coins/monero/src/frost.rs @@ -135,7 +135,7 @@ impl DLEqProof { // the proper order if they want to reach consensus // It'd be a poor API to have CLSAG define a new transcript solely to pass here, just to try to // merge later in some form, when it should instead just merge xH (as it does) - let mut transcript = Transcript::new(b"DLEq Proof".to_vec()); + let mut transcript = Transcript::new(b"DLEq Proof"); // Bit redundant, keeps things consistent transcript.domain_separate(b"DLEq"); // Doesn't include G which is constant, does include H which isn't, even though H manipulation diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/src/tests/clsag.rs index 20398117..102b64be 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/src/tests/clsag.rs @@ -96,7 +96,7 @@ fn clsag_multisig() -> Result<(), MultisigError> { algorithm_machines( &mut OsRng, ClsagMultisig::new( - Transcript::new(b"Monero Serai CLSAG Test".to_vec()), + Transcript::new(b"Monero Serai CLSAG Test"), Rc::new(RefCell::new(Some( ClsagDetails::new( ClsagInput::new( diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index 1656b1ee..c1305cde 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -37,7 +37,7 @@ pub struct TransactionMachine { impl SignableTransaction { pub async fn multisig( mut self, - label: Vec, + mut transcript: Transcript, rng: &mut R, rpc: &Rpc, height: usize, @@ -56,8 +56,9 @@ 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 // depending on how these transactions are coordinated + // Being every sender would already let you note rings which happen to use your transactions + // multiple times, already breaking privacy there - let mut transcript = Transcript::new(label); transcript.domain_separate(b"monero_transaction"); // Include the height we're using for our data // The data itself will be included, making this unnecessary, yet a lot of this is technically diff --git a/coins/monero/tests/send.rs b/coins/monero/tests/send.rs index 19cc6fdf..a50ffd33 100644 --- a/coins/monero/tests/send.rs +++ b/coins/monero/tests/send.rs @@ -25,7 +25,7 @@ mod rpc; use crate::rpc::{rpc, mine_block}; #[cfg(feature = "multisig")] -use monero_serai::frost::Ed25519; +use monero_serai::frost::{Transcript, Ed25519}; lazy_static! { static ref SEQUENTIAL: Mutex<()> = Mutex::new(()); @@ -145,7 +145,7 @@ async fn send_core(test: usize, multisig: bool) { machines.insert( i, signable.clone().multisig( - b"Monero Serai Test Transaction".to_vec(), + Transcript::new(b"Monero Serai Test Transaction"), &mut OsRng, &rpc, rpc.get_height().await.unwrap() - 10, diff --git a/crypto/transcript/src/lib.rs b/crypto/transcript/src/lib.rs index 5a04ada8..b324cc31 100644 --- a/crypto/transcript/src/lib.rs +++ b/crypto/transcript/src/lib.rs @@ -8,7 +8,7 @@ pub use merlin::MerlinTranscript; use digest::Digest; pub trait Transcript { - fn domain_separate(&mut self, label: &[u8]); + fn domain_separate(&mut self, label: &'static [u8]); fn append_message(&mut self, label: &'static [u8], message: &[u8]); fn challenge(&mut self, label: &'static [u8]) -> Vec; fn rng_seed(&mut self, label: &'static [u8]) -> [u8; 32]; @@ -24,15 +24,12 @@ impl PartialEq for DigestTranscript { } impl DigestTranscript { - pub fn new(label: Vec) -> Self { - DigestTranscript(label, PhantomData) + pub fn new(label: &'static [u8]) -> Self { + DigestTranscript(label.to_vec(), PhantomData) } } impl Transcript for DigestTranscript { - // It may be beneficial for each domain to be a nested transcript which is itself length prefixed - // This would go further than Merlin though and require an accurate end_domain function which has - // frustrations not worth bothering with when this shouldn't actually be meaningful fn domain_separate(&mut self, label: &[u8]) { self.append_message(b"domain", label); } diff --git a/crypto/transcript/src/merlin.rs b/crypto/transcript/src/merlin.rs index 18671545..b3d2ab50 100644 --- a/crypto/transcript/src/merlin.rs +++ b/crypto/transcript/src/merlin.rs @@ -10,7 +10,7 @@ impl Debug for MerlinTranscript { } impl Transcript for MerlinTranscript { - fn domain_separate(&mut self, label: &[u8]) { + fn domain_separate(&mut self, label: &'static [u8]) { self.append_message(b"dom-sep", label); } diff --git a/processor/Cargo.toml b/processor/Cargo.toml index 14c0c487..19171348 100644 --- a/processor/Cargo.toml +++ b/processor/Cargo.toml @@ -14,6 +14,7 @@ thiserror = "1" curve25519-dalek = { version = "3", features = ["std"] } blake2 = "0.10" +transcript = { path = "../crypto/transcript" } dalek-ff-group = { path = "../crypto/dalek-ff-group" } frost = { path = "../crypto/frost" } diff --git a/processor/src/wallet.rs b/processor/src/wallet.rs index c83a7f5a..d55c3a11 100644 --- a/processor/src/wallet.rs +++ b/processor/src/wallet.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; +use transcript::{Transcript, DigestTranscript}; use frost::{Curve, MultisigKeys}; use crate::{CoinError, Coin}; @@ -21,17 +22,14 @@ impl WalletKeys { // potential ETH group key. While this shouldn't be an issue, as this isn't a private // system, there are potentially other benefits to binding this to a specific group key // It's no longer possible to influence group key gen to key cancel without breaking the hash - // function, although that degree of influence means key gen is broken already + // function as well, although that degree of influence means key gen is broken already fn bind(&self, chain: &[u8]) -> MultisigKeys { - self.keys.offset( - C::hash_to_F( - &[ - b"Serai Processor Wallet", - chain, - &C::G_to_bytes(&self.keys.group_key()) - ].concat() - ) - ) + const DST: &[u8] = b"Serai Processor Wallet Chain Bind"; + let mut transcript = DigestTranscript::::new(DST); + transcript.append_message(b"chain", chain); + transcript.append_message(b"curve", C::id()); + transcript.append_message(b"group_key", &C::G_to_bytes(&self.keys.group_key())); + self.keys.offset(C::hash_to_F(DST, &transcript.challenge(b"offset"))) } }