DKG Blame (#196)

* Standardize the DLEq serialization function naming

They mismatched from the rest of the project.

This commit is technically incomplete as it doesn't update the dkg crate.

* Rewrite DKG encryption to enable per-message decryption without side effects

This isn't technically true as I already know a break in this which I'll
correct for shortly.

Does update documentation to explain the new scheme. Required for blame.

* Add a verifiable system for blame during the FROST DKG

Previously, if sent an invalid key share, the participant would realize that
and could accuse the sender. Without further evidence, either the accuser
or the accused could be guilty. Now, the accuser has a proof the accused is
in the wrong.

Reworks KeyMachine to return BlameMachine. This explicitly acknowledges how
locally complete keys still need group acknowledgement before the protocol
can be complete and provides a way for others to verify blame, even after a
locally successful run.

If any blame is cast, the protocol is no longer considered complete-able
(instead aborting). Further accusations of blame can still be handled however.

Updates documentation on network behavior.

Also starts to remove "OnDrop". We now use Zeroizing for anything which should
be zeroized on drop. This is a lot more piece-meal and reduces clones.

* Tweak Zeroizing and Debug impls

Expands Zeroizing to be more comprehensive.

Also updates Zeroizing<CachedPreprocess([u8; 32])> to
CachedPreprocess(Zeroizing<[u8; 32]>) so zeroizing is the first thing done
and last step before exposing the copy-able [u8; 32].

Removes private keys from Debug.

* Fix a bug where adversaries could claim to be using another user's encryption keys to learn their messages

Mentioned a few commits ago, now fixed.

This wouldn't have affected Serai, which aborts on failure, nor any DKG
currently supported. It's just about ensuring the DKG encryption is robust and
proper.

* Finish moving dleq from ser/deser to write/read

* Add tests for dkg blame

* Add a FROST test for invalid signature shares

* Batch verify encrypted messages' ephemeral keys' PoP

crypto/dkg/src/encryption.rs

@@ -1,10 +1,12 @@
-use core::{hash::Hash, fmt::Debug};
+use core::fmt::Debug;
 use std::{
   ops::Deref,
   io::{self, Read, Write},
   collections::HashMap,
 };
 
+use thiserror::Error;
+
 use zeroize::{Zeroize, Zeroizing};
 use rand_core::{RngCore, CryptoRng};
 
@@ -13,12 +15,17 @@ use chacha20::{
   Key as Cc20Key, Nonce as Cc20Iv, ChaCha20,
 };
 
-use group::GroupEncoding;
-
-use ciphersuite::Ciphersuite;
-
 use transcript::{Transcript, RecommendedTranscript};
 
+#[cfg(test)]
+use group::ff::Field;
+use group::GroupEncoding;
+use ciphersuite::Ciphersuite;
+use multiexp::BatchVerifier;
+
+use schnorr::SchnorrSignature;
+use dleq::DLEqProof;
+
 use crate::ThresholdParams;
 
 pub trait ReadWrite: Sized {
@@ -35,6 +42,7 @@ pub trait ReadWrite: Sized {
 pub trait Message: Clone + PartialEq + Eq + Debug + Zeroize + ReadWrite {}
 impl<M: Clone + PartialEq + Eq + Debug + Zeroize + ReadWrite> Message for M {}
 
+/// Wraps a message with a key to use for encryption in the future.
 #[derive(Clone, PartialEq, Eq, Debug, Zeroize)]
 pub struct EncryptionKeyMessage<C: Ciphersuite, M: Message> {
   msg: M,
@@ -57,20 +65,115 @@ impl<C: Ciphersuite, M: Message> EncryptionKeyMessage<C, M> {
     self.write(&mut buf).unwrap();
     buf
   }
+
+  // Used by tests
+  pub(crate) fn enc_key(&self) -> C::G {
+    self.enc_key
+  }
 }
 
-pub trait Encryptable: Clone + AsMut<[u8]> + Zeroize + ReadWrite {}
-impl<E: Clone + AsMut<[u8]> + Zeroize + ReadWrite> Encryptable for E {}
-#[derive(Clone, Zeroize)]
-pub struct EncryptedMessage<E: Encryptable>(Zeroizing<E>);
+pub trait Encryptable: Clone + AsRef<[u8]> + AsMut<[u8]> + Zeroize + ReadWrite {}
+impl<E: Clone + AsRef<[u8]> + AsMut<[u8]> + Zeroize + ReadWrite> Encryptable for E {}
 
-impl<E: Encryptable> EncryptedMessage<E> {
+/// An encrypted message, with a per-message encryption key enabling revealing specific messages
+/// without side effects.
+#[derive(Clone, Zeroize)]
+pub struct EncryptedMessage<C: Ciphersuite, E: Encryptable> {
+  key: C::G,
+  // Also include a proof-of-possession for the key.
+  // If this proof-of-possession wasn't here, Eve could observe Alice encrypt to Bob with key X,
+  // then send Bob a message also claiming to use X.
+  // While Eve's message would fail to meaningfully decrypt, Bob would then use this to create a
+  // blame argument against Eve. When they do, they'd reveal bX, revealing Alice's message to Bob.
+  // This is a massive side effect which could break some protocols, in the worst case.
+  // While Eve can still reuse their own keys, causing Bob to leak all messages by revealing for
+  // any single one, that's effectively Eve revealing themselves, and not considered relevant.
+  pop: SchnorrSignature<C>,
+  msg: Zeroizing<E>,
+}
+
+fn ecdh<C: Ciphersuite>(private: &Zeroizing<C::F>, public: C::G) -> Zeroizing<C::G> {
+  Zeroizing::new(public * private.deref())
+}
+
+fn cipher<C: Ciphersuite>(dst: &'static [u8], ecdh: &Zeroizing<C::G>) -> ChaCha20 {
+  // Ideally, we'd box this transcript with ZAlloc, yet that's only possible on nightly
+  // TODO: https://github.com/serai-dex/serai/issues/151
+  let mut transcript = RecommendedTranscript::new(b"DKG Encryption v0.2");
+  transcript.domain_separate(dst);
+
+  let mut ecdh = ecdh.to_bytes();
+  transcript.append_message(b"shared_key", ecdh.as_ref());
+  ecdh.as_mut().zeroize();
+
+  let zeroize = |buf: &mut [u8]| buf.zeroize();
+
+  let mut key = Cc20Key::default();
+  let mut challenge = transcript.challenge(b"key");
+  key.copy_from_slice(&challenge[.. 32]);
+  zeroize(challenge.as_mut());
+
+  // The RecommendedTranscript isn't vulnerable to length extension attacks, yet if it was,
+  // it'd make sense to clone it (and fork it) just to hedge against that
+  let mut iv = Cc20Iv::default();
+  let mut challenge = transcript.challenge(b"iv");
+  iv.copy_from_slice(&challenge[.. 12]);
+  zeroize(challenge.as_mut());
+
+  // Same commentary as the transcript regarding ZAlloc
+  // TODO: https://github.com/serai-dex/serai/issues/151
+  let res = ChaCha20::new(&key, &iv);
+  zeroize(key.as_mut());
+  zeroize(iv.as_mut());
+  res
+}
+
+fn encrypt<R: RngCore + CryptoRng, C: Ciphersuite, E: Encryptable>(
+  rng: &mut R,
+  dst: &'static [u8],
+  from: u16,
+  to: C::G,
+  mut msg: Zeroizing<E>,
+) -> EncryptedMessage<C, E> {
+  /*
+  The following code could be used to replace the requirement on an RNG here.
+  It's just currently not an issue to require taking in an RNG here.
+  let last = self.last_enc_key.to_bytes();
+  self.last_enc_key = C::hash_to_F(b"encryption_base", last.as_ref());
+  let key = C::hash_to_F(b"encryption_key", last.as_ref());
+  last.as_mut().zeroize();
+  */
+
+  let key = Zeroizing::new(C::random_nonzero_F(rng));
+  cipher::<C>(dst, &ecdh::<C>(&key, to)).apply_keystream(msg.as_mut().as_mut());
+
+  let pub_key = C::generator() * key.deref();
+  let nonce = Zeroizing::new(C::random_nonzero_F(rng));
+  let pub_nonce = C::generator() * nonce.deref();
+  EncryptedMessage {
+    key: pub_key,
+    pop: SchnorrSignature::sign(
+      &key,
+      nonce,
+      pop_challenge::<C>(pub_nonce, pub_key, from, msg.deref().as_ref()),
+    ),
+    msg,
+  }
+}
+
+impl<C: Ciphersuite, E: Encryptable> EncryptedMessage<C, E> {
   pub fn read<R: Read>(reader: &mut R, params: ThresholdParams) -> io::Result<Self> {
-    Ok(Self(Zeroizing::new(E::read(reader, params)?)))
+    Ok(Self {
+      key: C::read_G(reader)?,
+      pop: SchnorrSignature::<C>::read(reader)?,
+      msg: Zeroizing::new(E::read(reader, params)?),
+    })
   }
 
   pub fn write<W: Write>(&self, writer: &mut W) -> io::Result<()> {
-    self.0.write(writer)
+    writer.write_all(self.key.to_bytes().as_ref())?;
+    self.pop.write(writer)?;
+    self.msg.write(writer)
   }
 
   pub fn serialize(&self) -> Vec<u8> {
@@ -78,17 +181,150 @@ impl<E: Encryptable> EncryptedMessage<E> {
     self.write(&mut buf).unwrap();
     buf
   }
+
+  #[cfg(test)]
+  pub(crate) fn invalidate_pop(&mut self) {
+    self.pop.s += C::F::one();
+  }
+
+  #[cfg(test)]
+  pub(crate) fn invalidate_msg<R: RngCore + CryptoRng>(&mut self, rng: &mut R, from: u16) {
+    // Invalidate the message by specifying a new key/Schnorr PoP
+    // This will cause all initial checks to pass, yet a decrypt to gibberish
+    let key = Zeroizing::new(C::random_nonzero_F(rng));
+    let pub_key = C::generator() * key.deref();
+    let nonce = Zeroizing::new(C::random_nonzero_F(rng));
+    let pub_nonce = C::generator() * nonce.deref();
+    self.key = pub_key;
+    self.pop = SchnorrSignature::sign(
+      &key,
+      nonce,
+      pop_challenge::<C>(pub_nonce, pub_key, from, self.msg.deref().as_ref()),
+    );
+  }
+
+  // Assumes the encrypted message is a secret share.
+  #[cfg(test)]
+  pub(crate) fn invalidate_share_serialization<R: RngCore + CryptoRng>(
+    &mut self,
+    rng: &mut R,
+    dst: &'static [u8],
+    from: u16,
+    to: C::G,
+  ) {
+    use group::ff::PrimeField;
+
+    let mut repr = <C::F as PrimeField>::Repr::default();
+    for b in repr.as_mut().iter_mut() {
+      *b = 255;
+    }
+    // Tries to guarantee the above assumption.
+    assert_eq!(repr.as_ref().len(), self.msg.as_ref().len());
+    // Checks that this isn't over a field where this is somehow valid
+    assert!(!bool::from(C::F::from_repr(repr).is_some()));
+
+    self.msg.as_mut().as_mut().copy_from_slice(repr.as_ref());
+    *self = encrypt(rng, dst, from, to, self.msg.clone());
+  }
+
+  // Assumes the encrypted message is a secret share.
+  #[cfg(test)]
+  pub(crate) fn invalidate_share_value<R: RngCore + CryptoRng>(
+    &mut self,
+    rng: &mut R,
+    dst: &'static [u8],
+    from: u16,
+    to: C::G,
+  ) {
+    use group::ff::PrimeField;
+
+    // Assumes the share isn't randomly 1
+    let repr = C::F::one().to_repr();
+    self.msg.as_mut().as_mut().copy_from_slice(repr.as_ref());
+    *self = encrypt(rng, dst, from, to, self.msg.clone());
+  }
 }
 
+/// A proof that the provided point is the legitimately derived shared key for some message.
+#[derive(Clone, PartialEq, Eq, Debug, Zeroize)]
+pub struct EncryptionKeyProof<C: Ciphersuite> {
+  key: Zeroizing<C::G>,
+  dleq: DLEqProof<C::G>,
+}
+
+impl<C: Ciphersuite> EncryptionKeyProof<C> {
+  pub fn read<R: Read>(reader: &mut R) -> io::Result<Self> {
+    Ok(Self { key: Zeroizing::new(C::read_G(reader)?), dleq: DLEqProof::read(reader)? })
+  }
+
+  pub fn write<W: Write>(&self, writer: &mut W) -> io::Result<()> {
+    writer.write_all(self.key.to_bytes().as_ref())?;
+    self.dleq.write(writer)
+  }
+
+  pub fn serialize(&self) -> Vec<u8> {
+    let mut buf = vec![];
+    self.write(&mut buf).unwrap();
+    buf
+  }
+
+  #[cfg(test)]
+  pub(crate) fn invalidate_key(&mut self) {
+    *self.key += C::generator();
+  }
+
+  #[cfg(test)]
+  pub(crate) fn invalidate_dleq(&mut self) {
+    let mut buf = vec![];
+    self.dleq.write(&mut buf).unwrap();
+    // Adds one to c since this is serialized c, s
+    // Adding one to c will leave a validly serialized c
+    // Adding one to s may leave an invalidly serialized s
+    buf[0] = buf[0].wrapping_add(1);
+    self.dleq = DLEqProof::read::<&[u8]>(&mut buf.as_ref()).unwrap();
+  }
+}
+
+// This doesn't need to take the msg. It just doesn't hurt as an extra layer.
+// This still doesn't mean the DKG offers an authenticated channel. The per-message keys have no
+// root of trust other than their existence in the assumed-to-exist external authenticated channel.
+fn pop_challenge<C: Ciphersuite>(nonce: C::G, key: C::G, sender: u16, msg: &[u8]) -> C::F {
+  let mut transcript = RecommendedTranscript::new(b"DKG Encryption Key Proof of Possession v0.2");
+  transcript.append_message(b"nonce", nonce.to_bytes());
+  transcript.append_message(b"key", key.to_bytes());
+  // This is sufficient to prevent the attack this is meant to stop
+  transcript.append_message(b"sender", sender.to_le_bytes());
+  // This, as written above, doesn't hurt
+  transcript.append_message(b"message", msg);
+  // While this is a PoK and a PoP, it's called a PoP here since the important part is its owner
+  // Elsewhere, where we use the term PoK, the important part is that it isn't some inverse, with
+  // an unknown to anyone discrete log, breaking the system
+  C::hash_to_F(b"DKG-encryption-proof_of_possession", &transcript.challenge(b"schnorr"))
+}
+
+fn encryption_key_transcript() -> RecommendedTranscript {
+  RecommendedTranscript::new(b"DKG Encryption Key Correctness Proof v0.2")
+}
+
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Error)]
+pub(crate) enum DecryptionError {
+  #[error("accused provided an invalid signature")]
+  InvalidSignature,
+  #[error("accuser provided an invalid decryption key")]
+  InvalidProof,
+}
+
+// A simple box for managing encryption.
 #[derive(Clone)]
-pub(crate) struct Encryption<Id: Eq + Hash, C: Ciphersuite> {
+pub(crate) struct Encryption<C: Ciphersuite> {
   dst: &'static [u8],
+  i: u16,
   enc_key: Zeroizing<C::F>,
   enc_pub_key: C::G,
-  enc_keys: HashMap<Id, C::G>,
+  enc_keys: HashMap<u16, C::G>,
 }
 
-impl<Id: Eq + Hash, C: Ciphersuite> Zeroize for Encryption<Id, C> {
+impl<C: Ciphersuite> Zeroize for Encryption<C> {
   fn zeroize(&mut self) {
     self.enc_key.zeroize();
     self.enc_pub_key.zeroize();
@@ -98,10 +334,16 @@ impl<Id: Eq + Hash, C: Ciphersuite> Zeroize for Encryption<Id, C> {
   }
 }
 
-impl<Id: Eq + Hash, C: Ciphersuite> Encryption<Id, C> {
-  pub(crate) fn new<R: RngCore + CryptoRng>(dst: &'static [u8], rng: &mut R) -> Self {
+impl<C: Ciphersuite> Encryption<C> {
+  pub(crate) fn new<R: RngCore + CryptoRng>(dst: &'static [u8], i: u16, rng: &mut R) -> Self {
     let enc_key = Zeroizing::new(C::random_nonzero_F(rng));
-    Self { dst, enc_pub_key: C::generator() * enc_key.deref(), enc_key, enc_keys: HashMap::new() }
+    Self {
+      dst,
+      i,
+      enc_pub_key: C::generator() * enc_key.deref(),
+      enc_key,
+      enc_keys: HashMap::new(),
+    }
   }
 
   pub(crate) fn registration<M: Message>(&self, msg: M) -> EncryptionKeyMessage<C, M> {
@@ -110,7 +352,7 @@ impl<Id: Eq + Hash, C: Ciphersuite> Encryption<Id, C> {
 
   pub(crate) fn register<M: Message>(
     &mut self,
-    participant: Id,
+    participant: u16,
     msg: EncryptionKeyMessage<C, M>,
   ) -> M {
     if self.enc_keys.contains_key(&participant) {
@@ -120,62 +362,81 @@ impl<Id: Eq + Hash, C: Ciphersuite> Encryption<Id, C> {
     msg.msg
   }
 
-  fn cipher(&self, participant: Id, encrypt: bool) -> ChaCha20 {
-    // Ideally, we'd box this transcript with ZAlloc, yet that's only possible on nightly
-    // TODO: https://github.com/serai-dex/serai/issues/151
-    let mut transcript = RecommendedTranscript::new(b"DKG Encryption v0.2");
-    transcript.domain_separate(self.dst);
+  pub(crate) fn encrypt<R: RngCore + CryptoRng, E: Encryptable>(
+    &self,
+    rng: &mut R,
+    participant: u16,
+    msg: Zeroizing<E>,
+  ) -> EncryptedMessage<C, E> {
+    encrypt(rng, self.dst, self.i, self.enc_keys[&participant], msg)
+  }
 
-    let other = self.enc_keys[&participant];
-    if encrypt {
-      transcript.append_message(b"sender", self.enc_pub_key.to_bytes());
-      transcript.append_message(b"receiver", other.to_bytes());
-    } else {
-      transcript.append_message(b"sender", other.to_bytes());
-      transcript.append_message(b"receiver", self.enc_pub_key.to_bytes());
+  pub(crate) fn decrypt<R: RngCore + CryptoRng, I: Copy + Zeroize, E: Encryptable>(
+    &self,
+    rng: &mut R,
+    batch: &mut BatchVerifier<I, C::G>,
+    // Uses a distinct batch ID so if this batch verifier is reused, we know its the PoP aspect
+    // which failed, and therefore to use None for the blame
+    batch_id: I,
+    from: u16,
+    mut msg: EncryptedMessage<C, E>,
+  ) -> (Zeroizing<E>, EncryptionKeyProof<C>) {
+    msg.pop.batch_verify(
+      rng,
+      batch,
+      batch_id,
+      msg.key,
+      pop_challenge::<C>(msg.pop.R, msg.key, from, msg.msg.deref().as_ref()),
+    );
+
+    let key = ecdh::<C>(&self.enc_key, msg.key);
+    cipher::<C>(self.dst, &key).apply_keystream(msg.msg.as_mut().as_mut());
+    (
+      msg.msg,
+      EncryptionKeyProof {
+        key,
+        dleq: DLEqProof::prove(
+          rng,
+          &mut encryption_key_transcript(),
+          &[C::generator(), msg.key],
+          &self.enc_key,
+        ),
+      },
+    )
+  }
+
+  // Given a message, and the intended decryptor, and a proof for its key, decrypt the message.
+  // Returns None if the key was wrong.
+  pub(crate) fn decrypt_with_proof<E: Encryptable>(
+    &self,
+    from: u16,
+    decryptor: u16,
+    mut msg: EncryptedMessage<C, E>,
+    // There's no encryption key proof if the accusation is of an invalid signature
+    proof: Option<EncryptionKeyProof<C>>,
+  ) -> Result<Zeroizing<E>, DecryptionError> {
+    if !msg
+      .pop
+      .verify(msg.key, pop_challenge::<C>(msg.pop.R, msg.key, from, msg.msg.deref().as_ref()))
+    {
+      Err(DecryptionError::InvalidSignature)?;
     }
 
-    let mut shared = Zeroizing::new(other * self.enc_key.deref()).deref().to_bytes();
-    transcript.append_message(b"shared_key", shared.as_ref());
-    shared.as_mut().zeroize();
+    if let Some(proof) = proof {
+      // Verify this is the decryption key for this message
+      proof
+        .dleq
+        .verify(
+          &mut encryption_key_transcript(),
+          &[C::generator(), msg.key],
+          &[self.enc_keys[&decryptor], *proof.key],
+        )
+        .map_err(|_| DecryptionError::InvalidProof)?;
 
-    let zeroize = |buf: &mut [u8]| buf.zeroize();
-
-    let mut key = Cc20Key::default();
-    let mut challenge = transcript.challenge(b"key");
-    key.copy_from_slice(&challenge[.. 32]);
-    zeroize(challenge.as_mut());
-
-    // The RecommendedTranscript isn't vulnerable to length extension attacks, yet if it was,
-    // it'd make sense to clone it (and fork it) just to hedge against that
-    let mut iv = Cc20Iv::default();
-    let mut challenge = transcript.challenge(b"iv");
-    iv.copy_from_slice(&challenge[.. 12]);
-    zeroize(challenge.as_mut());
-
-    // Same commentary as the transcript regarding ZAlloc
-    // TODO: https://github.com/serai-dex/serai/issues/151
-    let res = ChaCha20::new(&key, &iv);
-    zeroize(key.as_mut());
-    zeroize(iv.as_mut());
-    res
-  }
-
-  pub(crate) fn encrypt<E: Encryptable>(
-    &self,
-    participant: Id,
-    mut msg: Zeroizing<E>,
-  ) -> EncryptedMessage<E> {
-    self.cipher(participant, true).apply_keystream(msg.as_mut().as_mut());
-    EncryptedMessage(msg)
-  }
-
-  pub(crate) fn decrypt<E: Encryptable>(
-    &self,
-    participant: Id,
-    mut msg: EncryptedMessage<E>,
-  ) -> Zeroizing<E> {
-    self.cipher(participant, false).apply_keystream(msg.0.as_mut().as_mut());
-    msg.0
+      cipher::<C>(self.dst, &proof.key).apply_keystream(msg.msg.as_mut().as_mut());
+      Ok(msg.msg)
+    } else {
+      Err(DecryptionError::InvalidProof)
+    }
   }
 }

crypto/dkg/src/frost.rs

@@ -1,6 +1,9 @@
-use std::{
+use core::{
   marker::PhantomData,
   ops::Deref,
+  fmt::{Debug, Formatter},
+};
+use std::{
   io::{self, Read, Write},
   collections::HashMap,
 };
@@ -13,7 +16,7 @@ use transcript::{Transcript, RecommendedTranscript};
 
 use group::{
   ff::{Field, PrimeField},
-  GroupEncoding,
+  Group, GroupEncoding,
 };
 use ciphersuite::Ciphersuite;
 use multiexp::{multiexp_vartime, BatchVerifier};
@@ -22,9 +25,14 @@ use schnorr::SchnorrSignature;
 
 use crate::{
   DkgError, ThresholdParams, ThresholdCore, validate_map,
-  encryption::{ReadWrite, EncryptionKeyMessage, EncryptedMessage, Encryption},
+  encryption::{
+    ReadWrite, EncryptionKeyMessage, EncryptedMessage, Encryption, EncryptionKeyProof,
+    DecryptionError,
+  },
 };
 
+type FrostError<C> = DkgError<EncryptionKeyProof<C>>;
+
 #[allow(non_snake_case)]
 fn challenge<C: Ciphersuite>(context: &str, l: u16, R: &[u8], Am: &[u8]) -> C::F {
   let mut transcript = RecommendedTranscript::new(b"DKG FROST v0.2");
@@ -33,10 +41,15 @@ fn challenge<C: Ciphersuite>(context: &str, l: u16, R: &[u8], Am: &[u8]) -> C::F
   transcript.append_message(b"participant", l.to_le_bytes());
   transcript.append_message(b"nonce", R);
   transcript.append_message(b"commitments", Am);
-  C::hash_to_F(b"PoK 0", &transcript.challenge(b"challenge"))
+  C::hash_to_F(b"DKG-FROST-proof_of_knowledge-0", &transcript.challenge(b"schnorr"))
 }
 
-/// Commitments message to be broadcast to all other parties.
+/// The commitments message, intended to be broadcast to all other parties.
+/// Every participant should only provide one set of commitments to all parties.
+/// If any participant sends multiple sets of commitments, they are faulty and should be presumed
+/// malicious.
+/// As this library does not handle networking, it is also unable to detect if any participant is
+/// so faulty. That responsibility lies with the caller.
 #[derive(Clone, PartialEq, Eq, Debug, Zeroize)]
 pub struct Commitments<C: Ciphersuite> {
   commitments: Vec<C::G>,
@@ -119,7 +132,7 @@ impl<C: Ciphersuite> KeyGenMachine<C> {
     );
 
     // Additionally create an encryption mechanism to protect the secret shares
-    let encryption = Encryption::new(b"FROST", rng);
+    let encryption = Encryption::new(b"FROST", self.params.i, rng);
 
     // Step 4: Broadcast
     let msg =
@@ -149,19 +162,39 @@ fn polynomial<F: PrimeField + Zeroize>(coefficients: &[Zeroizing<F>], l: u16) ->
   share
 }
 
-/// Secret share to be sent to the party it's intended for over an authenticated channel.
-#[derive(Clone, PartialEq, Eq, Debug)]
+/// The secret share message, to be sent to the party it's intended for over an authenticated
+/// channel.
+/// If any participant sends multiple secret shares to another participant, they are faulty.
+// This should presumably be written as SecretShare(Zeroizing<F::Repr>).
+// It's unfortunately not possible as F::Repr doesn't have Zeroize as a bound.
+// The encryption system also explicitly uses Zeroizing<M> so it can ensure anything being
+// encrypted is within Zeroizing. Accordingly, internally having Zeroizing would be redundant.
+#[derive(Clone, PartialEq, Eq)]
 pub struct SecretShare<F: PrimeField>(F::Repr);
+impl<F: PrimeField> AsRef<[u8]> for SecretShare<F> {
+  fn as_ref(&self) -> &[u8] {
+    self.0.as_ref()
+  }
+}
 impl<F: PrimeField> AsMut<[u8]> for SecretShare<F> {
   fn as_mut(&mut self) -> &mut [u8] {
     self.0.as_mut()
   }
 }
+impl<F: PrimeField> Debug for SecretShare<F> {
+  fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), core::fmt::Error> {
+    fmt.debug_struct("SecretShare").finish_non_exhaustive()
+  }
+}
 impl<F: PrimeField> Zeroize for SecretShare<F> {
   fn zeroize(&mut self) {
     self.0.as_mut().zeroize()
   }
 }
+// Still manually implement ZeroizeOnDrop to ensure these don't stick around.
+// We could replace Zeroizing<M> with a bound M: ZeroizeOnDrop.
+// Doing so would potentially fail to highlight thr expected behavior with these and remove a layer
+// of depth.
 impl<F: PrimeField> Drop for SecretShare<F> {
   fn drop(&mut self) {
     self.zeroize();
@@ -188,7 +221,7 @@ pub struct SecretShareMachine<C: Ciphersuite> {
   context: String,
   coefficients: Vec<Zeroizing<C::F>>,
   our_commitments: Vec<C::G>,
-  encryption: Encryption<u16, C>,
+  encryption: Encryption<C>,
 }
 
 impl<C: Ciphersuite> SecretShareMachine<C> {
@@ -198,7 +231,7 @@ impl<C: Ciphersuite> SecretShareMachine<C> {
     &mut self,
     rng: &mut R,
     mut commitments: HashMap<u16, EncryptionKeyMessage<C, Commitments<C>>>,
-  ) -> Result<HashMap<u16, Vec<C::G>>, DkgError> {
+  ) -> Result<HashMap<u16, Vec<C::G>>, FrostError<C>> {
     validate_map(&commitments, &(1 ..= self.params.n()).collect::<Vec<_>>(), self.params.i())?;
 
     let mut batch = BatchVerifier::<u16, C::G>::new(commitments.len());
@@ -221,21 +254,23 @@ impl<C: Ciphersuite> SecretShareMachine<C> {
       })
       .collect::<HashMap<_, _>>();
 
-    batch.verify_with_vartime_blame().map_err(DkgError::InvalidProofOfKnowledge)?;
+    batch.verify_with_vartime_blame().map_err(FrostError::InvalidProofOfKnowledge)?;
 
     commitments.insert(self.params.i, self.our_commitments.drain(..).collect());
     Ok(commitments)
   }
 
   /// Continue generating a key.
-  /// Takes in everyone else's commitments. Returns a HashMap of secret shares to be sent over
-  /// authenticated channels to their relevant counterparties.
+  /// Takes in everyone else's commitments. Returns a HashMap of encrypted secret shares to be sent
+  /// over authenticated channels to their relevant counterparties.
+  /// If any participant sends multiple secret shares to another participant, they are faulty.
   #[allow(clippy::type_complexity)]
   pub fn generate_secret_shares<R: RngCore + CryptoRng>(
     mut self,
     rng: &mut R,
     commitments: HashMap<u16, EncryptionKeyMessage<C, Commitments<C>>>,
-  ) -> Result<(KeyMachine<C>, HashMap<u16, EncryptedMessage<SecretShare<C::F>>>), DkgError> {
+  ) -> Result<(KeyMachine<C>, HashMap<u16, EncryptedMessage<C, SecretShare<C::F>>>), FrostError<C>>
+  {
     let commitments = self.verify_r1(&mut *rng, commitments)?;
 
     // Step 1: Generate secret shares for all other parties
@@ -250,7 +285,7 @@ impl<C: Ciphersuite> SecretShareMachine<C> {
       let mut share = polynomial(&self.coefficients, l);
       let share_bytes = Zeroizing::new(SecretShare::<C::F>(share.to_repr()));
       share.zeroize();
-      res.insert(l, self.encryption.encrypt(l, share_bytes));
+      res.insert(l, self.encryption.encrypt(rng, l, share_bytes));
     }
 
     // Calculate our own share
@@ -264,13 +299,18 @@ impl<C: Ciphersuite> SecretShareMachine<C> {
   }
 }
 
-/// Final step of the key generation protocol.
+/// Advancement of the the secret share state machine protocol.
+/// This machine will 'complete' the protocol, by a local perspective, and can be the last
+/// interactive component. In order to be secure, the parties must confirm having successfully
+/// completed the protocol (an effort out of scope to this library), yet this is modelled by one
+/// more state transition.
 pub struct KeyMachine<C: Ciphersuite> {
   params: ThresholdParams,
   secret: Zeroizing<C::F>,
   commitments: HashMap<u16, Vec<C::G>>,
-  encryption: Encryption<u16, C>,
+  encryption: Encryption<C>,
 }
+
 impl<C: Ciphersuite> Zeroize for KeyMachine<C> {
   fn zeroize(&mut self) {
     self.params.zeroize();
@@ -281,59 +321,84 @@ impl<C: Ciphersuite> Zeroize for KeyMachine<C> {
     self.encryption.zeroize();
   }
 }
-impl<C: Ciphersuite> Drop for KeyMachine<C> {
-  fn drop(&mut self) {
-    self.zeroize()
-  }
+
+// Calculate the exponent for a given participant and apply it to a series of commitments
+// Initially used with the actual commitments to verify the secret share, later used with
+// stripes to generate the verification shares
+fn exponential<C: Ciphersuite>(i: u16, values: &[C::G]) -> Vec<(C::F, C::G)> {
+  let i = C::F::from(i.into());
+  let mut res = Vec::with_capacity(values.len());
+  (0 .. values.len()).into_iter().fold(C::F::one(), |exp, l| {
+    res.push((exp, values[l]));
+    exp * i
+  });
+  res
+}
+
+fn share_verification_statements<C: Ciphersuite>(
+  target: u16,
+  commitments: &[C::G],
+  mut share: Zeroizing<C::F>,
+) -> Vec<(C::F, C::G)> {
+  // This can be insecurely linearized from n * t to just n using the below sums for a given
+  // stripe. Doing so uses naive addition which is subject to malleability. The only way to
+  // ensure that malleability isn't present is to use this n * t algorithm, which runs
+  // per sender and not as an aggregate of all senders, which also enables blame
+  let mut values = exponential::<C>(target, commitments);
+
+  // Perform the share multiplication outside of the multiexp to minimize stack copying
+  // While the multiexp BatchVerifier does zeroize its flattened multiexp, and itself, it still
+  // converts whatever we give to an iterator and then builds a Vec internally, welcoming copies
+  let neg_share_pub = C::generator() * -*share;
+  share.zeroize();
+  values.push((C::F::one(), neg_share_pub));
+
+  values
+}
+
+#[derive(Clone, Copy, Hash, Debug, Zeroize)]
+enum BatchId {
+  Decryption(u16),
+  Share(u16),
 }
-impl<C: Ciphersuite> ZeroizeOnDrop for KeyMachine<C> {}
 
 impl<C: Ciphersuite> KeyMachine<C> {
-  /// Complete key generation.
-  /// Takes in everyone elses' shares submitted to us. Returns a ThresholdCore object representing
-  /// the generated keys. Successful protocol completion MUST be confirmed by all parties before
-  /// these keys may be safely used.
-  pub fn complete<R: RngCore + CryptoRng>(
+  /// Calculate our share given the shares sent to us.
+  /// Returns a BlameMachine usable to determine if faults in the protocol occurred.
+  /// Will error on, and return a blame proof for, the first-observed case of faulty behavior.
+  pub fn calculate_share<R: RngCore + CryptoRng>(
     mut self,
     rng: &mut R,
-    mut shares: HashMap<u16, EncryptedMessage<SecretShare<C::F>>>,
-  ) -> Result<ThresholdCore<C>, DkgError> {
+    mut shares: HashMap<u16, EncryptedMessage<C, SecretShare<C::F>>>,
+  ) -> Result<BlameMachine<C>, FrostError<C>> {
     validate_map(&shares, &(1 ..= self.params.n()).collect::<Vec<_>>(), self.params.i())?;
 
-    // Calculate the exponent for a given participant and apply it to a series of commitments
-    // Initially used with the actual commitments to verify the secret share, later used with
-    // stripes to generate the verification shares
-    let exponential = |i: u16, values: &[_]| {
-      let i = C::F::from(i.into());
-      let mut res = Vec::with_capacity(self.params.t().into());
-      (0 .. usize::from(self.params.t())).into_iter().fold(C::F::one(), |exp, l| {
-        res.push((exp, values[l]));
-        exp * i
-      });
-      res
-    };
-
     let mut batch = BatchVerifier::new(shares.len());
+    let mut blames = HashMap::new();
     for (l, share_bytes) in shares.drain() {
-      let mut share_bytes = self.encryption.decrypt(l, share_bytes);
-      let mut share = Zeroizing::new(
-        Option::<C::F>::from(C::F::from_repr(share_bytes.0)).ok_or(DkgError::InvalidShare(l))?,
-      );
+      let (mut share_bytes, blame) =
+        self.encryption.decrypt(rng, &mut batch, BatchId::Decryption(l), l, share_bytes);
+      let share =
+        Zeroizing::new(Option::<C::F>::from(C::F::from_repr(share_bytes.0)).ok_or_else(|| {
+          FrostError::InvalidShare { participant: l, blame: Some(blame.clone()) }
+        })?);
       share_bytes.zeroize();
       *self.secret += share.deref();
 
-      // This can be insecurely linearized from n * t to just n using the below sums for a given
-      // stripe. Doing so uses naive addition which is subject to malleability. The only way to
-      // ensure that malleability isn't present is to use this n * t algorithm, which runs
-      // per sender and not as an aggregate of all senders, which also enables blame
-      let mut values = exponential(self.params.i, &self.commitments[&l]);
-      // multiexp will Zeroize this when it's done with it
-      values.push((-*share.deref(), C::generator()));
-      share.zeroize();
-
-      batch.queue(rng, l, values);
+      blames.insert(l, blame);
+      batch.queue(
+        rng,
+        BatchId::Share(l),
+        share_verification_statements::<C>(self.params.i(), &self.commitments[&l], share),
+      );
     }
-    batch.verify_with_vartime_blame().map_err(DkgError::InvalidShare)?;
+    batch.verify_with_vartime_blame().map_err(|id| {
+      let (l, blame) = match id {
+        BatchId::Decryption(l) => (l, None),
+        BatchId::Share(l) => (l, Some(blames.remove(&l).unwrap())),
+      };
+      FrostError::InvalidShare { participant: l, blame }
+    })?;
 
     // Stripe commitments per t and sum them in advance. Calculating verification shares relies on
     // these sums so preprocessing them is a massive speedup
@@ -352,16 +417,136 @@ impl<C: Ciphersuite> KeyMachine<C> {
         if i == self.params.i() {
           C::generator() * self.secret.deref()
         } else {
-          multiexp_vartime(&exponential(i, &stripes))
+          multiexp_vartime(&exponential::<C>(i, &stripes))
         },
       );
     }
 
-    Ok(ThresholdCore {
-      params: self.params,
-      secret_share: self.secret.clone(),
-      group_key: stripes[0],
-      verification_shares,
+    let KeyMachine { commitments, encryption, params, secret } = self;
+    Ok(BlameMachine {
+      commitments,
+      encryption,
+      result: ThresholdCore {
+        params,
+        secret_share: secret,
+        group_key: stripes[0],
+        verification_shares,
+      },
     })
   }
 }
+
+pub struct BlameMachine<C: Ciphersuite> {
+  commitments: HashMap<u16, Vec<C::G>>,
+  encryption: Encryption<C>,
+  result: ThresholdCore<C>,
+}
+
+impl<C: Ciphersuite> Zeroize for BlameMachine<C> {
+  fn zeroize(&mut self) {
+    for (_, commitments) in self.commitments.iter_mut() {
+      commitments.zeroize();
+    }
+    self.encryption.zeroize();
+    self.result.zeroize();
+  }
+}
+
+impl<C: Ciphersuite> BlameMachine<C> {
+  /// Mark the protocol as having been successfully completed, returning the generated keys.
+  /// This should only be called after having confirmed, with all participants, successful
+  /// completion.
+  ///
+  /// Confirming successful completion is not necessarily as simple as everyone reporting their
+  /// completion. Everyone must also receive everyone's report of completion, entering into the
+  /// territory of consensus protocols. This library does not handle that nor does it provide any
+  /// tooling to do so. This function is solely intended to force users to acknowledge they're
+  /// completing the protocol, not processing any blame.
+  pub fn complete(self) -> ThresholdCore<C> {
+    self.result
+  }
+
+  fn blame_internal(
+    &self,
+    sender: u16,
+    recipient: u16,
+    msg: EncryptedMessage<C, SecretShare<C::F>>,
+    proof: Option<EncryptionKeyProof<C>>,
+  ) -> u16 {
+    let share_bytes = match self.encryption.decrypt_with_proof(sender, recipient, msg, proof) {
+      Ok(share_bytes) => share_bytes,
+      // If there's an invalid signature, the sender did not send a properly formed message
+      Err(DecryptionError::InvalidSignature) => return sender,
+      // Decryption will fail if the provided ECDH key wasn't correct for the given message
+      Err(DecryptionError::InvalidProof) => return recipient,
+    };
+
+    let share = match Option::<C::F>::from(C::F::from_repr(share_bytes.0)) {
+      Some(share) => share,
+      // If this isn't a valid scalar, the sender is faulty
+      None => return sender,
+    };
+
+    // If this isn't a valid share, the sender is faulty
+    if !bool::from(
+      multiexp_vartime(&share_verification_statements::<C>(
+        recipient,
+        &self.commitments[&sender],
+        Zeroizing::new(share),
+      ))
+      .is_identity(),
+    ) {
+      return sender;
+    }
+
+    // The share was canonical and valid
+    recipient
+  }
+
+  /// Given an accusation of fault, determine the faulty party (either the sender, who sent an
+  /// invalid secret share, or the receiver, who claimed a valid secret share was invalid). No
+  /// matter which, prevent completion of the machine, forcing an abort of the protocol.
+  ///
+  /// The message should be a copy of the encrypted secret share from the accused sender to the
+  /// accusing recipient. This message must have been authenticated as actually having come from
+  /// the sender in question.
+  ///
+  /// In order to enable detecting multiple faults, an `AdditionalBlameMachine` is returned, which
+  /// can be used to determine further blame. These machines will process the same blame statements
+  /// multiple times, always identifying blame. It is the caller's job to ensure they're unique in
+  /// order to prevent multiple instances of blame over a single incident.
+  pub fn blame(
+    self,
+    sender: u16,
+    recipient: u16,
+    msg: EncryptedMessage<C, SecretShare<C::F>>,
+    proof: Option<EncryptionKeyProof<C>>,
+  ) -> (AdditionalBlameMachine<C>, u16) {
+    let faulty = self.blame_internal(sender, recipient, msg, proof);
+    (AdditionalBlameMachine(self), faulty)
+  }
+}
+
+#[derive(Zeroize)]
+pub struct AdditionalBlameMachine<C: Ciphersuite>(BlameMachine<C>);
+impl<C: Ciphersuite> AdditionalBlameMachine<C> {
+  /// Given an accusation of fault, determine the faulty party (either the sender, who sent an
+  /// invalid secret share, or the receiver, who claimed a valid secret share was invalid).
+  ///
+  /// The message should be a copy of the encrypted secret share from the accused sender to the
+  /// accusing recipient. This message must have been authenticated as actually having come from
+  /// the sender in question.
+  ///
+  /// This will process the same blame statement multiple times, always identifying blame. It is
+  /// the caller's job to ensure they're unique in order to prevent multiple instances of blame
+  /// over a single incident.
+  pub fn blame(
+    self,
+    sender: u16,
+    recipient: u16,
+    msg: EncryptedMessage<C, SecretShare<C::F>>,
+    proof: Option<EncryptionKeyProof<C>>,
+  ) -> u16 {
+    self.0.blame_internal(sender, recipient, msg, proof)
+  }
+}

crypto/dkg/src/lib.rs

@@ -6,7 +6,10 @@
 //! Additional utilities around them, such as promotion from one generator to another, are also
 //! provided.
 
-use core::{fmt::Debug, ops::Deref};
+use core::{
+  fmt::{Debug, Formatter},
+  ops::Deref,
+};
 use std::{io::Read, sync::Arc, collections::HashMap};
 
 use thiserror::Error;
@@ -34,8 +37,8 @@ pub mod promote;
 pub mod tests;
 
 /// Various errors possible during key generation/signing.
-#[derive(Clone, Copy, PartialEq, Eq, Debug, Error)]
-pub enum DkgError {
+#[derive(Clone, PartialEq, Eq, Debug, Error)]
+pub enum DkgError<B: Clone + PartialEq + Eq + Debug> {
   #[error("a parameter was 0 (required {0}, participants {1})")]
   ZeroParameter(u16, u16),
   #[error("invalid amount of required participants (max {1}, got {0})")]
@@ -54,19 +57,19 @@ pub enum DkgError {
 
   #[error("invalid proof of knowledge (participant {0})")]
   InvalidProofOfKnowledge(u16),
-  #[error("invalid share (participant {0})")]
-  InvalidShare(u16),
+  #[error("invalid share (participant {participant}, blame {blame})")]
+  InvalidShare { participant: u16, blame: Option<B> },
 
   #[error("internal error ({0})")]
   InternalError(&'static str),
 }
 
 // Validate a map of values to have the expected included participants
-pub(crate) fn validate_map<T>(
+pub(crate) fn validate_map<T, B: Clone + PartialEq + Eq + Debug>(
   map: &HashMap<u16, T>,
   included: &[u16],
   ours: u16,
-) -> Result<(), DkgError> {
+) -> Result<(), DkgError<B>> {
   if (map.len() + 1) != included.len() {
     Err(DkgError::InvalidParticipantQuantity(included.len(), map.len() + 1))?;
   }
@@ -100,7 +103,7 @@ pub struct ThresholdParams {
 }
 
 impl ThresholdParams {
-  pub fn new(t: u16, n: u16, i: u16) -> Result<ThresholdParams, DkgError> {
+  pub fn new(t: u16, n: u16, i: u16) -> Result<ThresholdParams, DkgError<()>> {
     if (t == 0) || (n == 0) {
       Err(DkgError::ZeroParameter(t, n))?;
     }
@@ -149,7 +152,7 @@ pub fn lagrange<F: PrimeField>(i: u16, included: &[u16]) -> F {
 
 /// Keys and verification shares generated by a DKG.
 /// Called core as they're expected to be wrapped into an Arc before usage in various operations.
-#[derive(Clone, PartialEq, Eq, Debug)]
+#[derive(Clone, PartialEq, Eq)]
 pub struct ThresholdCore<C: Ciphersuite> {
   /// Threshold Parameters.
   params: ThresholdParams,
@@ -162,6 +165,17 @@ pub struct ThresholdCore<C: Ciphersuite> {
   verification_shares: HashMap<u16, C::G>,
 }
 
+impl<C: Ciphersuite> Debug for ThresholdCore<C> {
+  fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), core::fmt::Error> {
+    fmt
+      .debug_struct("ThresholdCore")
+      .field("params", &self.params)
+      .field("group_key", &self.group_key)
+      .field("verification_shares", &self.verification_shares)
+      .finish_non_exhaustive()
+  }
+}
+
 impl<C: Ciphersuite> Zeroize for ThresholdCore<C> {
   fn zeroize(&mut self) {
     self.params.zeroize();
@@ -179,8 +193,12 @@ impl<C: Ciphersuite> ThresholdCore<C> {
     secret_share: Zeroizing<C::F>,
     verification_shares: HashMap<u16, C::G>,
   ) -> ThresholdCore<C> {
-    #[cfg(debug_assertions)]
-    validate_map(&verification_shares, &(0 ..= params.n).collect::<Vec<_>>(), 0).unwrap();
+    debug_assert!(validate_map::<_, ()>(
+      &verification_shares,
+      &(0 ..= params.n).collect::<Vec<_>>(),
+      0
+    )
+    .is_ok());
 
     let t = (1 ..= params.t).collect::<Vec<_>>();
     ThresholdCore {
@@ -220,15 +238,15 @@ impl<C: Ciphersuite> ThresholdCore<C> {
     serialized
   }
 
-  pub fn deserialize<R: Read>(reader: &mut R) -> Result<ThresholdCore<C>, DkgError> {
+  pub fn deserialize<R: Read>(reader: &mut R) -> Result<ThresholdCore<C>, DkgError<()>> {
     {
       let missing = DkgError::InternalError("ThresholdCore serialization is missing its curve");
       let different = DkgError::InternalError("deserializing ThresholdCore for another curve");
 
       let mut id_len = [0; 4];
-      reader.read_exact(&mut id_len).map_err(|_| missing)?;
+      reader.read_exact(&mut id_len).map_err(|_| missing.clone())?;
       if u32::try_from(C::ID.len()).unwrap().to_be_bytes() != id_len {
-        Err(different)?;
+        Err(different.clone())?;
       }
 
       let mut id = vec![0; C::ID.len()];
@@ -273,27 +291,42 @@ impl<C: Ciphersuite> ThresholdCore<C> {
 /// Threshold keys usable for signing.
 #[derive(Clone, Debug, Zeroize)]
 pub struct ThresholdKeys<C: Ciphersuite> {
-  /// Core keys.
+  // Core keys.
+  // If this is the last reference, the underlying keys will be dropped. When that happens, the
+  // private key present within it will be zeroed out (as it's within Zeroizing).
   #[zeroize(skip)]
   core: Arc<ThresholdCore<C>>,
 
-  /// Offset applied to these keys.
+  // Offset applied to these keys.
   pub(crate) offset: Option<C::F>,
 }
 
 /// View of keys passed to algorithm implementations.
-#[derive(Clone, Zeroize)]
+#[derive(Clone)]
 pub struct ThresholdView<C: Ciphersuite> {
   offset: C::F,
   group_key: C::G,
   included: Vec<u16>,
   secret_share: Zeroizing<C::F>,
-  #[zeroize(skip)]
   original_verification_shares: HashMap<u16, C::G>,
-  #[zeroize(skip)]
   verification_shares: HashMap<u16, C::G>,
 }
 
+impl<C: Ciphersuite> Zeroize for ThresholdView<C> {
+  fn zeroize(&mut self) {
+    self.offset.zeroize();
+    self.group_key.zeroize();
+    self.included.zeroize();
+    self.secret_share.zeroize();
+    for (_, share) in self.original_verification_shares.iter_mut() {
+      share.zeroize();
+    }
+    for (_, share) in self.verification_shares.iter_mut() {
+      share.zeroize();
+    }
+  }
+}
+
 impl<C: Ciphersuite> ThresholdKeys<C> {
   pub fn new(core: ThresholdCore<C>) -> ThresholdKeys<C> {
     ThresholdKeys { core: Arc::new(core), offset: None }
@@ -338,7 +371,7 @@ impl<C: Ciphersuite> ThresholdKeys<C> {
     self.core.serialize()
   }
 
-  pub fn view(&self, included: &[u16]) -> Result<ThresholdView<C>, DkgError> {
+  pub fn view(&self, included: &[u16]) -> Result<ThresholdView<C>, DkgError<()>> {
     if (included.len() < self.params().t.into()) || (usize::from(self.params().n) < included.len())
     {
       Err(DkgError::InvalidSigningSet)?;

crypto/dkg/src/promote.rs

@@ -44,13 +44,13 @@ pub struct GeneratorProof<C: Ciphersuite> {
 impl<C: Ciphersuite> GeneratorProof<C> {
   pub fn write<W: Write>(&self, writer: &mut W) -> io::Result<()> {
     writer.write_all(self.share.to_bytes().as_ref())?;
-    self.proof.serialize(writer)
+    self.proof.write(writer)
   }
 
   pub fn read<R: Read>(reader: &mut R) -> io::Result<GeneratorProof<C>> {
     Ok(GeneratorProof {
       share: <C as Ciphersuite>::read_G(reader)?,
-      proof: DLEqProof::deserialize(reader)?,
+      proof: DLEqProof::read(reader)?,
     })
   }
 
@@ -98,7 +98,7 @@ where
   pub fn complete(
     self,
     proofs: &HashMap<u16, GeneratorProof<C1>>,
-  ) -> Result<ThresholdKeys<C2>, DkgError> {
+  ) -> Result<ThresholdKeys<C2>, DkgError<()>> {
     let params = self.base.params();
     validate_map(proofs, &(1 ..= params.n).collect::<Vec<_>>(), params.i)?;
 

crypto/dkg/src/tests/frost.rs

@@ -4,17 +4,23 @@ use rand_core::{RngCore, CryptoRng};
 
 use crate::{
   Ciphersuite, ThresholdParams, ThresholdCore,
-  frost::KeyGenMachine,
+  frost::{KeyGenMachine, SecretShare, KeyMachine},
   encryption::{EncryptionKeyMessage, EncryptedMessage},
   tests::{THRESHOLD, PARTICIPANTS, clone_without},
 };
 
-/// Fully perform the FROST key generation algorithm.
-pub fn frost_gen<R: RngCore + CryptoRng, C: Ciphersuite>(
+// Needed so rustfmt doesn't fail to format on line length issues
+type FrostEncryptedMessage<C> = EncryptedMessage<C, SecretShare<<C as Ciphersuite>::F>>;
+type FrostSecretShares<C> = HashMap<u16, FrostEncryptedMessage<C>>;
+
+// Commit, then return enc key and shares
+#[allow(clippy::type_complexity)]
+fn commit_enc_keys_and_shares<R: RngCore + CryptoRng, C: Ciphersuite>(
   rng: &mut R,
-) -> HashMap<u16, ThresholdCore<C>> {
+) -> (HashMap<u16, KeyMachine<C>>, HashMap<u16, C::G>, HashMap<u16, FrostSecretShares<C>>) {
   let mut machines = HashMap::new();
   let mut commitments = HashMap::new();
+  let mut enc_keys = HashMap::new();
   for i in 1 ..= PARTICIPANTS {
     let machine = KeyGenMachine::<C>::new(
       ThresholdParams::new(THRESHOLD, PARTICIPANTS, i).unwrap(),
@@ -31,10 +37,11 @@ pub fn frost_gen<R: RngCore + CryptoRng, C: Ciphersuite>(
       )
       .unwrap(),
     );
+    enc_keys.insert(i, commitments[&i].enc_key());
   }
 
   let mut secret_shares = HashMap::new();
-  let mut machines = machines
+  let machines = machines
     .drain()
     .map(|(l, machine)| {
       let (machine, mut shares) =
@@ -57,19 +64,36 @@ pub fn frost_gen<R: RngCore + CryptoRng, C: Ciphersuite>(
     })
     .collect::<HashMap<_, _>>();
 
+  (machines, enc_keys, secret_shares)
+}
+
+fn generate_secret_shares<C: Ciphersuite>(
+  shares: &HashMap<u16, FrostSecretShares<C>>,
+  recipient: u16,
+) -> FrostSecretShares<C> {
+  let mut our_secret_shares = HashMap::new();
+  for (i, shares) in shares {
+    if recipient == *i {
+      continue;
+    }
+    our_secret_shares.insert(*i, shares[&recipient].clone());
+  }
+  our_secret_shares
+}
+
+/// Fully perform the FROST key generation algorithm.
+pub fn frost_gen<R: RngCore + CryptoRng, C: Ciphersuite>(
+  rng: &mut R,
+) -> HashMap<u16, ThresholdCore<C>> {
+  let (mut machines, _, secret_shares) = commit_enc_keys_and_shares::<_, C>(rng);
+
   let mut verification_shares = None;
   let mut group_key = None;
   machines
     .drain()
     .map(|(i, machine)| {
-      let mut our_secret_shares = HashMap::new();
-      for (l, shares) in &secret_shares {
-        if i == *l {
-          continue;
-        }
-        our_secret_shares.insert(*l, shares[&i].clone());
-      }
-      let these_keys = machine.complete(rng, our_secret_shares).unwrap();
+      let our_secret_shares = generate_secret_shares(&secret_shares, i);
+      let these_keys = machine.calculate_share(rng, our_secret_shares).unwrap().complete();
 
       // Verify the verification_shares are agreed upon
       if verification_shares.is_none() {
@@ -87,3 +111,188 @@ pub fn frost_gen<R: RngCore + CryptoRng, C: Ciphersuite>(
     })
     .collect::<HashMap<_, _>>()
 }
+
+#[cfg(test)]
+mod literal {
+  use rand_core::OsRng;
+
+  use ciphersuite::Ristretto;
+
+  use crate::{DkgError, encryption::EncryptionKeyProof, frost::BlameMachine};
+
+  use super::*;
+
+  fn test_blame(
+    machines: Vec<BlameMachine<Ristretto>>,
+    msg: FrostEncryptedMessage<Ristretto>,
+    blame: Option<EncryptionKeyProof<Ristretto>>,
+  ) {
+    for machine in machines {
+      let (additional, blamed) = machine.blame(1, 2, msg.clone(), blame.clone());
+      assert_eq!(blamed, 1);
+      // Verify additional blame also works
+      assert_eq!(additional.blame(1, 2, msg.clone(), blame.clone()), 1);
+    }
+  }
+
+  // TODO: Write a macro which expands to the following
+  #[test]
+  fn invalid_encryption_pop_blame() {
+    let (mut machines, _, mut secret_shares) =
+      commit_enc_keys_and_shares::<_, Ristretto>(&mut OsRng);
+
+    // Mutate the PoP of the encrypted message from 1 to 2
+    secret_shares.get_mut(&1).unwrap().get_mut(&2).unwrap().invalidate_pop();
+
+    let mut blame = None;
+    let machines = machines
+      .drain()
+      .filter_map(|(i, machine)| {
+        let our_secret_shares = generate_secret_shares(&secret_shares, i);
+        let machine = machine.calculate_share(&mut OsRng, our_secret_shares);
+        if i == 2 {
+          assert_eq!(machine.err(), Some(DkgError::InvalidShare { participant: 1, blame: None }));
+          // Explicitly declare we have a blame object, which happens to be None since invalid PoP
+          // is self-explainable
+          blame = Some(None);
+          None
+        } else {
+          Some(machine.unwrap())
+        }
+      })
+      .collect::<Vec<_>>();
+
+    test_blame(machines, secret_shares[&1][&2].clone(), blame.unwrap());
+  }
+
+  #[test]
+  fn invalid_ecdh_blame() {
+    let (mut machines, _, mut secret_shares) =
+      commit_enc_keys_and_shares::<_, Ristretto>(&mut OsRng);
+
+    // Mutate the share to trigger a blame event
+    // Mutates from 2 to 1, as 1 is expected to end up malicious for test_blame to pass
+    // While here, 2 is malicious, this is so 1 creates the blame proof
+    // We then malleate 1's blame proof, so 1 ends up malicious
+    // Doesn't simply invalidate the PoP as that won't have a blame statement
+    // By mutating the encrypted data, we do ensure a blame statement is created
+    secret_shares.get_mut(&2).unwrap().get_mut(&1).unwrap().invalidate_msg(&mut OsRng, 2);
+
+    let mut blame = None;
+    let machines = machines
+      .drain()
+      .filter_map(|(i, machine)| {
+        let our_secret_shares = generate_secret_shares(&secret_shares, i);
+        let machine = machine.calculate_share(&mut OsRng, our_secret_shares);
+        if i == 1 {
+          blame = Some(match machine.err() {
+            Some(DkgError::InvalidShare { participant: 2, blame: Some(blame) }) => Some(blame),
+            _ => panic!(),
+          });
+          None
+        } else {
+          Some(machine.unwrap())
+        }
+      })
+      .collect::<Vec<_>>();
+
+    blame.as_mut().unwrap().as_mut().unwrap().invalidate_key();
+    test_blame(machines, secret_shares[&2][&1].clone(), blame.unwrap());
+  }
+
+  // This should be largely equivalent to the prior test
+  #[test]
+  fn invalid_dleq_blame() {
+    let (mut machines, _, mut secret_shares) =
+      commit_enc_keys_and_shares::<_, Ristretto>(&mut OsRng);
+
+    secret_shares.get_mut(&2).unwrap().get_mut(&1).unwrap().invalidate_msg(&mut OsRng, 2);
+
+    let mut blame = None;
+    let machines = machines
+      .drain()
+      .filter_map(|(i, machine)| {
+        let our_secret_shares = generate_secret_shares(&secret_shares, i);
+        let machine = machine.calculate_share(&mut OsRng, our_secret_shares);
+        if i == 1 {
+          blame = Some(match machine.err() {
+            Some(DkgError::InvalidShare { participant: 2, blame: Some(blame) }) => Some(blame),
+            _ => panic!(),
+          });
+          None
+        } else {
+          Some(machine.unwrap())
+        }
+      })
+      .collect::<Vec<_>>();
+
+    blame.as_mut().unwrap().as_mut().unwrap().invalidate_dleq();
+    test_blame(machines, secret_shares[&2][&1].clone(), blame.unwrap());
+  }
+
+  #[test]
+  fn invalid_share_serialization_blame() {
+    let (mut machines, enc_keys, mut secret_shares) =
+      commit_enc_keys_and_shares::<_, Ristretto>(&mut OsRng);
+
+    secret_shares.get_mut(&1).unwrap().get_mut(&2).unwrap().invalidate_share_serialization(
+      &mut OsRng,
+      b"FROST",
+      1,
+      enc_keys[&2],
+    );
+
+    let mut blame = None;
+    let machines = machines
+      .drain()
+      .filter_map(|(i, machine)| {
+        let our_secret_shares = generate_secret_shares(&secret_shares, i);
+        let machine = machine.calculate_share(&mut OsRng, our_secret_shares);
+        if i == 2 {
+          blame = Some(match machine.err() {
+            Some(DkgError::InvalidShare { participant: 1, blame: Some(blame) }) => Some(blame),
+            _ => panic!(),
+          });
+          None
+        } else {
+          Some(machine.unwrap())
+        }
+      })
+      .collect::<Vec<_>>();
+
+    test_blame(machines, secret_shares[&1][&2].clone(), blame.unwrap());
+  }
+
+  #[test]
+  fn invalid_share_value_blame() {
+    let (mut machines, enc_keys, mut secret_shares) =
+      commit_enc_keys_and_shares::<_, Ristretto>(&mut OsRng);
+
+    secret_shares.get_mut(&1).unwrap().get_mut(&2).unwrap().invalidate_share_value(
+      &mut OsRng,
+      b"FROST",
+      1,
+      enc_keys[&2],
+    );
+
+    let mut blame = None;
+    let machines = machines
+      .drain()
+      .filter_map(|(i, machine)| {
+        let our_secret_shares = generate_secret_shares(&secret_shares, i);
+        let machine = machine.calculate_share(&mut OsRng, our_secret_shares);
+        if i == 2 {
+          blame = Some(match machine.err() {
+            Some(DkgError::InvalidShare { participant: 1, blame: Some(blame) }) => Some(blame),
+            _ => panic!(),
+          });
+          None
+        } else {
+          Some(machine.unwrap())
+        }
+      })
+      .collect::<Vec<_>>();
+
+    test_blame(machines, secret_shares[&1][&2].clone(), blame.unwrap());
+  }
+}