Support optional cheater detection (#564)

* Add feature for cheater detection (#355) Create option for aggregating without cheater detection Some renaming * avoid duplicating aggregate() * Make cheater detection feature on by default (#355) * Update changelog for cheater detection feature addition (#355) * Default to cheater detection feature in ciphersuites (#355) * Remove unnecessary cheater-detection gates in tests (#355) --------- Co-authored-by: Conrado Gouvea <[email protected]>
ZcashFoundation · Oct 25, 2023 · e0db615 · e0db615
1 parent 3d25db1
commit e0db615
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 16 deletions.
diff --git a/frost-core/CHANGELOG.md b/frost-core/CHANGELOG.md
@@ -13,6 +13,7 @@ Entries are listed in reverse chronological order.
   struct, which affects self-describing formats like JSON. The ciphersuite ID
   string was also changed for all ciphersuites: it is now equal to the
   `contextString` of each ciphersuite per the FROST spec.
+* An option to disable cheater detection during aggregation of signatures has been added.
 
 ## Released
 

diff --git a/frost-core/Cargo.toml b/frost-core/Cargo.toml
@@ -50,7 +50,7 @@ rand_chacha = "0.3"
 serde_json = "1.0"
 
 [features]
-default = ["serialization"]
+default = ["serialization", "cheater-detection"]
 #! ## Features
 ## Expose internal types, which do not have SemVer guarantees. This is an advanced
 ## feature which can be useful if you need to build a modified version of FROST.
@@ -63,6 +63,8 @@ serde = ["dep:serde", "dep:serdect"]
 serialization = ["serde", "dep:postcard"]
 # Exposes ciphersuite-generic tests for other crates to use
 test-impl = ["proptest", "serde_json", "criterion"]
+# Enable cheater detection
+cheater-detection = []
 
 [lib]
 bench = false
diff --git a/frost-core/src/frost.rs b/frost-core/src/frost.rs
@@ -406,6 +406,7 @@ where
 /// signature, if the coordinator themselves is a signer and misbehaves, they
 /// can avoid that step. However, at worst, this results in a denial of
 /// service attack due to publishing an invalid signature.
+
 pub fn aggregate<C>(
     signing_package: &SigningPackage<C>,
     signature_shares: &BTreeMap<Identifier<C>, round2::SignatureShare<C>>,
@@ -419,11 +420,12 @@ where
     if signing_package.signing_commitments().len() != signature_shares.len() {
         return Err(Error::UnknownIdentifier);
     }
-    if !signing_package
-        .signing_commitments()
-        .keys()
-        .all(|id| signature_shares.contains_key(id) && pubkeys.verifying_shares().contains_key(id))
-    {
+    if !signing_package.signing_commitments().keys().all(|id| {
+        #[cfg(feature = "cheater-detection")]
+        return signature_shares.contains_key(id) && pubkeys.verifying_shares().contains_key(id);
+        #[cfg(not(feature = "cheater-detection"))]
+        return signature_shares.contains_key(id);
+    }) {
         return Err(Error::UnknownIdentifier);
     }
 
@@ -460,6 +462,7 @@ where
     // Only if the verification of the aggregate signature failed; verify each share to find the cheater.
     // This approach is more efficient since we don't need to verify all shares
     // if the aggregate signature is valid (which should be the common case).
+    #[cfg(feature = "cheater-detection")]
     if let Err(err) = verification_result {
         // Compute the per-message challenge.
         let challenge = crate::challenge::<C>(
@@ -504,5 +507,8 @@ where
         return Err(err);
     }
 
+    #[cfg(not(feature = "cheater-detection"))]
+    verification_result?;
+
     Ok(signature)
 }
diff --git a/frost-core/src/frost/round2.rs b/frost-core/src/frost/round2.rs
@@ -88,12 +88,12 @@ where
         &self,
         identifier: Identifier<C>,
         group_commitment_share: &round1::GroupCommitmentShare<C>,
-        public_key: &frost::keys::VerifyingShare<C>,
+        verifying_share: &frost::keys::VerifyingShare<C>,
         lambda_i: Scalar<C>,
         challenge: &Challenge<C>,
     ) -> Result<(), Error<C>> {
         if (<C::Group>::generator() * self.share)
-            != (group_commitment_share.0 + (public_key.0 * challenge.0 * lambda_i))
+            != (group_commitment_share.0 + (verifying_share.0 * challenge.0 * lambda_i))
         {
             return Err(Error::InvalidSignatureShare {
                 culprit: identifier,

diff --git a/frost-core/src/tests/ciphersuite_generic.rs b/frost-core/src/tests/ciphersuite_generic.rs
@@ -4,7 +4,7 @@
 use std::{collections::BTreeMap, convert::TryFrom};
 
 use crate::{
-    frost::{self, Identifier},
+    frost::{self, keys::PublicKeyPackage, Identifier},
     Error, Field, Group, Signature, SigningKey, VerifyingKey,
 };
 use rand_core::{CryptoRng, RngCore};
@@ -191,7 +191,7 @@ pub fn check_sign<C: Ciphersuite + PartialEq, R: RngCore + CryptoRng>(
     min_signers: u16,
     key_packages: BTreeMap<frost::Identifier<C>, frost::keys::KeyPackage<C>>,
     mut rng: R,
-    pubkey_package: frost::keys::PublicKeyPackage<C>,
+    pubkey_package: PublicKeyPackage<C>,
 ) -> Result<(Vec<u8>, Signature<C>, VerifyingKey<C>), Error<C>> {
     let mut nonces_map: BTreeMap<frost::Identifier<C>, frost::round1::SigningNonces<C>> =
         BTreeMap::new();
@@ -248,6 +248,13 @@ pub fn check_sign<C: Ciphersuite + PartialEq, R: RngCore + CryptoRng>(
     // generates the final signature.
     ////////////////////////////////////////////////////////////////////////////
 
+    #[cfg(not(feature = "cheater-detection"))]
+    let pubkey_package = PublicKeyPackage {
+        header: pubkey_package.header,
+        verifying_shares: BTreeMap::new(),
+        verifying_key: pubkey_package.verifying_key,
+    };
+
     check_aggregate_errors(
         signing_package.clone(),
         signature_shares.clone(),
@@ -305,11 +312,13 @@ fn check_aggregate_errors<C: Ciphersuite + PartialEq>(
     signature_shares: BTreeMap<frost::Identifier<C>, frost::round2::SignatureShare<C>>,
     pubkey_package: frost::keys::PublicKeyPackage<C>,
 ) {
+    #[cfg(feature = "cheater-detection")]
     check_aggregate_corrupted_share(
         signing_package.clone(),
         signature_shares.clone(),
         pubkey_package.clone(),
     );
+
     check_aggregate_invalid_share_identifier_for_verifying_shares(
         signing_package,
         signature_shares,

diff --git a/frost-ed25519/Cargo.toml b/frost-ed25519/Cargo.toml
@@ -45,13 +45,15 @@ serde_json = "1.0"
 
 [features]
 nightly = []
-default = ["serialization"]
+default = ["serialization", "cheater-detection"]
 serialization = ["serde", "frost-core/serialization"]
 #! ## Features
 ## Enable `serde` support for types that need to be communicated. You
 ## can use `serde` to serialize structs with any encoder that supports
 ## `serde` (e.g. JSON with `serde_json`).
 serde = ["frost-core/serde"]
+## Enable cheater detection
+cheater-detection = ["frost-core/cheater-detection"]
 
 [lib]
 # Disables non-criterion benchmark which is not used; prevents errors

diff --git a/frost-ed448/Cargo.toml b/frost-ed448/Cargo.toml
@@ -43,13 +43,15 @@ serde_json = "1.0"
 
 [features]
 nightly = []
-default = ["serialization"]
+default = ["serialization", "cheater-detection"]
 serialization = ["serde", "frost-core/serialization"]
 #! ## Features
 ## Enable `serde` support for types that need to be communicated. You
 ## can use `serde` to serialize structs with any encoder that supports
 ## `serde` (e.g. JSON with `serde_json`).
 serde = ["frost-core/serde"]
+## Enable cheater detection
+cheater-detection = ["frost-core/cheater-detection"]
 
 [lib]
 # Disables non-criterion benchmark which is not used; prevents errors

diff --git a/frost-p256/Cargo.toml b/frost-p256/Cargo.toml
@@ -44,13 +44,15 @@ serde_json = "1.0"
 
 [features]
 nightly = []
-default = ["serialization"]
+default = ["serialization", "cheater-detection"]
 serialization = ["serde", "frost-core/serialization"]
 #! ## Features
 ## Enable `serde` support for types that need to be communicated. You
 ## can use `serde` to serialize structs with any encoder that supports
 ## `serde` (e.g. JSON with `serde_json`).
 serde = ["frost-core/serde"]
+## Enable cheater detection
+cheater-detection = ["frost-core/cheater-detection"]
 
 [lib]
 # Disables non-criterion benchmark which is not used; prevents errors

diff --git a/frost-rerandomized/Cargo.toml b/frost-rerandomized/Cargo.toml
@@ -29,7 +29,7 @@ rand_core = "0.6"
 
 [features]
 nightly = []
-default = ["serialization"]
+default = ["serialization", "cheater-detection"]
 serialization = ["serde", "frost-core/serialization"]
 #! ## Features
 ## Enable `serde` support for types that need to be communicated. You
@@ -38,3 +38,5 @@ serialization = ["serde", "frost-core/serialization"]
 serde = ["frost-core/serde"]
 # Exposes ciphersuite-generic tests for other crates to use
 test-impl = ["frost-core/test-impl"]
+## Enable cheater detection
+cheater-detection = ["frost-core/cheater-detection"]
diff --git a/frost-ristretto255/Cargo.toml b/frost-ristretto255/Cargo.toml
@@ -41,13 +41,15 @@ serde_json = "1.0"
 
 [features]
 nightly = []
-default = ["serialization"]
+default = ["serialization", "cheater-detection"]
 serialization = ["serde", "frost-core/serialization"]
 #! ## Features
 ## Enable `serde` support for types that need to be communicated. You
 ## can use `serde` to serialize structs with any encoder that supports
 ## `serde` (e.g. JSON with `serde_json`).
 serde = ["frost-core/serde"]
+## Enable cheater detection
+cheater-detection = ["frost-core/cheater-detection"]
 
 [lib]
 # Disables non-criterion benchmark which is not used; prevents errors

diff --git a/frost-secp256k1/Cargo.toml b/frost-secp256k1/Cargo.toml
@@ -43,13 +43,15 @@ serde_json = "1.0"
 
 [features]
 nightly = []
-default = ["serialization"]
+default = ["serialization", "cheater-detection"]
 serialization = ["serde", "frost-core/serialization"]
 #! ## Features
 ## Enable `serde` support for types that need to be communicated. You
 ## can use `serde` to serialize structs with any encoder that supports
 ## `serde` (e.g. JSON with `serde_json`).
 serde = ["frost-core/serde"]
+## Enable cheater detection
+cheater-detection = ["frost-core/cheater-detection"]
 
 [lib]
 # Disables non-criterion benchmark which is not used; prevents errors