diff --git a/coins/monero/src/clsag/multisig.rs b/coins/monero/src/clsag/multisig.rs index 211c7653..5cb04987 100644 --- a/coins/monero/src/clsag/multisig.rs +++ b/coins/monero/src/clsag/multisig.rs @@ -162,7 +162,7 @@ impl Algorithm for Multisig { // 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", &u64::try_from(l).unwrap().to_le_bytes()); + 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)] diff --git a/coins/monero/src/frost.rs b/coins/monero/src/frost.rs index c10641c7..bdf2f336 100644 --- a/coins/monero/src/frost.rs +++ b/coins/monero/src/frost.rs @@ -77,15 +77,14 @@ impl Curve for Ed25519 { 32 } - fn F_from_le_slice(slice: &[u8]) -> Result { + fn F_from_slice(slice: &[u8]) -> Result { let scalar = Self::F::from_repr( slice.try_into().map_err(|_| CurveError::InvalidLength(32, slice.len()))? ); - if scalar.is_some().unwrap_u8() == 1 { - Ok(scalar.unwrap()) - } else { - Err(CurveError::InvalidScalar) + if scalar.is_some().unwrap_u8() == 0 { + Err(CurveError::InvalidScalar)?; } + Ok(scalar.unwrap()) } fn G_from_slice(slice: &[u8]) -> Result { @@ -105,7 +104,7 @@ impl Curve for Ed25519 { } } - fn F_to_le_bytes(f: &Self::F) -> Vec { + fn F_to_bytes(f: &Self::F) -> Vec { f.to_repr().to_vec() } diff --git a/coins/monero/src/transaction/multisig.rs b/coins/monero/src/transaction/multisig.rs index d9f2685f..fcf2abc0 100644 --- a/coins/monero/src/transaction/multisig.rs +++ b/coins/monero/src/transaction/multisig.rs @@ -64,6 +64,8 @@ impl SignableTransaction { // These outputs can only be spent once. Therefore, it forces all RNGs derived from this // transcript (such as the one used to create one time keys) to be unique transcript.append_message(b"input_hash", &input.tx.0); + // TODO: Should this be u8, u16, or u32? Right now, outputs are solely up to 16, but what + // about the future? transcript.append_message(b"input_output_index", &u64::try_from(input.o).unwrap().to_le_bytes()); // Not including this, with a doxxed list of payments, would allow brute forcing the inputs // to determine RNG seeds and therefore the true spends diff --git a/crypto/frost/src/key_gen.rs b/crypto/frost/src/key_gen.rs index 4354856a..4680fdc7 100644 --- a/crypto/frost/src/key_gen.rs +++ b/crypto/frost/src/key_gen.rs @@ -9,8 +9,8 @@ use crate::{Curve, MultisigParams, MultisigKeys, FrostError}; #[allow(non_snake_case)] fn challenge(l: usize, context: &str, R: &[u8], Am: &[u8]) -> C::F { - let mut c = Vec::with_capacity(8 + context.len() + R.len() + Am.len()); - c.extend(&u64::try_from(l).unwrap().to_le_bytes()); + let mut c = Vec::with_capacity(2 + context.len() + R.len() + Am.len()); + c.extend(&u16::try_from(l).unwrap().to_be_bytes()); c.extend(context.as_bytes()); c.extend(R); // R c.extend(Am); // A of the first commitment, which is what we're proving we have the private key @@ -59,7 +59,7 @@ fn generate_key_r1( let s = k + (coefficients[0] * c); serialized.extend(&C::G_to_bytes(&R)); - serialized.extend(&C::F_to_le_bytes(&s)); + serialized.extend(&C::F_to_bytes(&s)); // Step 4: Broadcast (coefficients, commitments, serialized) @@ -148,7 +148,7 @@ fn verify_r1( ); scalars.push( - -C::F_from_le_slice( + -C::F_from_slice( &serialized[l][commitments_len + C::G_len() .. serialized[l].len()] ).map_err(|_| FrostError::InvalidProofOfKnowledge(l))? * u ); @@ -184,7 +184,7 @@ fn verify_r1( &serialized[l][commitments_len .. commitments_len + C::G_len()] ).map_err(|_| FrostError::InvalidProofOfKnowledge(l))?; - let s = C::F_from_le_slice( + let s = C::F_from_slice( &serialized[l][commitments_len + C::G_len() .. serialized[l].len()] ).map_err(|_| FrostError::InvalidProofOfKnowledge(l))?; @@ -248,7 +248,7 @@ fn generate_key_r2( continue } - res.push(C::F_to_le_bytes(&polynomial(&coefficients, i))); + res.push(C::F_to_bytes(&polynomial(&coefficients, i))); } // Calculate our own share @@ -298,7 +298,7 @@ fn complete_r2( shares.push(C::F::zero()); continue; } - shares.push(C::F_from_le_slice(&serialized[i]).map_err(|_| FrostError::InvalidShare(i))?); + shares.push(C::F_from_slice(&serialized[i]).map_err(|_| FrostError::InvalidShare(i))?); } diff --git a/crypto/frost/src/lib.rs b/crypto/frost/src/lib.rs index 275c5276..5d7f9af3 100644 --- a/crypto/frost/src/lib.rs +++ b/crypto/frost/src/lib.rs @@ -98,7 +98,7 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { // While they do technically exist, their usage of Self::Repr breaks all potential library usage // without helper functions like this #[allow(non_snake_case)] - fn F_from_le_slice(slice: &[u8]) -> Result; + fn F_from_slice(slice: &[u8]) -> Result; /// Group element from slice. Must require canonicity or risks differing binding factors #[allow(non_snake_case)] @@ -106,7 +106,7 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { /// Obtain a vector of the byte encoding of F #[allow(non_snake_case)] - fn F_to_le_bytes(f: &Self::F) -> Vec; + fn F_to_bytes(f: &Self::F) -> Vec; /// Obtain a vector of the byte encoding of G #[allow(non_snake_case)] @@ -135,8 +135,8 @@ impl MultisigParams { Err(FrostError::ZeroParameter(t, n))?; } - if u64::try_from(n).is_err() { - Err(FrostError::TooManyParticipants(n, u64::MAX))?; + if u16::try_from(n).is_err() { + Err(FrostError::TooManyParticipants(n, u16::MAX))?; } // When t == n, this shouldn't be used (MuSig2 and other variants of MuSig exist for a reason), @@ -161,7 +161,7 @@ pub enum FrostError { #[error("a parameter was 0 (required {0}, participants {1})")] ZeroParameter(usize, usize), #[error("too many participants (max {1}, got {0})")] - TooManyParticipants(usize, u64), + TooManyParticipants(usize, u16), #[error("invalid amount of required participants (max {1}, got {0})")] InvalidRequiredQuantity(usize, usize), #[error("invalid participant index (0 < index <= {0}, yet index is {1})")] @@ -296,7 +296,7 @@ impl MultisigKeys { serialized.extend(&(self.params.n as u64).to_le_bytes()); serialized.extend(&(self.params.t as u64).to_le_bytes()); serialized.extend(&(self.params.i as u64).to_le_bytes()); - serialized.extend(&C::F_to_le_bytes(&self.secret_share)); + serialized.extend(&C::F_to_bytes(&self.secret_share)); serialized.extend(&C::G_to_bytes(&self.group_key)); for i in 1 ..= self.params.n { serialized.extend(&C::G_to_bytes(&self.verification_shares[i])); @@ -345,7 +345,7 @@ impl MultisigKeys { .map_err(|_| FrostError::InternalError("parameter doesn't fit into usize".to_string()))?; cursor += 8; - let secret_share = C::F_from_le_slice(&serialized[cursor .. (cursor + C::F_len())]) + let secret_share = C::F_from_slice(&serialized[cursor .. (cursor + C::F_len())]) .map_err(|_| FrostError::InternalError("invalid secret share".to_string()))?; cursor += C::F_len(); let group_key = C::G_from_slice(&serialized[cursor .. (cursor + C::G_len())]) diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index bfbe8ff5..91faebd1 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -149,7 +149,7 @@ fn sign_with_share>( let transcript = params.algorithm.transcript(); transcript.domain_separate(b"FROST"); if params.keys.offset.is_some() { - transcript.append_message(b"offset", &C::F_to_le_bytes(¶ms.keys.offset.unwrap())); + transcript.append_message(b"offset", &C::F_to_bytes(¶ms.keys.offset.unwrap())); } } @@ -170,7 +170,7 @@ fn sign_with_share>( B.push(Some(our_preprocess.commitments)); { let transcript = params.algorithm.transcript(); - transcript.append_message(b"participant", &u16::try_from(l).unwrap().to_le_bytes()); + transcript.append_message(b"participant", &u16::try_from(l).unwrap().to_be_bytes()); transcript.append_message( b"commitments", &our_preprocess.serialized[0 .. (C::G_len() * 2)] @@ -206,7 +206,7 @@ fn sign_with_share>( B.push(Some([D, E])); { let transcript = params.algorithm.transcript(); - transcript.append_message(b"participant", &u16::try_from(l).unwrap().to_le_bytes()); + transcript.append_message(b"participant", &u16::try_from(l).unwrap().to_be_bytes()); transcript.append_message(b"commitments", &commitments[0 .. commitments_len]); } } @@ -255,7 +255,7 @@ fn sign_with_share>( our_preprocess.nonces[0] + (our_preprocess.nonces[1] * binding), msg ); - Ok((Package { Ris, R, share }, C::F_to_le_bytes(&share))) + Ok((Package { Ris, R, share }, C::F_to_bytes(&share))) } // This doesn't check the signing set is as expected and unexpected changes can cause false blames @@ -291,7 +291,7 @@ fn complete>( Err(FrostError::InvalidShare(l))?; } - let part = C::F_from_le_slice(serialized[l].as_ref().unwrap()) + let part = C::F_from_slice(serialized[l].as_ref().unwrap()) .map_err(|_| FrostError::InvalidShare(l))?; sum += part; responses.push(Some(part)); diff --git a/crypto/frost/tests/common.rs b/crypto/frost/tests/common.rs index 057927d4..8983badf 100644 --- a/crypto/frost/tests/common.rs +++ b/crypto/frost/tests/common.rs @@ -61,11 +61,9 @@ impl Curve for Secp256k1 { 33 } - fn F_from_le_slice(slice: &[u8]) -> Result { - let mut bytes: [u8; 32] = slice.try_into().map_err( - |_| CurveError::InvalidLength(32, slice.len()) - )?; - bytes.reverse(); + fn F_from_slice(slice: &[u8]) -> Result { + let bytes: [u8; 32] = slice.try_into() + .map_err(|_| CurveError::InvalidLength(32, slice.len()))?; let scalar = Scalar::from_repr(bytes.into()); if scalar.is_none().unwrap_u8() == 1 { Err(CurveError::InvalidScalar)?; @@ -81,10 +79,8 @@ impl Curve for Secp256k1 { Ok(point.unwrap()) } - fn F_to_le_bytes(f: &Self::F) -> Vec { - let mut res: [u8; 32] = f.to_bytes().into(); - res.reverse(); - res.to_vec() + fn F_to_bytes(f: &Self::F) -> Vec { + (&f.to_bytes()).to_vec() } fn G_to_bytes(g: &Self::G) -> Vec {