From 4ee0d32867c9e281dfdb9fc99b2f537bdf68371a Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Wed, 6 Sep 2023 06:48:30 -0300 Subject: [PATCH] check number of commitments in sign() (#480) * check number of commitments in sign() * make comment clearer --- frost-core/CHANGELOG.md | 2 + frost-core/src/error.rs | 5 +- frost-core/src/frost/keys.rs | 4 + frost-core/src/frost/keys/dkg.rs | 9 ++ frost-core/src/frost/round2.rs | 4 + frost-core/src/tests/ciphersuite_generic.rs | 95 +++++++++++++++----- frost-core/src/tests/vectors.rs | 12 ++- frost-ed25519/tests/helpers/samples.rs | 2 +- frost-ed25519/tests/recreation_tests.rs | 2 + frost-ed25519/tests/serde_tests.rs | 1 + frost-ed448/tests/helpers/samples.rs | 2 +- frost-ed448/tests/recreation_tests.rs | 2 + frost-ed448/tests/serde_tests.rs | 1 + frost-p256/tests/helpers/samples.rs | 2 +- frost-p256/tests/recreation_tests.rs | 2 + frost-p256/tests/serde_tests.rs | 1 + frost-rerandomized/src/lib.rs | 1 + frost-ristretto255/tests/helpers/samples.rs | 2 +- frost-ristretto255/tests/recreation_tests.rs | 2 + frost-ristretto255/tests/serde_tests.rs | 1 + frost-secp256k1/tests/helpers/samples.rs | 2 +- frost-secp256k1/tests/recreation_tests.rs | 2 + frost-secp256k1/tests/serde_tests.rs | 1 + 23 files changed, 124 insertions(+), 33 deletions(-) diff --git a/frost-core/CHANGELOG.md b/frost-core/CHANGELOG.md index 76c0d6f..099a3a9 100644 --- a/frost-core/CHANGELOG.md +++ b/frost-core/CHANGELOG.md @@ -9,6 +9,8 @@ Entries are listed in reverse chronological order. * Challenge hashing during DKG computation was changed to match the paper. This means that code running this version won't interoperate with code running previous versions. +* A new `min_signers` field was added to `KeyPackage`, which changes its + `new()` method and its serde serialization. ## Released diff --git a/frost-core/src/error.rs b/frost-core/src/error.rs index f1eb98b..86465ec 100644 --- a/frost-core/src/error.rs +++ b/frost-core/src/error.rs @@ -56,10 +56,12 @@ pub enum Error { /// The participant's commitment is missing from the Signing Package #[error("The Signing Package must contain the participant's Commitment.")] MissingCommitment, - /// The participant's commitment is incorrect #[error("The participant's commitment is incorrect.")] IncorrectCommitment, + /// Incorrect number of commitments. + #[error("Incorrect number of commitments.")] + IncorrectNumberOfCommitments, /// Signature share verification failed. #[error("Invalid signature share.")] InvalidSignatureShare { @@ -144,6 +146,7 @@ where | Error::InvalidCoefficient | Error::UnknownIdentifier | Error::IncorrectNumberOfIdentifiers + | Error::IncorrectNumberOfCommitments | Error::IdentifierDerivationNotSupported => None, } } diff --git a/frost-core/src/frost/keys.rs b/frost-core/src/frost/keys.rs index 1322484..2b7e624 100644 --- a/frost-core/src/frost/keys.rs +++ b/frost-core/src/frost/keys.rs @@ -576,6 +576,7 @@ pub struct KeyPackage { /// The public verifying key that represents the entire group. #[zeroize(skip)] pub(crate) group_public: VerifyingKey, + pub(crate) min_signers: u16, /// Ciphersuite ID for serialization #[cfg_attr( feature = "serde", @@ -599,12 +600,14 @@ where secret_share: SigningShare, public: VerifyingShare, group_public: VerifyingKey, + min_signers: u16, ) -> Self { Self { identifier, secret_share, public, group_public, + min_signers, ciphersuite: (), } } @@ -632,6 +635,7 @@ where secret_share: secret_share.value, public, group_public, + min_signers: secret_share.commitment.0.len() as u16, ciphersuite: (), }) } diff --git a/frost-core/src/frost/keys/dkg.rs b/frost-core/src/frost/keys/dkg.rs index 5648fef..5f98c9f 100644 --- a/frost-core/src/frost/keys/dkg.rs +++ b/frost-core/src/frost/keys/dkg.rs @@ -107,6 +107,8 @@ pub mod round1 { pub(crate) coefficients: Vec>, /// The public commitment for the participant (C_i) pub(crate) commitment: VerifiableSecretSharingCommitment, + /// The minimum number of signers. + pub(crate) min_signers: u16, /// The total number of signers. pub(crate) max_signers: u16, } @@ -120,6 +122,7 @@ pub mod round1 { .field("identifier", &self.identifier) .field("coefficients", &"") .field("commitment", &self.commitment) + .field("min_signers", &self.min_signers) .field("max_signers", &self.max_signers) .finish() } @@ -197,6 +200,8 @@ pub mod round2 { pub(crate) commitment: VerifiableSecretSharingCommitment, /// The participant's own secret share (f_i(i)). pub(crate) secret_share: Scalar, + /// The minimum number of signers. + pub(crate) min_signers: u16, /// The total number of signers. pub(crate) max_signers: u16, } @@ -210,6 +215,7 @@ pub mod round2 { .field("identifier", &self.identifier) .field("commitment", &self.commitment) .field("secret_share", &"") + .field("min_signers", &self.min_signers) .field("max_signers", &self.max_signers) .finish() } @@ -273,6 +279,7 @@ pub fn part1( identifier, coefficients, commitment: commitment.clone(), + min_signers, max_signers, }; let package = round1::Package { @@ -368,6 +375,7 @@ pub fn part2( identifier: secret_package.identifier, commitment: secret_package.commitment, secret_share: fii, + min_signers: secret_package.min_signers, max_signers: secret_package.max_signers, }, round2_packages, @@ -518,6 +526,7 @@ pub fn part3( secret_share: signing_share, public: verifying_key, group_public, + min_signers: round2_secret_package.min_signers, ciphersuite: (), }; let public_key_package = PublicKeyPackage { diff --git a/frost-core/src/frost/round2.rs b/frost-core/src/frost/round2.rs index 55d72b2..9a42d72 100644 --- a/frost-core/src/frost/round2.rs +++ b/frost-core/src/frost/round2.rs @@ -189,6 +189,10 @@ pub fn sign( signer_nonces: &round1::SigningNonces, key_package: &frost::keys::KeyPackage, ) -> Result, Error> { + if signing_package.signing_commitments().len() < key_package.min_signers as usize { + return Err(Error::IncorrectNumberOfCommitments); + } + // Validate the signer's commitment is present in the signing package let commitment = signing_package .signing_commitments diff --git a/frost-core/src/tests/ciphersuite_generic.rs b/frost-core/src/tests/ciphersuite_generic.rs index 9ec1f20..bb1f5b7 100644 --- a/frost-core/src/tests/ciphersuite_generic.rs +++ b/frost-core/src/tests/ciphersuite_generic.rs @@ -1,4 +1,6 @@ //! Ciphersuite-generic test functions. +#![allow(clippy::type_complexity)] + use std::{ collections::{BTreeMap, HashMap}, convert::TryFrom, @@ -117,8 +119,31 @@ pub fn check_sign_with_dealer( let key_package = frost::keys::KeyPackage::try_from(v).unwrap(); key_packages.insert(k, key_package); } + // Check if it fails with not enough signers. Usually this would return an + // error before even running the signing procedure, because `KeyPackage` + // contains the correct `min_signers` value and the signing procedure checks + // if the number of shares is at least `min_signers`. To bypass the check + // and test if the protocol itself fails with not enough signers, we modify + // the `KeyPackages`s, decrementing their saved `min_signers` value before + // running the signing procedure. + let r = check_sign( + min_signers - 1, + key_packages + .iter() + .map(|(id, k)| { + // Decrement `min_signers` as explained above and use + // the updated `KeyPackage`. + let mut k = k.clone(); + k.min_signers -= 1; + (*id, k) + }) + .collect(), + &mut rng, + pubkeys.clone(), + ); + assert_eq!(r, Err(Error::InvalidSignature)); - check_sign(min_signers, key_packages, rng, pubkeys) + check_sign(min_signers, key_packages, rng, pubkeys).unwrap() } /// Test FROST signing with trusted dealer fails with invalid numbers of signers. @@ -163,7 +188,7 @@ pub fn check_sign( key_packages: HashMap, frost::keys::KeyPackage>, mut rng: R, pubkey_package: frost::keys::PublicKeyPackage, -) -> (Vec, Signature, VerifyingKey) { +) -> Result<(Vec, Signature, VerifyingKey), Error> { let mut nonces_map: HashMap, frost::round1::SigningNonces> = HashMap::new(); let mut commitments_map: BTreeMap, frost::round1::SigningCommitments> = @@ -201,11 +226,16 @@ pub fn check_sign( for participant_identifier in nonces_map.keys() { let key_package = key_packages.get(participant_identifier).unwrap(); - let nonces_to_use = &nonces_map.get(participant_identifier).unwrap(); + let nonces_to_use = nonces_map.get(participant_identifier).unwrap(); + + check_sign_errors( + signing_package.clone(), + nonces_to_use.clone(), + key_package.clone(), + ); // Each participant generates their signature share. - let signature_share = - frost::round2::sign(&signing_package, nonces_to_use, key_package).unwrap(); + let signature_share = frost::round2::sign(&signing_package, nonces_to_use, key_package)?; signature_shares.insert(*participant_identifier, signature_share); } @@ -221,33 +251,47 @@ pub fn check_sign( ); // Aggregate (also verifies the signature shares) - let group_signature = - frost::aggregate(&signing_package, &signature_shares, &pubkey_package).unwrap(); + let group_signature = frost::aggregate(&signing_package, &signature_shares, &pubkey_package)?; // Check that the threshold signature can be verified by the group public // key (the verification key). - let is_signature_valid = pubkey_package + pubkey_package .group_public - .verify(message, &group_signature) - .is_ok(); - assert!(is_signature_valid); + .verify(message, &group_signature)?; // Check that the threshold signature can be verified by the group public // key (the verification key) from KeyPackage.group_public for (participant_identifier, _) in nonces_map.clone() { let key_package = key_packages.get(&participant_identifier).unwrap(); - assert!(key_package - .group_public - .verify(message, &group_signature) - .is_ok()); + key_package.group_public.verify(message, &group_signature)?; } - ( + Ok(( message.to_owned(), group_signature, pubkey_package.group_public, - ) + )) +} + +fn check_sign_errors( + signing_package: frost::SigningPackage, + signing_nonces: frost::round1::SigningNonces, + key_package: frost::keys::KeyPackage, +) { + // Check if passing not enough commitments causes an error + + let mut commitments = signing_package.signing_commitments().clone(); + // Remove one commitment that's not from the key_package owner + let id = *commitments + .keys() + .find(|&&id| id != key_package.identifier) + .unwrap(); + commitments.remove(&id); + let signing_package = frost::SigningPackage::new(commitments, signing_package.message()); + + let r = frost::round2::sign(&signing_package, &signing_nonces, &key_package); + assert_eq!(r, Err(Error::IncorrectNumberOfCommitments)); } fn check_aggregate_errors( @@ -461,7 +505,7 @@ where let pubkeys = frost::keys::PublicKeyPackage::new(verifying_keys, group_public.unwrap()); // Proceed with the signing test. - check_sign(min_signers, key_packages, rng, pubkeys) + check_sign(min_signers, key_packages, rng, pubkeys).unwrap() } /// Check that calling dkg::part3() with distinct sets of participants fail. @@ -571,7 +615,7 @@ pub fn check_sign_with_dealer_and_identifiers( @@ -653,6 +697,7 @@ pub fn check_sign_with_missing_identifier::try_from(1).unwrap(); let id_2 = Identifier::::try_from(2).unwrap(); let id_3 = Identifier::::try_from(3).unwrap(); + let id_4 = Identifier::::try_from(4).unwrap(); let key_packages_inc = vec![id_1, id_2, id_3]; for participant_identifier in key_packages_inc { @@ -666,11 +711,14 @@ pub fn check_sign_with_missing_identifier(json_vectors: &Value) -> TestVectors = inputs["share_polynomial_coefficients"] .as_array() .unwrap() .iter() @@ -67,8 +67,14 @@ pub fn parse_test_vectors(json_vectors: &Value) -> TestVectors::new(i.try_into().unwrap(), secret, signer_public, group_public); + let min_signers = share_polynomial_coefficients.len() + 1; + let key_package = KeyPackage::::new( + i.try_into().unwrap(), + secret, + signer_public, + group_public, + min_signers as u16, + ); key_packages.insert(*key_package.identifier(), key_package); } diff --git a/frost-ed25519/tests/helpers/samples.rs b/frost-ed25519/tests/helpers/samples.rs index eb3e86f..cb021dc 100644 --- a/frost-ed25519/tests/helpers/samples.rs +++ b/frost-ed25519/tests/helpers/samples.rs @@ -80,7 +80,7 @@ pub fn key_package() -> KeyPackage { let serialized_element = ::Group::serialize(&element1()); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); - KeyPackage::new(identifier, signing_share, verifying_share, verifying_key) + KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) } /// Generate a sample PublicKeyPackage. diff --git a/frost-ed25519/tests/recreation_tests.rs b/frost-ed25519/tests/recreation_tests.rs index a6dfc0a..a76d5f0 100644 --- a/frost-ed25519/tests/recreation_tests.rs +++ b/frost-ed25519/tests/recreation_tests.rs @@ -71,12 +71,14 @@ fn check_key_package_recreation() { let signing_share = key_package.secret_share(); let verifying_share = key_package.public(); let verifying_key = key_package.group_public(); + let min_signers = key_package.min_signers(); let new_key_package = KeyPackage::new( *identifier, *signing_share, *verifying_share, *verifying_key, + *min_signers, ); assert!(key_package == new_key_package); diff --git a/frost-ed25519/tests/serde_tests.rs b/frost-ed25519/tests/serde_tests.rs index f29f1ad..a9576f4 100644 --- a/frost-ed25519/tests/serde_tests.rs +++ b/frost-ed25519/tests/serde_tests.rs @@ -284,6 +284,7 @@ fn check_key_package_serialization() { "secret_share": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a", "public": "5866666666666666666666666666666666666666666666666666666666666666", "group_public": "5866666666666666666666666666666666666666666666666666666666666666", + "min_signers": 2, "ciphersuite": "FROST(Ed25519, SHA-512)" }"#; let decoded_key_package: KeyPackage = serde_json::from_str(json).unwrap(); diff --git a/frost-ed448/tests/helpers/samples.rs b/frost-ed448/tests/helpers/samples.rs index 679a5d9..f6d523d 100644 --- a/frost-ed448/tests/helpers/samples.rs +++ b/frost-ed448/tests/helpers/samples.rs @@ -80,7 +80,7 @@ pub fn key_package() -> KeyPackage { let serialized_element = ::Group::serialize(&element1()); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); - KeyPackage::new(identifier, signing_share, verifying_share, verifying_key) + KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) } /// Generate a sample PublicKeyPackage. diff --git a/frost-ed448/tests/recreation_tests.rs b/frost-ed448/tests/recreation_tests.rs index c3a07e1..a901e83 100644 --- a/frost-ed448/tests/recreation_tests.rs +++ b/frost-ed448/tests/recreation_tests.rs @@ -71,12 +71,14 @@ fn check_key_package_recreation() { let signing_share = key_package.secret_share(); let verifying_share = key_package.public(); let verifying_key = key_package.group_public(); + let min_signers = key_package.min_signers(); let new_key_package = KeyPackage::new( *identifier, *signing_share, *verifying_share, *verifying_key, + *min_signers, ); assert!(key_package == new_key_package); diff --git a/frost-ed448/tests/serde_tests.rs b/frost-ed448/tests/serde_tests.rs index 1ba91f7..cbfe800 100644 --- a/frost-ed448/tests/serde_tests.rs +++ b/frost-ed448/tests/serde_tests.rs @@ -284,6 +284,7 @@ fn check_key_package_serialization() { "secret_share": "4d83e51cb78150c2380ad9b3a18148166024e4c9db3cdf82466d3153aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa2a00", "public": "14fa30f25b790898adc8d74e2c13bdfdc4397ce61cffd33ad7c2a0051e9c78874098a36c7373ea4b62c7c9563720768824bcb66e71463f6900", "group_public": "14fa30f25b790898adc8d74e2c13bdfdc4397ce61cffd33ad7c2a0051e9c78874098a36c7373ea4b62c7c9563720768824bcb66e71463f6900", + "min_signers": 2, "ciphersuite": "FROST(Ed448, SHAKE256)" }"#; let decoded_key_package: KeyPackage = serde_json::from_str(json).unwrap(); diff --git a/frost-p256/tests/helpers/samples.rs b/frost-p256/tests/helpers/samples.rs index eec57a8..9c2edb1 100644 --- a/frost-p256/tests/helpers/samples.rs +++ b/frost-p256/tests/helpers/samples.rs @@ -80,7 +80,7 @@ pub fn key_package() -> KeyPackage { let serialized_element = ::Group::serialize(&element1()); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); - KeyPackage::new(identifier, signing_share, verifying_share, verifying_key) + KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) } /// Generate a sample PublicKeyPackage. diff --git a/frost-p256/tests/recreation_tests.rs b/frost-p256/tests/recreation_tests.rs index 5f6a0e8..9b8a912 100644 --- a/frost-p256/tests/recreation_tests.rs +++ b/frost-p256/tests/recreation_tests.rs @@ -71,12 +71,14 @@ fn check_key_package_recreation() { let signing_share = key_package.secret_share(); let verifying_share = key_package.public(); let verifying_key = key_package.group_public(); + let min_signers = key_package.min_signers(); let new_key_package = KeyPackage::new( *identifier, *signing_share, *verifying_share, *verifying_key, + *min_signers, ); assert!(key_package == new_key_package); diff --git a/frost-p256/tests/serde_tests.rs b/frost-p256/tests/serde_tests.rs index 5a791cf..7d977ca 100644 --- a/frost-p256/tests/serde_tests.rs +++ b/frost-p256/tests/serde_tests.rs @@ -284,6 +284,7 @@ fn check_key_package_serialization() { "secret_share": "aaaaaaaa00000000aaaaaaaaaaaaaaaa7def51c91a0fbf034d26872ca84218e1", "public": "036b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296", "group_public": "036b17d1f2e12c4247f8bce6e563a440f277037d812deb33a0f4a13945d898c296", + "min_signers": 2, "ciphersuite": "FROST(P-256, SHA-256)" }"#; let decoded_key_package: KeyPackage = serde_json::from_str(json).unwrap(); diff --git a/frost-rerandomized/src/lib.rs b/frost-rerandomized/src/lib.rs index 718b3c9..38c4bce 100644 --- a/frost-rerandomized/src/lib.rs +++ b/frost-rerandomized/src/lib.rs @@ -72,6 +72,7 @@ impl Randomize for KeyPackage { randomized_signing_share, randomized_verifying_share, randomized_params.randomized_verifying_key, + *self.min_signers(), ); Ok(randomized_key_package) } diff --git a/frost-ristretto255/tests/helpers/samples.rs b/frost-ristretto255/tests/helpers/samples.rs index 3f07402..ca4143a 100644 --- a/frost-ristretto255/tests/helpers/samples.rs +++ b/frost-ristretto255/tests/helpers/samples.rs @@ -80,7 +80,7 @@ pub fn key_package() -> KeyPackage { let serialized_element = ::Group::serialize(&element1()); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); - KeyPackage::new(identifier, signing_share, verifying_share, verifying_key) + KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) } /// Generate a sample PublicKeyPackage. diff --git a/frost-ristretto255/tests/recreation_tests.rs b/frost-ristretto255/tests/recreation_tests.rs index 5103e6f..5e03a7d 100644 --- a/frost-ristretto255/tests/recreation_tests.rs +++ b/frost-ristretto255/tests/recreation_tests.rs @@ -71,12 +71,14 @@ fn check_key_package_recreation() { let signing_share = key_package.secret_share(); let verifying_share = key_package.public(); let verifying_key = key_package.group_public(); + let min_signers = key_package.min_signers(); let new_key_package = KeyPackage::new( *identifier, *signing_share, *verifying_share, *verifying_key, + *min_signers, ); assert!(key_package == new_key_package); diff --git a/frost-ristretto255/tests/serde_tests.rs b/frost-ristretto255/tests/serde_tests.rs index d22da94..afaff78 100644 --- a/frost-ristretto255/tests/serde_tests.rs +++ b/frost-ristretto255/tests/serde_tests.rs @@ -284,6 +284,7 @@ fn check_key_package_serialization() { "secret_share": "498d4e9311420c903913a56c94a694b8aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0a", "public": "e2f2ae0a6abc4e71a884a961c500515f58e30b6aa582dd8db6a65945e08d2d76", "group_public": "e2f2ae0a6abc4e71a884a961c500515f58e30b6aa582dd8db6a65945e08d2d76", + "min_signers": 2, "ciphersuite": "FROST(ristretto255, SHA-512)" }"#; let decoded_key_package: KeyPackage = serde_json::from_str(json).unwrap(); diff --git a/frost-secp256k1/tests/helpers/samples.rs b/frost-secp256k1/tests/helpers/samples.rs index f8f960c..67e2cdb 100644 --- a/frost-secp256k1/tests/helpers/samples.rs +++ b/frost-secp256k1/tests/helpers/samples.rs @@ -80,7 +80,7 @@ pub fn key_package() -> KeyPackage { let serialized_element = ::Group::serialize(&element1()); let verifying_key = VerifyingKey::deserialize(serialized_element).unwrap(); - KeyPackage::new(identifier, signing_share, verifying_share, verifying_key) + KeyPackage::new(identifier, signing_share, verifying_share, verifying_key, 2) } /// Generate a sample PublicKeyPackage. diff --git a/frost-secp256k1/tests/recreation_tests.rs b/frost-secp256k1/tests/recreation_tests.rs index c70c978..6b390ac 100644 --- a/frost-secp256k1/tests/recreation_tests.rs +++ b/frost-secp256k1/tests/recreation_tests.rs @@ -71,12 +71,14 @@ fn check_key_package_recreation() { let signing_share = key_package.secret_share(); let verifying_share = key_package.public(); let verifying_key = key_package.group_public(); + let min_signers = key_package.min_signers(); let new_key_package = KeyPackage::new( *identifier, *signing_share, *verifying_share, *verifying_key, + *min_signers, ); assert!(key_package == new_key_package); diff --git a/frost-secp256k1/tests/serde_tests.rs b/frost-secp256k1/tests/serde_tests.rs index 7fa8237..0b8ba92 100644 --- a/frost-secp256k1/tests/serde_tests.rs +++ b/frost-secp256k1/tests/serde_tests.rs @@ -284,6 +284,7 @@ fn check_key_package_serialization() { "secret_share": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa9d1c9e899ca306ad27fe1945de0242b81", "public": "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", "group_public": "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + "min_signers": 2, "ciphersuite": "FROST(secp256k1, SHA-256)" }"#; let decoded_key_package: KeyPackage = serde_json::from_str(json).unwrap();