diff --git a/.gitignore b/.gitignore index 75e2c55..aae833a 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /wasm/no_std/target /.helix +.cspell.config.yaml diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 8ddf1b9..c45491d 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -7,8 +7,6 @@ //! * [Ed25519], requires `ciphersuite-ed25519` feature //! * [Bitcoin], requires `ciphersuite-bitcoin` feature -use alloc::vec::Vec; - use generic_ec::{ errors::{InvalidPoint, InvalidScalar}, Curve, NonZero, Point, Scalar, SecretScalar, @@ -47,14 +45,14 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// Preferred [multiscalar multiplication](generic_ec::multiscalar) algorithm /// - /// Multiscalar multiplication optimization greatly improves performace of FROST protocol. + /// Multiscalar multiplication optimization greatly improves performance of FROST protocol. /// By default, we set it to [`generic_ec::multiscalar::Default`] which uses the fastest /// algorithm available in [`generic_ec`] crate. type MultiscalarMul: generic_ec::multiscalar::MultiscalarMul; /// `H1` hash function as defined in the draft /// - /// Accepts a list of bytestring, that'll be contatenated before hashing. + /// Accepts a list of bytestring, that'll be concatenated before hashing. /// Returns `H1(data[0] || data[1] || ... || data[data.len() - 1])`. fn h1(msg: &[&[u8]]) -> Scalar; /// Computes the challenge according to Schnorr scheme @@ -67,18 +65,18 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { ) -> Scalar; /// `H3` hash function as defined in the draft /// - /// Accepts a list of bytestring, that'll be contatenated before hashing. + /// Accepts a list of bytestring, that'll be concatenated before hashing. /// Returns `H3(data[0] || data[1] || ... || data[data.len() - 1])`. fn h3(msg: &[&[u8]]) -> Scalar; /// `H4` hash function as defined in the draft /// - /// Accepts a list of bytestring, that'll be contatenated before hashing. + /// Accepts a list of bytestring, that'll be concatenated before hashing. /// Returns `H4(data[0] || data[1] || ... || data[data.len() - 1])`. fn h4() -> Self::Digest; /// `H5` hash function as defined in the draft /// - /// Accepts a list of bytestring, that'll be contatenated before hashing. + /// Accepts a list of bytestring, that'll be concatenated before hashing. /// Returns `H5(data[0] || data[1] || ... || data[data.len() - 1])`. fn h5() -> Self::Digest; @@ -115,9 +113,9 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// /// Our implementation of FROST requires that if point $X$ isn't normalized, then $-X$ is normalized. Zero point /// (aka point at infinity) is always normalized. Note that certain parts of the protocol may enforce this property - /// via debug assertations. + /// via debug assertions. /// - /// The protocol always outputs sigantures with normalized R-component. We also require that public key is + /// The protocol always outputs signatures with normalized R-component. We also require that public key is /// normalized. If it isn't, signing fails. You can use [`normalize_key_share`] function to normalize any key. /// /// If Schnorr scheme doesn't have a notion of normalized points, this function should always return `true`. @@ -176,7 +174,7 @@ pub trait AdditionalEntropy { where Self: 'b; - /// Returns bytes representation of the entropy encoded in complience with [`C`](Ciphersuite) + /// Returns bytes representation of the entropy encoded in compliance with [`C`](Ciphersuite) fn to_bytes(&self) -> Self::Bytes<'_>; } @@ -231,7 +229,7 @@ impl> AdditionalEntropy for &T { pub struct NormalizedPoint(P, core::marker::PhantomData); impl>> NormalizedPoint { - /// Serialzies the normalized point in a space-efficient manner + /// Serializes the normalized point in a space-efficient manner /// /// Alias to [`Ciphersuite::serialize_normalized_point`] pub fn to_bytes(&self) -> C::NormalizedPointBytes { @@ -310,49 +308,6 @@ where } } -/// Normalizes the key share -/// -/// Some Schnorr signing schemes require public key to be [normalized](Ciphersuite::is_normalized). -/// Generally, DKG protocols output arbitrary public keys which are not necessarily normalized. If -/// you want to use this key in the Schnorr scheme that requires public keys to be normalized, each -/// key share needs to be normalized using this function. -/// -/// ## Error -/// Returns an error if output key share is inconsistent. That should never happen in practice. If -/// it happened, it's probably a bug. -pub fn normalize_key_share( - key_share: crate::key_share::KeyShare, -) -> Result, crate::key_share::InvalidKeyShare> { - use crate::key_share::Validate; - - let Err(new_pk) = NormalizedPoint::::try_normalize(key_share.shared_public_key) else { - // Public key is already normalized - return Ok(key_share); - }; - let key_share = key_share.into_inner(); - - // PK is negated. Now we need to negate the key share, and each public key share - let new_sk = -key_share.x; - let public_shares = key_share - .key_info - .public_shares - .iter() - .map(|p| -p) - .collect::>(); - - crate::key_share::DirtyKeyShare { - i: key_share.i, - key_info: crate::key_share::DirtyKeyInfo { - shared_public_key: *new_pk, - public_shares, - ..key_share.key_info - }, - x: new_sk, - } - .validate() - .map_err(|e| e.into_error()) -} - /// Checks whether `key_share` is normalized /// /// `key_share` must be normalized, otherwise the signing will return error. See [`Ciphersuite::is_normalized`] diff --git a/givre/src/ciphersuite/bitcoin.rs b/givre/src/ciphersuite/bitcoin.rs index 56486ed..7fdb6ce 100644 --- a/givre/src/ciphersuite/bitcoin.rs +++ b/givre/src/ciphersuite/bitcoin.rs @@ -3,7 +3,7 @@ use sha2::Digest; use super::{Ciphersuite, Secp256k1}; -/// FROST ciphersuite that outputs [BIP-340] compliant sigantures +/// FROST ciphersuite that outputs [BIP-340] compliant signatures /// /// # Normalized public keys /// BIP-340 requires that public keys are normalized, meaning that they must have diff --git a/givre/src/lib.rs b/givre/src/lib.rs index 9495ca0..348b22b 100644 --- a/givre/src/lib.rs +++ b/givre/src/lib.rs @@ -6,7 +6,7 @@ //! This crate provides: //! * Distributed Key Generation (DKG) \ //! FROST does not define DKG protocol to be used. We simply re-export DKG based on [CGGMP21] implementation -//! when `cggmp21-keygen` feature is enabled, which is a fairly reasonalbe choice as it's proven to be UC-secure. +//! when `cggmp21-keygen` feature is enabled, which is a fairly reasonable choice as it's proven to be UC-secure. //! Alternatively, you can use any other UC-secure DKG protocol. //! * FROST Signing \ //! We provide API for both manual signing execution (for better flexibility and efficiency) and interactive protocol @@ -23,7 +23,7 @@ //! //! ## Distributed Key Generation (DKG) //! First of all, you need to generate a key. For that purpose, you can use any secure -//! (preferrably, UC-secure) DKG protocol. FROST IETF Draft does not define any DKG +//! (preferably, UC-secure) DKG protocol. FROST IETF Draft does not define any DKG //! protocol or requirements it needs to meet, so the choice is up to you. This library //! re-exports CGGMP21 DKG from [`cggmp21-keygen`] crate when `cggmp21-keygen` feature //! is enabled which is proven to be UC-secure and should be a reasonable default. @@ -39,7 +39,7 @@ //! //! where: //! * `Msg` is a protocol message (e.g., [`keygen::msg::threshold::Msg`]) -//! * [`round_based::Incoming`] and [`round_based::Outgoing`] wrap `Msg` and provide additional data (e.g., sender/recepient) +//! * [`round_based::Incoming`] and [`round_based::Outgoing`] wrap `Msg` and provide additional data (e.g., sender/recipient) //! * [`futures::Stream`] and [`futures::Sink`] are well-known async primitives. //! //! [`futures::Stream`]: https://docs.rs/futures/latest/futures/stream/trait.Stream.html @@ -262,7 +262,7 @@ pub type SignerIndex = u16; /// /// This can be less efficient than doing signing manually, when you can commit nonces before /// a message to be signed is known, but more secure, as using this function ensures that -/// protocol isn't misused (e.g. that nonce is never resused). +/// protocol isn't misused (e.g. that nonce is never reused). /// /// ## Inputs /// * Signer index in *signing protocol* $0 \le i < \\text{min\\_signers}$ diff --git a/givre/src/signing/aggregate.rs b/givre/src/signing/aggregate.rs index d78e154..0eb5934 100644 --- a/givre/src/signing/aggregate.rs +++ b/givre/src/signing/aggregate.rs @@ -100,11 +100,23 @@ pub fn aggregate( msg: &[u8], ) -> Result, AggregateError> { // --- Retrieve and Validate Data + let crate::key_share::DirtyKeyInfo { + shared_public_key: pk, + vss_setup, + .. + } = &**key_info; + // Make sure we never use (potentially non-normalized) `key_info` anywhere + // below by shadowing the variable. + #[allow(unused_variables)] + let key_info = (); + + // Normalize the public key + let pk = C::normalize_point(*pk); + let mut comm_list = signers .iter() .map(|(j, comm, _sig_share)| { - key_info - .share_preimage(*j) + utils::share_preimage(vss_setup, *j) .map(|id| (id, *comm)) .ok_or(Reason::UnknownSigner(*j)) }) @@ -122,8 +134,7 @@ pub fn aggregate( } // --- The Aggregation - let binding_factor_list = - utils::compute_binding_factors::(key_info.shared_public_key, &comm_list, msg); + let binding_factor_list = utils::compute_binding_factors::(*pk, &comm_list, msg); let group_commitment = utils::compute_group_commitment::(&comm_list, &binding_factor_list); let z = signers .iter() @@ -134,8 +145,7 @@ pub fn aggregate( r: C::normalize_point(group_commitment), z, }; - sig.verify(&C::normalize_point(key_info.shared_public_key), msg) - .map_err(|_| Reason::InvalidSig)?; + sig.verify(&pk, msg).map_err(|_| Reason::InvalidSig)?; Ok(sig) } diff --git a/givre/src/signing/round2.rs b/givre/src/signing/round2.rs index 39b1988..ca4ed44 100644 --- a/givre/src/signing/round2.rs +++ b/givre/src/signing/round2.rs @@ -6,10 +6,10 @@ //! //! [Section 5.2]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-15.html#name-round-two-signature-share-g -use alloc::vec::Vec; +use alloc::{borrow::Cow, vec::Vec}; use core::{fmt, iter}; -use generic_ec::{Curve, NonZero, Scalar}; +use generic_ec::{Curve, NonZero, Scalar, SecretScalar}; use crate::{ ciphersuite::{Ciphersuite, NormalizedPoint}, @@ -49,27 +49,54 @@ pub fn sign( signers: &[(SignerIndex, PublicCommitments)], ) -> Result, SigningError> { // --- Retrieve and Validate Data - let pk = NormalizedPoint::::try_normalize(key_share.shared_public_key) - .map_err(|_| Reason::ShareNotNormalized)?; - if signers.len() < usize::from(key_share.min_signers()) { + let t = key_share.min_signers(); + let crate::key_share::DirtyKeyShare { + i, + key_info: + crate::key_share::DirtyKeyInfo { + shared_public_key: pk, + vss_setup, + .. + }, + x, + } = &**key_share; + // Make sure we never use (potentially non-normalized) `key_share` anywhere + // below by shadowing the variable. + #[allow(unused_variables)] + let key_share = (); + + // Normalize the key share + let (x, pk): (Cow>>, NormalizedPoint) = + match NormalizedPoint::::try_normalize(*pk) { + Ok(pk) => { + // public key is already normalized, there's nothing to do + (Cow::Borrowed(&x), pk) + } + + Err(neg_pk) => { + // public key was not normalized. we need to negate the key share. + // note that we do not negate `public_shares` as they are not used + // anywhere in this function. + (Cow::Owned(-x), neg_pk) + } + }; + + if signers.len() < usize::from(t) { return Err(Reason::TooFewSigners { - min_signers: key_share.min_signers(), + min_signers: t, n: signers.len(), } .into()); } - let signer_id = key_share - .share_preimage(key_share.i) - .ok_or(Bug::RetrieveOwnShareId)?; + let signer_id = utils::share_preimage(vss_setup, *i).ok_or(Bug::RetrieveOwnShareId)?; let mut comm_list = signers .iter() .map(|(j, comm)| { - if key_share.i == *j && nonce.public_commitments() != *comm { + if i == j && nonce.public_commitments() != *comm { // Commitments don't match provided nonces - invalid inputs Err(Reason::NoncesDontMatchComm) } else { - key_share - .share_preimage(*j) + utils::share_preimage(vss_setup, *j) .map(|id| (id, *comm)) .ok_or(Reason::UnknownSigner(*j)) } @@ -102,8 +129,7 @@ pub fn sign( }; // --- The Signing - let binding_factor_list = - utils::compute_binding_factors::(key_share.shared_public_key, &comm_list, msg); + let binding_factor_list = utils::compute_binding_factors::(*pk, &comm_list, msg); let binding_factor = binding_factor_list.get(i).ok_or(Bug::OwnBindingFactor)?.1; debug_assert_eq!(binding_factor_list[i].0, signer_id); @@ -123,7 +149,7 @@ pub fn sign( }; let signers_list = comm_list.iter().map(|(i, _)| *i).collect::>(); - let lambda_i = if key_share.vss_setup.is_some() { + let lambda_i = if vss_setup.is_some() { derive_interpolating_value(&signers_list, &signer_id) .ok_or(Reason::DeriveInterpolationValue)? } else { @@ -132,9 +158,7 @@ pub fn sign( let challenge = C::compute_challenge(&group_commitment, &pk, msg); - Ok(SigShare( - nonce_share + (lambda_i * &key_share.x * challenge), - )) + Ok(SigShare(nonce_share + (lambda_i * &*x * challenge))) } /// Computes an interpolation value as described in [Section 4.2] @@ -189,7 +213,6 @@ pub struct SigningError(Reason); #[derive(Debug)] enum Reason { - ShareNotNormalized, TooFewSigners { min_signers: u16, n: usize }, UnknownSigner(SignerIndex), SameSignerTwice, @@ -208,7 +231,6 @@ enum Bug { impl fmt::Display for SigningError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.0 { - Reason::ShareNotNormalized => f.write_str("key share is not normalized, see the docs"), Reason::TooFewSigners { min_signers, n } => write!( f, "signers list contains {n} singners, although at \ @@ -244,8 +266,7 @@ impl fmt::Display for Bug { impl std::error::Error for SigningError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match &self.0 { - Reason::ShareNotNormalized - | Reason::TooFewSigners { .. } + Reason::TooFewSigners { .. } | Reason::UnknownSigner(_) | Reason::NoncesDontMatchComm | Reason::DeriveInterpolationValue diff --git a/givre/src/signing/utils.rs b/givre/src/signing/utils.rs index ec354cf..e8b578f 100644 --- a/givre/src/signing/utils.rs +++ b/givre/src/signing/utils.rs @@ -109,3 +109,22 @@ where { slice.windows(2).all(|win| f(&win[0]) <= f(&win[1])) } + +/// Returns share preimage associated with j-th signer +/// +/// * For additive shares, share preimage is defined as `j+1` +/// * For VSS-shares, share preimage is scalar $I_j$ such that $x_j = F(I_j)$ where +/// $F(x)$ is polynomial co-shared by the signers and $x_j$ is secret share of j-th +/// signer +pub fn share_preimage( + vss_setup: &Option>, + j: u16, +) -> Option>> { + match vss_setup { + Some(v) => v.I.get(usize::from(j)).copied(), + None => Some( + #[allow(clippy::expect_used)] + NonZero::from_scalar(Scalar::from(j + 1)).expect("j+1 is guaranteed to be non-zero"), + ), + } +} diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 605c435..4c5f447 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -1,5 +1,4 @@ use givre::{ - ciphersuite::NormalizedPoint, generic_ec::{NonZero, Point}, signing::aggregate::Signature, Ciphersuite, @@ -13,7 +12,7 @@ pub trait ExternalVerifier: Ciphersuite { type InvalidSig: core::fmt::Debug; fn verify_sig( - pk: &NormalizedPoint>>, + pk: &NonZero>, sig: &Signature, msg: &[u8], ) -> Result<(), Self::InvalidSig>; @@ -26,7 +25,7 @@ impl ExternalVerifier for givre::ciphersuite::Ed25519 { type InvalidSig = ed25519::SignatureError; fn verify_sig( - pk: &NormalizedPoint>>, + pk: &NonZero>, sig: &Signature, msg: &[u8], ) -> Result<(), ed25519::SignatureError> { @@ -50,7 +49,7 @@ impl ExternalVerifier for givre::ciphersuite::Secp256k1 { type InvalidSig = core::convert::Infallible; fn verify_sig( - _pk: &NormalizedPoint>>, + _pk: &NonZero>, _sig: &Signature, _msg: &[u8], ) -> Result<(), Self::InvalidSig> { @@ -65,10 +64,11 @@ impl ExternalVerifier for givre::ciphersuite::Bitcoin { type InvalidSig = secp256k1::Error; fn verify_sig( - pk: &NormalizedPoint>>, + pk: &NonZero>, sig: &Signature, msg: &[u8], ) -> Result<(), Self::InvalidSig> { + let pk = Self::normalize_point(*pk); let pk = secp256k1::XOnlyPublicKey::from_slice(pk.to_bytes().as_ref())?; let mut signature = [0u8; 64]; diff --git a/tests/tests/it/interactive.rs b/tests/tests/it/interactive.rs index 546ba28..abd3164 100644 --- a/tests/tests/it/interactive.rs +++ b/tests/tests/it/interactive.rs @@ -2,7 +2,7 @@ mod generic { use std::iter; - use givre::{ciphersuite::NormalizedPoint, Ciphersuite}; + use givre::Ciphersuite; use givre_tests::ExternalVerifier; use rand::{seq::SliceRandom, Rng, RngCore}; @@ -49,16 +49,7 @@ mod generic { futures::future::try_join_all(keygen_executions) .await .unwrap(); - // Normalize key shares - let key_shares = key_shares - .into_iter() - .map(givre::ciphersuite::normalize_key_share::) - .collect::, _>>() - .expect("normalize key share"); - let key_shares = key_shares.as_slice(); - let key_info: &givre::KeyInfo<_> = key_shares[0].as_ref(); - let pk = NormalizedPoint::try_normalize(key_info.shared_public_key) - .expect("output public key isn't normalized"); + let pk = key_shares[0].shared_public_key; // --- Signing @@ -81,10 +72,13 @@ mod generic { let signing_executions = (0..t) .zip(signers) .zip(iter::repeat_with(|| (rng.fork(), simulation.add_party()))) - .map(|((j, &index_at_keygen), (mut rng, party))| async move { - givre::signing::(j, &key_shares[usize::from(index_at_keygen)], signers, msg) - .sign(&mut rng, party) - .await + .map(|((j, &index_at_keygen), (mut rng, party))| { + let key_share = &key_shares[usize::from(index_at_keygen)]; + async move { + givre::signing::(j, key_share, signers, msg) + .sign(&mut rng, party) + .await + } }); let sigs: Vec> = @@ -92,7 +86,7 @@ mod generic { .await .unwrap(); - sigs[0].verify(&pk, msg).unwrap(); + sigs[0].verify(&C::normalize_point(pk), msg).unwrap(); C::verify_sig(&pk, &sigs[0], msg).unwrap(); for sig in &sigs[1..] { diff --git a/tests/tests/it/main.rs b/tests/tests/it/main.rs index 1e0d62d..566fd42 100644 --- a/tests/tests/it/main.rs +++ b/tests/tests/it/main.rs @@ -3,7 +3,7 @@ mod test_vectors; #[generic_tests::define(attrs(test_case::case, test))] mod generic { - use givre::{ciphersuite::NormalizedPoint, Ciphersuite}; + use givre::Ciphersuite; use givre_tests::ExternalVerifier; use rand::{seq::SliceRandom, Rng, RngCore}; @@ -21,14 +21,8 @@ mod generic { .set_threshold(t) .generate_shares(&mut rng) .unwrap(); - let key_shares = key_shares - .into_iter() - .map(givre::ciphersuite::normalize_key_share::) - .collect::, _>>() - .expect("normalize key shares"); let key_info: &givre::key_share::KeyInfo<_> = key_shares[0].as_ref(); - let pk = NormalizedPoint::try_normalize(key_info.shared_public_key) - .expect("public key is not normalized"); + let pk = key_info.shared_public_key; // List of indexes of signers who co-hold the key let key_holders = (0..n).collect::>(); @@ -76,7 +70,8 @@ mod generic { let sig = givre::signing::aggregate::aggregate::(key_info, &partial_sigs, &message) .expect("aggregation failed"); - sig.verify(&pk, &message).expect("invalid signature"); + sig.verify(&C::normalize_point(pk), &message) + .expect("invalid signature"); C::verify_sig(&pk, &sig, &message).expect("external verifier: invalid signature") }