From 179347a1b511c76aa145f4ec6fe03455b067950f Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Fri, 7 Jun 2024 21:08:53 -0300 Subject: [PATCH] return error when deserializing the identity --- frost-core/CHANGELOG.md | 17 +++ frost-core/src/batch.rs | 16 ++- frost-core/src/benches.rs | 3 +- frost-core/src/keys.rs | 131 +++++++----------- frost-core/src/keys/dkg.rs | 21 +-- frost-core/src/lib.rs | 42 +++--- frost-core/src/round1.rs | 66 ++++----- frost-core/src/round2.rs | 6 +- frost-core/src/serialization.rs | 29 ++-- frost-core/src/signature.rs | 23 ++- frost-core/src/signing_key.rs | 21 ++- frost-core/src/tests/batch.rs | 6 +- .../src/tests/coefficient_commitment.rs | 12 +- frost-core/src/tests/proptests.rs | 4 +- frost-core/src/tests/vectors.rs | 13 +- frost-core/src/tests/vss_commitment.rs | 34 ++--- frost-core/src/traits.rs | 9 +- frost-core/src/verifying_key.rs | 64 +++------ frost-ed25519/src/lib.rs | 7 +- frost-ed25519/tests/helpers/mod.rs | 4 +- frost-ed25519/tests/helpers/samples.rs | 16 +-- frost-ed448/src/lib.rs | 7 +- frost-ed448/tests/helpers/samples.rs | 16 +-- frost-p256/src/lib.rs | 18 +-- frost-p256/src/tests/deserialize.rs | 3 +- frost-p256/tests/helpers/samples.rs | 16 +-- frost-rerandomized/src/lib.rs | 4 +- frost-ristretto255/src/lib.rs | 7 +- frost-ristretto255/tests/helpers/samples.rs | 16 +-- frost-secp256k1/src/lib.rs | 18 +-- frost-secp256k1/src/tests/deserialize.rs | 3 +- frost-secp256k1/tests/helpers/samples.rs | 16 +-- 32 files changed, 337 insertions(+), 331 deletions(-) diff --git a/frost-core/CHANGELOG.md b/frost-core/CHANGELOG.md index 725a6de3..7ddbe73e 100644 --- a/frost-core/CHANGELOG.md +++ b/frost-core/CHANGELOG.md @@ -4,6 +4,23 @@ Entries are listed in reverse chronological order. ## Unreleased +* Changed the `deserialize()` function of Elements and structs containing + Elements to return an error if the element is the identity. This is a + requirement in the FROST specification that wasn't being followed. We are not + aware of any possible security issues that could be caused by this; in the + unlikely case that the identity was being serialized, this would be caught by + deserialization methods. However, we consider this change the right thing to + do as a defense-in-depth mechanism. This entails the following changes: + * `Group::serialize()` now returns an error. When implementing it, you must + return an error if it attempts to serialize the identity. + * `VerifyingShare::serialize()`, `CoefficientCommitment::serialize()`, + `VerifiableSecretSharingCommitment::serialize()`, + `NonceCommitment::serialize()`, `Signature::serialize()`, + `VerifyingKey::serialize()` can now all return an error. +* Removed `batch::Item::into()` which created a batch Item from a triple of + VerifyingKey, Signature and message. Use the new `batch::Item::new()` instead + (which can return an error). + ## 1.0.1 * Fixed `no-default-features`, previously it wouldn't compile. diff --git a/frost-core/src/batch.rs b/frost-core/src/batch.rs index 37d25bdf..9aa81999 100644 --- a/frost-core/src/batch.rs +++ b/frost-core/src/batch.rs @@ -25,16 +25,20 @@ pub struct Item { c: Challenge, } -impl<'msg, C, M> From<(VerifyingKey, Signature, &'msg M)> for Item +impl Item where C: Ciphersuite, - M: AsRef<[u8]>, { - fn from((vk, sig, msg): (VerifyingKey, Signature, &'msg M)) -> Self { + /// Create a new batch [`Item`] from a [`VerifyingKey`], [`Signature`] + /// and a message to be verified. + pub fn new(vk: VerifyingKey, sig: Signature, msg: M) -> Result> + where + M: AsRef<[u8]>, + { // Compute c now to avoid dependency on the msg lifetime. - let c = crate::challenge(&sig.R, &vk, msg.as_ref()); + let c = crate::challenge(&sig.R, &vk, msg.as_ref())?; - Self { vk, sig, c } + Ok(Self { vk, sig, c }) } } @@ -129,7 +133,7 @@ where Rs.push(R); VK_coeffs.push(<::Field>::zero() + (blind * item.c.0)); - VKs.push(item.vk.element); + VKs.push(item.vk.to_element()); } let scalars = once(&P_coeff_acc) diff --git a/frost-core/src/benches.rs b/frost-core/src/benches.rs index 74931bc7..16810cc4 100644 --- a/frost-core/src/benches.rs +++ b/frost-core/src/benches.rs @@ -67,7 +67,8 @@ pub fn bench_batch_verify( let msg = b"Bench"; let Item { vk, sig } = item; - batch.queue((*vk, *sig, msg)); + let item = batch::Item::::new(*vk, *sig, msg).unwrap(); + batch.queue(item); } batch.verify(&mut rng) }) diff --git a/frost-core/src/keys.rs b/frost-core/src/keys.rs index 0ec6fb24..75ffc8fd 100644 --- a/frost-core/src/keys.rs +++ b/frost-core/src/keys.rs @@ -17,11 +17,12 @@ use rand_core::{CryptoRng, RngCore}; use zeroize::{DefaultIsZeroes, Zeroize}; use crate::{ - Ciphersuite, Element, Error, Field, Group, Header, Identifier, Scalar, SigningKey, VerifyingKey, + serialization::SerializableElement, Ciphersuite, Element, Error, Field, Group, GroupError, + Header, Identifier, Scalar, SigningKey, VerifyingKey, }; #[cfg(feature = "serde")] -use crate::serialization::{ElementSerialization, ScalarSerialization}; +use crate::serialization::ScalarSerialization; #[cfg(feature = "serialization")] use crate::serialization::{Deserialize, Serialize}; @@ -38,7 +39,7 @@ pub(crate) fn sum_commitments( commitments: &[&VerifiableSecretSharingCommitment], ) -> Result, Error> { let mut group_commitment = vec![ - CoefficientCommitment(::identity()); + CoefficientCommitment::new(::identity()); commitments .first() .ok_or(Error::IncorrectNumberOfCommitments)? @@ -47,7 +48,7 @@ pub(crate) fn sum_commitments( ]; for commitment in commitments { for (i, c) in group_commitment.iter_mut().enumerate() { - *c = CoefficientCommitment( + *c = CoefficientCommitment::new( c.value() + commitment .0 @@ -186,9 +187,8 @@ where #[derive(Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(bound = "C: Ciphersuite"))] -#[cfg_attr(feature = "serde", serde(try_from = "ElementSerialization"))] -#[cfg_attr(feature = "serde", serde(into = "ElementSerialization"))] -pub struct VerifyingShare(pub(super) Element) +#[cfg_attr(feature = "serde", serde(transparent))] +pub struct VerifyingShare(pub(super) SerializableElement) where C: Ciphersuite; @@ -197,27 +197,30 @@ where C: Ciphersuite, { /// Create a new [`VerifyingShare`] from a element. - #[cfg(feature = "internals")] - pub fn new(element: Element) -> Self { - Self(element) + #[cfg_attr(feature = "internals", visibility::make(pub))] + #[cfg_attr(docsrs, doc(cfg(feature = "internals")))] + pub(crate) fn new(element: Element) -> Self { + Self(SerializableElement(element)) } /// Get the inner element. - #[cfg(feature = "internals")] - pub fn to_element(&self) -> Element { - self.0 + #[cfg_attr(feature = "internals", visibility::make(pub))] + #[cfg_attr(docsrs, doc(cfg(feature = "internals")))] + #[allow(dead_code)] + pub(crate) fn to_element(&self) -> Element { + self.0 .0 } /// Deserialize from bytes pub fn deserialize(bytes: ::Serialization) -> Result> { ::deserialize(&bytes) - .map(|element| Self(element)) + .map(|element| Self(SerializableElement(element))) .map_err(|e| e.into()) } /// Serialize to bytes - pub fn serialize(&self) -> ::Serialization { - ::serialize(&self.0) + pub fn serialize(&self) -> Result<::Serialization, Error> { + Ok(::serialize(&self.0 .0)?) } /// Computes a verifying share for a peer given the group commitment. @@ -237,7 +240,7 @@ where // Y_i = ∏_{k=0}^{t−1} (∏_{j=1}^n φ_{jk})^{i^k mod q} // i.e. we can operate on the sum of all φ_j commitments, which is // what is passed to the functions. - VerifyingShare(evaluate_vss(identifier, commitment)) + VerifyingShare::new(evaluate_vss(identifier, commitment)) } } @@ -247,7 +250,12 @@ where { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_tuple("VerifyingShare") - .field(&hex::encode(self.serialize())) + .field( + &self + .serialize() + .map(hex::encode) + .unwrap_or("".to_string()), + ) .finish() } } @@ -257,29 +265,7 @@ where C: Ciphersuite, { fn from(secret: SigningShare) -> VerifyingShare { - VerifyingShare(::generator() * secret.0 as Scalar) - } -} - -#[cfg(feature = "serde")] -impl TryFrom> for VerifyingShare -where - C: Ciphersuite, -{ - type Error = Error; - - fn try_from(value: ElementSerialization) -> Result { - Self::deserialize(value.0) - } -} - -#[cfg(feature = "serde")] -impl From> for ElementSerialization -where - C: Ciphersuite, -{ - fn from(value: VerifyingShare) -> Self { - Self(value.serialize()) + VerifyingShare::new(::generator() * secret.0 as Scalar) } } @@ -290,9 +276,7 @@ where #[derive(Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(bound = "C: Ciphersuite"))] -#[cfg_attr(feature = "serde", serde(try_from = "ElementSerialization"))] -#[cfg_attr(feature = "serde", serde(into = "ElementSerialization"))] -pub struct CoefficientCommitment(pub(crate) Element); +pub struct CoefficientCommitment(pub(crate) SerializableElement); impl CoefficientCommitment where @@ -301,12 +285,12 @@ where /// Create a new CoefficientCommitment. #[cfg_attr(feature = "internals", visibility::make(pub))] pub(crate) fn new(value: Element) -> Self { - Self(value) + Self(SerializableElement(value)) } /// returns serialized element - pub fn serialize(&self) -> ::Serialization { - ::serialize(&self.0) + pub fn serialize(&self) -> Result<::Serialization, Error> { + Ok(::serialize(&self.0 .0)?) } /// Creates a new commitment from a coefficient input @@ -318,7 +302,7 @@ where /// Returns inner element value pub fn value(&self) -> Element { - self.0 + self.0 .0 } } @@ -328,33 +312,16 @@ where { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { f.debug_tuple("CoefficientCommitment") - .field(&hex::encode(self.serialize())) + .field( + &self + .serialize() + .map(hex::encode) + .unwrap_or("".to_string()), + ) .finish() } } -#[cfg(feature = "serde")] -impl TryFrom> for CoefficientCommitment -where - C: Ciphersuite, -{ - type Error = Error; - - fn try_from(value: ElementSerialization) -> Result { - Self::deserialize(value.0) - } -} - -#[cfg(feature = "serde")] -impl From> for ElementSerialization -where - C: Ciphersuite, -{ - fn from(value: CoefficientCommitment) -> Self { - Self(value.serialize()) - } -} - /// Contains the commitments to the coefficients for our secret polynomial _f_, /// used to generate participants' key shares. /// @@ -385,11 +352,12 @@ where } /// Returns serialized coefficent commitments - pub fn serialize(&self) -> Vec<::Serialization> { - self.0 + pub fn serialize(&self) -> Result::Serialization>, Error> { + Ok(self + .0 .iter() - .map(|cc| <::Group as Group>::serialize(&cc.0)) - .collect() + .map(|cc| <::Group as Group>::serialize(&cc.value())) + .collect::>()?) } /// Returns VerifiableSecretSharingCommitment from a vector of serialized CoefficientCommitments @@ -408,7 +376,7 @@ where /// element in the vector), or an error if the vector is empty. pub(crate) fn verifying_key(&self) -> Result, Error> { Ok(VerifyingKey::new( - self.0.first().ok_or(Error::MissingCommitment)?.0, + self.0.first().ok_or(Error::MissingCommitment)?.0 .0, )) } @@ -488,7 +456,10 @@ where return Err(Error::InvalidSecretShare); } - Ok((VerifyingShare(result), self.commitment.verifying_key()?)) + Ok(( + VerifyingShare::new(result), + self.commitment.verifying_key()?, + )) } } @@ -633,7 +604,9 @@ fn evaluate_vss( let (_, result) = commitment.0.iter().fold( (<::Field>::one(), ::identity()), - |(i_to_the_k, sum_so_far), comm_k| (i * i_to_the_k, sum_so_far + comm_k.0 * i_to_the_k), + |(i_to_the_k, sum_so_far), comm_k| { + (i * i_to_the_k, sum_so_far + comm_k.value() * i_to_the_k) + }, ); result } @@ -862,7 +835,7 @@ pub(crate) fn generate_secret_polynomial( // Create the vector of commitments let commitment: Vec<_> = coefficients .iter() - .map(|c| CoefficientCommitment(::generator() * *c)) + .map(|c| CoefficientCommitment::new(::generator() * *c)) .collect(); let commitment: VerifiableSecretSharingCommitment = VerifiableSecretSharingCommitment(commitment); diff --git a/frost-core/src/keys/dkg.rs b/frost-core/src/keys/dkg.rs index e6e10ac6..dd09bda5 100644 --- a/frost-core/src/keys/dkg.rs +++ b/frost-core/src/keys/dkg.rs @@ -318,17 +318,19 @@ fn challenge( identifier: Identifier, verifying_key: &VerifyingKey, R: &Element, -) -> Option> +) -> Result, Error> where C: Ciphersuite, { let mut preimage = vec![]; preimage.extend_from_slice(identifier.serialize().as_ref()); - preimage.extend_from_slice(::serialize(&verifying_key.element).as_ref()); - preimage.extend_from_slice(::serialize(R).as_ref()); + preimage.extend_from_slice(::serialize(&verifying_key.to_element())?.as_ref()); + preimage.extend_from_slice(::serialize(R)?.as_ref()); - Some(Challenge(C::HDKG(&preimage[..])?)) + Ok(Challenge( + C::HDKG(&preimage[..]).ok_or(Error::DKGNotSupported)?, + )) } /// Compute the proof of knowledge of the secret coefficients used to generate @@ -348,8 +350,7 @@ pub(crate) fn compute_proof_of_knowledge // > a context string to prevent replay attacks. let k = <::Field>::random(&mut rng); let R_i = ::generator() * k; - let c_i = challenge::(identifier, &commitment.verifying_key()?, &R_i) - .ok_or(Error::DKGNotSupported)?; + let c_i = challenge::(identifier, &commitment.verifying_key()?, &R_i)?; let a_i0 = *coefficients .first() .expect("coefficients must have at least one element"); @@ -363,7 +364,7 @@ pub(crate) fn compute_proof_of_knowledge pub(crate) fn verify_proof_of_knowledge( identifier: Identifier, commitment: &VerifiableSecretSharingCommitment, - proof_of_knowledge: Signature, + proof_of_knowledge: &Signature, ) -> Result<(), Error> { // Round 1, Step 5 // @@ -374,8 +375,8 @@ pub(crate) fn verify_proof_of_knowledge( let R_ell = proof_of_knowledge.R; let mu_ell = proof_of_knowledge.z; let phi_ell0 = commitment.verifying_key()?; - let c_ell = challenge::(ell, &phi_ell0, &R_ell).ok_or(Error::DKGNotSupported)?; - if R_ell != ::generator() * mu_ell - phi_ell0.element * c_ell.0 { + let c_ell = challenge::(ell, &phi_ell0, &R_ell)?; + if R_ell != ::generator() * mu_ell - phi_ell0.to_element() * c_ell.0 { return Err(Error::InvalidProofOfKnowledge { culprit: ell }); } Ok(()) @@ -422,7 +423,7 @@ pub fn part2( verify_proof_of_knowledge( ell, &round1_package.commitment, - round1_package.proof_of_knowledge, + &round1_package.proof_of_knowledge, )?; // Round 2, Step 1 diff --git a/frost-core/src/lib.rs b/frost-core/src/lib.rs index bee7cbbc..96c5cba6 100644 --- a/frost-core/src/lib.rs +++ b/frost-core/src/lib.rs @@ -107,17 +107,21 @@ where /// [RFC]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-14.html#section-3.2 #[cfg_attr(feature = "internals", visibility::make(pub))] #[cfg_attr(docsrs, doc(cfg(feature = "internals")))] -fn challenge(R: &Element, verifying_key: &VerifyingKey, msg: &[u8]) -> Challenge +fn challenge( + R: &Element, + verifying_key: &VerifyingKey, + msg: &[u8], +) -> Result, Error> where C: Ciphersuite, { let mut preimage = vec![]; - preimage.extend_from_slice(::serialize(R).as_ref()); - preimage.extend_from_slice(::serialize(&verifying_key.element).as_ref()); + preimage.extend_from_slice(::serialize(R)?.as_ref()); + preimage.extend_from_slice(::serialize(&verifying_key.to_element())?.as_ref()); preimage.extend_from_slice(msg); - Challenge(C::H2(&preimage[..])) + Ok(Challenge(C::H2(&preimage[..]))) } /// Generates a random nonzero scalar. @@ -233,13 +237,13 @@ pub(crate) fn compute_binding_factor_list( signing_package: &SigningPackage, verifying_key: &VerifyingKey, additional_prefix: &[u8], -) -> BindingFactorList +) -> Result, Error> where C: Ciphersuite, { - let preimages = signing_package.binding_factor_preimages(verifying_key, additional_prefix); + let preimages = signing_package.binding_factor_preimages(verifying_key, additional_prefix)?; - BindingFactorList( + Ok(BindingFactorList( preimages .iter() .map(|(identifier, preimage)| { @@ -247,7 +251,7 @@ where (*identifier, BindingFactor(binding_factor)) }) .collect(), - ) + )) } #[cfg(any(test, feature = "test-impl"))] @@ -399,28 +403,30 @@ where // We separate this out into its own method so it can be tested #[cfg_attr(feature = "internals", visibility::make(pub))] #[cfg_attr(docsrs, doc(cfg(feature = "internals")))] + #[allow(clippy::type_complexity)] pub fn binding_factor_preimages( &self, verifying_key: &VerifyingKey, additional_prefix: &[u8], - ) -> Vec<(Identifier, Vec)> { + ) -> Result, Vec)>, Error> { let mut binding_factor_input_prefix = vec![]; // The length of a serialized verifying key of the same cipersuite does // not change between runs of the protocol, so we don't need to hash to // get a fixed length. - binding_factor_input_prefix.extend_from_slice(verifying_key.serialize().as_ref()); + binding_factor_input_prefix.extend_from_slice(verifying_key.serialize()?.as_ref()); // The message is hashed with H4 to force the variable-length message // into a fixed-length byte string, same for hashing the variable-sized // (between runs of the protocol) set of group commitments, but with H5. binding_factor_input_prefix.extend_from_slice(C::H4(self.message.as_slice()).as_ref()); binding_factor_input_prefix.extend_from_slice( - C::H5(&round1::encode_group_commitments(self.signing_commitments())[..]).as_ref(), + C::H5(&round1::encode_group_commitments(self.signing_commitments())?[..]).as_ref(), ); binding_factor_input_prefix.extend_from_slice(additional_prefix); - self.signing_commitments() + Ok(self + .signing_commitments() .keys() .map(|identifier| { let mut binding_factor_input = vec![]; @@ -429,7 +435,7 @@ where binding_factor_input.extend_from_slice(identifier.serialize().as_ref()); (*identifier, binding_factor_input) }) - .collect() + .collect()) } } @@ -496,7 +502,7 @@ where for (commitment_identifier, commitment) in signing_package.signing_commitments() { // The following check prevents a party from accidentally revealing their share. // Note that the '&&' operator would be sufficient. - if identity == commitment.binding.0 || identity == commitment.hiding.0 { + if identity == commitment.binding.value() || identity == commitment.hiding.value() { return Err(Error::IdentityCommitment); } @@ -506,10 +512,10 @@ where // Collect the binding commitments and their binding factors for one big // multiscalar multiplication at the end. - binding_elements.push(commitment.binding.0); + binding_elements.push(commitment.binding.value()); binding_scalars.push(binding_factor.0); - group_commitment = group_commitment + commitment.hiding.0; + group_commitment = group_commitment + commitment.hiding.value(); } let accumulated_binding_commitment: Element = @@ -568,7 +574,7 @@ where // Encodes the signing commitment list produced in round one as part of generating [`BindingFactor`], the // binding factor. let binding_factor_list: BindingFactorList = - compute_binding_factor_list(signing_package, &pubkeys.verifying_key, &[]); + compute_binding_factor_list(signing_package, &pubkeys.verifying_key, &[])?; // Compute the group commitment from signing commitments produced in round one. let group_commitment = compute_group_commitment(signing_package, &binding_factor_list)?; @@ -605,7 +611,7 @@ where &group_commitment.0, &pubkeys.verifying_key, signing_package.message().as_slice(), - ); + )?; // Verify the signature shares. for (signature_share_identifier, signature_share) in signature_shares { diff --git a/frost-core/src/round1.rs b/frost-core/src/round1.rs index 1baf304b..bfc11c1b 100644 --- a/frost-core/src/round1.rs +++ b/frost-core/src/round1.rs @@ -12,10 +12,12 @@ use hex::FromHex; use rand_core::{CryptoRng, RngCore}; use zeroize::Zeroize; -use crate::{Ciphersuite, Element, Error, Field, Group, Header, Scalar}; +use crate::{ + serialization::SerializableElement, Ciphersuite, Element, Error, Field, Group, Header, Scalar, +}; #[cfg(feature = "serde")] -use crate::serialization::{ElementSerialization, ScalarSerialization}; +use crate::serialization::ScalarSerialization; #[cfg(feature = "serialization")] use crate::serialization::{Deserialize, Serialize}; @@ -136,46 +138,31 @@ where #[derive(Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(bound = "C: Ciphersuite"))] -#[cfg_attr(feature = "serde", serde(try_from = "ElementSerialization"))] -#[cfg_attr(feature = "serde", serde(into = "ElementSerialization"))] -pub struct NonceCommitment(pub(super) Element); +pub struct NonceCommitment(pub(super) SerializableElement); impl NonceCommitment where C: Ciphersuite, { + /// Create a new [`NonceCommitment`] from an [`Element`] + pub(crate) fn new(value: Element) -> Self { + Self(SerializableElement(value)) + } + + pub(crate) fn value(&self) -> Element { + self.0 .0 + } + /// Deserialize [`NonceCommitment`] from bytes pub fn deserialize(bytes: ::Serialization) -> Result> { ::deserialize(&bytes) - .map(|element| Self(element)) + .map(|element| Self::new(element)) .map_err(|e| e.into()) } /// Serialize [`NonceCommitment`] to bytes - pub fn serialize(&self) -> ::Serialization { - ::serialize(&self.0) - } -} - -#[cfg(feature = "serde")] -impl TryFrom> for NonceCommitment -where - C: Ciphersuite, -{ - type Error = Error; - - fn try_from(value: ElementSerialization) -> Result { - Self::deserialize(value.0) - } -} - -#[cfg(feature = "serde")] -impl From> for ElementSerialization -where - C: Ciphersuite, -{ - fn from(value: NonceCommitment) -> Self { - Self(value.serialize()) + pub fn serialize(&self) -> Result<::Serialization, Error> { + Ok(::serialize(&self.0 .0)?) } } @@ -185,7 +172,12 @@ where { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_tuple("NonceCommitment") - .field(&hex::encode(self.serialize())) + .field( + &self + .serialize() + .map(hex::encode) + .unwrap_or("".to_string()), + ) .finish() } } @@ -204,7 +196,7 @@ where C: Ciphersuite, { fn from(nonce: &Nonce) -> Self { - Self(::generator() * nonce.0) + Self::new(::generator() * nonce.0) } } @@ -359,7 +351,7 @@ where self, binding_factor: &crate::BindingFactor, ) -> GroupCommitmentShare { - GroupCommitmentShare::(self.hiding.0 + (self.binding.0 * binding_factor.0)) + GroupCommitmentShare::(self.hiding.value() + (self.binding.value() * binding_factor.0)) } } @@ -406,16 +398,16 @@ pub struct GroupCommitmentShare(pub(super) Element); /// [`encode_group_commitment_list()`]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-14.html#name-list-operations pub(super) fn encode_group_commitments( signing_commitments: &BTreeMap, SigningCommitments>, -) -> Vec { +) -> Result, Error> { let mut bytes = vec![]; for (item_identifier, item) in signing_commitments { bytes.extend_from_slice(item_identifier.serialize().as_ref()); - bytes.extend_from_slice(::serialize(&item.hiding.0).as_ref()); - bytes.extend_from_slice(::serialize(&item.binding.0).as_ref()); + bytes.extend_from_slice(::serialize(&item.hiding.value())?.as_ref()); + bytes.extend_from_slice(::serialize(&item.binding.value())?.as_ref()); } - bytes + Ok(bytes) } /// Done once by each participant, to generate _their_ nonces and commitments diff --git a/frost-core/src/round2.rs b/frost-core/src/round2.rs index 20412a0e..f71bddef 100644 --- a/frost-core/src/round2.rs +++ b/frost-core/src/round2.rs @@ -93,7 +93,7 @@ where challenge: &Challenge, ) -> Result<(), Error> { if (::generator() * self.share) - != (group_commitment_share.0 + (verifying_share.0 * challenge.0 * lambda_i)) + != (group_commitment_share.0 + (verifying_share.to_element() * challenge.0 * lambda_i)) { return Err(Error::InvalidSignatureShare { culprit: identifier, @@ -202,7 +202,7 @@ pub fn sign( // Encodes the signing commitment list produced in round one as part of generating [`BindingFactor`], the // binding factor. let binding_factor_list: BindingFactorList = - compute_binding_factor_list(signing_package, &key_package.verifying_key, &[]); + compute_binding_factor_list(signing_package, &key_package.verifying_key, &[])?; let binding_factor: frost::BindingFactor = binding_factor_list .get(&key_package.identifier) .ok_or(Error::UnknownIdentifier)? @@ -219,7 +219,7 @@ pub fn sign( &group_commitment.0, &key_package.verifying_key, signing_package.message.as_slice(), - ); + )?; // Compute the Schnorr signature share. let signature_share = compute_signature_share( diff --git a/frost-core/src/serialization.rs b/frost-core/src/serialization.rs index e3127c75..eda0bdec 100644 --- a/frost-core/src/serialization.rs +++ b/frost-core/src/serialization.rs @@ -1,11 +1,15 @@ //! Serialization support. +use crate::Ciphersuite; + #[cfg(feature = "serde")] -use crate::{Ciphersuite, Field, Group}; +use crate::{Field, Group}; #[cfg(feature = "serialization")] use crate::Error; +use crate::Element; + #[cfg(feature = "serde")] #[cfg_attr(feature = "internals", visibility::make(pub))] #[cfg_attr(docsrs, doc(cfg(feature = "internals")))] @@ -51,13 +55,11 @@ where } } -#[cfg(feature = "serde")] -pub(crate) struct ElementSerialization( - pub(crate) <::Group as Group>::Serialization, -); +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) struct SerializableElement(pub(crate) Element); #[cfg(feature = "serde")] -impl serde::Serialize for ElementSerialization +impl serde::Serialize for SerializableElement where C: Ciphersuite, { @@ -65,12 +67,14 @@ where where S: serde::Serializer, { - serdect::array::serialize_hex_lower_or_bin(&self.0.as_ref(), serializer) + let serialized = + ::serialize(&self.0).map_err(serde::ser::Error::custom)?; + serdect::array::serialize_hex_lower_or_bin(&serialized.as_ref(), serializer) } } #[cfg(feature = "serde")] -impl<'de, C> serde::Deserialize<'de> for ElementSerialization +impl<'de, C> serde::Deserialize<'de> for SerializableElement where C: Ciphersuite, { @@ -80,14 +84,19 @@ where { // Get size from the size of the generator let generator = ::generator(); - let len = ::serialize(&generator).as_ref().len(); + let len = ::serialize(&generator) + .expect("serializing the generator always works") + .as_ref() + .len(); let mut bytes = vec![0u8; len]; serdect::array::deserialize_hex_or_bin(&mut bytes[..], deserializer)?; let array = bytes .try_into() .map_err(|_| serde::de::Error::custom("invalid byte length"))?; - Ok(Self(array)) + ::deserialize(&array) + .map(|element| Self(element)) + .map_err(serde::de::Error::custom) } } diff --git a/frost-core/src/signature.rs b/frost-core/src/signature.rs index e913c7de..a55b0db2 100644 --- a/frost-core/src/signature.rs +++ b/frost-core/src/signature.rs @@ -35,7 +35,7 @@ where // and get its length. Note that we can't use the identity because it can be encoded // shorter in some cases (e.g. P-256, which uses SEC1 encoding). let generator = ::generator(); - let mut R_bytes = Vec::from(::serialize(&generator).as_ref()); + let mut R_bytes = Vec::from(::serialize(&generator)?.as_ref()); let R_bytes_len = R_bytes.len(); @@ -71,13 +71,13 @@ where } /// Converts this signature to its [`Ciphersuite::SignatureSerialization`] in bytes. - pub fn serialize(&self) -> C::SignatureSerialization { + pub fn serialize(&self) -> Result> { let mut bytes = vec![]; - bytes.extend(::serialize(&self.R).as_ref()); + bytes.extend(::serialize(&self.R)?.as_ref()); bytes.extend(<::Field>::serialize(&self.z).as_ref()); - bytes.try_into().debugless_unwrap() + Ok(bytes.try_into().debugless_unwrap()) } } @@ -92,7 +92,13 @@ where where S: serde::Serializer, { - serdect::slice::serialize_hex_lower_or_bin(&self.serialize().as_ref(), serializer) + serdect::slice::serialize_hex_lower_or_bin( + &self + .serialize() + .map_err(serde::ser::Error::custom)? + .as_ref(), + serializer, + ) } } @@ -120,7 +126,12 @@ where impl std::fmt::Debug for Signature { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { f.debug_struct("Signature") - .field("R", &hex::encode(::serialize(&self.R).as_ref())) + .field( + "R", + &::serialize(&self.R) + .map(|s| hex::encode(s.as_ref())) + .unwrap_or("".to_string()), + ) .field( "z", &hex::encode(<::Field>::serialize(&self.z).as_ref()), diff --git a/frost-core/src/signing_key.rs b/frost-core/src/signing_key.rs index 26d812f2..03e13156 100644 --- a/frost-core/src/signing_key.rs +++ b/frost-core/src/signing_key.rs @@ -31,11 +31,7 @@ where let scalar = <::Field as Field>::deserialize(&bytes).map_err(Error::from)?; - if scalar == <::Field as Field>::zero() { - return Err(Error::MalformedSigningKey); - } - - Ok(Self { scalar }) + Self::from_scalar(scalar) } /// Serialize `SigningKey` to bytes @@ -50,18 +46,21 @@ where let R = ::generator() * k; // Generate Schnorr challenge - let c = crate::challenge::(&R, &VerifyingKey::::from(*self), msg); + let c = crate::challenge::(&R, &VerifyingKey::::from(*self), msg).expect("should not return error since that happens only if one of the inputs is the identity. R is not since k is nonzero. The verifying_key is not because signing keys are not allowed to be zero."); let z = k + (c.0 * self.scalar); Signature { R, z } } - /// Creates a SigningKey from a scalar. + /// Creates a SigningKey from a scalar. Returns an error if the scalar is zero. pub fn from_scalar( scalar: <<::Group as Group>::Field as Field>::Scalar, - ) -> Self { - Self { scalar } + ) -> Result> { + if scalar == <::Field as Field>::zero() { + return Err(Error::MalformedSigningKey); + } + Ok(Self { scalar }) } /// Return the underlying scalar. @@ -84,9 +83,7 @@ where C: Ciphersuite, { fn from(signing_key: &SigningKey) -> Self { - VerifyingKey { - element: C::Group::generator() * signing_key.scalar, - } + VerifyingKey::new(C::Group::generator() * signing_key.scalar) } } diff --git a/frost-core/src/tests/batch.rs b/frost-core/src/tests/batch.rs index 2f40433b..008e0cb7 100644 --- a/frost-core/src/tests/batch.rs +++ b/frost-core/src/tests/batch.rs @@ -10,7 +10,7 @@ pub fn batch_verify(mut rng: R) { let msg = b"BatchVerifyTest"; let sig = sk.sign(&mut rng, &msg[..]); assert!(vk.verify(msg, &sig).is_ok()); - batch.queue((vk, sig, msg)); + batch.queue(batch::Item::::new(vk, sig, msg).unwrap()); } assert!(batch.verify(rng).is_ok()); } @@ -31,14 +31,14 @@ pub fn bad_batch_verify(mut rng: R) { } else { sk.sign(&mut rng, b"bad") }; - (vk, sig, msg).into() + batch::Item::::new(vk, sig, msg).unwrap() } 1 => { let sk = SigningKey::new(&mut rng); let vk = VerifyingKey::::from(&sk); let msg = b"BatchVerifyTest"; let sig = sk.sign(&mut rng, &msg[..]); - (vk, sig, msg).into() + batch::Item::::new(vk, sig, msg).unwrap() } _ => unreachable!(), }; diff --git a/frost-core/src/tests/coefficient_commitment.rs b/frost-core/src/tests/coefficient_commitment.rs index 36c241c8..6218d167 100644 --- a/frost-core/src/tests/coefficient_commitment.rs +++ b/frost-core/src/tests/coefficient_commitment.rs @@ -16,9 +16,11 @@ pub fn check_serialization_of_coefficient_commitment(&mut rng); - let expected = ::serialize(&element); + let expected = ::serialize(&element).unwrap(); - let data = frost::keys::CoefficientCommitment::(element).serialize(); + let data = frost::keys::CoefficientCommitment::::new(element) + .serialize() + .unwrap(); assert!(expected.as_ref() == data.as_ref()); } @@ -27,9 +29,9 @@ pub fn check_serialization_of_coefficient_commitment(mut rng: R) { let element = generate_element::(&mut rng); - let expected = CoefficientCommitment::(element); + let expected = CoefficientCommitment::::new(element); - let serialized_element = ::serialize(&element); + let serialized_element = ::serialize(&element).unwrap(); let coeff_commitment = frost::keys::CoefficientCommitment::::deserialize(serialized_element).unwrap(); @@ -59,7 +61,7 @@ pub fn check_get_value_of_coefficient_commitment(&mut rng); - let coeff_commitment = frost::keys::CoefficientCommitment::(element); + let coeff_commitment = frost::keys::CoefficientCommitment::::new(element); let value = coeff_commitment.value(); assert!(value == element) diff --git a/frost-core/src/tests/proptests.rs b/frost-core/src/tests/proptests.rs index f7c70897..06a3925f 100644 --- a/frost-core/src/tests/proptests.rs +++ b/frost-core/src/tests/proptests.rs @@ -59,12 +59,12 @@ where // The signature data is stored in (refined) byte types, but do a round trip // conversion to raw bytes to exercise those code paths. let _sig = { - let bytes = self.sig.serialize(); + let bytes = self.sig.serialize().unwrap(); Signature::::deserialize(bytes) }; // Check that the verification key is a valid key. - let _pub_key = VerifyingKey::::deserialize(self.vk.serialize()) + let _pub_key = VerifyingKey::::deserialize(self.vk.serialize().unwrap()) .expect("The test verification key to be well-formed."); // Check that signature validation has the expected result. diff --git a/frost-core/src/tests/vectors.rs b/frost-core/src/tests/vectors.rs index d38f7af3..837e8929 100644 --- a/frost-core/src/tests/vectors.rs +++ b/frost-core/src/tests/vectors.rs @@ -263,13 +263,14 @@ pub fn check_sign_with_test_vectors(json_vectors: &Value) { for (identifier, input) in signing_package .binding_factor_preimages(&verifying_key, &[]) + .unwrap() .iter() { assert_eq!(*input, binding_factor_inputs[identifier]); } let binding_factor_list: frost::BindingFactorList = - compute_binding_factor_list(&signing_package, &verifying_key, &[]); + compute_binding_factor_list(&signing_package, &verifying_key, &[]).unwrap(); for (identifier, binding_factor) in binding_factor_list.0.iter() { assert_eq!(*binding_factor, binding_factors[identifier]); @@ -311,7 +312,10 @@ pub fn check_sign_with_test_vectors(json_vectors: &Value) { // Check that the generated signature matches the test vector signature let group_signature = group_signature_result.unwrap(); - assert_eq!(group_signature.serialize().as_ref(), signature_bytes); + assert_eq!( + group_signature.serialize().unwrap().as_ref(), + signature_bytes + ); // Aggregate the FROST signature from our signature shares let group_signature_result = @@ -322,5 +326,8 @@ pub fn check_sign_with_test_vectors(json_vectors: &Value) { // Check that the generated signature matches the test vector signature let group_signature = group_signature_result.unwrap(); - assert_eq!(group_signature.serialize().as_ref(), signature_bytes); + assert_eq!( + group_signature.serialize().unwrap().as_ref(), + signature_bytes + ); } diff --git a/frost-core/src/tests/vss_commitment.rs b/frost-core/src/tests/vss_commitment.rs index 9a0c40d2..31b5d4cb 100644 --- a/frost-core/src/tests/vss_commitment.rs +++ b/frost-core/src/tests/vss_commitment.rs @@ -24,20 +24,22 @@ pub fn check_serialize_vss_commitment(mu let input_3 = generate_element::(&mut rng); let coeff_comms = vec![ - CoefficientCommitment::(input_1), - CoefficientCommitment(input_2), - CoefficientCommitment(input_3), + CoefficientCommitment::::new(input_1), + CoefficientCommitment::new(input_2), + CoefficientCommitment::new(input_3), ]; // --- let expected = [ - ::serialize(&input_1), - ::serialize(&input_2), - ::serialize(&input_3), + ::serialize(&input_1).unwrap(), + ::serialize(&input_2).unwrap(), + ::serialize(&input_3).unwrap(), ]; - let vss_commitment = VerifiableSecretSharingCommitment(coeff_comms).serialize(); + let vss_commitment = VerifiableSecretSharingCommitment(coeff_comms) + .serialize() + .unwrap(); assert!(expected.len() == vss_commitment.len()); assert!(expected @@ -56,18 +58,18 @@ pub fn check_deserialize_vss_commitment( let input_3 = generate_element::(&mut rng); let coeff_comms = vec![ - CoefficientCommitment::(input_1), - CoefficientCommitment(input_2), - CoefficientCommitment(input_3), + CoefficientCommitment::::new(input_1), + CoefficientCommitment::new(input_2), + CoefficientCommitment::new(input_3), ]; // --- let expected = VerifiableSecretSharingCommitment(coeff_comms); let data = vec![ - ::serialize(&input_1), - ::serialize(&input_2), - ::serialize(&input_3), + ::serialize(&input_1).unwrap(), + ::serialize(&input_2).unwrap(), + ::serialize(&input_3).unwrap(), ]; let vss_value = VerifiableSecretSharingCommitment::deserialize(data); @@ -98,9 +100,9 @@ pub fn check_deserialize_vss_commitment_error::serialize(&input_1), - ::serialize(&input_2), - ::serialize(&input_3), + ::serialize(&input_1).unwrap(), + ::serialize(&input_2).unwrap(), + ::serialize(&input_3).unwrap(), serialized, ]; diff --git a/frost-core/src/traits.rs b/frost-core/src/traits.rs index 817caf60..4221d96b 100644 --- a/frost-core/src/traits.rs +++ b/frost-core/src/traits.rs @@ -113,11 +113,12 @@ pub trait Group: Copy + Clone + PartialEq { /// [`ScalarBaseMult()`]: https://www.ietf.org/archive/id/draft-irtf-cfrg-frost-14.html#section-3.1-3.5 fn generator() -> Self::Element; - /// A member function of a group _G_ that maps an [`Element`] to a unique byte array buf of - /// fixed length Ne. + /// A member function of a group _G_ that maps an [`Element`] to a unique + /// byte array buf of fixed length Ne. This function raises an error if the + /// element is the identity element of the group. /// /// - fn serialize(element: &Self::Element) -> Self::Serialization; + fn serialize(element: &Self::Element) -> Result; /// A member function of a [`Group`] that attempts to map a byte array `buf` to an [`Element`]. /// @@ -224,7 +225,7 @@ pub trait Ciphersuite: Copy + Clone + PartialEq + Debug { signature: &Signature, public_key: &VerifyingKey, ) -> Result<(), Error> { - let c = crate::challenge::(&signature.R, public_key, msg); + let c = crate::challenge::(&signature.R, public_key, msg)?; public_key.verify_prehashed(c, signature) } diff --git a/frost-core/src/verifying_key.rs b/frost-core/src/verifying_key.rs index 519020b5..2effd05e 100644 --- a/frost-core/src/verifying_key.rs +++ b/frost-core/src/verifying_key.rs @@ -3,22 +3,18 @@ use std::fmt::{self, Debug}; #[cfg(any(test, feature = "test-impl"))] use hex::FromHex; -use crate::{Challenge, Ciphersuite, Element, Error, Group, Signature}; - -#[cfg(feature = "serde")] -use crate::serialization::ElementSerialization; +use crate::{serialization::SerializableElement, Challenge, Ciphersuite, Error, Group, Signature}; /// A valid verifying key for Schnorr signatures over a FROST [`Ciphersuite::Group`]. #[derive(Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(bound = "C: Ciphersuite"))] -#[cfg_attr(feature = "serde", serde(try_from = "ElementSerialization"))] -#[cfg_attr(feature = "serde", serde(into = "ElementSerialization"))] +#[cfg_attr(feature = "serde", serde(transparent))] pub struct VerifyingKey where C: Ciphersuite, { - pub(crate) element: Element, + pub(crate) element: SerializableElement, } impl VerifyingKey @@ -29,13 +25,16 @@ where #[cfg_attr(feature = "internals", visibility::make(pub))] #[cfg_attr(docsrs, doc(cfg(feature = "internals")))] pub(crate) fn new(element: ::Element) -> Self { - Self { element } + Self { + element: SerializableElement(element), + } } /// Return the underlying element. - #[cfg(feature = "internals")] - pub fn to_element(self) -> ::Element { - self.element + #[cfg_attr(feature = "internals", visibility::make(pub))] + #[cfg_attr(docsrs, doc(cfg(feature = "internals")))] + pub(crate) fn to_element(self) -> ::Element { + self.element.0 } /// Deserialize from bytes @@ -43,13 +42,13 @@ where bytes: ::Serialization, ) -> Result, Error> { ::deserialize(&bytes) - .map(|element| VerifyingKey { element }) + .map(|element| VerifyingKey::new(element)) .map_err(|e| e.into()) } /// Serialize `VerifyingKey` to bytes - pub fn serialize(&self) -> ::Serialization { - ::serialize(&self.element) + pub fn serialize(&self) -> Result<::Serialization, Error> { + Ok(::serialize(&self.element.0)?) } /// Verify a purported `signature` with a pre-hashed [`Challenge`] made by this verification @@ -64,7 +63,7 @@ where // // where h is the cofactor let zB = C::Group::generator() * signature.z; - let cA = self.element * challenge.0; + let cA = self.element.0 * challenge.0; let check = (zB - cA - signature.R) * C::Group::cofactor(); if check == C::Group::identity() { @@ -84,13 +83,13 @@ where pub(crate) fn from_commitment( commitment: &crate::keys::VerifiableSecretSharingCommitment, ) -> Result, Error> { - Ok(VerifyingKey { - element: commitment + Ok(VerifyingKey::new( + commitment .coefficients() .first() .ok_or(Error::IncorrectCommitment)? .value(), - }) + )) } } @@ -100,7 +99,12 @@ where { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_tuple("VerifyingKey") - .field(&hex::encode(self.serialize())) + .field( + &self + .serialize() + .map(hex::encode) + .unwrap_or("".to_string()), + ) .finish() } } @@ -120,25 +124,3 @@ where } } } - -#[cfg(feature = "serde")] -impl TryFrom> for VerifyingKey -where - C: Ciphersuite, -{ - type Error = Error; - - fn try_from(value: ElementSerialization) -> Result { - Self::deserialize(value.0) - } -} - -#[cfg(feature = "serde")] -impl From> for ElementSerialization -where - C: Ciphersuite, -{ - fn from(value: VerifyingKey) -> Self { - Self(value.serialize()) - } -} diff --git a/frost-ed25519/src/lib.rs b/frost-ed25519/src/lib.rs index b774c17d..c7644edb 100644 --- a/frost-ed25519/src/lib.rs +++ b/frost-ed25519/src/lib.rs @@ -99,8 +99,11 @@ impl Group for Ed25519Group { ED25519_BASEPOINT_POINT } - fn serialize(element: &Self::Element) -> Self::Serialization { - element.compress().to_bytes() + fn serialize(element: &Self::Element) -> Result { + if *element == Self::identity() { + return Err(GroupError::InvalidIdentityElement); + } + Ok(element.compress().to_bytes()) } fn deserialize(buf: &Self::Serialization) -> Result { diff --git a/frost-ed25519/tests/helpers/mod.rs b/frost-ed25519/tests/helpers/mod.rs index 8f0f795d..ec68f588 100644 --- a/frost-ed25519/tests/helpers/mod.rs +++ b/frost-ed25519/tests/helpers/mod.rs @@ -14,11 +14,11 @@ pub fn verify_signature( group_pubkey: frost_core::VerifyingKey, ) { let sig = { - let bytes: [u8; 64] = group_signature.serialize(); + let bytes: [u8; 64] = group_signature.serialize().unwrap(); ed25519_dalek::Signature::from(bytes) }; let pub_key = { - let bytes = group_pubkey.serialize(); + let bytes = group_pubkey.serialize().unwrap(); ed25519_dalek::VerifyingKey::from_bytes(&bytes).unwrap() }; // Check that signature validation has the expected result. diff --git a/frost-ed25519/tests/helpers/samples.rs b/frost-ed25519/tests/helpers/samples.rs index 3080df01..9f9cc79f 100644 --- a/frost-ed25519/tests/helpers/samples.rs +++ b/frost-ed25519/tests/helpers/samples.rs @@ -44,8 +44,8 @@ pub fn signing_nonces() -> SigningNonces { /// Generate a sample SigningCommitments. pub fn signing_commitments() -> SigningCommitments { - let serialized_element1 = ::Group::serialize(&element1()); - let serialized_element2 = ::Group::serialize(&element2()); + let serialized_element1 = ::Group::serialize(&element1()).unwrap(); + let serialized_element2 = ::Group::serialize(&element2()).unwrap(); let hiding_nonce_commitment = NonceCommitment::deserialize(serialized_element1).unwrap(); let binding_nonce_commitment = NonceCommitment::deserialize(serialized_element2).unwrap(); @@ -72,7 +72,7 @@ pub fn signature_share() -> SignatureShare { pub fn secret_share() -> SecretShare { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let vss_commitment = VerifiableSecretSharingCommitment::deserialize(vec![serialized_element]).unwrap(); @@ -84,10 +84,10 @@ pub fn secret_share() -> SecretShare { pub fn key_package() -> KeyPackage { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) @@ -96,9 +96,9 @@ pub fn key_package() -> KeyPackage { /// Generate a sample PublicKeyPackage. pub fn public_key_package() -> PublicKeyPackage { let identifier = 42u16.try_into().unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); let verifying_shares = BTreeMap::from([(identifier, verifying_share)]); @@ -108,7 +108,7 @@ pub fn public_key_package() -> PublicKeyPackage { /// Generate a sample round1::Package. pub fn round1_package() -> round1::Package { let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let serialized_signature = serialized_element .as_ref() .iter() diff --git a/frost-ed448/src/lib.rs b/frost-ed448/src/lib.rs index a73a1de1..74a6f10d 100644 --- a/frost-ed448/src/lib.rs +++ b/frost-ed448/src/lib.rs @@ -98,8 +98,11 @@ impl Group for Ed448Group { Self::Element::generator() } - fn serialize(element: &Self::Element) -> Self::Serialization { - element.compress().0 + fn serialize(element: &Self::Element) -> Result { + if *element == Self::identity() { + return Err(GroupError::InvalidIdentityElement); + } + Ok(element.compress().0) } fn deserialize(buf: &Self::Serialization) -> Result { diff --git a/frost-ed448/tests/helpers/samples.rs b/frost-ed448/tests/helpers/samples.rs index 435196f4..10d077f2 100644 --- a/frost-ed448/tests/helpers/samples.rs +++ b/frost-ed448/tests/helpers/samples.rs @@ -44,8 +44,8 @@ pub fn signing_nonces() -> SigningNonces { /// Generate a sample SigningCommitments. pub fn signing_commitments() -> SigningCommitments { - let serialized_element1 = ::Group::serialize(&element1()); - let serialized_element2 = ::Group::serialize(&element2()); + let serialized_element1 = ::Group::serialize(&element1()).unwrap(); + let serialized_element2 = ::Group::serialize(&element2()).unwrap(); let hiding_nonce_commitment = NonceCommitment::deserialize(serialized_element1).unwrap(); let binding_nonce_commitment = NonceCommitment::deserialize(serialized_element2).unwrap(); @@ -72,7 +72,7 @@ pub fn signature_share() -> SignatureShare { pub fn secret_share() -> SecretShare { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let vss_commitment = VerifiableSecretSharingCommitment::deserialize(vec![serialized_element]).unwrap(); @@ -84,10 +84,10 @@ pub fn secret_share() -> SecretShare { pub fn key_package() -> KeyPackage { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) @@ -96,9 +96,9 @@ pub fn key_package() -> KeyPackage { /// Generate a sample PublicKeyPackage. pub fn public_key_package() -> PublicKeyPackage { let identifier = 42u16.try_into().unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); let verifying_shares = BTreeMap::from([(identifier, verifying_share)]); @@ -108,7 +108,7 @@ pub fn public_key_package() -> PublicKeyPackage { /// Generate a sample round1::Package. pub fn round1_package() -> round1::Package { let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let serialized_signature = serialized_element .as_ref() .iter() diff --git a/frost-p256/src/lib.rs b/frost-p256/src/lib.rs index 90f616d8..cfbf880d 100644 --- a/frost-p256/src/lib.rs +++ b/frost-p256/src/lib.rs @@ -112,21 +112,15 @@ impl Group for P256Group { ProjectivePoint::GENERATOR } - fn serialize(element: &Self::Element) -> Self::Serialization { + fn serialize(element: &Self::Element) -> Result { + if *element == Self::identity() { + return Err(GroupError::InvalidIdentityElement); + } let mut fixed_serialized = [0; 33]; let serialized_point = element.to_encoded_point(true); let serialized = serialized_point.as_bytes(); - // Sanity check; either it takes all bytes or a single byte (identity). - assert!(serialized.len() == fixed_serialized.len() || serialized.len() == 1); - // Copy to the left of the buffer (i.e. pad the identity with zeroes). - // Note that identity elements shouldn't be serialized in FROST, but we - // do this padding so that this function doesn't have to return an error. - // If this encodes the identity, it will fail when deserializing. - { - let (left, _right) = fixed_serialized.split_at_mut(serialized.len()); - left.copy_from_slice(serialized); - } - fixed_serialized + fixed_serialized.copy_from_slice(serialized); + Ok(fixed_serialized) } fn deserialize(buf: &Self::Serialization) -> Result { diff --git a/frost-p256/src/tests/deserialize.rs b/frost-p256/src/tests/deserialize.rs index 38aa9e65..ab7d8bf2 100644 --- a/frost-p256/src/tests/deserialize.rs +++ b/frost-p256/src/tests/deserialize.rs @@ -4,7 +4,8 @@ use crate::*; fn check_deserialize_non_canonical() { let mut encoded_generator = ::Group::serialize( &::Group::generator(), - ); + ) + .unwrap(); let r = ::Group::deserialize(&encoded_generator); assert!(r.is_ok()); diff --git a/frost-p256/tests/helpers/samples.rs b/frost-p256/tests/helpers/samples.rs index 0158a8bc..8e5cc484 100644 --- a/frost-p256/tests/helpers/samples.rs +++ b/frost-p256/tests/helpers/samples.rs @@ -44,8 +44,8 @@ pub fn signing_nonces() -> SigningNonces { /// Generate a sample SigningCommitments. pub fn signing_commitments() -> SigningCommitments { - let serialized_element1 = ::Group::serialize(&element1()); - let serialized_element2 = ::Group::serialize(&element2()); + let serialized_element1 = ::Group::serialize(&element1()).unwrap(); + let serialized_element2 = ::Group::serialize(&element2()).unwrap(); let hiding_nonce_commitment = NonceCommitment::deserialize(serialized_element1).unwrap(); let binding_nonce_commitment = NonceCommitment::deserialize(serialized_element2).unwrap(); @@ -72,7 +72,7 @@ pub fn signature_share() -> SignatureShare { pub fn secret_share() -> SecretShare { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let vss_commitment = VerifiableSecretSharingCommitment::deserialize(vec![serialized_element]).unwrap(); @@ -84,10 +84,10 @@ pub fn secret_share() -> SecretShare { pub fn key_package() -> KeyPackage { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) @@ -96,9 +96,9 @@ pub fn key_package() -> KeyPackage { /// Generate a sample PublicKeyPackage. pub fn public_key_package() -> PublicKeyPackage { let identifier = 42u16.try_into().unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); let verifying_shares = BTreeMap::from([(identifier, verifying_share)]); @@ -108,7 +108,7 @@ pub fn public_key_package() -> PublicKeyPackage { /// Generate a sample round1::Package. pub fn round1_package() -> round1::Package { let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let serialized_signature = serialized_element .as_ref() .iter() diff --git a/frost-rerandomized/src/lib.rs b/frost-rerandomized/src/lib.rs index 97e87590..72d4f7de 100644 --- a/frost-rerandomized/src/lib.rs +++ b/frost-rerandomized/src/lib.rs @@ -326,7 +326,9 @@ where .field("randomizer", &self.randomizer) .field( "randomizer_element", - &hex::encode(::serialize(&self.randomizer_element).as_ref()), + &::serialize(&self.randomizer_element) + .map(hex::encode) + .unwrap_or("".to_string()), ) .field("randomized_verifying_key", &self.randomized_verifying_key) .finish() diff --git a/frost-ristretto255/src/lib.rs b/frost-ristretto255/src/lib.rs index eabb403d..d402c917 100644 --- a/frost-ristretto255/src/lib.rs +++ b/frost-ristretto255/src/lib.rs @@ -96,8 +96,11 @@ impl Group for RistrettoGroup { RISTRETTO_BASEPOINT_POINT } - fn serialize(element: &Self::Element) -> Self::Serialization { - element.compress().to_bytes() + fn serialize(element: &Self::Element) -> Result { + if *element == Self::identity() { + return Err(GroupError::InvalidIdentityElement); + } + Ok(element.compress().to_bytes()) } fn deserialize(buf: &Self::Serialization) -> Result { diff --git a/frost-ristretto255/tests/helpers/samples.rs b/frost-ristretto255/tests/helpers/samples.rs index ee1bfbc4..97651126 100644 --- a/frost-ristretto255/tests/helpers/samples.rs +++ b/frost-ristretto255/tests/helpers/samples.rs @@ -44,8 +44,8 @@ pub fn signing_nonces() -> SigningNonces { /// Generate a sample SigningCommitments. pub fn signing_commitments() -> SigningCommitments { - let serialized_element1 = ::Group::serialize(&element1()); - let serialized_element2 = ::Group::serialize(&element2()); + let serialized_element1 = ::Group::serialize(&element1()).unwrap(); + let serialized_element2 = ::Group::serialize(&element2()).unwrap(); let hiding_nonce_commitment = NonceCommitment::deserialize(serialized_element1).unwrap(); let binding_nonce_commitment = NonceCommitment::deserialize(serialized_element2).unwrap(); @@ -72,7 +72,7 @@ pub fn signature_share() -> SignatureShare { pub fn secret_share() -> SecretShare { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let vss_commitment = VerifiableSecretSharingCommitment::deserialize(vec![serialized_element]).unwrap(); @@ -84,10 +84,10 @@ pub fn secret_share() -> SecretShare { pub fn key_package() -> KeyPackage { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) @@ -96,9 +96,9 @@ pub fn key_package() -> KeyPackage { /// Generate a sample PublicKeyPackage. pub fn public_key_package() -> PublicKeyPackage { let identifier = 42u16.try_into().unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); let verifying_shares = BTreeMap::from([(identifier, verifying_share)]); @@ -108,7 +108,7 @@ pub fn public_key_package() -> PublicKeyPackage { /// Generate a sample round1::Package. pub fn round1_package() -> round1::Package { let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let serialized_signature = serialized_element .as_ref() .iter() diff --git a/frost-secp256k1/src/lib.rs b/frost-secp256k1/src/lib.rs index 25501bf7..3ddd5ecf 100644 --- a/frost-secp256k1/src/lib.rs +++ b/frost-secp256k1/src/lib.rs @@ -112,21 +112,15 @@ impl Group for Secp256K1Group { ProjectivePoint::GENERATOR } - fn serialize(element: &Self::Element) -> Self::Serialization { + fn serialize(element: &Self::Element) -> Result { + if *element == Self::identity() { + return Err(GroupError::InvalidIdentityElement); + } let mut fixed_serialized = [0; 33]; let serialized_point = element.to_affine().to_encoded_point(true); let serialized = serialized_point.as_bytes(); - // Sanity check; either it takes all bytes or a single byte (identity). - assert!(serialized.len() == fixed_serialized.len() || serialized.len() == 1); - // Copy to the left of the buffer (i.e. pad the identity with zeroes). - // Note that identity elements shouldn't be serialized in FROST, but we - // do this padding so that this function doesn't have to return an error. - // If this encodes the identity, it will fail when deserializing. - { - let (left, _right) = fixed_serialized.split_at_mut(serialized.len()); - left.copy_from_slice(serialized); - } - fixed_serialized + fixed_serialized.copy_from_slice(serialized); + Ok(fixed_serialized) } fn deserialize(buf: &Self::Serialization) -> Result { diff --git a/frost-secp256k1/src/tests/deserialize.rs b/frost-secp256k1/src/tests/deserialize.rs index bce82001..a744832b 100644 --- a/frost-secp256k1/src/tests/deserialize.rs +++ b/frost-secp256k1/src/tests/deserialize.rs @@ -4,7 +4,8 @@ use crate::*; fn check_deserialize_non_canonical() { let mut encoded_generator = ::Group::serialize( &::Group::generator(), - ); + ) + .unwrap(); let r = ::Group::deserialize(&encoded_generator); assert!(r.is_ok()); diff --git a/frost-secp256k1/tests/helpers/samples.rs b/frost-secp256k1/tests/helpers/samples.rs index e8446fe7..ff22aec8 100644 --- a/frost-secp256k1/tests/helpers/samples.rs +++ b/frost-secp256k1/tests/helpers/samples.rs @@ -44,8 +44,8 @@ pub fn signing_nonces() -> SigningNonces { /// Generate a sample SigningCommitments. pub fn signing_commitments() -> SigningCommitments { - let serialized_element1 = ::Group::serialize(&element1()); - let serialized_element2 = ::Group::serialize(&element2()); + let serialized_element1 = ::Group::serialize(&element1()).unwrap(); + let serialized_element2 = ::Group::serialize(&element2()).unwrap(); let hiding_nonce_commitment = NonceCommitment::deserialize(serialized_element1).unwrap(); let binding_nonce_commitment = NonceCommitment::deserialize(serialized_element2).unwrap(); @@ -72,7 +72,7 @@ pub fn signature_share() -> SignatureShare { pub fn secret_share() -> SecretShare { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let vss_commitment = VerifiableSecretSharingCommitment::deserialize(vec![serialized_element]).unwrap(); @@ -84,10 +84,10 @@ pub fn secret_share() -> SecretShare { pub fn key_package() -> KeyPackage { let identifier = 42u16.try_into().unwrap(); let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let signing_share = SigningShare::deserialize(serialized_scalar).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) @@ -96,9 +96,9 @@ pub fn key_package() -> KeyPackage { /// Generate a sample PublicKeyPackage. pub fn public_key_package() -> PublicKeyPackage { let identifier = 42u16.try_into().unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_share = VerifyingShare::deserialize(serialized_element).unwrap(); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); let verifying_shares = BTreeMap::from([(identifier, verifying_share)]); @@ -108,7 +108,7 @@ pub fn public_key_package() -> PublicKeyPackage { /// Generate a sample round1::Package. pub fn round1_package() -> round1::Package { let serialized_scalar = <::Group as Group>::Field::serialize(&scalar1()); - let serialized_element = ::Group::serialize(&element1()); + let serialized_element = ::Group::serialize(&element1()).unwrap(); let serialized_signature = serialized_element .as_ref() .iter()