diff --git a/frost-core/src/frost/round1.rs b/frost-core/src/frost/round1.rs index 86c336d6..969d57d1 100644 --- a/frost-core/src/frost/round1.rs +++ b/frost-core/src/frost/round1.rs @@ -207,6 +207,12 @@ pub struct SigningNonces { pub(crate) hiding: Nonce, /// The binding [`Nonce`]. pub(crate) binding: Nonce, + /// The commitments to the nonces. This is precomputed to improve + /// sign() performance, since it needs to check if the commitments + /// to the participant's nonces are included in the commitments sent + /// by the Coordinator, and this prevents having to recompute them. + #[zeroize(skip)] + pub(crate) commitments: SigningCommitments, } impl SigningNonces @@ -221,12 +227,26 @@ where where R: CryptoRng + RngCore, { - // The values of 'hiding' and 'binding' must be non-zero so that commitments are - // not the identity. let hiding = Nonce::::new(secret, rng); let binding = Nonce::::new(secret, rng); - Self { hiding, binding } + Self::from_nonces(hiding, binding) + } + + /// Generates a new [`SigningNonces`] from a pair of [`Nonce`]. This is + /// useful internally since [`SigningNonces`] precompute the respective + /// commitments. + #[cfg_attr(test, visibility::make(pub))] + pub(crate) fn from_nonces(hiding: Nonce, binding: Nonce) -> Self { + let hiding_commitment = (&hiding).into(); + let binding_commitment = (&binding).into(); + let commitments = SigningCommitments::new(hiding_commitment, binding_commitment); + + Self { + hiding, + binding, + commitments, + } } /// Gets the hiding [`Nonce`] @@ -295,11 +315,7 @@ where C: Ciphersuite, { fn from(nonces: &SigningNonces) -> Self { - Self { - hiding: nonces.hiding.clone().into(), - binding: nonces.binding.clone().into(), - ciphersuite: (), - } + nonces.commitments } } diff --git a/frost-core/src/frost/round2.rs b/frost-core/src/frost/round2.rs index 2e38f301..22724773 100644 --- a/frost-core/src/frost/round2.rs +++ b/frost-core/src/frost/round2.rs @@ -11,8 +11,6 @@ use crate::{ #[cfg(feature = "serde")] use crate::ScalarSerialization; -use super::round1::SigningCommitments; - // Used to help encoding a SignatureShare. Since it has a Scalar it can't // be directly encoded with serde, so we use this struct to wrap the scalar. #[cfg(feature = "serde")] @@ -195,10 +193,8 @@ pub fn sign( .get(&key_package.identifier) .ok_or(Error::MissingCommitment)?; - let signing_commitments = SigningCommitments::from(signer_nonces); - // Validate if the signer's commitment exists - if &signing_commitments != commitment { + if &signer_nonces.commitments != commitment { return Err(Error::IncorrectCommitment); } diff --git a/frost-core/src/signing_key.rs b/frost-core/src/signing_key.rs index 04323e1f..a9b7517c 100644 --- a/frost-core/src/signing_key.rs +++ b/frost-core/src/signing_key.rs @@ -28,9 +28,14 @@ where pub fn deserialize( bytes: <::Field as Field>::Serialization, ) -> Result, Error> { - <::Field as Field>::deserialize(&bytes) - .map(|scalar| SigningKey { scalar }) - .map_err(|e| e.into()) + let scalar = + <::Field as Field>::deserialize(&bytes).map_err(Error::from)?; + + if scalar == <::Field as Field>::zero() { + return Err(Error::MalformedSigningKey); + } + + Ok(Self { scalar }) } /// Serialize `SigningKey` to bytes diff --git a/frost-core/src/tests/ciphersuite_generic.rs b/frost-core/src/tests/ciphersuite_generic.rs index 523777a4..08a7e9ef 100644 --- a/frost-core/src/tests/ciphersuite_generic.rs +++ b/frost-core/src/tests/ciphersuite_generic.rs @@ -6,12 +6,20 @@ use std::{ use crate::{ frost::{self, Identifier}, - Error, Field, Group, Signature, VerifyingKey, + Error, Field, Group, Signature, SigningKey, VerifyingKey, }; use rand_core::{CryptoRng, RngCore}; use crate::Ciphersuite; +/// Test if creating a zero SigningKey fails +pub fn check_zero_key_fails() { + let zero = <<::Group as Group>::Field>::zero(); + let encoded_zero = <<::Group as Group>::Field>::serialize(&zero); + let r = SigningKey::::deserialize(encoded_zero); + assert_eq!(r, Err(Error::MalformedSigningKey)); +} + /// Test share generation with a Ciphersuite pub fn check_share_generation(mut rng: R) { let secret = crate::SigningKey::::new(&mut rng); diff --git a/frost-core/src/tests/vectors.rs b/frost-core/src/tests/vectors.rs index 4e1edb6c..5bf0665c 100644 --- a/frost-core/src/tests/vectors.rs +++ b/frost-core/src/tests/vectors.rs @@ -96,10 +96,10 @@ pub fn parse_test_vectors(json_vectors: &Value) -> TestVectors { - hiding: Nonce::::from_hex(signer["hiding_nonce"].as_str().unwrap()).unwrap(), - binding: Nonce::::from_hex(signer["binding_nonce"].as_str().unwrap()).unwrap(), - }; + let signing_nonces = SigningNonces::::from_nonces( + Nonce::::from_hex(signer["hiding_nonce"].as_str().unwrap()).unwrap(), + Nonce::::from_hex(signer["binding_nonce"].as_str().unwrap()).unwrap(), + ); signer_nonces.insert(identifier, signing_nonces); diff --git a/frost-ed25519/tests/integration_tests.rs b/frost-ed25519/tests/integration_tests.rs index c729c75e..3dd8a2eb 100644 --- a/frost-ed25519/tests/integration_tests.rs +++ b/frost-ed25519/tests/integration_tests.rs @@ -3,6 +3,11 @@ use lazy_static::lazy_static; use rand::thread_rng; use serde_json::Value; +#[test] +fn check_zero_key_fails() { + frost_core::tests::ciphersuite_generic::check_zero_key_fails::(); +} + #[test] fn check_sign_with_dkg() { let rng = thread_rng(); diff --git a/frost-ed448/tests/integration_tests.rs b/frost-ed448/tests/integration_tests.rs index 90956ccb..323048ec 100644 --- a/frost-ed448/tests/integration_tests.rs +++ b/frost-ed448/tests/integration_tests.rs @@ -3,6 +3,11 @@ use lazy_static::lazy_static; use rand::thread_rng; use serde_json::Value; +#[test] +fn check_zero_key_fails() { + frost_core::tests::ciphersuite_generic::check_zero_key_fails::(); +} + #[test] fn check_sign_with_dkg() { let rng = thread_rng(); diff --git a/frost-p256/tests/integration_tests.rs b/frost-p256/tests/integration_tests.rs index 406181b5..317f5746 100644 --- a/frost-p256/tests/integration_tests.rs +++ b/frost-p256/tests/integration_tests.rs @@ -3,6 +3,11 @@ use lazy_static::lazy_static; use rand::thread_rng; use serde_json::Value; +#[test] +fn check_zero_key_fails() { + frost_core::tests::ciphersuite_generic::check_zero_key_fails::(); +} + #[test] fn check_sign_with_dkg() { let rng = thread_rng(); diff --git a/frost-ristretto255/tests/integration_tests.rs b/frost-ristretto255/tests/integration_tests.rs index 9d479a7d..3150ba7c 100644 --- a/frost-ristretto255/tests/integration_tests.rs +++ b/frost-ristretto255/tests/integration_tests.rs @@ -3,6 +3,11 @@ use lazy_static::lazy_static; use rand::thread_rng; use serde_json::Value; +#[test] +fn check_zero_key_fails() { + frost_core::tests::ciphersuite_generic::check_zero_key_fails::(); +} + #[test] fn check_sign_with_dkg() { let rng = thread_rng(); diff --git a/frost-secp256k1/tests/integration_tests.rs b/frost-secp256k1/tests/integration_tests.rs index f6cb05da..7607e351 100644 --- a/frost-secp256k1/tests/integration_tests.rs +++ b/frost-secp256k1/tests/integration_tests.rs @@ -3,6 +3,11 @@ use lazy_static::lazy_static; use rand::thread_rng; use serde_json::Value; +#[test] +fn check_zero_key_fails() { + frost_core::tests::ciphersuite_generic::check_zero_key_fails::(); +} + #[test] fn check_sign_with_dkg() { let rng = thread_rng();