From f3a1da85d5065473a4524d345fb55c93f8a9d5ef Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Mon, 11 Mar 2024 10:58:01 +0100 Subject: [PATCH 01/14] Change features names --- givre/Cargo.toml | 6 +++--- givre/src/ciphersuite.rs | 12 ++++++------ tests/Cargo.toml | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/givre/Cargo.toml b/givre/Cargo.toml index c561048..eb81d26 100644 --- a/givre/Cargo.toml +++ b/givre/Cargo.toml @@ -41,6 +41,6 @@ spof = ["key-share/spof"] # Library doesn't have support of HD signing yet. hd-wallets = ["key-share/hd-wallets", "cggmp21-keygen?/hd-wallets"] -all-curves = ["curve-secp256k1", "curve-ed25519"] -curve-secp256k1 = ["generic-ec/curve-secp256k1", "k256", "sha2", "static_assertions"] -curve-ed25519 = ["generic-ec/curve-ed25519", "sha2"] +all-ciphersuites = ["ciphersuite-secp256k1", "ciphersuite-ed25519"] +ciphersuite-secp256k1 = ["generic-ec/curve-secp256k1", "k256", "sha2", "static_assertions"] +ciphersuite-ed25519 = ["generic-ec/curve-ed25519", "sha2"] diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 4ce1d0b..af79d38 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -3,22 +3,22 @@ //! Ciphersuite specifies which curve and hash primitives to use during the signing. //! //! Out of the box, we provide ciphersuites defined the in the draft: -//! * [Secp256k1], requires `curve-secp256k1` feature -//! * [Ed25519], requires `curve-ed25519` feature +//! * [Secp256k1], requires `ciphersuite-secp256k1` feature +//! * [Ed25519], requires `ciphersuite-ed25519` feature use generic_ec::{ errors::{InvalidPoint, InvalidScalar}, Curve, Point, Scalar, SecretScalar, }; use rand_core::{CryptoRng, RngCore}; -#[cfg(feature = "curve-ed25519")] +#[cfg(feature = "ciphersuite-ed25519")] mod ed25519; -#[cfg(feature = "curve-secp256k1")] +#[cfg(feature = "ciphersuite-secp256k1")] mod secp256k1; -#[cfg(feature = "curve-ed25519")] +#[cfg(feature = "ciphersuite-ed25519")] pub use ed25519::Ed25519; -#[cfg(feature = "curve-secp256k1")] +#[cfg(feature = "ciphersuite-secp256k1")] pub use secp256k1::Secp256k1; /// Ciphersuite determines an underlying curve and set of cryptographic primitives diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 11e7ad1..d5b2c45 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -7,7 +7,7 @@ publish = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -givre = { path = "../givre", features = ["all-curves", "cggmp21-keygen", "spof", "hd-wallets", "full-signing"] } +givre = { path = "../givre", features = ["all-ciphersuites", "cggmp21-keygen", "spof", "hd-wallets", "full-signing"] } generic-tests = "0.1" test-case = "3.3" From d16e73cfa5883533bcc3f0d5832cf7c3bcae3c6e Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Mon, 11 Mar 2024 11:17:53 +0100 Subject: [PATCH 02/14] Move compute_challenge to ciphersuite --- givre/src/ciphersuite.rs | 11 +++++++---- givre/src/ciphersuite/ed25519.rs | 16 ++++++++++------ givre/src/ciphersuite/secp256k1.rs | 15 +++++++++++++-- givre/src/signing/aggregate.rs | 4 +--- givre/src/signing/round2.rs | 19 ++----------------- 5 files changed, 33 insertions(+), 32 deletions(-) diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index af79d38..737b8c3 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -42,11 +42,14 @@ pub trait Ciphersuite { /// Accepts a list of bytestring, that'll be contatenated before hashing. /// Returns `H1(data[0] || data[1] || ... || data[data.len() - 1])`. fn h1(msg: &[&[u8]]) -> Scalar; - /// `H2` hash function as defined in the draft + /// Computes the challenge according to Schnorr scheme /// - /// Accepts a list of bytestring, that'll be contatenated before hashing. - /// Returns `H2(data[0] || data[1] || ... || data[data.len() - 1])`. - fn h2(msg: &[&[u8]]) -> Scalar; + /// Implementation should be based on `H2` hash function as defined in the draft. + fn compute_challenge( + group_commitment: &Point, + group_public_key: &Point, + msg: &[u8], + ) -> Scalar; /// `H3` hash function as defined in the draft /// /// Accepts a list of bytestring, that'll be contatenated before hashing. diff --git a/givre/src/ciphersuite/ed25519.rs b/givre/src/ciphersuite/ed25519.rs index 3ee3022..3e7151d 100644 --- a/givre/src/ciphersuite/ed25519.rs +++ b/givre/src/ciphersuite/ed25519.rs @@ -24,12 +24,16 @@ impl Ciphersuite for Ed25519 { generic_ec::Scalar::from_le_bytes_mod_order(hash) } - fn h2(msg: &[&[u8]]) -> generic_ec::Scalar { - let mut hash = sha2::Sha512::new(); - for msg in msg { - hash.update(msg); - } - let hash = hash.finalize(); + fn compute_challenge( + group_commitment: &generic_ec::Point, + group_public_key: &generic_ec::Point, + msg: &[u8], + ) -> generic_ec::Scalar { + let hash = sha2::Sha512::new() + .chain_update(Self::serialize_point(group_commitment)) + .chain_update(Self::serialize_point(&group_public_key)) + .chain_update(msg) + .finalize(); generic_ec::Scalar::from_le_bytes_mod_order(hash) } diff --git a/givre/src/ciphersuite/secp256k1.rs b/givre/src/ciphersuite/secp256k1.rs index 399da08..8e7e4cc 100644 --- a/givre/src/ciphersuite/secp256k1.rs +++ b/givre/src/ciphersuite/secp256k1.rs @@ -16,8 +16,19 @@ impl Ciphersuite for Secp256k1 { hash_to_scalar(msg, &[Self::NAME.as_bytes(), b"rho"]) } - fn h2(msg: &[&[u8]]) -> generic_ec::Scalar { - hash_to_scalar(msg, &[Self::NAME.as_bytes(), b"chal"]) + fn compute_challenge( + group_commitment: &generic_ec::Point, + group_public_key: &generic_ec::Point, + msg: &[u8], + ) -> generic_ec::Scalar { + hash_to_scalar( + &[ + Self::serialize_point(group_commitment).as_ref(), + Self::serialize_point(group_public_key).as_ref(), + msg, + ], + &[Self::NAME.as_bytes(), b"chal"], + ) } fn h3(msg: &[&[u8]]) -> generic_ec::Scalar { diff --git a/givre/src/signing/aggregate.rs b/givre/src/signing/aggregate.rs index 47f0103..807106c 100644 --- a/givre/src/signing/aggregate.rs +++ b/givre/src/signing/aggregate.rs @@ -35,9 +35,7 @@ impl Signature { public_key: &Point, msg: &[u8], ) -> Result<(), InvalidSignature> { - let comm_bytes = C::serialize_point(&self.r); - let pk_bytes = C::serialize_point(public_key); - let challenge = C::h2(&[comm_bytes.as_ref(), pk_bytes.as_ref(), msg]); + let challenge = C::compute_challenge(&self.r, &public_key, msg); let lhs = Point::generator() * &self.z; let rhs = self.r + public_key * challenge; diff --git a/givre/src/signing/round2.rs b/givre/src/signing/round2.rs index 3c86142..2a3f82e 100644 --- a/givre/src/signing/round2.rs +++ b/givre/src/signing/round2.rs @@ -8,7 +8,7 @@ use core::{fmt, iter}; -use generic_ec::{Curve, NonZero, Point, Scalar}; +use generic_ec::{Curve, NonZero, Scalar}; use crate::{ciphersuite::Ciphersuite, KeyShare, SignerIndex}; @@ -111,7 +111,7 @@ pub fn sign( Scalar::one() }; - let challenge = compute_challenge::(&group_commitment, &key_share.shared_public_key, msg); + let challenge = C::compute_challenge(&group_commitment, &key_share.shared_public_key, msg); Ok(SigShare( nonce.hiding_nonce @@ -166,21 +166,6 @@ fn derive_interpolating_value( Some(num * denom.invert()) } -/// Computes a challenge as described in [Section 4.6] -/// -/// [Section 4.6]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-15.html#name-signature-challenge-computa -fn compute_challenge( - group_commitment: &Point, - group_pk: &Point, - msg: &[u8], -) -> Scalar { - C::h2(&[ - C::serialize_point(&group_commitment).as_ref(), - C::serialize_point(&group_pk).as_ref(), - msg, - ]) -} - /// Signing error #[derive(Debug)] pub struct SigningError(Reason); From 143da77462251f43f0d683f1975cd30030f36eae Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Tue, 12 Mar 2024 11:47:34 +0100 Subject: [PATCH 03/14] Add bitcoin support --- Cargo.lock | 20 ++++ givre/Cargo.toml | 5 +- givre/src/ciphersuite.rs | 146 ++++++++++++++++++++++++++++- givre/src/ciphersuite/bitcoin.rs | 81 ++++++++++++++++ givre/src/ciphersuite/ed25519.rs | 15 ++- givre/src/ciphersuite/secp256k1.rs | 15 ++- givre/src/signing/aggregate.rs | 22 ++--- givre/src/signing/full_signing.rs | 14 +-- givre/src/signing/round2.rs | 31 ++++-- tests/Cargo.toml | 1 + tests/src/lib.rs | 43 +++++++-- tests/tests/it/interactive.rs | 25 +++-- tests/tests/it/main.rs | 23 +++-- tests/tests/it/test_vectors.rs | 2 +- 14 files changed, 383 insertions(+), 60 deletions(-) create mode 100644 givre/src/ciphersuite/bitcoin.rs diff --git a/Cargo.lock b/Cargo.lock index 5bcd2ce..a01caf8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -568,6 +568,7 @@ dependencies = [ "generic-ec", "k256", "key-share", + "once_cell", "rand_core", "round-based", "serde", @@ -588,6 +589,7 @@ dependencies = [ "rand_core", "rand_dev", "round-based", + "secp256k1", "test-case", "tokio", ] @@ -967,6 +969,24 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secp256k1" +version = "0.28.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d24b59d129cdadea20aea4fb2352fa053712e5d713eee47d700cd4b2bc002f10" +dependencies = [ + "secp256k1-sys", +] + +[[package]] +name = "secp256k1-sys" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5d1746aae42c19d583c3c1a8c646bfad910498e2051c551a7f2e3c0c9fbb7eb" +dependencies = [ + "cc", +] + [[package]] name = "semver" version = "1.0.21" diff --git a/givre/Cargo.toml b/givre/Cargo.toml index eb81d26..8f403a1 100644 --- a/givre/Cargo.toml +++ b/givre/Cargo.toml @@ -23,6 +23,8 @@ sha2 = { version = "0.10", default-features = false, optional = true } serde = { version = "1", default-features = false, features = ["derive"], optional = true } +once_cell = { version = "1", optional = true } + [dev-dependencies] rand_core = { version = "0.6", default-features = false, features = ["getrandom"] } @@ -41,6 +43,7 @@ spof = ["key-share/spof"] # Library doesn't have support of HD signing yet. hd-wallets = ["key-share/hd-wallets", "cggmp21-keygen?/hd-wallets"] -all-ciphersuites = ["ciphersuite-secp256k1", "ciphersuite-ed25519"] +all-ciphersuites = ["ciphersuite-secp256k1", "ciphersuite-ed25519", "ciphersuite-bitcoin"] ciphersuite-secp256k1 = ["generic-ec/curve-secp256k1", "k256", "sha2", "static_assertions"] ciphersuite-ed25519 = ["generic-ec/curve-ed25519", "sha2"] +ciphersuite-bitcoin = ["ciphersuite-secp256k1", "dep:once_cell"] diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 737b8c3..4970619 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -11,11 +11,15 @@ use generic_ec::{ }; use rand_core::{CryptoRng, RngCore}; +#[cfg(feature = "ciphersuite-bitcoin")] +mod bitcoin; #[cfg(feature = "ciphersuite-ed25519")] mod ed25519; #[cfg(feature = "ciphersuite-secp256k1")] mod secp256k1; +#[cfg(feature = "ciphersuite-bitcoin")] +pub use bitcoin::Bitcoin; #[cfg(feature = "ciphersuite-ed25519")] pub use ed25519::Ed25519; #[cfg(feature = "ciphersuite-secp256k1")] @@ -27,7 +31,7 @@ pub use secp256k1::Secp256k1; /// For the details, refer to [Section 6] of the draft /// /// [Section 6]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-15.html#name-ciphersuites -pub trait Ciphersuite { +pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// Name of the ciphersuite, also known as `contextString` in the draft const NAME: &'static str; @@ -46,8 +50,8 @@ pub trait Ciphersuite { /// /// Implementation should be based on `H2` hash function as defined in the draft. fn compute_challenge( - group_commitment: &Point, - group_public_key: &Point, + group_commitment: &NormalizedPoint, + group_public_key: &NormalizedPoint, msg: &[u8], ) -> Scalar; /// `H3` hash function as defined in the draft @@ -85,6 +89,39 @@ pub trait Ciphersuite { let mut scalar = Self::deserialize_scalar(bytes)?; Ok(SecretScalar::new(&mut scalar)) } + + /// Determines if the point is normalized according to the Schnorr scheme definition + /// + /// Some Schnorr schemes choose to work with X-only points such as public key and R-component + /// of the signature. To enable that, Y coordinate of the points must be unambiguous. There are + /// several ways of accomplishing that: + /// + /// 1. Implicitly choosing the Y coordinate that is in the lower half. + /// 2. Implicitly choosing the Y coordinate that is even. + /// 3. Implicitly choosing the Y coordinate that is a quadratic residue (i.e. has a square root modulo $p$). + /// + /// Our implementation of FROST requires that if point X isn't normalized, then $-X$ is normalized. Protocol + /// ensures that public key and R-component of the signature are always normalized. + /// + /// If Schnorr scheme doesn't have a notion of normalized points, this function should always return `true`. + fn is_normalized(point: &Point) -> bool { + let _ = point; + true + } + /// Normalizes the point + /// + /// Returns either `point` if it's already normalized, or `-point` otherwise. See [Ciphersuite::is_normalized] + /// for more details. + fn normalize_point(point: Point) -> NormalizedPoint { + match NormalizedPoint::::try_from(point) { + Ok(point) => point, + Err(point) => point, + } + } + /// Byte array that contains bytes representation of the normalized point + type NormalizedPointBytes: AsRef<[u8]>; + /// Serializes a normalized point + fn serialize_normalized_point(point: &NormalizedPoint) -> Self::NormalizedPointBytes; } /// Nonce generation as defined in [Section 4.1](https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-15.html#name-nonce-generation) @@ -152,3 +189,106 @@ impl> AdditionalEntropy for &T { (*self).to_bytes() } } + +/// Normalized point +#[derive(Debug, Clone, Copy)] +pub struct NormalizedPoint(Point); + +impl TryFrom> for NormalizedPoint { + type Error = NormalizedPoint; + fn try_from(point: Point) -> Result { + if C::is_normalized(&point) { + Ok(Self(point)) + } else { + debug_assert!(C::is_normalized(&(-point))); + Err(Self(-point)) + } + } +} +impl core::ops::Deref for NormalizedPoint { + type Target = Point; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl core::cmp::PartialEq for NormalizedPoint { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } +} +impl core::cmp::Eq for NormalizedPoint {} + +#[cfg(feature = "serde")] +impl serde::Serialize for NormalizedPoint { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + // Normalized point is serialized as a regular point - we do not take advantage + // of shorter form is serde traits to keep impl simpler + (&**self).serialize(serializer) + } +} +#[cfg(feature = "serde")] +impl<'de, C: Ciphersuite> serde::Deserialize<'de> for NormalizedPoint { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let point = Point::::deserialize(deserializer)?; + NormalizedPoint::::try_from(point) + .map_err(|_| ::custom("point isn't normalized")) + } +} + +/// Normalizes the key share +/// +/// Some Schnorr signing schemes require public key to be [normalized](Ciphersuite::is_normalized), which means that if +/// you use a generic key generation protocol, half of the times it'll output not normalized public key. You can use +/// this function in order to normalize the key. Each signer must normalize their key share to make it work. +pub fn normalize_key_share( + key_share: crate::key_share::KeyShare, +) -> Result, NormalizeKeyShareError> { + use crate::key_share::Validate; + + let Err(new_pk) = NormalizedPoint::::try_from(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 = SecretScalar::new(&mut -(key_share.x.as_ref())); + let public_shares = key_share + .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| NormalizeKeyShareError(e.into_error())) +} + +/// Normalizing the key share failed +#[derive(Debug)] +pub struct NormalizeKeyShareError(crate::key_share::InvalidKeyShare); + +impl core::fmt::Display for NormalizeKeyShareError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("bug: output key share is invalid") + } +} +impl std::error::Error for NormalizeKeyShareError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.0) + } +} diff --git a/givre/src/ciphersuite/bitcoin.rs b/givre/src/ciphersuite/bitcoin.rs new file mode 100644 index 0000000..b2bea5d --- /dev/null +++ b/givre/src/ciphersuite/bitcoin.rs @@ -0,0 +1,81 @@ +use super::{Ciphersuite, Secp256k1}; + +/// FROST ciphersuite that outputs [BIP-340] compliant sigantures +/// +/// [BIP-340]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki +#[derive(Debug, Clone, Copy)] +pub struct Bitcoin; + +impl Ciphersuite for Bitcoin { + const NAME: &'static str = "DFNS-bitcoin-SHA256-v1"; + type Curve = ::Curve; + type Digest = ::Digest; + + fn h1(msg: &[&[u8]]) -> generic_ec::Scalar { + Secp256k1::h1(msg) + } + + fn compute_challenge( + group_commitment: &super::NormalizedPoint, + group_public_key: &super::NormalizedPoint, + msg: &[u8], + ) -> generic_ec::Scalar { + use sha2::{Digest, Sha256}; + static HASH: once_cell::sync::Lazy = once_cell::sync::Lazy::new(|| { + let tag = Sha256::digest("BIP0340/challenge"); + Sha256::new().chain_update(&tag).chain_update(&tag) + }); + let challenge = HASH + .clone() + .chain_update(Self::serialize_normalized_point(group_commitment)) + .chain_update(Self::serialize_normalized_point(group_public_key)) + .chain_update(msg) + .finalize(); + generic_ec::Scalar::from_be_bytes_mod_order(challenge) + } + + fn h3(msg: &[&[u8]]) -> generic_ec::Scalar { + Secp256k1::h3(msg) + } + fn h4() -> Self::Digest { + Secp256k1::h4() + } + fn h5() -> Self::Digest { + Secp256k1::h5() + } + + type PointBytes = ::PointBytes; + fn serialize_point(point: &generic_ec::Point) -> Self::PointBytes { + Secp256k1::serialize_point(point) + } + fn deserialize_point( + bytes: &[u8], + ) -> Result, generic_ec::errors::InvalidPoint> { + Secp256k1::deserialize_point(bytes) + } + + type ScalarBytes = ::ScalarBytes; + fn serialize_scalar(scalar: &generic_ec::Scalar) -> Self::ScalarBytes { + scalar.to_be_bytes() + } + fn deserialize_scalar( + bytes: &[u8], + ) -> Result, generic_ec::errors::InvalidScalar> { + generic_ec::Scalar::from_be_bytes(bytes) + } + + fn is_normalized(point: &generic_ec::Point) -> bool { + // First byte of compressed point is either 2 or 3. 2 means the Y coordinate is odd. + debug_assert!(matches!(point.to_bytes(true)[0], 2 | 3)); + point.to_bytes(true)[0] == 2 + } + + type NormalizedPointBytes = [u8; 32]; + fn serialize_normalized_point( + point: &super::NormalizedPoint, + ) -> Self::NormalizedPointBytes { + point.to_bytes(true)[1..] + .try_into() + .expect("the size doesn't match") + } +} diff --git a/givre/src/ciphersuite/ed25519.rs b/givre/src/ciphersuite/ed25519.rs index 3e7151d..8d76e8c 100644 --- a/givre/src/ciphersuite/ed25519.rs +++ b/givre/src/ciphersuite/ed25519.rs @@ -25,13 +25,13 @@ impl Ciphersuite for Ed25519 { } fn compute_challenge( - group_commitment: &generic_ec::Point, - group_public_key: &generic_ec::Point, + group_commitment: &super::NormalizedPoint, + group_public_key: &super::NormalizedPoint, msg: &[u8], ) -> generic_ec::Scalar { let hash = sha2::Sha512::new() - .chain_update(Self::serialize_point(group_commitment)) - .chain_update(Self::serialize_point(&group_public_key)) + .chain_update(Self::serialize_normalized_point(group_commitment)) + .chain_update(Self::serialize_normalized_point(group_public_key)) .chain_update(msg) .finalize(); @@ -81,4 +81,11 @@ impl Ciphersuite for Ed25519 { ) -> Result, generic_ec::errors::InvalidScalar> { generic_ec::Scalar::from_le_bytes(bytes) } + + type NormalizedPointBytes = Self::PointBytes; + fn serialize_normalized_point( + point: &super::NormalizedPoint, + ) -> Self::NormalizedPointBytes { + Self::serialize_point(point) + } } diff --git a/givre/src/ciphersuite/secp256k1.rs b/givre/src/ciphersuite/secp256k1.rs index 8e7e4cc..f7c6b15 100644 --- a/givre/src/ciphersuite/secp256k1.rs +++ b/givre/src/ciphersuite/secp256k1.rs @@ -17,14 +17,14 @@ impl Ciphersuite for Secp256k1 { } fn compute_challenge( - group_commitment: &generic_ec::Point, - group_public_key: &generic_ec::Point, + group_commitment: &super::NormalizedPoint, + group_public_key: &super::NormalizedPoint, msg: &[u8], ) -> generic_ec::Scalar { hash_to_scalar( &[ - Self::serialize_point(group_commitment).as_ref(), - Self::serialize_point(group_public_key).as_ref(), + Self::serialize_normalized_point(group_commitment).as_ref(), + Self::serialize_normalized_point(group_public_key).as_ref(), msg, ], &[Self::NAME.as_bytes(), b"chal"], @@ -66,6 +66,13 @@ impl Ciphersuite for Secp256k1 { ) -> Result, generic_ec::errors::InvalidScalar> { generic_ec::Scalar::from_be_bytes(bytes) } + + type NormalizedPointBytes = Self::PointBytes; + fn serialize_normalized_point( + point: &super::NormalizedPoint, + ) -> Self::NormalizedPointBytes { + Self::serialize_point(point) + } } fn hash_to_scalar( diff --git a/givre/src/signing/aggregate.rs b/givre/src/signing/aggregate.rs index 807106c..4f5ce0a 100644 --- a/givre/src/signing/aggregate.rs +++ b/givre/src/signing/aggregate.rs @@ -8,9 +8,9 @@ use core::fmt; -use generic_ec::{Curve, Point, Scalar}; +use generic_ec::{Point, Scalar}; -use crate::{Ciphersuite, SignerIndex}; +use crate::{ciphersuite::NormalizedPoint, Ciphersuite, SignerIndex}; use super::{round1::PublicCommitments, round2::SigShare, utils}; @@ -21,24 +21,24 @@ use super::{round1::PublicCommitments, round2::SigShare, utils}; serde(bound = "") )] /// Schnorr Signature -pub struct Signature { +pub struct Signature { /// $R$ component of the signature - pub r: Point, + pub r: crate::ciphersuite::NormalizedPoint, /// $z$ component of the signature - pub z: Scalar, + pub z: Scalar, } -impl Signature { +impl Signature { /// Verifies signature against a public key and a message - pub fn verify>( + pub fn verify( &self, - public_key: &Point, + public_key: &NormalizedPoint, msg: &[u8], ) -> Result<(), InvalidSignature> { let challenge = C::compute_challenge(&self.r, &public_key, msg); let lhs = Point::generator() * &self.z; - let rhs = self.r + public_key * challenge; + let rhs = *self.r + **public_key * challenge; if lhs == rhs { Ok(()) @@ -60,7 +60,7 @@ pub fn aggregate( key_info: &crate::key_share::KeyInfo, signers: &[(SignerIndex, PublicCommitments, SigShare)], msg: &[u8], -) -> Result, AggregateError> { +) -> Result, AggregateError> { // --- Retrieve and Validate Data let mut comm_list = signers .iter() @@ -93,7 +93,7 @@ pub fn aggregate( .sum(); Ok(Signature { - r: group_commitment, + r: C::normalize_point(group_commitment), z, }) } diff --git a/givre/src/signing/full_signing.rs b/givre/src/signing/full_signing.rs index b9b5f7b..2e14a31 100644 --- a/givre/src/signing/full_signing.rs +++ b/givre/src/signing/full_signing.rs @@ -82,11 +82,7 @@ impl<'a, C: Ciphersuite> SigningBuilder<'a, C> { } /// Executes Interactive Signing protocol - pub async fn sign( - self, - party: M, - rng: &mut R, - ) -> Result, FullSigningError> + pub async fn sign(self, party: M, rng: &mut R) -> Result, FullSigningError> where M: round_based::Mpc>, R: RngCore + CryptoRng, @@ -116,7 +112,7 @@ async fn signing( signers: &[SignerIndex], msg: &[u8], output_sig_share: bool, -) -> Result, FullSigningError> +) -> Result, FullSigningError> where C: Ciphersuite, M: round_based::Mpc>, @@ -185,9 +181,9 @@ where Ok(SigningOutput::Signature(sig)) } -enum SigningOutput { - Signature(Signature), - SigShare(SigShare), +enum SigningOutput { + Signature(Signature), + SigShare(SigShare), } /// Interactive Signing error diff --git a/givre/src/signing/round2.rs b/givre/src/signing/round2.rs index 2a3f82e..cc14ad3 100644 --- a/givre/src/signing/round2.rs +++ b/givre/src/signing/round2.rs @@ -10,7 +10,10 @@ use core::{fmt, iter}; use generic_ec::{Curve, NonZero, Scalar}; -use crate::{ciphersuite::Ciphersuite, KeyShare, SignerIndex}; +use crate::{ + ciphersuite::{Ciphersuite, NormalizedPoint}, + KeyShare, SignerIndex, +}; use super::{ round1::{PublicCommitments, SecretNonces}, @@ -45,6 +48,8 @@ pub fn sign( signers: &[(SignerIndex, PublicCommitments)], ) -> Result, SigningError> { // --- Retrieve and Validate Data + let pk = NormalizedPoint::::try_from(key_share.shared_public_key) + .map_err(|_| Reason::ShareNotNormalized)?; if signers.len() < usize::from(key_share.min_signers()) { return Err(Reason::TooFewSigners { min_signers: key_share.min_signers(), @@ -102,6 +107,19 @@ pub fn sign( debug_assert_eq!(binding_factor_list[i].0, signer_id); let group_commitment = utils::compute_group_commitment(&comm_list, &binding_factor_list); + let nonce_share = nonce.hiding_nonce + (nonce.binding_nonce * binding_factor); + + let (group_commitment, nonce_share) = match NormalizedPoint::try_from(group_commitment) { + Ok(r) => { + // Signature is normalized, no need to do anything else + (r, nonce_share) + } + Err(r) => { + // Signature is not normalized, we had to negate `r`. Each signer need to negate + // their `nonce_share` as well + (r, -nonce_share) + } + }; let signers_list = comm_list.iter().map(|(i, _)| *i).collect::>(); let lambda_i = if key_share.vss_setup.is_some() { @@ -111,12 +129,10 @@ pub fn sign( Scalar::one() }; - let challenge = C::compute_challenge(&group_commitment, &key_share.shared_public_key, msg); + let challenge = C::compute_challenge(&group_commitment, &pk, msg); Ok(SigShare( - nonce.hiding_nonce - + (nonce.binding_nonce * binding_factor) - + (lambda_i * &key_share.x * challenge), + nonce_share + (lambda_i * &key_share.x * challenge), )) } @@ -172,6 +188,7 @@ pub struct SigningError(Reason); #[derive(Debug)] enum Reason { + ShareNotNormalized, TooFewSigners { min_signers: u16, n: usize }, UnknownSigner(SignerIndex), SameSignerTwice, @@ -190,6 +207,7 @@ 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 \ @@ -224,7 +242,8 @@ impl fmt::Display for Bug { impl std::error::Error for SigningError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match &self.0 { - Reason::TooFewSigners { .. } + Reason::ShareNotNormalized + | Reason::TooFewSigners { .. } | Reason::UnknownSigner(_) | Reason::NoncesDontMatchComm | Reason::DeriveInterpolationValue diff --git a/tests/Cargo.toml b/tests/Cargo.toml index d5b2c45..b130d0b 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -23,3 +23,4 @@ round-based = { version = "0.2", features = ["dev"] } futures = "0.3" ed25519 = { package = "ed25519-dalek", version = "2.1" } +secp256k1 = { version = "0.28", features = ["global-context"] } diff --git a/tests/src/lib.rs b/tests/src/lib.rs index f5383af..0d4d70b 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -1,11 +1,15 @@ -use givre::{generic_ec::Point, signing::aggregate::Signature, Ciphersuite}; +use givre::{ciphersuite::NormalizedPoint, signing::aggregate::Signature, Ciphersuite}; pub trait ExternalVerifier: Ciphersuite { + /// Although Schnorr scheme can sign messages of arbitrary size, external verifier may require that + /// message to be signed to be hashed + const REQUIRED_MESSAGE_SIZE: Option = None; + type InvalidSig: core::fmt::Debug; fn verify_sig( - pk: &Point, - sig: &Signature, + pk: &NormalizedPoint, + sig: &Signature, msg: &[u8], ) -> Result<(), Self::InvalidSig>; } @@ -17,8 +21,8 @@ impl ExternalVerifier for givre::ciphersuite::Ed25519 { type InvalidSig = ed25519::SignatureError; fn verify_sig( - pk: &Point, - sig: &Signature, + pk: &NormalizedPoint, + sig: &Signature, msg: &[u8], ) -> Result<(), ed25519::SignatureError> { let pk = ed25519::VerifyingKey::from_bytes( @@ -41,11 +45,36 @@ impl ExternalVerifier for givre::ciphersuite::Secp256k1 { type InvalidSig = core::convert::Infallible; fn verify_sig( - _pk: &Point, - _sig: &Signature, + _pk: &NormalizedPoint, + _sig: &Signature, _msg: &[u8], ) -> Result<(), Self::InvalidSig> { // No external verifier for secp256k1 ciphersuite Ok(()) } } + +impl ExternalVerifier for givre::ciphersuite::Bitcoin { + const REQUIRED_MESSAGE_SIZE: Option = Some(32); + + type InvalidSig = secp256k1::Error; + + fn verify_sig( + pk: &NormalizedPoint, + sig: &Signature, + msg: &[u8], + ) -> Result<(), Self::InvalidSig> { + let pk = + secp256k1::XOnlyPublicKey::from_slice(Self::serialize_normalized_point(pk).as_ref())?; + + let mut signature = [0u8; 64]; + signature[..32].copy_from_slice(Self::serialize_normalized_point(&sig.r).as_ref()); + signature[32..].copy_from_slice(Self::serialize_scalar(&sig.z).as_ref()); + + let signature = secp256k1::schnorr::Signature::from_slice(&signature)?; + + let msg = secp256k1::Message::from_digest_slice(msg)?; + + signature.verify(&msg, &pk) + } +} diff --git a/tests/tests/it/interactive.rs b/tests/tests/it/interactive.rs index 82f72c3..075a464 100644 --- a/tests/tests/it/interactive.rs +++ b/tests/tests/it/interactive.rs @@ -2,9 +2,9 @@ mod generic { use std::iter; - use givre::Ciphersuite; + use givre::{ciphersuite::NormalizedPoint, Ciphersuite}; use givre_tests::ExternalVerifier; - use rand::{seq::SliceRandom, Rng}; + use rand::{seq::SliceRandom, Rng, RngCore}; #[test_case::case(Some(2), 3; "t2n3")] #[test_case::case(Some(3), 3; "t3n3")] @@ -49,13 +49,24 @@ 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_from(key_info.shared_public_key) + .expect("output public key isn't normalized"); // --- Signing // message to be signed - let msg: [u8; 29] = rng.gen(); + let msg_len = C::REQUIRED_MESSAGE_SIZE.unwrap_or_else(|| rng.gen_range(20..=100)); + let mut msg = vec![0u8; msg_len]; + rng.fill_bytes(&mut msg); + let msg = &msg; // Choose `t` signers to do signing let t = t.unwrap_or(n); @@ -81,10 +92,8 @@ mod generic { .await .unwrap(); - sigs[0] - .verify::(&key_info.shared_public_key, &msg) - .unwrap(); - C::verify_sig(&key_info.shared_public_key, &sigs[0], &msg).unwrap(); + sigs[0].verify(&pk, &msg).unwrap(); + C::verify_sig(&pk, &sigs[0], &msg).unwrap(); for sig in &sigs[1..] { assert_eq!(sigs[0].r, sig.r); @@ -92,6 +101,8 @@ mod generic { } } + #[instantiate_tests()] + mod bitcoin {} #[instantiate_tests()] mod secp256k1 {} #[instantiate_tests()] diff --git a/tests/tests/it/main.rs b/tests/tests/it/main.rs index 9415012..294cbde 100644 --- a/tests/tests/it/main.rs +++ b/tests/tests/it/main.rs @@ -3,9 +3,9 @@ mod test_vectors; #[generic_tests::define(attrs(test_case::case))] mod generic { - use givre::Ciphersuite; + use givre::{ciphersuite::NormalizedPoint, Ciphersuite}; use givre_tests::ExternalVerifier; - use rand::{seq::SliceRandom, Rng}; + use rand::{seq::SliceRandom, Rng, RngCore}; #[test_case::case(Some(2), 3; "t2n3")] #[test_case::case(Some(3), 3; "t3n3")] @@ -21,7 +21,14 @@ 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_from(key_info.shared_public_key) + .expect("public key is not normalized"); // List of indexes of signers who co-hold the key let key_holders = (0..n).collect::>(); @@ -46,7 +53,9 @@ mod generic { } // Round 2. Each signer signs a message - let message: [u8; 28] = rng.gen(); + let message_len = C::REQUIRED_MESSAGE_SIZE.unwrap_or_else(|| rng.gen_range(20..=100)); + let mut message = vec![0u8; message_len]; + rng.fill_bytes(&mut message); let partial_sigs = public_commitments .iter() @@ -67,12 +76,12 @@ mod generic { let sig = givre::signing::aggregate::aggregate::(&key_info, &partial_sigs, &message) .expect("aggregation failed"); - sig.verify::(&key_info.shared_public_key, &message) - .expect("invalid signature"); - C::verify_sig(&key_info.shared_public_key, &sig, &message) - .expect("external verifier: invalid signature") + sig.verify(&pk, &message).expect("invalid signature"); + C::verify_sig(&pk, &sig, &message).expect("external verifier: invalid signature") } + #[instantiate_tests()] + mod bitcoin {} #[instantiate_tests()] mod secp256k1 {} #[instantiate_tests()] diff --git a/tests/tests/it/test_vectors.rs b/tests/tests/it/test_vectors.rs index 8b83381..02fe613 100644 --- a/tests/tests/it/test_vectors.rs +++ b/tests/tests/it/test_vectors.rs @@ -162,7 +162,7 @@ impl TestVector { let r = C::deserialize_point(&self.expected_sig[0]).unwrap(); let z = C::deserialize_scalar(&self.expected_sig[1]).unwrap(); - assert_eq!(sig.r, r); + assert_eq!(*sig.r, r); assert_eq!(sig.z, z); } } From 2e631b11c942e40c7b7cbb942009b52e8d894d51 Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Tue, 12 Mar 2024 12:06:20 +0100 Subject: [PATCH 04/14] Fix clippy warns --- givre/src/ciphersuite.rs | 16 ++++++++-------- givre/src/ciphersuite/bitcoin.rs | 3 ++- givre/src/lib.rs | 1 + givre/src/signing/aggregate.rs | 4 ++-- givre/src/signing/full_signing.rs | 2 +- givre/src/signing/utils.rs | 9 ++++----- tests/tests/it/interactive.rs | 6 +++--- tests/tests/it/main.rs | 2 +- tests/tests/it/test_vectors.rs | 9 +++++---- 9 files changed, 27 insertions(+), 25 deletions(-) diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 4970619..967a0fc 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -150,42 +150,42 @@ pub trait AdditionalEntropy { Self: 'b; /// Returns bytes representation of the entropy encoded in complience with [`C`](Ciphersuite) - fn to_bytes<'b>(&'b self) -> Self::Bytes<'b>; + fn to_bytes(&self) -> Self::Bytes<'_>; } impl, E: Curve> AdditionalEntropy for crate::KeyShare { type Bytes<'b> = as AdditionalEntropy>::Bytes<'b>; - fn to_bytes<'b>(&'b self) -> Self::Bytes<'b> { + fn to_bytes(&self) -> Self::Bytes<'_> { AdditionalEntropy::::to_bytes(&self.x) } } impl, E: Curve> AdditionalEntropy for generic_ec::Scalar { type Bytes<'b> = C::ScalarBytes; - fn to_bytes<'b>(&'b self) -> Self::Bytes<'b> { + fn to_bytes(&self) -> Self::Bytes<'_> { C::serialize_scalar(self) } } impl, E: Curve> AdditionalEntropy for generic_ec::SecretScalar { type Bytes<'b> = as AdditionalEntropy>::Bytes<'b>; - fn to_bytes<'b>(&'b self) -> Self::Bytes<'b> { + fn to_bytes(&self) -> Self::Bytes<'_> { AdditionalEntropy::::to_bytes(self.as_ref()) } } impl AdditionalEntropy for [u8] { type Bytes<'b> = &'b [u8]; - fn to_bytes<'b>(&'b self) -> Self::Bytes<'b> { + fn to_bytes(&self) -> Self::Bytes<'_> { self } } impl AdditionalEntropy for [u8; N] { type Bytes<'b> = &'b [u8; N]; - fn to_bytes<'b>(&'b self) -> Self::Bytes<'b> { + fn to_bytes(&self) -> Self::Bytes<'_> { self } } impl> AdditionalEntropy for &T { type Bytes<'b> = >::Bytes<'b> where Self: 'b; - fn to_bytes<'b>(&'b self) -> Self::Bytes<'b> { + fn to_bytes(&self) -> Self::Bytes<'_> { (*self).to_bytes() } } @@ -226,7 +226,7 @@ impl serde::Serialize for NormalizedPoint { { // Normalized point is serialized as a regular point - we do not take advantage // of shorter form is serde traits to keep impl simpler - (&**self).serialize(serializer) + (**self).serialize(serializer) } } #[cfg(feature = "serde")] diff --git a/givre/src/ciphersuite/bitcoin.rs b/givre/src/ciphersuite/bitcoin.rs index b2bea5d..902d74f 100644 --- a/givre/src/ciphersuite/bitcoin.rs +++ b/givre/src/ciphersuite/bitcoin.rs @@ -23,7 +23,7 @@ impl Ciphersuite for Bitcoin { use sha2::{Digest, Sha256}; static HASH: once_cell::sync::Lazy = once_cell::sync::Lazy::new(|| { let tag = Sha256::digest("BIP0340/challenge"); - Sha256::new().chain_update(&tag).chain_update(&tag) + Sha256::new().chain_update(tag).chain_update(tag) }); let challenge = HASH .clone() @@ -74,6 +74,7 @@ impl Ciphersuite for Bitcoin { fn serialize_normalized_point( point: &super::NormalizedPoint, ) -> Self::NormalizedPointBytes { + #[allow(clippy::expect_used)] point.to_bytes(true)[1..] .try_into() .expect("the size doesn't match") diff --git a/givre/src/lib.rs b/givre/src/lib.rs index 898f07b..2c12cae 100644 --- a/givre/src/lib.rs +++ b/givre/src/lib.rs @@ -22,6 +22,7 @@ #![forbid(unsafe_code)] #![deny(clippy::expect_used, clippy::unwrap_used, clippy::panic)] #![deny(missing_docs)] +#![allow(clippy::type_complexity)] #![cfg_attr(docsrs, feature(doc_auto_cfg))] pub use generic_ec; diff --git a/givre/src/signing/aggregate.rs b/givre/src/signing/aggregate.rs index 4f5ce0a..7fa66b3 100644 --- a/givre/src/signing/aggregate.rs +++ b/givre/src/signing/aggregate.rs @@ -35,9 +35,9 @@ impl Signature { public_key: &NormalizedPoint, msg: &[u8], ) -> Result<(), InvalidSignature> { - let challenge = C::compute_challenge(&self.r, &public_key, msg); + let challenge = C::compute_challenge(&self.r, public_key, msg); - let lhs = Point::generator() * &self.z; + let lhs = Point::generator() * self.z; let rhs = *self.r + **public_key * challenge; if lhs == rhs { diff --git a/givre/src/signing/full_signing.rs b/givre/src/signing/full_signing.rs index 2e14a31..4430543 100644 --- a/givre/src/signing/full_signing.rs +++ b/givre/src/signing/full_signing.rs @@ -175,7 +175,7 @@ where .collect::>(); let key_info: &KeyInfo<_> = key_share.as_ref(); - let sig = crate::signing::aggregate::aggregate::(&key_info, &signers_list, msg) + let sig = crate::signing::aggregate::aggregate::(key_info, &signers_list, msg) .map_err(Reason::Aggregate)?; Ok(SigningOutput::Signature(sig)) diff --git a/givre/src/signing/utils.rs b/givre/src/signing/utils.rs index cffa0e2..011bb90 100644 --- a/givre/src/signing/utils.rs +++ b/givre/src/signing/utils.rs @@ -23,9 +23,9 @@ pub fn encode_group_commitment_list( }, ) in commitment_list { - output.update(C::serialize_scalar(&i).as_ref()); - output.update(C::serialize_point(&hiding_comm).as_ref()); - output.update(C::serialize_point(&binding_comm).as_ref()); + output.update(C::serialize_scalar(i).as_ref()); + output.update(C::serialize_point(hiding_comm).as_ref()); + output.update(C::serialize_point(binding_comm).as_ref()); } output } @@ -59,7 +59,7 @@ pub fn compute_binding_factors( pk_bytes.as_ref(), &msg_hash, &encoded_commitment_hash, - C::serialize_scalar(&i).as_ref(), + C::serialize_scalar(i).as_ref(), ]); binding_factor_list.push((*i, binding_factor)) } @@ -85,7 +85,6 @@ pub fn compute_group_commitment<'a, E: Curve>( debug_assert_eq!(i, _i); (*i, *comm, *factor) }) - .into_iter() .fold(Point::zero(), |acc, (_i, comm, binding_factor)| { let binding_nonce = comm.binding_comm * binding_factor; acc + comm.hiding_comm + binding_nonce diff --git a/tests/tests/it/interactive.rs b/tests/tests/it/interactive.rs index 075a464..699c35d 100644 --- a/tests/tests/it/interactive.rs +++ b/tests/tests/it/interactive.rs @@ -82,7 +82,7 @@ mod generic { .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) + givre::signing::(j, &key_shares[usize::from(index_at_keygen)], signers, msg) .sign(party, &mut rng) .await }); @@ -92,8 +92,8 @@ mod generic { .await .unwrap(); - sigs[0].verify(&pk, &msg).unwrap(); - C::verify_sig(&pk, &sigs[0], &msg).unwrap(); + sigs[0].verify(&pk, msg).unwrap(); + C::verify_sig(&pk, &sigs[0], msg).unwrap(); for sig in &sigs[1..] { assert_eq!(sigs[0].r, sig.r); diff --git a/tests/tests/it/main.rs b/tests/tests/it/main.rs index 294cbde..5f53b4f 100644 --- a/tests/tests/it/main.rs +++ b/tests/tests/it/main.rs @@ -73,7 +73,7 @@ mod generic { .expect("signing failed"); // Round 3. Aggregate sig shares - let sig = givre::signing::aggregate::aggregate::(&key_info, &partial_sigs, &message) + let sig = givre::signing::aggregate::aggregate::(key_info, &partial_sigs, &message) .expect("aggregation failed"); sig.verify(&pk, &message).expect("invalid signature"); diff --git a/tests/tests/it/test_vectors.rs b/tests/tests/it/test_vectors.rs index 02fe613..c441067 100644 --- a/tests/tests/it/test_vectors.rs +++ b/tests/tests/it/test_vectors.rs @@ -156,11 +156,11 @@ impl TestVector { // --- Aggregate let sig = - givre::signing::aggregate::aggregate::(&key_info, &sig_shares, self.msg).unwrap(); + givre::signing::aggregate::aggregate::(key_info, &sig_shares, self.msg).unwrap(); { - let r = C::deserialize_point(&self.expected_sig[0]).unwrap(); - let z = C::deserialize_scalar(&self.expected_sig[1]).unwrap(); + let r = C::deserialize_point(self.expected_sig[0]).unwrap(); + let z = C::deserialize_scalar(self.expected_sig[1]).unwrap(); assert_eq!(*sig.r, r); assert_eq!(sig.z, z); @@ -303,7 +303,8 @@ fn mocked_randomness(bytes: &[u8]) -> impl RngCore + CryptoRng + '_ { rand_core::impls::next_u64_via_fill(self) } fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand::Error> { - Ok(self.fill_bytes(dest)) + self.fill_bytes(dest); + Ok(()) } } impl<'b> CryptoRng for MockedRng<'b> {} From 6c18a79e51893cce8a0abfa440260078317a3de2 Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Tue, 12 Mar 2024 13:57:40 +0100 Subject: [PATCH 05/14] Final touches --- givre/src/ciphersuite.rs | 56 +++++++++++++++++------------- givre/src/ciphersuite/bitcoin.rs | 12 +++++-- givre/src/ciphersuite/secp256k1.rs | 4 +-- givre/src/lib.rs | 5 +-- 4 files changed, 46 insertions(+), 31 deletions(-) diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 967a0fc..8b79292 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -100,8 +100,11 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// 2. Implicitly choosing the Y coordinate that is even. /// 3. Implicitly choosing the Y coordinate that is a quadratic residue (i.e. has a square root modulo $p$). /// - /// Our implementation of FROST requires that if point X isn't normalized, then $-X$ is normalized. Protocol - /// ensures that public key and R-component of the signature are always normalized. + /// Our implementation of FROST requires that if point $X$ isn't normalized, then $-X$ is normalized. Note that + /// certain parts of the protocol may enforce this property via debug assertations. + /// + /// The protocol always outputs sigantures 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`. fn is_normalized(point: &Point) -> bool { @@ -120,7 +123,7 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { } /// Byte array that contains bytes representation of the normalized point type NormalizedPointBytes: AsRef<[u8]>; - /// Serializes a normalized point + /// Serializes a normalized point in a space-efficient manner as defined by Schnorr scheme fn serialize_normalized_point(point: &NormalizedPoint) -> Self::NormalizedPointBytes; } @@ -191,11 +194,26 @@ impl> AdditionalEntropy for &T { } /// Normalized point +/// +/// Point that satisfies [`Ciphersuite::is_normalized`]. #[derive(Debug, Clone, Copy)] pub struct NormalizedPoint(Point); +impl NormalizedPoint { + /// Serialzies the normalized point in a space-efficient manner + /// + /// Alias to [`Ciphersuite::serialize_normalized_point`] + pub fn to_bytes(&self) -> C::NormalizedPointBytes { + C::serialize_normalized_point(self) + } +} + impl TryFrom> for NormalizedPoint { type Error = NormalizedPoint; + + /// Normalizes the point + /// + /// Returns `Ok(point)` is point is already normalized, or `Err(-point)` otherwise. fn try_from(point: Point) -> Result { if C::is_normalized(&point) { Ok(Self(point)) @@ -225,7 +243,7 @@ impl serde::Serialize for NormalizedPoint { S: serde::Serializer, { // Normalized point is serialized as a regular point - we do not take advantage - // of shorter form is serde traits to keep impl simpler + // of shorter form in serde traits to keep impl simpler (**self).serialize(serializer) } } @@ -243,12 +261,17 @@ impl<'de, C: Ciphersuite> serde::Deserialize<'de> for NormalizedPoint { /// Normalizes the key share /// -/// Some Schnorr signing schemes require public key to be [normalized](Ciphersuite::is_normalized), which means that if -/// you use a generic key generation protocol, half of the times it'll output not normalized public key. You can use -/// this function in order to normalize the key. Each signer must normalize their key share to make it work. +/// 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, NormalizeKeyShareError> { +) -> Result, crate::key_share::InvalidKeyShare> { use crate::key_share::Validate; let Err(new_pk) = NormalizedPoint::::try_from(key_share.shared_public_key) else { @@ -275,20 +298,5 @@ pub fn normalize_key_share( x: new_sk, } .validate() - .map_err(|e| NormalizeKeyShareError(e.into_error())) -} - -/// Normalizing the key share failed -#[derive(Debug)] -pub struct NormalizeKeyShareError(crate::key_share::InvalidKeyShare); - -impl core::fmt::Display for NormalizeKeyShareError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("bug: output key share is invalid") - } -} -impl std::error::Error for NormalizeKeyShareError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - Some(&self.0) - } + .map_err(|e| e.into_error()) } diff --git a/givre/src/ciphersuite/bitcoin.rs b/givre/src/ciphersuite/bitcoin.rs index 902d74f..3d95024 100644 --- a/givre/src/ciphersuite/bitcoin.rs +++ b/givre/src/ciphersuite/bitcoin.rs @@ -2,6 +2,12 @@ use super::{Ciphersuite, Secp256k1}; /// FROST ciphersuite that outputs [BIP-340] compliant sigantures /// +/// # Normalized public keys +/// BIP-340 requires that public keys are normalized, meaning that they must have +/// odd Y coordinate. Generic DKG protocols output public key with both even and odd +/// Y coordinate. You can use [`normalize_key_share`](super::normalize_key_share) +/// to normalize the key share after it's generated. +/// /// [BIP-340]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki #[derive(Debug, Clone, Copy)] pub struct Bitcoin; @@ -27,8 +33,8 @@ impl Ciphersuite for Bitcoin { }); let challenge = HASH .clone() - .chain_update(Self::serialize_normalized_point(group_commitment)) - .chain_update(Self::serialize_normalized_point(group_public_key)) + .chain_update(group_commitment.to_bytes()) + .chain_update(group_public_key.to_bytes()) .chain_update(msg) .finalize(); generic_ec::Scalar::from_be_bytes_mod_order(challenge) @@ -75,7 +81,7 @@ impl Ciphersuite for Bitcoin { point: &super::NormalizedPoint, ) -> Self::NormalizedPointBytes { #[allow(clippy::expect_used)] - point.to_bytes(true)[1..] + (**point).to_bytes(true)[1..] .try_into() .expect("the size doesn't match") } diff --git a/givre/src/ciphersuite/secp256k1.rs b/givre/src/ciphersuite/secp256k1.rs index f7c6b15..8d062f7 100644 --- a/givre/src/ciphersuite/secp256k1.rs +++ b/givre/src/ciphersuite/secp256k1.rs @@ -23,8 +23,8 @@ impl Ciphersuite for Secp256k1 { ) -> generic_ec::Scalar { hash_to_scalar( &[ - Self::serialize_normalized_point(group_commitment).as_ref(), - Self::serialize_normalized_point(group_public_key).as_ref(), + group_commitment.to_bytes().as_ref(), + group_public_key.to_bytes().as_ref(), msg, ], &[Self::NAME.as_bytes(), b"chal"], diff --git a/givre/src/lib.rs b/givre/src/lib.rs index 2c12cae..6fa14c9 100644 --- a/givre/src/lib.rs +++ b/givre/src/lib.rs @@ -4,9 +4,10 @@ //! [commit nonces](signing::round1) ahead of time), and identifiable abort. //! //! This crate provides: -//! * Threshold and non-threshold Distributed Key Generation (DKG) \ +//! * Distributed Key Generation (DKG) \ //! Note that FROST does not define DKG protocol to be used. We simply re-export DKG based on [CGGMP21] implementation -//! when `cggmp21-keygen` feature is enabled. Alternatively, you can use any other UC-secure DKG protocol. +//! when `cggmp21-keygen` feature is enabled, which is a fairly reasonalbe 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 //! (for easier usability and fool-proof design), see [mod@signing] module for details. From b0cb08fdd326990ac4fd15849db1271ddf0b475e Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Tue, 12 Mar 2024 13:59:53 +0100 Subject: [PATCH 06/14] Update readme --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1f0b991..422c18d 100644 --- a/README.md +++ b/README.md @@ -4,9 +4,10 @@ FROST is state of art protocol for Threshold Schnorr Signatures that supports 1- commit nonces ahead of time), and identifiable abort. This crate provides: -* Threshold and non-threshold Distributed Key Generation (DKG) \ +* Distributed Key Generation (DKG) \ Note that FROST does not define DKG protocol to be used. We simply re-export DKG based on [CGGMP21] implementation - when `cggmp21-keygen` feature is enabled. Alternatively, you can use any other UC-secure DKG protocol. + when `cggmp21-keygen` feature is enabled, which is a fairly reasonalbe 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 (for easier usability and fool-proof design), see signing module for details. From 8935df7c5f69bcfac602956146f9934285082fce Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Tue, 12 Mar 2024 14:08:42 +0100 Subject: [PATCH 07/14] Mention bitcoin in the list of ciphersuites --- givre/src/ciphersuite.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 8b79292..b8e9b0e 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -5,6 +5,7 @@ //! Out of the box, we provide ciphersuites defined the in the draft: //! * [Secp256k1], requires `ciphersuite-secp256k1` feature //! * [Ed25519], requires `ciphersuite-ed25519` feature +//! * [Bitcoin], requires `ciphersuite-bitcoin` feature use generic_ec::{ errors::{InvalidPoint, InvalidScalar}, Curve, Point, Scalar, SecretScalar, From 8c17d6b247550b80cb79dee4711ee49d9484691a Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Wed, 13 Mar 2024 10:01:06 +0100 Subject: [PATCH 08/14] Zero point is always normalized --- givre/src/ciphersuite.rs | 5 +++-- givre/src/ciphersuite/bitcoin.rs | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index b8e9b0e..9d4bf2d 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -101,8 +101,9 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// 2. Implicitly choosing the Y coordinate that is even. /// 3. Implicitly choosing the Y coordinate that is a quadratic residue (i.e. has a square root modulo $p$). /// - /// Our implementation of FROST requires that if point $X$ isn't normalized, then $-X$ is normalized. Note that - /// certain parts of the protocol may enforce this property via debug assertations. + /// 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. /// /// The protocol always outputs sigantures 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. diff --git a/givre/src/ciphersuite/bitcoin.rs b/givre/src/ciphersuite/bitcoin.rs index 3d95024..96d83c5 100644 --- a/givre/src/ciphersuite/bitcoin.rs +++ b/givre/src/ciphersuite/bitcoin.rs @@ -71,9 +71,9 @@ impl Ciphersuite for Bitcoin { } fn is_normalized(point: &generic_ec::Point) -> bool { - // First byte of compressed point is either 2 or 3. 2 means the Y coordinate is odd. - debug_assert!(matches!(point.to_bytes(true)[0], 2 | 3)); - point.to_bytes(true)[0] == 2 + // First byte of compressed non-zero point is either 2 or 3. 2 means the Y coordinate is odd. + debug_assert!(point.is_zero() || matches!(point.to_bytes(true)[0], 2 | 3)); + point.is_zero() || point.to_bytes(true)[0] == 2 } type NormalizedPointBytes = [u8; 32]; From fa549d794665744b3dbd68b76d69d98d92618f6d Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Wed, 13 Mar 2024 13:40:03 +0100 Subject: [PATCH 09/14] Update deps --- Cargo.lock | 31 +++++++----- Cargo.toml | 4 -- givre/Cargo.toml | 6 +-- givre/src/ciphersuite.rs | 78 +++++++++++++++++++----------- givre/src/ciphersuite/bitcoin.rs | 18 ++++--- givre/src/ciphersuite/ed25519.rs | 11 +++-- givre/src/ciphersuite/secp256k1.rs | 17 ++++--- givre/src/signing/aggregate.rs | 6 +-- givre/src/signing/round2.rs | 6 +-- givre/src/signing/utils.rs | 2 +- tests/src/lib.rs | 15 ++++-- tests/tests/it/interactive.rs | 2 +- tests/tests/it/main.rs | 2 +- tests/tests/it/test_vectors.rs | 4 +- 14 files changed, 119 insertions(+), 83 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a01caf8..d58fe23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -110,7 +110,8 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "cggmp21-keygen" version = "0.1.0" -source = "git+https://github.com/dfns/cggmp21?branch=spof#18ac63bfc1849cd01259440ae340e3a71d01ee69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2a502901b03b7a77e31348435318ed5e0c01193322c145211d265e4b973f98e" dependencies = [ "digest", "futures", @@ -472,8 +473,9 @@ dependencies = [ [[package]] name = "generic-ec" -version = "0.1.4" -source = "git+https://github.com/dfns/generic-ec?branch=m#ba27154f93ec2502f71063056e1da35039140a69" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f315ffeaae6a05691c5a27028b4ae960d3a430e406d0109ff8502e2f84ae97b" dependencies = [ "generic-ec-core", "generic-ec-curves", @@ -489,8 +491,9 @@ dependencies = [ [[package]] name = "generic-ec-core" -version = "0.1.2" -source = "git+https://github.com/dfns/generic-ec?branch=m#ba27154f93ec2502f71063056e1da35039140a69" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cab102fc88bfc017c16e69d21edae6f41ab58bfe69eed09ed0a2cf10ec923f" dependencies = [ "generic-array", "rand_core", @@ -501,8 +504,9 @@ dependencies = [ [[package]] name = "generic-ec-curves" -version = "0.1.3" -source = "git+https://github.com/dfns/generic-ec?branch=m#ba27154f93ec2502f71063056e1da35039140a69" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a133d38cde4fef7aea4e367ca51f291db0248495a424ec4208cdace08ba59f4" dependencies = [ "crypto-bigint", "curve25519-dalek", @@ -518,9 +522,9 @@ dependencies = [ [[package]] name = "generic-ec-zkp" -version = "0.1.1" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f98645d68f62951789a4aec46ef4416878c2827b2353f5267cf96b096eae60d9" +checksum = "a9b631ea71fa0294e80d479c5b952e276776c9abf1b1a542b48e66b83fe1828c" dependencies = [ "generic-array", "generic-ec", @@ -702,8 +706,9 @@ dependencies = [ [[package]] name = "key-share" -version = "0.1.0" -source = "git+https://github.com/dfns/cggmp21?branch=spof#18ac63bfc1849cd01259440ae340e3a71d01ee69" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05c746d847a566beecf3927cd9dcb60a6c61ca7d24845d8025974a97fa011918" dependencies = [ "generic-ec", "generic-ec-zkp", @@ -1083,9 +1088,9 @@ dependencies = [ [[package]] name = "slip-10" -version = "0.1.0" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4670651d286d5714497971c12d36b83d75679c39da5d5ed757ff5e1e1d45855c" +checksum = "f20f0918d675ab26ca9fa3e2c42548356f54b7a09fb4633313756522ddd59f75" dependencies = [ "generic-array", "generic-ec", diff --git a/Cargo.toml b/Cargo.toml index ec2924e..c73fd9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,3 @@ members = [ "givre", "tests", ] - -[patch.crates-io.generic-ec] -git = "https://github.com/dfns/generic-ec" -branch = "m" diff --git a/givre/Cargo.toml b/givre/Cargo.toml index 8f403a1..637b459 100644 --- a/givre/Cargo.toml +++ b/givre/Cargo.toml @@ -6,10 +6,10 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -cggmp21-keygen = { git = "https://github.com/dfns/cggmp21", branch = "spof", optional = true } -key-share = { git = "https://github.com/dfns/cggmp21", branch = "spof" } +cggmp21-keygen = { version = "0.1", optional = true } +key-share = "0.2" -generic-ec = { version = "0.1", default-features = false } +generic-ec = { version = "0.2", default-features = false } rand_core = { version = "0.6", default-features = false } digest = { version = "0.10", default-features = false } diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 9d4bf2d..ea83e7a 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -8,7 +8,7 @@ //! * [Bitcoin], requires `ciphersuite-bitcoin` feature use generic_ec::{ errors::{InvalidPoint, InvalidScalar}, - Curve, Point, Scalar, SecretScalar, + Curve, NonZero, Point, Scalar, SecretScalar, }; use rand_core::{CryptoRng, RngCore}; @@ -51,8 +51,8 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// /// Implementation should be based on `H2` hash function as defined in the draft. fn compute_challenge( - group_commitment: &NormalizedPoint, - group_public_key: &NormalizedPoint, + group_commitment: &NormalizedPoint>, + group_public_key: &NormalizedPoint>>, msg: &[u8], ) -> Scalar; /// `H3` hash function as defined in the draft @@ -117,8 +117,10 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// /// Returns either `point` if it's already normalized, or `-point` otherwise. See [Ciphersuite::is_normalized] /// for more details. - fn normalize_point(point: Point) -> NormalizedPoint { - match NormalizedPoint::::try_from(point) { + fn normalize_point> + core::ops::Neg>( + point: P, + ) -> NormalizedPoint { + match NormalizedPoint::::try_normalize(point) { Ok(point) => point, Err(point) => point, } @@ -126,7 +128,9 @@ pub trait Ciphersuite: Sized + Clone + Copy + core::fmt::Debug { /// Byte array that contains bytes representation of the normalized point type NormalizedPointBytes: AsRef<[u8]>; /// Serializes a normalized point in a space-efficient manner as defined by Schnorr scheme - fn serialize_normalized_point(point: &NormalizedPoint) -> Self::NormalizedPointBytes; + fn serialize_normalized_point>>( + point: &NormalizedPoint, + ) -> Self::NormalizedPointBytes; } /// Nonce generation as defined in [Section 4.1](https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-15.html#name-nonce-generation) @@ -176,6 +180,12 @@ impl, E: Curve> AdditionalEntropy for generic_ec::S AdditionalEntropy::::to_bytes(self.as_ref()) } } +impl> AdditionalEntropy for generic_ec::NonZero { + type Bytes<'b> = >::Bytes<'b> where Self: 'b; + fn to_bytes(&self) -> Self::Bytes<'_> { + AdditionalEntropy::::to_bytes(self.as_ref()) + } +} impl AdditionalEntropy for [u8] { type Bytes<'b> = &'b [u8]; fn to_bytes(&self) -> Self::Bytes<'_> { @@ -197,11 +207,12 @@ impl> AdditionalEntropy for &T { /// Normalized point /// -/// Point that satisfies [`Ciphersuite::is_normalized`]. +/// Point that satisfies [`Ciphersuite::is_normalized`]. Can wrap both `Point` and +/// `NonZero>`. #[derive(Debug, Clone, Copy)] -pub struct NormalizedPoint(Point); +pub struct NormalizedPoint(P, core::marker::PhantomData); -impl NormalizedPoint { +impl>> NormalizedPoint { /// Serialzies the normalized point in a space-efficient manner /// /// Alias to [`Ciphersuite::serialize_normalized_point`] @@ -210,36 +221,44 @@ impl NormalizedPoint { } } -impl TryFrom> for NormalizedPoint { - type Error = NormalizedPoint; - +impl> + core::ops::Neg> NormalizedPoint { /// Normalizes the point /// /// Returns `Ok(point)` is point is already normalized, or `Err(-point)` otherwise. - fn try_from(point: Point) -> Result { - if C::is_normalized(&point) { - Ok(Self(point)) + pub fn try_normalize(point: P) -> Result { + if point.as_ref().is_zero() || C::is_normalized(point.as_ref()) { + Ok(Self(point, Default::default())) } else { - debug_assert!(C::is_normalized(&(-point))); - Err(Self(-point)) + let neg_point = -point; + debug_assert!(C::is_normalized(neg_point.as_ref())); + Ok(Self(neg_point, Default::default())) } } } -impl core::ops::Deref for NormalizedPoint { - type Target = Point; + +impl core::ops::Deref for NormalizedPoint { + type Target = P; fn deref(&self) -> &Self::Target { &self.0 } } -impl core::cmp::PartialEq for NormalizedPoint { +impl AsRef for NormalizedPoint +where + P: AsRef, +{ + fn as_ref(&self) -> &T { + self.0.as_ref() + } +} +impl core::cmp::PartialEq for NormalizedPoint { fn eq(&self, other: &Self) -> bool { self.0 == other.0 } } -impl core::cmp::Eq for NormalizedPoint {} +impl core::cmp::Eq for NormalizedPoint {} #[cfg(feature = "serde")] -impl serde::Serialize for NormalizedPoint { +impl serde::Serialize for NormalizedPoint { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer, @@ -250,13 +269,17 @@ impl serde::Serialize for NormalizedPoint { } } #[cfg(feature = "serde")] -impl<'de, C: Ciphersuite> serde::Deserialize<'de> for NormalizedPoint { +impl<'de, C, P> serde::Deserialize<'de> for NormalizedPoint +where + C: Ciphersuite, + P: AsRef> + serde::Deserialize<'de> + core::ops::Neg, +{ fn deserialize(deserializer: D) -> Result where D: serde::Deserializer<'de>, { - let point = Point::::deserialize(deserializer)?; - NormalizedPoint::::try_from(point) + let point = P::deserialize(deserializer)?; + NormalizedPoint::::try_normalize(point) .map_err(|_| ::custom("point isn't normalized")) } } @@ -276,15 +299,16 @@ pub fn normalize_key_share( ) -> Result, crate::key_share::InvalidKeyShare> { use crate::key_share::Validate; - let Err(new_pk) = NormalizedPoint::::try_from(key_share.shared_public_key) else { + 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 = SecretScalar::new(&mut -(key_share.x.as_ref())); + let new_sk = -key_share.x; let public_shares = key_share + .key_info .public_shares .iter() .map(|p| -p) diff --git a/givre/src/ciphersuite/bitcoin.rs b/givre/src/ciphersuite/bitcoin.rs index 96d83c5..3cd2cde 100644 --- a/givre/src/ciphersuite/bitcoin.rs +++ b/givre/src/ciphersuite/bitcoin.rs @@ -1,3 +1,5 @@ +use generic_ec::{NonZero, Point}; + use super::{Ciphersuite, Secp256k1}; /// FROST ciphersuite that outputs [BIP-340] compliant sigantures @@ -22,8 +24,8 @@ impl Ciphersuite for Bitcoin { } fn compute_challenge( - group_commitment: &super::NormalizedPoint, - group_public_key: &super::NormalizedPoint, + group_commitment: &super::NormalizedPoint>, + group_public_key: &super::NormalizedPoint>>, msg: &[u8], ) -> generic_ec::Scalar { use sha2::{Digest, Sha256}; @@ -51,12 +53,12 @@ impl Ciphersuite for Bitcoin { } type PointBytes = ::PointBytes; - fn serialize_point(point: &generic_ec::Point) -> Self::PointBytes { + fn serialize_point(point: &Point) -> Self::PointBytes { Secp256k1::serialize_point(point) } fn deserialize_point( bytes: &[u8], - ) -> Result, generic_ec::errors::InvalidPoint> { + ) -> Result, generic_ec::errors::InvalidPoint> { Secp256k1::deserialize_point(bytes) } @@ -70,18 +72,18 @@ impl Ciphersuite for Bitcoin { generic_ec::Scalar::from_be_bytes(bytes) } - fn is_normalized(point: &generic_ec::Point) -> bool { + fn is_normalized(point: &Point) -> bool { // First byte of compressed non-zero point is either 2 or 3. 2 means the Y coordinate is odd. debug_assert!(point.is_zero() || matches!(point.to_bytes(true)[0], 2 | 3)); point.is_zero() || point.to_bytes(true)[0] == 2 } type NormalizedPointBytes = [u8; 32]; - fn serialize_normalized_point( - point: &super::NormalizedPoint, + fn serialize_normalized_point>>( + point: &super::NormalizedPoint, ) -> Self::NormalizedPointBytes { #[allow(clippy::expect_used)] - (**point).to_bytes(true)[1..] + point.as_ref().to_bytes(true)[1..] .try_into() .expect("the size doesn't match") } diff --git a/givre/src/ciphersuite/ed25519.rs b/givre/src/ciphersuite/ed25519.rs index 8d76e8c..250ed9b 100644 --- a/givre/src/ciphersuite/ed25519.rs +++ b/givre/src/ciphersuite/ed25519.rs @@ -1,4 +1,5 @@ use digest::Digest; +use generic_ec::{NonZero, Point}; use crate::Ciphersuite; @@ -25,8 +26,8 @@ impl Ciphersuite for Ed25519 { } fn compute_challenge( - group_commitment: &super::NormalizedPoint, - group_public_key: &super::NormalizedPoint, + group_commitment: &super::NormalizedPoint>, + group_public_key: &super::NormalizedPoint>>, msg: &[u8], ) -> generic_ec::Scalar { let hash = sha2::Sha512::new() @@ -83,9 +84,9 @@ impl Ciphersuite for Ed25519 { } type NormalizedPointBytes = Self::PointBytes; - fn serialize_normalized_point( - point: &super::NormalizedPoint, + fn serialize_normalized_point>>( + point: &super::NormalizedPoint, ) -> Self::NormalizedPointBytes { - Self::serialize_point(point) + Self::serialize_point(point.as_ref()) } } diff --git a/givre/src/ciphersuite/secp256k1.rs b/givre/src/ciphersuite/secp256k1.rs index 8d062f7..3f8f7dc 100644 --- a/givre/src/ciphersuite/secp256k1.rs +++ b/givre/src/ciphersuite/secp256k1.rs @@ -1,4 +1,5 @@ use digest::Digest; +use generic_ec::{NonZero, Point}; use crate::Ciphersuite; @@ -17,8 +18,8 @@ impl Ciphersuite for Secp256k1 { } fn compute_challenge( - group_commitment: &super::NormalizedPoint, - group_public_key: &super::NormalizedPoint, + group_commitment: &super::NormalizedPoint>, + group_public_key: &super::NormalizedPoint>>, msg: &[u8], ) -> generic_ec::Scalar { hash_to_scalar( @@ -48,13 +49,13 @@ impl Ciphersuite for Secp256k1 { } type PointBytes = generic_ec::EncodedPoint; - fn serialize_point(point: &generic_ec::Point) -> Self::PointBytes { + fn serialize_point(point: &Point) -> Self::PointBytes { point.to_bytes(true) } fn deserialize_point( bytes: &[u8], - ) -> Result, generic_ec::errors::InvalidPoint> { - generic_ec::Point::from_bytes(bytes) + ) -> Result, generic_ec::errors::InvalidPoint> { + Point::from_bytes(bytes) } type ScalarBytes = generic_ec::EncodedScalar; @@ -68,10 +69,10 @@ impl Ciphersuite for Secp256k1 { } type NormalizedPointBytes = Self::PointBytes; - fn serialize_normalized_point( - point: &super::NormalizedPoint, + fn serialize_normalized_point>>( + point: &super::NormalizedPoint, ) -> Self::NormalizedPointBytes { - Self::serialize_point(point) + Self::serialize_point(point.as_ref()) } } diff --git a/givre/src/signing/aggregate.rs b/givre/src/signing/aggregate.rs index 7fa66b3..c736267 100644 --- a/givre/src/signing/aggregate.rs +++ b/givre/src/signing/aggregate.rs @@ -8,7 +8,7 @@ use core::fmt; -use generic_ec::{Point, Scalar}; +use generic_ec::{NonZero, Point, Scalar}; use crate::{ciphersuite::NormalizedPoint, Ciphersuite, SignerIndex}; @@ -23,7 +23,7 @@ use super::{round1::PublicCommitments, round2::SigShare, utils}; /// Schnorr Signature pub struct Signature { /// $R$ component of the signature - pub r: crate::ciphersuite::NormalizedPoint, + pub r: crate::ciphersuite::NormalizedPoint>, /// $z$ component of the signature pub z: Scalar, } @@ -32,7 +32,7 @@ impl Signature { /// Verifies signature against a public key and a message pub fn verify( &self, - public_key: &NormalizedPoint, + public_key: &NormalizedPoint>>, msg: &[u8], ) -> Result<(), InvalidSignature> { let challenge = C::compute_challenge(&self.r, public_key, msg); diff --git a/givre/src/signing/round2.rs b/givre/src/signing/round2.rs index cc14ad3..1b671e0 100644 --- a/givre/src/signing/round2.rs +++ b/givre/src/signing/round2.rs @@ -8,7 +8,7 @@ use core::{fmt, iter}; -use generic_ec::{Curve, NonZero, Scalar}; +use generic_ec::{Curve, NonZero, Point, Scalar}; use crate::{ ciphersuite::{Ciphersuite, NormalizedPoint}, @@ -48,7 +48,7 @@ pub fn sign( signers: &[(SignerIndex, PublicCommitments)], ) -> Result, SigningError> { // --- Retrieve and Validate Data - let pk = NormalizedPoint::::try_from(key_share.shared_public_key) + let pk = NormalizedPoint::::try_normalize(key_share.shared_public_key) .map_err(|_| Reason::ShareNotNormalized)?; if signers.len() < usize::from(key_share.min_signers()) { return Err(Reason::TooFewSigners { @@ -109,7 +109,7 @@ pub fn sign( let group_commitment = utils::compute_group_commitment(&comm_list, &binding_factor_list); let nonce_share = nonce.hiding_nonce + (nonce.binding_nonce * binding_factor); - let (group_commitment, nonce_share) = match NormalizedPoint::try_from(group_commitment) { + let (group_commitment, nonce_share) = match NormalizedPoint::try_normalize(group_commitment) { Ok(r) => { // Signature is normalized, no need to do anything else (r, nonce_share) diff --git a/givre/src/signing/utils.rs b/givre/src/signing/utils.rs index 011bb90..4e3b665 100644 --- a/givre/src/signing/utils.rs +++ b/givre/src/signing/utils.rs @@ -39,7 +39,7 @@ pub fn encode_group_commitment_list( /// /// Although it's not mentioned in the draft, but note that output list is sorted by signer ID. pub fn compute_binding_factors( - shared_pk: Point, + shared_pk: NonZero>, commitment_list: &[(NonZero>, PublicCommitments)], msg: &[u8], ) -> Vec<(NonZero>, Scalar)> { diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 0d4d70b..341b56a 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -1,4 +1,9 @@ -use givre::{ciphersuite::NormalizedPoint, signing::aggregate::Signature, Ciphersuite}; +use givre::{ + ciphersuite::NormalizedPoint, + generic_ec::{NonZero, Point}, + signing::aggregate::Signature, + Ciphersuite, +}; pub trait ExternalVerifier: Ciphersuite { /// Although Schnorr scheme can sign messages of arbitrary size, external verifier may require that @@ -8,7 +13,7 @@ pub trait ExternalVerifier: Ciphersuite { type InvalidSig: core::fmt::Debug; fn verify_sig( - pk: &NormalizedPoint, + pk: &NormalizedPoint>>, sig: &Signature, msg: &[u8], ) -> Result<(), Self::InvalidSig>; @@ -21,7 +26,7 @@ impl ExternalVerifier for givre::ciphersuite::Ed25519 { type InvalidSig = ed25519::SignatureError; fn verify_sig( - pk: &NormalizedPoint, + pk: &NormalizedPoint>>, sig: &Signature, msg: &[u8], ) -> Result<(), ed25519::SignatureError> { @@ -45,7 +50,7 @@ impl ExternalVerifier for givre::ciphersuite::Secp256k1 { type InvalidSig = core::convert::Infallible; fn verify_sig( - _pk: &NormalizedPoint, + _pk: &NormalizedPoint>>, _sig: &Signature, _msg: &[u8], ) -> Result<(), Self::InvalidSig> { @@ -60,7 +65,7 @@ impl ExternalVerifier for givre::ciphersuite::Bitcoin { type InvalidSig = secp256k1::Error; fn verify_sig( - pk: &NormalizedPoint, + pk: &NormalizedPoint>>, sig: &Signature, msg: &[u8], ) -> Result<(), Self::InvalidSig> { diff --git a/tests/tests/it/interactive.rs b/tests/tests/it/interactive.rs index 699c35d..a9fa566 100644 --- a/tests/tests/it/interactive.rs +++ b/tests/tests/it/interactive.rs @@ -57,7 +57,7 @@ mod generic { .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_from(key_info.shared_public_key) + let pk = NormalizedPoint::try_normalize(key_info.shared_public_key) .expect("output public key isn't normalized"); // --- Signing diff --git a/tests/tests/it/main.rs b/tests/tests/it/main.rs index 5f53b4f..85f7029 100644 --- a/tests/tests/it/main.rs +++ b/tests/tests/it/main.rs @@ -27,7 +27,7 @@ mod generic { .collect::, _>>() .expect("normalize key shares"); let key_info: &givre::key_share::KeyInfo<_> = key_shares[0].as_ref(); - let pk = NormalizedPoint::::try_from(key_info.shared_public_key) + let pk = NormalizedPoint::try_normalize(key_info.shared_public_key) .expect("public key is not normalized"); // List of indexes of signers who co-hold the key diff --git a/tests/tests/it/test_vectors.rs b/tests/tests/it/test_vectors.rs index c441067..e4888ba 100644 --- a/tests/tests/it/test_vectors.rs +++ b/tests/tests/it/test_vectors.rs @@ -33,6 +33,7 @@ impl TestVector { let (t, n): (u16, u16) = (T.try_into().unwrap(), N.try_into().unwrap()); let public_key = C::deserialize_point(self.public_key).unwrap(); + let public_key = NonZero::from_point(public_key).unwrap(); { let secret_key = C::deserialize_secret_scalar(self.secret_key).unwrap(); assert_eq!(Point::generator() * &secret_key, public_key); @@ -40,7 +41,8 @@ impl TestVector { let shares = self .shares - .map(|share| C::deserialize_secret_scalar(share).unwrap()); + .map(|share| C::deserialize_secret_scalar(share).unwrap()) + .map(|share| NonZero::from_secret_scalar(share).unwrap()); let public_shares = shares .clone() .map(|share| Point::generator() * share) From a713b9b6fbe976a83b192440dff1e74c02ddbf3c Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Wed, 13 Mar 2024 14:13:14 +0100 Subject: [PATCH 10/14] Fix a bug in point normalization --- givre/src/ciphersuite.rs | 2 +- givre/src/lib.rs | 4 ++-- givre/src/signing/round2.rs | 12 ++++++------ tests/src/lib.rs | 5 ++--- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index ea83e7a..0ab5c3b 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -231,7 +231,7 @@ impl> + core::ops::Neg> Nor } else { let neg_point = -point; debug_assert!(C::is_normalized(neg_point.as_ref())); - Ok(Self(neg_point, Default::default())) + Err(Self(neg_point, Default::default())) } } } diff --git a/givre/src/lib.rs b/givre/src/lib.rs index 6fa14c9..4160d45 100644 --- a/givre/src/lib.rs +++ b/givre/src/lib.rs @@ -77,9 +77,9 @@ pub use cggmp21_keygen::keygen; /// ```rust,no_run /// # use rand_core::OsRng; /// # let mut rng = OsRng; -/// use givre::generic_ec::{curves::Secp256k1, SecretScalar}; +/// use givre::generic_ec::{curves::Secp256k1, SecretScalar, NonZero}; /// -/// let secret_key_to_be_imported = SecretScalar::::random(&mut rng); +/// let secret_key_to_be_imported = NonZero::>::random(&mut rng); /// /// let key_shares = givre::trusted_dealer::builder::(5) /// .set_threshold(Some(3)) diff --git a/givre/src/signing/round2.rs b/givre/src/signing/round2.rs index 1b671e0..38571f3 100644 --- a/givre/src/signing/round2.rs +++ b/givre/src/signing/round2.rs @@ -8,7 +8,7 @@ use core::{fmt, iter}; -use generic_ec::{Curve, NonZero, Point, Scalar}; +use generic_ec::{Curve, NonZero, Scalar}; use crate::{ ciphersuite::{Ciphersuite, NormalizedPoint}, @@ -110,14 +110,14 @@ pub fn sign( let nonce_share = nonce.hiding_nonce + (nonce.binding_nonce * binding_factor); let (group_commitment, nonce_share) = match NormalizedPoint::try_normalize(group_commitment) { - Ok(r) => { + Ok(group_commitment) => { // Signature is normalized, no need to do anything else - (r, nonce_share) + (group_commitment, nonce_share) } - Err(r) => { - // Signature is not normalized, we had to negate `r`. Each signer need to negate + Err(neg_group_commitment) => { + // Signature is not normalized, we had to negate `group_commitment`. Each signer need to negate // their `nonce_share` as well - (r, -nonce_share) + (neg_group_commitment, -nonce_share) } }; diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 341b56a..3dda00d 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -69,11 +69,10 @@ impl ExternalVerifier for givre::ciphersuite::Bitcoin { sig: &Signature, msg: &[u8], ) -> Result<(), Self::InvalidSig> { - let pk = - secp256k1::XOnlyPublicKey::from_slice(Self::serialize_normalized_point(pk).as_ref())?; + let pk = secp256k1::XOnlyPublicKey::from_slice(pk.to_bytes().as_ref())?; let mut signature = [0u8; 64]; - signature[..32].copy_from_slice(Self::serialize_normalized_point(&sig.r).as_ref()); + signature[..32].copy_from_slice(sig.r.to_bytes().as_ref()); signature[32..].copy_from_slice(Self::serialize_scalar(&sig.z).as_ref()); let signature = secp256k1::schnorr::Signature::from_slice(&signature)?; From 21c67ff7d0266fe3d413ba6270adf2494efe2e11 Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Wed, 13 Mar 2024 14:14:46 +0100 Subject: [PATCH 11/14] Change phrasing Co-authored-by: maurges --- tests/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/lib.rs b/tests/src/lib.rs index 3dda00d..605c435 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -7,7 +7,7 @@ use givre::{ pub trait ExternalVerifier: Ciphersuite { /// Although Schnorr scheme can sign messages of arbitrary size, external verifier may require that - /// message to be signed to be hashed + /// the message needs to be an output of a hash function with fixed size const REQUIRED_MESSAGE_SIZE: Option = None; type InvalidSig: core::fmt::Debug; From 7e05894e3f925f07f990c1193f0ecf16d8847d2b Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Wed, 13 Mar 2024 14:17:49 +0100 Subject: [PATCH 12/14] Add `is_key_share_normalized` --- givre/src/ciphersuite.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/givre/src/ciphersuite.rs b/givre/src/ciphersuite.rs index 0ab5c3b..5f7beef 100644 --- a/givre/src/ciphersuite.rs +++ b/givre/src/ciphersuite.rs @@ -326,3 +326,13 @@ pub fn normalize_key_share( .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`] +/// for more details. +pub fn is_key_share_normalized( + key_share: &crate::key_share::KeyShare, +) -> bool { + C::is_normalized(&key_share.shared_public_key) +} From 8da6c1d47dec3f4a5e902ef99e1353a36043ecb2 Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Thu, 14 Mar 2024 10:54:21 +0100 Subject: [PATCH 13/14] Use std::sync::OnceLock --- givre/src/ciphersuite/bitcoin.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/givre/src/ciphersuite/bitcoin.rs b/givre/src/ciphersuite/bitcoin.rs index 3cd2cde..8cb7516 100644 --- a/givre/src/ciphersuite/bitcoin.rs +++ b/givre/src/ciphersuite/bitcoin.rs @@ -1,4 +1,5 @@ use generic_ec::{NonZero, Point}; +use sha2::Digest; use super::{Ciphersuite, Secp256k1}; @@ -28,13 +29,7 @@ impl Ciphersuite for Bitcoin { group_public_key: &super::NormalizedPoint>>, msg: &[u8], ) -> generic_ec::Scalar { - use sha2::{Digest, Sha256}; - static HASH: once_cell::sync::Lazy = once_cell::sync::Lazy::new(|| { - let tag = Sha256::digest("BIP0340/challenge"); - Sha256::new().chain_update(tag).chain_update(tag) - }); - let challenge = HASH - .clone() + let challenge = challenge_hash() .chain_update(group_commitment.to_bytes()) .chain_update(group_public_key.to_bytes()) .chain_update(msg) @@ -88,3 +83,13 @@ impl Ciphersuite for Bitcoin { .expect("the size doesn't match") } } + +fn challenge_hash() -> sha2::Sha256 { + static PRECOMPUTED: std::sync::OnceLock = std::sync::OnceLock::new(); + PRECOMPUTED + .get_or_init(|| { + let tag = sha2::Sha256::digest("BIP0340/challenge"); + sha2::Sha256::new().chain_update(&tag).chain_update(&tag) + }) + .clone() +} From 680d44c497370a21d392659a5cdacf6c74d9b17d Mon Sep 17 00:00:00 2001 From: Denis Varlakov Date: Thu, 14 Mar 2024 12:39:59 +0100 Subject: [PATCH 14/14] Remove `once_cell` dep --- Cargo.lock | 1 - givre/Cargo.toml | 4 +--- givre/src/lib.rs | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d58fe23..b32ed95 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -572,7 +572,6 @@ dependencies = [ "generic-ec", "k256", "key-share", - "once_cell", "rand_core", "round-based", "serde", diff --git a/givre/Cargo.toml b/givre/Cargo.toml index 637b459..2a5cb49 100644 --- a/givre/Cargo.toml +++ b/givre/Cargo.toml @@ -23,8 +23,6 @@ sha2 = { version = "0.10", default-features = false, optional = true } serde = { version = "1", default-features = false, features = ["derive"], optional = true } -once_cell = { version = "1", optional = true } - [dev-dependencies] rand_core = { version = "0.6", default-features = false, features = ["getrandom"] } @@ -46,4 +44,4 @@ hd-wallets = ["key-share/hd-wallets", "cggmp21-keygen?/hd-wallets"] all-ciphersuites = ["ciphersuite-secp256k1", "ciphersuite-ed25519", "ciphersuite-bitcoin"] ciphersuite-secp256k1 = ["generic-ec/curve-secp256k1", "k256", "sha2", "static_assertions"] ciphersuite-ed25519 = ["generic-ec/curve-ed25519", "sha2"] -ciphersuite-bitcoin = ["ciphersuite-secp256k1", "dep:once_cell"] +ciphersuite-bitcoin = ["ciphersuite-secp256k1"] diff --git a/givre/src/lib.rs b/givre/src/lib.rs index 4160d45..b3f46c5 100644 --- a/givre/src/lib.rs +++ b/givre/src/lib.rs @@ -20,7 +20,7 @@ //! [CGGMP21]: https://github.com/dfns/cggmp21 //! [draft]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-15.html -#![forbid(unsafe_code)] +#![forbid(unsafe_code, unused_crate_dependencies)] #![deny(clippy::expect_used, clippy::unwrap_used, clippy::panic)] #![deny(missing_docs)] #![allow(clippy::type_complexity)]