diff --git a/coins/monero/Cargo.toml b/coins/monero/Cargo.toml index e9fdeaf0..22e1dfe5 100644 --- a/coins/monero/Cargo.toml +++ b/coins/monero/Cargo.toml @@ -40,4 +40,5 @@ experimental = [] multisig = ["ff", "group", "rand_chacha", "transcript", "frost", "dalek-ff-group"] [dev-dependencies] +sha2 = "0.10" tokio = { version = "1", features = ["full"] } diff --git a/coins/monero/src/frost.rs b/coins/monero/src/frost.rs index 0a276288..6b44a296 100644 --- a/coins/monero/src/frost.rs +++ b/coins/monero/src/frost.rs @@ -1,9 +1,10 @@ -use core::convert::TryInto; +use core::{convert::TryInto, fmt::{Formatter, Debug}}; +use std::marker::PhantomData; use thiserror::Error; use rand_core::{RngCore, CryptoRng}; -use blake2::{digest::Update, Digest, Blake2b512}; +use blake2::{digest::{generic_array::typenum::U64, Digest}, Blake2b512}; use curve25519_dalek::{ constants::ED25519_BASEPOINT_TABLE as DTable, @@ -11,7 +12,7 @@ use curve25519_dalek::{ edwards::EdwardsPoint as DPoint }; -use ff::{Field, PrimeField}; +use ff::PrimeField; use group::Group; use transcript::{Transcript as TranscriptTrait, DigestTranscript}; @@ -32,9 +33,26 @@ pub enum MultisigError { InvalidKeyImage(u16) } -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub struct Ed25519; -impl Curve for Ed25519 { +// Accept a parameterized hash function in order to check against the FROST vectors while still +// allowing Blake2b to be used with wide reduction in practice +pub struct Ed25519Internal, const WIDE: bool> { + _digest: PhantomData +} + +// Removed requirements for D to have all of these +impl, const WIDE: bool> Clone for Ed25519Internal { + fn clone(&self) -> Self { *self } +} +impl, const WIDE: bool> Copy for Ed25519Internal {} +impl, const WIDE: bool> PartialEq for Ed25519Internal { + fn eq(&self, _: &Self) -> bool { true } +} +impl, const WIDE: bool> Eq for Ed25519Internal {} +impl, const WIDE: bool> Debug for Ed25519Internal { + fn fmt(&self, _: &mut Formatter<'_>) -> Result<(), core::fmt::Error> { Ok(()) } +} + +impl, const WIDE: bool> Curve for Ed25519Internal { type F = dfg::Scalar; type G = dfg::EdwardsPoint; type T = &'static dfg::EdwardsBasepointTable; @@ -44,7 +62,7 @@ impl Curve for Ed25519 { } fn id() -> &'static [u8] { - b"Ed25519" + b"edwards25519" } fn generator() -> Self::G { @@ -59,15 +77,15 @@ impl Curve for Ed25519 { true } - fn random_nonce(_secret: Self::F, rng: &mut R) -> Self::F { - dfg::Scalar::random(rng) + fn random_nonce(secret: Self::F, rng: &mut R) -> Self::F { + let mut seed = vec![0; 32]; + rng.fill_bytes(&mut seed); + seed.extend(&secret.to_bytes()); + Self::hash_to_F(b"nonce", &seed) } - // This will already be a keccak256 hash in the case of CLSAG signing, making it fine to simply - // return as-is, yet this ensures it's fixed size (a security requirement) and unique regardless - // of how it's called/what it's called with fn hash_msg(msg: &[u8]) -> Vec { - Blake2b512::digest(msg).to_vec() + D::digest(msg).to_vec() } fn hash_binding_factor(binding: &[u8]) -> Self::F { @@ -75,7 +93,12 @@ impl Curve for Ed25519 { } fn hash_to_F(dst: &[u8], msg: &[u8]) -> Self::F { - dfg::Scalar::from_hash(Blake2b512::new().chain(dst).chain(msg)) + let digest = D::new().chain_update(dst).chain_update(msg); + if WIDE { + dfg::Scalar::from_hash(digest) + } else { + dfg::Scalar::from_bytes_mod_order(digest.finalize()[32 ..].try_into().unwrap()) + } } fn F_len() -> usize { @@ -101,8 +124,8 @@ impl Curve for Ed25519 { let point = dfg::CompressedEdwardsY::new(bytes).decompress(); if let Some(point) = point { - // Ban torsioned points - if !point.is_torsion_free() { + // Ban identity and torsioned points + if point.is_identity().into() || (!bool::from(point.is_torsion_free())) { Err(CurveError::InvalidPoint)?; } // Ban points which weren't canonically encoded @@ -124,6 +147,8 @@ impl Curve for Ed25519 { } } +pub type Ed25519 = Ed25519Internal; + // Used to prove legitimacy of key images and nonces which both involve other basepoints #[derive(Clone)] pub struct DLEqProof { @@ -225,6 +250,7 @@ pub fn read_dleq( xG: &DPoint ) -> Result { // Not using G_from_slice here would enable non-canonical points and break blame + // This does also ban identity points, yet those should never be a concern let other = ::G_from_slice( &serialized[(start + 0) .. (start + 32)] ).map_err(|_| MultisigError::InvalidDLEqProof(l))?; diff --git a/coins/monero/src/tests/frost.rs b/coins/monero/src/tests/frost.rs index 423d23d2..59f2b707 100644 --- a/coins/monero/src/tests/frost.rs +++ b/coins/monero/src/tests/frost.rs @@ -1,11 +1,67 @@ use rand::rngs::OsRng; -use frost::tests::{curve::test_curve, key_gen}; +use sha2::Sha512; -use crate::frost::Ed25519; +use dalek_ff_group as dfg; +use frost::{Curve, algorithm::Hram, tests::{curve::test_curve, vectors::{Vectors, vectors}}}; + +use crate::frost::{Ed25519, Ed25519Internal}; #[test] fn frost_ed25519() { test_curve::<_, Ed25519>(&mut OsRng); - key_gen::<_, Ed25519>(&mut OsRng); +} + +// Not spec-compliant, as this shouldn't use wide reduction +// Is vectors compliant, which is why the below tests pass +// See https://github.com/cfrg/draft-irtf-cfrg-frost/issues/204 +//type TestEd25519 = Ed25519Internal; +// If this is kept, we can remove WIDE +type TestEd25519 = Ed25519Internal; + +#[derive(Copy, Clone)] +struct IetfEd25519Hram {} +impl Hram for IetfEd25519Hram { + #[allow(non_snake_case)] + fn hram(R: &dfg::EdwardsPoint, A: &dfg::EdwardsPoint, m: &[u8]) -> dfg::Scalar { + TestEd25519::hash_to_F( + b"", + &[&R.compress().to_bytes(), &A.compress().to_bytes(), m].concat() + ) + } +} + +#[test] +fn frost_ed25519_vectors() { + vectors::( + Vectors { + threshold: 2, + shares: &[ + "929dcc590407aae7d388761cddb0c0db6f5627aea8e217f4a033f2ec83d93509", + "a91e66e012e4364ac9aaa405fcafd370402d9859f7b6685c07eed76bf409e80d", + "d3cb090a075eb154e82fdb4b3cb507f110040905468bb9c46da8bdea643a9a02" + ], + group_secret: "7b1c33d3f5291d85de664833beb1ad469f7fb6025a0ec78b3a790c6e13a98304", + group_key: "15d21ccd7ee42959562fc8aa63224c8851fb3ec85a3faf66040d380fb9738673", + + msg: "74657374", + included: &[1, 3], + nonces: &[ + [ + "8c76af04340e83bb5fc427c117d38347fc8ef86d5397feea9aa6412d96c05b0a", + "14a37ddbeae8d9e9687369e5eb3c6d54f03dc19d76bb54fb5425131bc37a600b" + ], + [ + "5ca39ebab6874f5e7b5089f3521819a2aa1e2cf738bae6974ee80555de2ef70e", + "0afe3650c4815ff37becd3c6948066e906e929ea9b8f546c74e10002dbcc150c" + ] + ], + sig_shares: &[ + "4369474a398aa10357b60d683da91ea6a767dcf53fd541a8ed6b4d780827ea0a", + "32fcc690d926075e45d2dfb746bab71447943cddbefe80d122c39174aa2e1004" + ], + sig: "2b8d9c6995333c5990e3a3dd6568785539d3322f7f0376452487ea35cfda587b".to_owned() + + "75650edb12b1a8619c88ed1f8463d6baeefb18d3fed3c279102fdfecb255fa0e" + } + ); } diff --git a/crypto/frost/src/tests/literal/p256.rs b/crypto/frost/src/tests/literal/p256.rs index c36a046d..c2668329 100644 --- a/crypto/frost/src/tests/literal/p256.rs +++ b/crypto/frost/src/tests/literal/p256.rs @@ -179,7 +179,6 @@ fn p256_curve() { test_curve::<_, P256>(&mut OsRng); } -#[allow(non_snake_case)] #[derive(Clone)] pub struct IetfP256Hram {} impl Hram for IetfP256Hram { @@ -217,7 +216,6 @@ fn p256_vectors() { "486e2ee25a3fbc8e6399d748b077a2755fde99fa85cc24fa647ea4ebf5811a15" ] ], - binding: "cf7ffe4b8ad6edb6237efaa8cbfb2dfb2fd08d163b6ad9063720f14779a9e143", sig_shares: &[ "9e4d8865faf8c7b3193a3b35eda3d9e12118447114b1e7d5b4809ea28067f8a9", "b7d094eab6305ae74daeed1acd31abba9ab81f638d38b72c132cb25a5dfae1fc" diff --git a/crypto/frost/src/tests/vectors.rs b/crypto/frost/src/tests/vectors.rs index 5c984aa3..0e9f3396 100644 --- a/crypto/frost/src/tests/vectors.rs +++ b/crypto/frost/src/tests/vectors.rs @@ -16,7 +16,6 @@ pub struct Vectors { pub msg: &'static str, pub included: &'static [u16], pub nonces: &'static [[&'static str; 2]], - pub binding: &'static str, pub sig_shares: &'static [&'static str], pub sig: String }