Addressing audits findings

src/chaum_pedersen_signature.rs

@@ -13,7 +13,7 @@ use crate::dual_scalar_mul::DualScalarMultiplication;
 use crate::engine::EngineBLS;
 use crate::nugget::{NuggetBLS, NuggetPublicKey, NuggetSignature};
 use crate::serialize::SerializableToBytes;
-use crate::{Message, SecretKeyVT};
+use crate::{Message, SecretKey, SecretKeyVT};
 
 pub type DLEQProof<E> = (<E as EngineBLS>::Scalar, <E as EngineBLS>::Scalar);
 pub type ChaumPedersenSignature<E> = NuggetSignature<E>;
@@ -322,10 +322,12 @@ where
         &self,
         message_point_as_bytes: &Vec<u8>,
     ) -> <<E as EngineBLS>::PublicKeyGroup as PrimeGroup>::ScalarField {
-        let secret_key_as_bytes = self.to_bytes();
-
         let mut secret_key_hasher = H::default();
+
+        let mut secret_key_as_bytes = self.to_bytes();
         secret_key_hasher.update(secret_key_as_bytes.as_slice());
+        ::zeroize::Zeroize::zeroize(secret_key_as_bytes.as_mut_slice());
+
         let hashed_secret_key = secret_key_hasher.finalize_fixed_reset().to_vec();
 
         let hasher = <DefaultFieldHasher<H> as HashToField<
@@ -336,6 +338,53 @@ where
     }
 }
 
+/// Side-channel-protected variant: BLS signing goes through
+/// `SecretKey::seeded_sign` (resplits the key with deterministic
+/// randomness), while DLEQ proof generation and witness derivation
+/// delegate to the vartime form, which is acceptable because they
+/// operate over auxiliary scalars rather than the long-term key.
+impl<E: EngineBLS, S: CurveGroup, H: FixedOutputReset + Default + Clone>
+    ChaumPedersenSigner<E, S, H> for SecretKey<E>
+where
+    S: PrimeGroup<ScalarField = E::Scalar> + SerializableToBytes,
+{
+    fn generate_cp_signature(&mut self, message: &Message) -> ChaumPedersenSignature<E> {
+        let bls_signature = self.seeded_sign(message);
+        let dleq = <Self as ChaumPedersenSigner<E, S, H>>::generate_dleq_proof(
+            self,
+            message,
+            bls_signature.0,
+        );
+        NuggetSignature(bls_signature.0, dleq)
+    }
+
+    fn generate_dleq_proof(
+        &mut self,
+        message: &Message,
+        bls_signature: E::SignatureGroup,
+    ) -> DLEQProof<E> {
+        <SecretKeyVT<E> as ChaumPedersenSigner<E, S, H>>::generate_dleq_proof(
+            &mut self.into_vartime(),
+            message,
+            bls_signature,
+        )
+    }
+
+    fn generate_witness_scaler(
+        &self,
+        _message_point_as_bytes: &Vec<u8>,
+    ) -> <<E as EngineBLS>::PublicKeyGroup as PrimeGroup>::ScalarField {
+        // Unreachable: `generate_dleq_proof` for `SecretKey` delegates to
+        // the `SecretKeyVT` impl, which calls its own `generate_witness_scaler`.
+        // Trait coherence forces us to provide a body here, but no caller
+        // ever lands on it.
+        unimplemented!(
+            "SecretKey::generate_witness_scaler is never called; \
+             dleq generation delegates to SecretKeyVT"
+        )
+    }
+}
+
 #[cfg(all(test, feature = "std"))]
 mod tests {
     use rand::thread_rng;

src/engine.rs

@@ -32,7 +32,7 @@ use ark_ec::{
 };
 use ark_ff::field_hashers::{DefaultFieldHasher, HashToField};
 use ark_ff::{Field, PrimeField, UniformRand, Zero};
-use ark_serialize::CanonicalSerialize;
+use ark_serialize::{CanonicalSerialize, Valid};
 use rand::Rng;
 use rand_core::RngCore;
 
@@ -198,8 +198,7 @@ pub trait EngineBLS {
             signature,
         )];
         Self::final_exponentiation(Self::miller_loop(inputs.into_iter().map(|t| t).chain(&lhs)))
-            .unwrap()
-            == (PairingOutput::<Self::Engine>::zero()) //zero is the target_field::one !!
+            == Some(PairingOutput::<Self::Engine>::zero()) //zero is the target_field::one !!
     }
 
     /// Prepared negative of the generator of the public key curve.
@@ -219,6 +218,17 @@ pub trait EngineBLS {
         Self::PublicKeyPrepared::from(g_affine)
     }
 
+    /// Reject public keys that are the identity element or not in the
+    /// prime-order subgroup. Defends verification against inputs that
+    /// bypass the deserialization-time check — e.g. direct tuple-struct
+    /// construction, or aggregates that sum to the identity.
+    ///
+    /// Implements `KeyValidate` from
+    /// <https://www.ietf.org/archive/id/draft-irtf-cfrg-bls-signature-06.html#name-validating-public-keys>.
+    fn validate_public_key(g: &Self::PublicKeyGroupAffine) -> bool {
+        !g.is_zero() && g.check().is_ok()
+    }
+
     /// Process the signature to be use in pairing. This has to be
     /// implemented by the type of BLS system implementing the engine
     /// by calling either prepare_g1 or prepare_g2 based on which group

