From 60254a017175ff15e36683ebe92d4e53355d6b04 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 24 Jun 2022 19:47:19 -0400 Subject: [PATCH] Reorganize FROST's handling of curves --- coins/monero/src/frost.rs | 3 +- coins/monero/src/ringct/clsag/multisig.rs | 4 +- coins/monero/src/tests/clsag.rs | 4 +- coins/monero/src/wallet/send/multisig.rs | 2 +- coins/monero/tests/send.rs | 9 +- crypto/frost/src/{curves => curve}/dalek.rs | 34 +++--- crypto/frost/src/{curves => curve}/kp256.rs | 2 +- crypto/frost/src/curve/mod.rs | 121 ++++++++++++++++++++ crypto/frost/src/curves/mod.rs | 5 - crypto/frost/src/key_gen.rs | 3 +- crypto/frost/src/lib.rs | 110 +----------------- crypto/frost/src/sign.rs | 2 +- crypto/frost/src/tests/literal/dalek.rs | 6 +- crypto/frost/src/tests/literal/kp256.rs | 4 +- processor/src/coins/monero.rs | 3 +- processor/src/lib.rs | 3 +- processor/src/tests/mod.rs | 2 +- processor/src/wallet.rs | 2 +- 18 files changed, 165 insertions(+), 154 deletions(-) rename crypto/frost/src/{curves => curve}/dalek.rs (98%) rename crypto/frost/src/{curves => curve}/kp256.rs (98%) create mode 100644 crypto/frost/src/curve/mod.rs delete mode 100644 crypto/frost/src/curves/mod.rs diff --git a/coins/monero/src/frost.rs b/coins/monero/src/frost.rs index ef557384..49d4e2da 100644 --- a/coins/monero/src/frost.rs +++ b/coins/monero/src/frost.rs @@ -10,8 +10,7 @@ use curve25519_dalek::{ }; use transcript::{Transcript, RecommendedTranscript}; -use frost::curves::Curve; -pub use frost::curves::dalek::Ed25519; +use frost::curve::{Curve, Ed25519}; use dalek_ff_group as dfg; use crate::random_scalar; diff --git a/coins/monero/src/ringct/clsag/multisig.rs b/coins/monero/src/ringct/clsag/multisig.rs index dfbd64ad..dfeeda5f 100644 --- a/coins/monero/src/ringct/clsag/multisig.rs +++ b/coins/monero/src/ringct/clsag/multisig.rs @@ -14,12 +14,12 @@ use curve25519_dalek::{ use group::Group; use transcript::{Transcript, RecommendedTranscript}; -use frost::{FrostError, MultisigView, algorithm::Algorithm}; +use frost::{curve::Ed25519, FrostError, MultisigView, algorithm::Algorithm}; use dalek_ff_group as dfg; use crate::{ hash_to_point, - frost::{MultisigError, Ed25519, DLEqProof, read_dleq}, + frost::{MultisigError, DLEqProof, read_dleq}, ringct::clsag::{ClsagInput, Clsag} }; diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/src/tests/clsag.rs index e35c7972..66644a50 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/src/tests/clsag.rs @@ -7,6 +7,8 @@ use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar}; #[cfg(feature = "multisig")] use transcript::RecommendedTranscript; +#[cfg(feature = "multisig")] +use frost::curve::Ed25519; use crate::{ Commitment, @@ -15,7 +17,7 @@ use crate::{ ringct::clsag::{ClsagInput, Clsag} }; #[cfg(feature = "multisig")] -use crate::{frost::{Ed25519, MultisigError}, ringct::clsag::{ClsagDetails, ClsagMultisig}}; +use crate::{frost::MultisigError, ringct::clsag::{ClsagDetails, ClsagMultisig}}; #[cfg(feature = "multisig")] use frost::tests::{key_gen, algorithm_machines, sign}; diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index ca9db422..89eaa6d0 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -7,6 +7,7 @@ use curve25519_dalek::{traits::Identity, scalar::Scalar, edwards::{EdwardsPoint, use transcript::{Transcript, RecommendedTranscript}; use frost::{ + curve::Ed25519, FrostError, MultisigKeys, sign::{ PreprocessMachine, SignMachine, SignatureMachine, @@ -15,7 +16,6 @@ use frost::{ }; use crate::{ - frost::Ed25519, random_scalar, ringct::{clsag::{ClsagInput, ClsagDetails, ClsagMultisig}, bulletproofs::Bulletproofs, RctPrunable}, transaction::{Input, Transaction}, rpc::Rpc, diff --git a/coins/monero/tests/send.rs b/coins/monero/tests/send.rs index ba05f338..c8543d9f 100644 --- a/coins/monero/tests/send.rs +++ b/coins/monero/tests/send.rs @@ -14,7 +14,9 @@ use curve25519_dalek::constants::ED25519_BASEPOINT_TABLE; #[cfg(feature = "multisig")] use dalek_ff_group::Scalar; #[cfg(feature = "multisig")] -use frost::tests::{THRESHOLD, key_gen, sign}; +use transcript::RecommendedTranscript; +#[cfg(feature = "multisig")] +use frost::{curve::Ed25519, tests::{THRESHOLD, key_gen, sign}}; use monero::{ network::Network, @@ -26,11 +28,6 @@ use monero_serai::{random_scalar, wallet::SignableTransaction}; mod rpc; use crate::rpc::{rpc, mine_block}; -#[cfg(feature = "multisig")] -use transcript::RecommendedTranscript; -#[cfg(feature = "multisig")] -use monero_serai::frost::Ed25519; - lazy_static! { static ref SEQUENTIAL: Mutex<()> = Mutex::new(()); } diff --git a/crypto/frost/src/curves/dalek.rs b/crypto/frost/src/curve/dalek.rs similarity index 98% rename from crypto/frost/src/curves/dalek.rs rename to crypto/frost/src/curve/dalek.rs index 1ba41918..362a9614 100644 --- a/crypto/frost/src/curves/dalek.rs +++ b/crypto/frost/src/curve/dalek.rs @@ -8,7 +8,7 @@ use group::{ff::PrimeField, Group}; use dalek_ff_group::Scalar; -use crate::{CurveError, Curve, algorithm::Hram}; +use crate::{curve::{CurveError, Curve}, algorithm::Hram}; macro_rules! dalek_curve { ( @@ -125,22 +125,6 @@ macro_rules! dalek_curve { } } -#[cfg(feature = "ed25519")] -dalek_curve!( - Ed25519, - IetfEd25519Hram, - EdwardsPoint, - CompressedEdwardsY, - EdwardsBasepointTable, - ED25519_BASEPOINT_POINT, - ED25519_BASEPOINT_TABLE, - |point: EdwardsPoint| !bool::from(point.is_torsion_free()), - b"edwards25519", - b"", - b"", - b"", -); - #[cfg(any(test, feature = "ristretto"))] dalek_curve!( Ristretto, @@ -156,3 +140,19 @@ dalek_curve!( b"chal", b"digest", ); + +#[cfg(feature = "ed25519")] +dalek_curve!( + Ed25519, + IetfEd25519Hram, + EdwardsPoint, + CompressedEdwardsY, + EdwardsBasepointTable, + ED25519_BASEPOINT_POINT, + ED25519_BASEPOINT_TABLE, + |point: EdwardsPoint| !bool::from(point.is_torsion_free()), + b"edwards25519", + b"", + b"", + b"", +); diff --git a/crypto/frost/src/curves/kp256.rs b/crypto/frost/src/curve/kp256.rs similarity index 98% rename from crypto/frost/src/curves/kp256.rs rename to crypto/frost/src/curve/kp256.rs index c7568708..1b762978 100644 --- a/crypto/frost/src/curves/kp256.rs +++ b/crypto/frost/src/curve/kp256.rs @@ -8,7 +8,7 @@ use group::{ff::{Field, PrimeField}, Group, GroupEncoding}; use elliptic_curve::{bigint::{Encoding, U384}, hash2curve::{Expander, ExpandMsg, ExpandMsgXmd}}; -use crate::{curves::{CurveError, Curve}, algorithm::Hram}; +use crate::{curve::{CurveError, Curve}, algorithm::Hram}; macro_rules! kp_curve { ( diff --git a/crypto/frost/src/curve/mod.rs b/crypto/frost/src/curve/mod.rs new file mode 100644 index 00000000..f6ad5cf9 --- /dev/null +++ b/crypto/frost/src/curve/mod.rs @@ -0,0 +1,121 @@ +use core::{ops::Mul, fmt::Debug}; + +use thiserror::Error; + +use rand_core::{RngCore, CryptoRng}; + +use group::{ff::PrimeField, Group, GroupOps}; + +#[cfg(any(test, feature = "dalek"))] +mod dalek; +#[cfg(any(test, feature = "ristretto"))] +pub use dalek::{Ristretto, IetfRistrettoHram}; +#[cfg(feature = "ed25519")] +pub use dalek::{Ed25519, IetfEd25519Hram}; + +#[cfg(feature = "kp256")] +mod kp256; +#[cfg(feature = "secp256k1")] +pub use kp256::{Secp256k1, NonIetfSecp256k1Hram}; +#[cfg(feature = "p256")] +pub use kp256::{P256, IetfP256Hram}; + +/// Set of errors for curve-related operations, namely encoding and decoding +#[derive(Clone, Error, Debug)] +pub enum CurveError { + #[error("invalid length for data (expected {0}, got {0})")] + InvalidLength(usize, usize), + #[error("invalid scalar")] + InvalidScalar, + #[error("invalid point")] + InvalidPoint, +} + +/// Unified trait to manage a field/group +// This should be moved into its own crate if the need for generic cryptography over ff/group +// continues, which is the exact reason ff/group exists (to provide a generic interface) +// elliptic-curve exists, yet it doesn't really serve the same role, nor does it use &[u8]/Vec +// It uses GenericArray which will hopefully be deprecated as Rust evolves and doesn't offer enough +// advantages in the modern day to be worth the hassle -- Kayaba +pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { + /// Scalar field element type + // This is available via G::Scalar yet `C::G::Scalar` is ambiguous, forcing horrific accesses + type F: PrimeField; + /// Group element type + type G: Group + GroupOps; + /// Precomputed table type + type T: Mul; + + /// ID for this curve + const ID: &'static [u8]; + + /// Generator for the group + // While group does provide this in its API, privacy coins will want to use a custom basepoint + const GENERATOR: Self::G; + + /// Table for the generator for the group + /// If there isn't a precomputed table available, the generator itself should be used + const GENERATOR_TABLE: Self::T; + + /// If little endian is used for the scalar field's Repr + const LITTLE_ENDIAN: bool; + + /// Securely generate a random nonce. H4 from the IETF draft + fn random_nonce(secret: Self::F, rng: &mut R) -> Self::F; + + /// Hash the message for the binding factor. H3 from the IETF draft + // This doesn't actually need to be part of Curve as it does nothing with the curve + // This also solely relates to FROST and with a proper Algorithm/HRAM, all projects using + // aggregatable signatures over this curve will work without issue + // It is kept here as Curve + H{1, 2, 3} is effectively a ciphersuite according to the IETF draft + // and moving it to Schnorr would force all of them into being ciphersuite-specific + // H2 is left to the Schnorr Algorithm as H2 is the H used in HRAM, which Schnorr further + // modularizes + fn hash_msg(msg: &[u8]) -> Vec; + + /// Hash the commitments and message to calculate the binding factor. H1 from the IETF draft + fn hash_binding_factor(binding: &[u8]) -> Self::F; + + // The following methods would optimally be F:: and G:: yet developers can't control F/G + // They can control a trait they pass into this library + + /// Field element from hash. Used during key gen and by other crates under Serai as a general + /// utility + // Not parameterized by Digest as it's fine for it to use its own hash function as relevant to + // hash_msg and hash_binding_factor + #[allow(non_snake_case)] + fn hash_to_F(dst: &[u8], msg: &[u8]) -> Self::F; + + /// Constant size of a serialized scalar field element + // The alternative way to grab this would be either serializing a junk element and getting its + // length or doing a naive division of its BITS property by 8 and assuming a lack of padding + #[allow(non_snake_case)] + fn F_len() -> usize; + + /// Constant size of a serialized group element + // We could grab the serialization as described above yet a naive developer may use a + // non-constant size encoding, proving yet another reason to force this to be a provided constant + // A naive developer could still provide a constant for a variable length encoding, yet at least + // that is on them + #[allow(non_snake_case)] + fn G_len() -> usize; + + /// Field element from slice. Preferred to be canonical yet does not have to be + // Required due to the lack of standardized encoding functions provided by ff/group + // 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_slice(slice: &[u8]) -> Result; + + /// Group element from slice. Must require canonicity or risks differing binding factors + #[allow(non_snake_case)] + fn G_from_slice(slice: &[u8]) -> Result; + + /// Obtain a vector of the byte encoding of F + #[allow(non_snake_case)] + fn F_to_bytes(f: &Self::F) -> Vec; + + /// Obtain a vector of the byte encoding of G + #[allow(non_snake_case)] + fn G_to_bytes(g: &Self::G) -> Vec; +} diff --git a/crypto/frost/src/curves/mod.rs b/crypto/frost/src/curves/mod.rs deleted file mode 100644 index 3742e1f9..00000000 --- a/crypto/frost/src/curves/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -#[cfg(any(test, feature = "dalek"))] -pub mod dalek; - -#[cfg(feature = "kp256")] -pub mod kp256; diff --git a/crypto/frost/src/key_gen.rs b/crypto/frost/src/key_gen.rs index 061260c6..f48a82fd 100644 --- a/crypto/frost/src/key_gen.rs +++ b/crypto/frost/src/key_gen.rs @@ -7,7 +7,8 @@ use group::ff::{Field, PrimeField}; use multiexp::{multiexp_vartime, BatchVerifier}; use crate::{ - Curve, MultisigParams, MultisigKeys, FrostError, + curve::Curve, + FrostError, MultisigParams, MultisigKeys, schnorr::{self, SchnorrSignature}, validate_map }; diff --git a/crypto/frost/src/lib.rs b/crypto/frost/src/lib.rs index f3b8b2bd..2e53b723 100644 --- a/crypto/frost/src/lib.rs +++ b/crypto/frost/src/lib.rs @@ -1,122 +1,20 @@ -use core::{ops::Mul, fmt::Debug}; +use core::fmt::Debug; use std::collections::HashMap; use thiserror::Error; -use rand_core::{RngCore, CryptoRng}; - -use group::{ff::{Field, PrimeField}, Group, GroupOps}; +use group::ff::{Field, PrimeField}; mod schnorr; +pub mod curve; +use curve::Curve; pub mod key_gen; pub mod algorithm; pub mod sign; -#[cfg(any(test, feature = "curves"))] -pub mod curves; pub mod tests; -/// Set of errors for curve-related operations, namely encoding and decoding -#[derive(Clone, Error, Debug)] -pub enum CurveError { - #[error("invalid length for data (expected {0}, got {0})")] - InvalidLength(usize, usize), - #[error("invalid scalar")] - InvalidScalar, - #[error("invalid point")] - InvalidPoint, -} - -/// Unified trait to manage a field/group -// This should be moved into its own crate if the need for generic cryptography over ff/group -// continues, which is the exact reason ff/group exists (to provide a generic interface) -// elliptic-curve exists, yet it doesn't really serve the same role, nor does it use &[u8]/Vec -// It uses GenericArray which will hopefully be deprecated as Rust evolves and doesn't offer enough -// advantages in the modern day to be worth the hassle -- Kayaba -pub trait Curve: Clone + Copy + PartialEq + Eq + Debug { - /// Scalar field element type - // This is available via G::Scalar yet `C::G::Scalar` is ambiguous, forcing horrific accesses - type F: PrimeField; - /// Group element type - type G: Group + GroupOps; - /// Precomputed table type - type T: Mul; - - /// ID for this curve - const ID: &'static [u8]; - - /// Generator for the group - // While group does provide this in its API, privacy coins will want to use a custom basepoint - const GENERATOR: Self::G; - - /// Table for the generator for the group - /// If there isn't a precomputed table available, the generator itself should be used - const GENERATOR_TABLE: Self::T; - - /// If little endian is used for the scalar field's Repr - const LITTLE_ENDIAN: bool; - - /// Securely generate a random nonce. H4 from the IETF draft - fn random_nonce(secret: Self::F, rng: &mut R) -> Self::F; - - /// Hash the message for the binding factor. H3 from the IETF draft - // This doesn't actually need to be part of Curve as it does nothing with the curve - // This also solely relates to FROST and with a proper Algorithm/HRAM, all projects using - // aggregatable signatures over this curve will work without issue - // It is kept here as Curve + H{1, 2, 3} is effectively a ciphersuite according to the IETF draft - // and moving it to Schnorr would force all of them into being ciphersuite-specific - // H2 is left to the Schnorr Algorithm as H2 is the H used in HRAM, which Schnorr further - // modularizes - fn hash_msg(msg: &[u8]) -> Vec; - - /// Hash the commitments and message to calculate the binding factor. H1 from the IETF draft - fn hash_binding_factor(binding: &[u8]) -> Self::F; - - // The following methods would optimally be F:: and G:: yet developers can't control F/G - // They can control a trait they pass into this library - - /// Field element from hash. Used during key gen and by other crates under Serai as a general - /// utility - // Not parameterized by Digest as it's fine for it to use its own hash function as relevant to - // hash_msg and hash_binding_factor - #[allow(non_snake_case)] - fn hash_to_F(dst: &[u8], msg: &[u8]) -> Self::F; - - /// Constant size of a serialized scalar field element - // The alternative way to grab this would be either serializing a junk element and getting its - // length or doing a naive division of its BITS property by 8 and assuming a lack of padding - #[allow(non_snake_case)] - fn F_len() -> usize; - - /// Constant size of a serialized group element - // We could grab the serialization as described above yet a naive developer may use a - // non-constant size encoding, proving yet another reason to force this to be a provided constant - // A naive developer could still provide a constant for a variable length encoding, yet at least - // that is on them - #[allow(non_snake_case)] - fn G_len() -> usize; - - /// Field element from slice. Preferred to be canonical yet does not have to be - // Required due to the lack of standardized encoding functions provided by ff/group - // 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_slice(slice: &[u8]) -> Result; - - /// Group element from slice. Must require canonicity or risks differing binding factors - #[allow(non_snake_case)] - fn G_from_slice(slice: &[u8]) -> Result; - - /// Obtain a vector of the byte encoding of F - #[allow(non_snake_case)] - fn F_to_bytes(f: &Self::F) -> Vec; - - /// Obtain a vector of the byte encoding of G - #[allow(non_snake_case)] - fn G_to_bytes(g: &Self::G) -> Vec; -} - /// Parameters for a multisig // These fields can not be made public as they should be static #[derive(Clone, Copy, PartialEq, Eq, Debug)] diff --git a/crypto/frost/src/sign.rs b/crypto/frost/src/sign.rs index 289165aa..8ea0f61c 100644 --- a/crypto/frost/src/sign.rs +++ b/crypto/frost/src/sign.rs @@ -8,7 +8,7 @@ use group::ff::Field; use transcript::Transcript; use crate::{ - Curve, + curve::Curve, FrostError, MultisigParams, MultisigKeys, MultisigView, algorithm::Algorithm, diff --git a/crypto/frost/src/tests/literal/dalek.rs b/crypto/frost/src/tests/literal/dalek.rs index 7cd3e92f..fdcc0c0f 100644 --- a/crypto/frost/src/tests/literal/dalek.rs +++ b/crypto/frost/src/tests/literal/dalek.rs @@ -1,11 +1,11 @@ use rand::rngs::OsRng; -use crate::{curves::dalek, tests::vectors::{Vectors, test_with_vectors}}; +use crate::{curve, tests::vectors::{Vectors, test_with_vectors}}; #[cfg(any(test, feature = "ristretto"))] #[test] fn ristretto_vectors() { - test_with_vectors::<_, dalek::Ristretto, dalek::IetfRistrettoHram>( + test_with_vectors::<_, curve::Ristretto, curve::IetfRistrettoHram>( &mut OsRng, Vectors { threshold: 2, @@ -42,7 +42,7 @@ fn ristretto_vectors() { #[cfg(feature = "ed25519")] #[test] fn ed25519_vectors() { - test_with_vectors::<_, dalek::Ed25519, dalek::IetfEd25519Hram>( + test_with_vectors::<_, curve::Ed25519, curve::IetfEd25519Hram>( &mut OsRng, Vectors { threshold: 2, diff --git a/crypto/frost/src/tests/literal/kp256.rs b/crypto/frost/src/tests/literal/kp256.rs index 07db5073..dee20157 100644 --- a/crypto/frost/src/tests/literal/kp256.rs +++ b/crypto/frost/src/tests/literal/kp256.rs @@ -3,12 +3,12 @@ use rand::rngs::OsRng; #[cfg(feature = "secp256k1")] use crate::tests::{curve::test_curve, schnorr::test_schnorr}; #[cfg(feature = "secp256k1")] -use crate::curves::kp256::Secp256k1; +use crate::curve::Secp256k1; #[cfg(feature = "p256")] use crate::tests::vectors::{Vectors, test_with_vectors}; #[cfg(feature = "p256")] -use crate::curves::kp256::{P256, IetfP256Hram}; +use crate::curve::{P256, IetfP256Hram}; #[cfg(feature = "secp256k1")] #[test] diff --git a/processor/src/coins/monero.rs b/processor/src/coins/monero.rs index f7156cb1..06dc2f75 100644 --- a/processor/src/coins/monero.rs +++ b/processor/src/coins/monero.rs @@ -6,11 +6,10 @@ use curve25519_dalek::{constants::ED25519_BASEPOINT_TABLE, scalar::Scalar}; use dalek_ff_group as dfg; use transcript::RecommendedTranscript; -use frost::MultisigKeys; +use frost::{curve::Ed25519, MultisigKeys}; use monero::{PublicKey, network::Network, util::address::Address}; use monero_serai::{ - frost::Ed25519, transaction::{Timelock, Transaction}, rpc::Rpc, wallet::{Fee, SpendableOutput, SignableTransaction as MSignableTransaction, TransactionMachine} diff --git a/processor/src/lib.rs b/processor/src/lib.rs index 678cb288..caf92d2f 100644 --- a/processor/src/lib.rs +++ b/processor/src/lib.rs @@ -3,9 +3,8 @@ use std::{marker::Send, sync::Arc, collections::HashMap}; use async_trait::async_trait; use thiserror::Error; -use frost::{Curve, FrostError, MultisigKeys, sign::PreprocessMachine}; - use transcript::RecommendedTranscript; +use frost::{curve::Curve, FrostError, MultisigKeys, sign::PreprocessMachine}; mod coins; mod wallet; diff --git a/processor/src/tests/mod.rs b/processor/src/tests/mod.rs index e7214b97..eacb8223 100644 --- a/processor/src/tests/mod.rs +++ b/processor/src/tests/mod.rs @@ -6,7 +6,7 @@ use rand::rngs::OsRng; use group::Group; -use frost::Curve; +use frost::curve::Curve; use crate::{ NetworkError, Network, diff --git a/processor/src/wallet.rs b/processor/src/wallet.rs index e7bdb50e..0680ea53 100644 --- a/processor/src/wallet.rs +++ b/processor/src/wallet.rs @@ -4,7 +4,7 @@ use rand_core::OsRng; use transcript::{Transcript, RecommendedTranscript}; -use frost::{Curve, MultisigKeys, sign::{PreprocessMachine, SignMachine, SignatureMachine}}; +use frost::{curve::Curve, MultisigKeys, sign::{PreprocessMachine, SignMachine, SignatureMachine}}; use crate::{CoinError, SignError, Output, Coin, Network};