fix: reject identity keys and commitments in signatures

src/ristretto/ristretto_com_and_pub_sig.rs

@@ -82,10 +82,11 @@ pub type RistrettoComAndPubSig = CommitmentAndPublicKeySignature<RistrettoPublic
 mod test {
     use blake2::Blake2b;
     use digest::{consts::U64, Digest};
+    use rand_core::RngCore;
     use tari_utilities::ByteArray;
 
     use crate::{
-        commitment::HomomorphicCommitmentFactory,
+        commitment::{HomomorphicCommitment, HomomorphicCommitmentFactory},
         keys::{PublicKey, SecretKey},
         ristretto::{
             pedersen::{commitment_factory::PedersenCommitmentFactory, PedersenCommitment},
@@ -359,4 +360,70 @@ mod test {
         assert_eq!(bytes.capacity(), bytes.len());
         assert!(bytes.iter().all(|b| *b == 0x00));
     }
+
+    #[test]
+    fn zero_commitment() {
+        let mut rng = rand::thread_rng();
+        let factory = PedersenCommitmentFactory::default();
+
+        // Generate a zero commitment opening and a random key
+        let a = RistrettoSecretKey::default();
+        let x = RistrettoSecretKey::default();
+        let commitment = factory.commit(&x, &a);
+        assert_eq!(commitment, HomomorphicCommitment::<RistrettoPublicKey>::default());
+
+        let y = RistrettoSecretKey::random(&mut rng);
+        let public_key = RistrettoPublicKey::from_secret_key(&y);
+
+        // Generate a signature with the zero opening and key
+        let mut challenge = [0u8; RistrettoSecretKey::WIDE_REDUCTION_LEN];
+        rng.fill_bytes(&mut challenge);
+        let sig = RistrettoComAndPubSig::sign(
+            &a,
+            &x,
+            &y,
+            &RistrettoSecretKey::random(&mut rng),
+            &RistrettoSecretKey::random(&mut rng),
+            &RistrettoSecretKey::random(&mut rng),
+            &challenge,
+            &factory,
+        )
+        .unwrap();
+
+        // The signature should fail to verify
+        assert!(!sig.verify_challenge(&commitment, &public_key, &challenge, &factory, &mut rng));
+    }
+
+    #[test]
+    fn zero_public_key() {
+        let mut rng = rand::thread_rng();
+        let factory = PedersenCommitmentFactory::default();
+
+        // Generate a random commitment opening and a zero key
+        let a = RistrettoSecretKey::random(&mut rng);
+        let x = RistrettoSecretKey::random(&mut rng);
+        let commitment = factory.commit(&x, &a);
+
+        let y = RistrettoSecretKey::default();
+        let public_key = RistrettoPublicKey::from_secret_key(&y);
+        assert_eq!(public_key, RistrettoPublicKey::default());
+
+        // Generate a signature with the opening and zero key
+        let mut challenge = [0u8; RistrettoSecretKey::WIDE_REDUCTION_LEN];
+        rng.fill_bytes(&mut challenge);
+        let sig = RistrettoComAndPubSig::sign(
+            &a,
+            &x,
+            &y,
+            &RistrettoSecretKey::random(&mut rng),
+            &RistrettoSecretKey::random(&mut rng),
+            &RistrettoSecretKey::random(&mut rng),
+            &challenge,
+            &factory,
+        )
+        .unwrap();
+
+        // The signature should fail to verify
+        assert!(!sig.verify_challenge(&commitment, &public_key, &challenge, &factory, &mut rng));
+    }
 }

src/ristretto/ristretto_com_sig.rs

@@ -107,10 +107,11 @@ pub type RistrettoComSig = CommitmentSignature<RistrettoPublicKey, RistrettoSecr
 mod test {
     use blake2::Blake2b;
     use digest::{consts::U64, Digest};
+    use rand_core::RngCore;
     use tari_utilities::ByteArray;
 
     use crate::{
-        commitment::HomomorphicCommitmentFactory,
+        commitment::{HomomorphicCommitment, HomomorphicCommitmentFactory},
         keys::{PublicKey, SecretKey},
         ristretto::{
             pedersen::{commitment_factory::PedersenCommitmentFactory, PedersenCommitment},
@@ -228,4 +229,32 @@ mod test {
         assert_eq!(bytes.capacity(), bytes.len());
         assert!(bytes.iter().all(|b| *b == 0x00));
     }
+
+    #[test]
+    fn zero_commitment() {
+        let mut rng = rand::thread_rng();
+        let factory = PedersenCommitmentFactory::default();
+
+        // Generate a zero commitment opening
+        let secret_a = RistrettoSecretKey::default();
+        let secret_x = RistrettoSecretKey::default();
+        let commitment = factory.commit(&secret_x, &secret_a);
+        assert_eq!(commitment, HomomorphicCommitment::<RistrettoPublicKey>::default());
+
+        // Generate a signature with the zero opening
+        let mut challenge = [0u8; RistrettoSecretKey::WIDE_REDUCTION_LEN];
+        rng.fill_bytes(&mut challenge);
+        let sig = RistrettoComSig::sign(
+            &secret_a,
+            &secret_x,
+            &RistrettoSecretKey::random(&mut rng),
+            &RistrettoSecretKey::random(&mut rng),
+            &challenge,
+            &factory,
+        )
+        .unwrap();
+
+        // The signature should fail to verify
+        assert!(!sig.verify_challenge(&commitment, &challenge, &factory));
+    }
 }

src/ristretto/ristretto_sig.rs

@@ -263,4 +263,21 @@ mod test {
         assert!(!sig.verify(&P, "Qs are things that happen to other people"));
         assert!(!sig.verify(&(&P + &P), "Queues are things that happen to other people"));
     }
+
+    #[test]
+    fn zero_public_key() {
+        let mut rng = rand::thread_rng();
+
+        // Generate a zero key
+        let secret_key = RistrettoSecretKey::default();
+        let public_key = RistrettoPublicKey::from_secret_key(&secret_key);
+        assert_eq!(public_key, RistrettoPublicKey::default());
+
+        // Sign a message with the zero key
+        let message = "A secret message";
+        let sig = RistrettoSchnorr::sign(&secret_key, message, &mut rng).unwrap();
+
+        // The signature should fail to verify
+        assert!(!sig.verify(&public_key, message,));
+    }
 }

src/signatures/commitment_and_public_key_signature.rs

@@ -176,6 +176,11 @@ where
         C: HomomorphicCommitmentFactory<P = P>,
         R: RngCore + CryptoRng,
     {
+        // Reject a zero commitment and public key
+        if commitment.as_public_key() == &P::default() || pubkey == &P::default() {
+            return false;
+        }
+
         // The challenge cannot be zero
         if *challenge == K::default() {
             return false;

src/signatures/commitment_signature.rs

@@ -136,6 +136,11 @@ where
         for<'b> &'b HomomorphicCommitment<P>: Add<&'b HomomorphicCommitment<P>, Output = HomomorphicCommitment<P>>,
         C: HomomorphicCommitmentFactory<P = P>,
     {
+        // Reject a zero commitment
+        if public_commitment.as_public_key() == &P::default() {
+            return false;
+        }
+
         // v*H + u*G
         let lhs = self.calc_signature_verifier(factory);
         // R + e.C

src/signatures/schnorr.rs

@@ -24,7 +24,8 @@ use crate::{
     keys::{PublicKey, SecretKey},
 };
 
-// Define the hashing domain for Schnorr signatures
+// Define a default hashing domain for Schnorr signatures
+// You almost certainly want to define your own that is specific to signature context!
 hash_domain!(SchnorrSigChallenge, "com.tari.schnorr_signature", 1);
 
 /// An error occurred during construction of a SchnorrSignature
@@ -231,6 +232,11 @@ where
         for<'b> &'b K: Mul<&'a P, Output = P>,
         for<'b> &'b P: Add<P, Output = P>,
     {
+        // Reject a zero key
+        if public_key == &P::default() {
+            return false;
+        }
+
         let lhs = self.calc_signature_verifier();
         let rhs = &self.public_nonce + challenge * public_key;
         // Implementors should make this a constant time comparison