src/experimental/bit.rs

@@ -632,7 +632,9 @@ mod tests {
             .collect::<Vec<_>>();
 
         let mut bitsig1 = BitSignedMessage::<ZBLS, _>::new(pop.clone(), &msg1);
-        assert!(bitsig1.verify()); // verifiers::verify_with_distinct_messages(&dms,true)
+        // Empty aggregate: the summed public key is the identity element,
+        // which `validate_public_key` rejects per IETF KeyValidate.
+        assert!(!bitsig1.verify());
         for (i, sig) in sigs1.iter().enumerate().take(2) {
             assert!(bitsig1.add(sig).is_ok() == (i < 4));
             assert!(bitsig1.verify()); // verifiers::verify_with_distinct_messages(&dms,true)
@@ -687,7 +689,9 @@ mod tests {
 
         let mut countsig = CountSignedMessage::<ZBLS, _>::new(pop.clone(), msg1);
         assert!(countsig.signers.len() == 1);
-        assert!(countsig.verify()); // verifiers::verify_with_distinct_messages(&dms,true)
+        // Empty aggregate: the summed public key is the identity element,
+        // which `validate_public_key` rejects per IETF KeyValidate.
+        assert!(!countsig.verify());
         assert!(countsig.add_bitsig(&bitsig1).is_ok());
         assert!(bitsig1.signature == countsig.signature);
         assert!(countsig.signers.len() == 1);

src/experimental/schnorr_pop.rs

@@ -41,11 +41,13 @@ impl<E: EngineBLS, H: FixedOutputReset + Default + Clone + 'static> BLSSchnorrPo
     //The pseudo random witness is generated similar to eddsa witness
     //hash(secret_key|publick_key)
     fn witness_scalar(&self) -> <<E as EngineBLS>::PublicKeyGroup as PrimeGroup>::ScalarField {
-        let secret_key_as_bytes = self.secret.to_bytes();
+        let mut secret_key_as_bytes = self.secret.to_bytes();
         let public_key_as_bytes = <E as EngineBLS>::public_key_point_to_byte(&self.public.0);
 
         let mut secret_key_hasher = H::default();
         secret_key_hasher.update(secret_key_as_bytes.as_slice());
+        ::zeroize::Zeroize::zeroize(secret_key_as_bytes.as_mut_slice());
+
         let hashed_secret_key = secret_key_hasher.finalize_fixed_reset().to_vec();
 
         let hasher = <DefaultFieldHasher<H> as HashToField<

src/lib.rs

@@ -215,7 +215,8 @@ impl Message {
     fn cipher_suite<E: EngineBLS>(&self) -> Vec<u8> {
         let id = match self.2 {
             MessageType::ProofOfPossession => PROOF_OF_POSSESSION_ID,
-            _ => NORMAL_MESSAGE_SIGNATURE_ID,
+            MessageType::NormalAssumingPoP => NORMAL_MESSAGE_SIGNATURE_ID,
+            MessageType::NormalBasic => NORMAL_MESSAGE_SIGNATURE_ID,
         };
 
         let h2c_suite_id = [
@@ -228,7 +229,7 @@ impl Message {
         let sc_tag = match self.2 {
             MessageType::ProofOfPossession => POP_MESSAGE,
             MessageType::NormalAssumingPoP => NORMAL_MESSAGE_SIGNATURE_ASSUMING_POP,
-            _ => NORMAL_MESSAGE_SIGNATURE_BASIC,
+            MessageType::NormalBasic => NORMAL_MESSAGE_SIGNATURE_BASIC,
         };
 
         [id, &h2c_suite_id[..], sc_tag].concat()

src/nugget.rs

@@ -25,7 +25,7 @@ use crate::chaum_pedersen_signature::{ChaumPedersenSigner, ChaumPedersenVerifier
 use crate::dual_scalar_mul::DualScalarMultiplication;
 use crate::chaum_pedersen_signature::DLEQProof;
 use crate::serialize::SerializableToBytes;
-use crate::single::{Keypair, KeypairVT, PublicKey, SecretKeyVT, Signature};
+use crate::single::{Keypair, KeypairVT, PublicKey, SecretKey, SecretKeyVT, Signature};
 use crate::{EngineBLS, Message, Signed};
 
 /// Wrapper for a point in the signature group which is supposed to
@@ -89,6 +89,26 @@ where
     }
 }
 
+/// Side-channel-protected variant: signing goes through the
+/// `ChaumPedersenSigner` impl for `SecretKey`, so the resplit happens
+/// on the split key (no `into_vartime` conversion is done here).
+impl<E: EngineBLS, S: CurveGroup> NuggetBLS<E, S> for SecretKey<E>
+where
+    S: PrimeGroup<ScalarField = E::Scalar> + SerializableToBytes,
+{
+    fn into_public_key_in_signature_group(&self) -> PublicKeyInSignatureGroup<E> {
+        NuggetBLS::<E, S>::into_public_key_in_signature_group(&self.into_vartime())
+    }
+
+    fn into_public_key_in_sister_group(&self) -> PublicKeyInSisterGroup<S> {
+        self.into_vartime().into_public_key_in_sister_group()
+    }
+
+    fn sign(&mut self, message: &Message) -> NuggetSignature<E> {
+        ChaumPedersenSigner::<E, S, Sha256>::generate_cp_signature(self, &message)
+    }
+}
+
 impl<E: EngineBLS, S: CurveGroup> NuggetBLS<E, S> for KeypairVT<E>
 where
     S: PrimeGroup<ScalarField = E::Scalar> + SerializableToBytes,
@@ -112,16 +132,16 @@ where
     S: PrimeGroup<ScalarField = E::Scalar> + SerializableToBytes,
 {
     fn into_public_key_in_signature_group(&self) -> PublicKeyInSignatureGroup<E> {
-        NuggetBLS::<E, S>::into_public_key_in_signature_group(&self.into_vartime())
+        NuggetBLS::<E, S>::into_public_key_in_signature_group(&self.secret)
     }
 
     fn into_public_key_in_sister_group(&self) -> PublicKeyInSisterGroup<S> {
-        self.into_vartime().into_public_key_in_sister_group()
+        NuggetBLS::<E, S>::into_public_key_in_sister_group(&self.secret)
     }
 
     /// Sign a message using a Seedabale RNG created from a seed derived from the message and key
     fn sign(&mut self, message: &Message) -> NuggetSignature<E> {
-        NuggetBLS::<E, S>::sign(&mut self.into_vartime(), message)
+        NuggetBLS::<E, S>::sign(&mut self.secret, message)
     }
 }
 

src/nugget_pop.rs

@@ -92,7 +92,11 @@ impl<E: EngineBLS, H: FixedOutputReset + Default + Clone + 'static>
         let mut randomized_pub_in_g1 = public_key_in_signature_group;
         randomized_pub_in_g1 *= randomization_coefficient;
         let signature = E::prepare_signature(self.0 + randomized_pub_in_g1);
-        let prepared_public_key = E::prepare_public_key(public_key_of_prover.1);
+        let Some(prepared_public_key) =
+            crate::verifiers::validate_and_prepare_public_key::<E>(public_key_of_prover.1)
+        else {
+            return false;
+        };
         let prepared = [
             (
                 prepared_public_key.clone(),
@@ -162,12 +166,13 @@ where
 #[cfg(all(test, feature = "std"))]
 mod tests {
     use crate::double_nugget::DoubleNuggetBLS;
-    use crate::engine::TinyBLS381;
+    use crate::engine::{EngineBLS, TinyBLS381};
     use crate::serialize::SerializableToBytes;
     use crate::single::Keypair;
     use crate::{nugget_pop::NuggetBLSPoP, NuggetDoublePublicKey};
     use crate::{ProofOfPossession, ProofOfPossessionGenerator};
 
+    use ark_ff::Zero;
     use rand::thread_rng;
     use sha2::Sha256;
 
@@ -324,4 +329,35 @@ mod tests {
             NuggetBLSnCPPoP<TinyBLS381>,
         >();
     }
+
+    /// A keypair with secret scalar zero and identity public key.
+    /// `SecretKeyVT::sign` returns `sk * H(msg) = identity` for such
+    /// a keypair, so the PoP produced by `generate_pok` is itself
+    /// identity. The pairing equation in `NuggetBLSPoP::verify` then
+    /// reduces to `identity == identity`, so without `validate_public_key`
+    /// the verifier would accept — rejection here is attributable to
+    /// the public key check.
+    #[test]
+    fn nugget_bls_pop_rejects_identity_pk() {
+        use crate::single::{PublicKey, SecretKeyVT};
+        let mut keypair = Keypair::<TinyBLS381> {
+            public: PublicKey::<TinyBLS381>(
+                <TinyBLS381 as EngineBLS>::PublicKeyGroup::zero(),
+            ),
+            secret: SecretKeyVT::<TinyBLS381>(<TinyBLS381 as EngineBLS>::Scalar::zero())
+                .into_split(thread_rng()),
+        };
+        let pop = <Keypair<TinyBLS381> as ProofOfPossessionGenerator<
+            TinyBLS381,
+            Sha256,
+            NuggetDoublePublicKey<TinyBLS381>,
+            NuggetBLSPoP<TinyBLS381>,
+        >>::generate_pok(&mut keypair);
+        let double_pk = DoubleNuggetBLS::into_nugget_double_public_key(&keypair);
+        assert!(!<NuggetBLSPoP<TinyBLS381> as ProofOfPossession<
+            TinyBLS381,
+            Sha256,
+            NuggetDoublePublicKey<TinyBLS381>,
+        >>::verify(&pop, &double_pk));
+    }
 }