add Error::culprit() to make it easier to find the identifier of misbehaving participants (#420)

frost-core/src/error.rs

@@ -4,6 +4,9 @@ use thiserror::Error;
 
 use crate::{frost::Identifier, Ciphersuite};
 
+#[derive(Error, Debug, Clone, Copy, Eq, PartialEq)]
+pub struct ParticipantError<C: Ciphersuite>(Identifier<C>);
+
 /// An error related to FROST.
 #[non_exhaustive]
 #[derive(Error, Debug, Copy, Clone, Eq, PartialEq)]
@@ -45,14 +48,11 @@ pub enum Error<C: Ciphersuite> {
     #[error("Invalid signature share.")]
     InvalidSignatureShare {
         /// The identifier of the signer whose share validation failed.
-        signer: Identifier<C>,
+        culprit: Identifier<C>,
     },
     /// Secret share verification failed.
     #[error("Invalid secret share.")]
-    InvalidSecretShare {
-        /// The identifier of the signer whose share validation failed.
-        identifier: Identifier<C>,
-    },
+    InvalidSecretShare,
     /// Round 1 package not found for Round 2 participant.
     #[error("Round 1 package not found for Round 2 participant.")]
     PackageNotFound,
@@ -69,7 +69,7 @@ pub enum Error<C: Ciphersuite> {
     #[error("The proof of knowledge is not valid.")]
     InvalidProofOfKnowledge {
         /// The identifier of the signer whose share validation failed.
-        sender: Identifier<C>,
+        culprit: Identifier<C>,
     },
     /// Error in scalar Field.
     #[error("Error in scalar Field.")]
@@ -82,6 +82,48 @@ pub enum Error<C: Ciphersuite> {
     InvalidCoefficient,
 }
 
+impl<C> Error<C>
+where
+    C: Ciphersuite,
+{
+    /// Return the identifier of the participant that caused the error.
+    /// Returns None if not applicable for the error.
+    ///
+    /// This can be used to penalize a participant that does not follow the
+    /// protocol correctly, e.g. removing them from further signings.
+    pub fn culprit(&self) -> Option<Identifier<C>> {
+        // Use an exhaustive match to make sure that if we add new enum items
+        // then we will explicitly check if they should be added here.
+        match self {
+            Error::InvalidSignatureShare {
+                culprit: identifier,
+            }
+            | Error::InvalidProofOfKnowledge {
+                culprit: identifier,
+            } => Some(*identifier),
+            Error::InvalidSecretShare
+            | Error::InvalidMinSigners
+            | Error::InvalidMaxSigners
+            | Error::InvalidCoefficients
+            | Error::MalformedIdentifier
+            | Error::MalformedSigningKey
+            | Error::MalformedVerifyingKey
+            | Error::MalformedSignature
+            | Error::InvalidSignature
+            | Error::DuplicatedShares
+            | Error::IncorrectNumberOfShares
+            | Error::IdentityCommitment
+            | Error::PackageNotFound
+            | Error::IncorrectNumberOfPackages
+            | Error::IncorrectPackage
+            | Error::DKGNotSupported
+            | Error::FieldError(_)
+            | Error::GroupError(_)
+            | Error::InvalidCoefficient => None,
+        }
+    }
+}
+
 /// An error related to a scalar Field.
 #[non_exhaustive]
 #[derive(Error, Debug, Copy, Clone, Eq, PartialEq)]

frost-core/src/frost/keys.rs

@@ -366,9 +366,7 @@ where
         let result = evaluate_vss(&self.commitment, self.identifier);
 
         if !(f_result == result) {
-            return Err(Error::InvalidSecretShare {
-                identifier: self.identifier,
-            });
+            return Err(Error::InvalidSecretShare);
         }
 
         let group_public = VerifyingKey {
@@ -564,7 +562,7 @@ where
 /// group public key.
 ///
 /// Used for verification purposes before publishing a signature.
-#[derive(PartialEq, Eq, Getters)]
+#[derive(Clone, PartialEq, Eq, Getters)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "serde", serde(deny_unknown_fields))]
 pub struct PublicKeyPackage<C: Ciphersuite> {

frost-core/src/frost/keys/dkg.rs

@@ -290,7 +290,7 @@ pub fn part2<C: Ciphersuite>(
         let c_ell = challenge::<C>(ell, &R_ell, &phi_ell0).ok_or(Error::DKGNotSupported)?;
 
         if R_ell != <C::Group>::generator() * mu_ell - phi_ell0 * c_ell.0 {
-            return Err(Error::InvalidProofOfKnowledge { sender: ell });
+            return Err(Error::InvalidProofOfKnowledge { culprit: ell });
         }
 
         // Round 2, Step 1

frost-core/src/frost/round2.rs

@@ -137,7 +137,7 @@ where
             != (group_commitment_share.0 + (public_key.0 * challenge.0 * lambda_i))
         {
             return Err(Error::InvalidSignatureShare {
-                signer: self.identifier,
+                culprit: self.identifier,
             });
         }
 

frost-core/src/tests/ciphersuite_generic.rs

@@ -3,7 +3,7 @@ use std::{collections::HashMap, convert::TryFrom};
 
 use crate::{
     frost::{self},
-    Error, Signature, VerifyingKey,
+    Error, Field, Group, Signature, VerifyingKey,
 };
 use debugless_unwrap::DebuglessUnwrapErr;
 use rand_core::{CryptoRng, RngCore};
@@ -136,6 +136,12 @@ fn check_sign<C: Ciphersuite + PartialEq, R: RngCore + CryptoRng>(
     // generates the final signature.
     ////////////////////////////////////////////////////////////////////////////
 
+    check_aggregate_error(
+        signing_package.clone(),
+        signature_shares.clone(),
+        pubkey_package.clone(),
+    );
+
     // Aggregate (also verifies the signature shares)
     let group_signature =
         frost::aggregate(&signing_package, &signature_shares[..], &pubkey_package).unwrap();
@@ -166,6 +172,24 @@ fn check_sign<C: Ciphersuite + PartialEq, R: RngCore + CryptoRng>(
     )
 }
 
+fn check_aggregate_error<C: Ciphersuite + PartialEq>(
+    signing_package: frost::SigningPackage<C>,
+    mut signature_shares: Vec<frost::round2::SignatureShare<C>>,
+    pubkey_package: frost::keys::PublicKeyPackage<C>,
+) {
+    let one = <<C as Ciphersuite>::Group as Group>::Field::one();
+    // Corrupt a share
+    signature_shares[0].signature.z_share = signature_shares[0].signature.z_share + one;
+    let e = frost::aggregate(&signing_package, &signature_shares[..], &pubkey_package).unwrap_err();
+    assert_eq!(e.culprit(), Some(*signature_shares[0].identifier()));
+    assert_eq!(
+        e,
+        Error::InvalidSignatureShare {
+            culprit: *signature_shares[0].identifier()
+        }
+    );
+}
+
 /// Test FROST signing with trusted dealer with a Ciphersuite.
 pub fn check_sign_with_dkg<C: Ciphersuite + PartialEq, R: RngCore + CryptoRng>(
     mut rng: R,
@@ -247,6 +271,7 @@ where
             .remove(&participant_identifier)
             .unwrap();
         let round1_packages = &received_round1_packages[&participant_identifier];
+        check_part2_error(round1_secret_package.clone(), round1_packages.clone());
         let (round2_secret_package, round2_packages) =
             frost::keys::dkg::part2(round1_secret_package, round1_packages).expect("should work");
 
@@ -320,3 +345,38 @@ where
     // Proceed with the signing test.
     check_sign(min_signers, key_packages, rng, pubkeys)
 }
+
+fn check_part2_error<C: Ciphersuite>(
+    round1_secret_package: frost::keys::dkg::round1::SecretPackage<C>,
+    mut round1_packages: Vec<frost::keys::dkg::round1::Package<C>>,
+) {
+    let one = <<C as Ciphersuite>::Group as Group>::Field::one();
+    // Corrupt a PoK
+    round1_packages[0].proof_of_knowledge.z = round1_packages[0].proof_of_knowledge.z + one;
+    let e = frost::keys::dkg::part2(round1_secret_package, &round1_packages).debugless_unwrap_err();
+    assert_eq!(e.culprit(), Some(*round1_packages[0].sender_identifier()));
+    assert_eq!(
+        e,
+        Error::InvalidProofOfKnowledge {
+            culprit: *round1_packages[0].sender_identifier()
+        }
+    );
+}
+
+/// Test Error culprit method.
+pub fn check_error_culprit<C: Ciphersuite>() {
+    let identifier: frost::Identifier<C> = 42u16.try_into().unwrap();
+
+    let e = Error::InvalidSignatureShare {
+        culprit: identifier,
+    };
+    assert_eq!(e.culprit(), Some(identifier));
+
+    let e = Error::InvalidProofOfKnowledge {
+        culprit: identifier,
+    };
+    assert_eq!(e.culprit(), Some(identifier));
+
+    let e: Error<C> = Error::InvalidSignature;
+    assert_eq!(e.culprit(), None);
+}

frost-ristretto255/tests/integration_tests.rs

@@ -48,3 +48,8 @@ fn check_sign_with_test_vectors() {
         &VECTORS_BIG_IDENTIFIER,
     );
 }
+
+#[test]
+fn check_error_culprit() {
+    frost_core::tests::ciphersuite_generic::check_error_culprit::<Ristretto255Sha512>();
+}