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,66 @@ 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();
+
+        // Manually generate a signature on a zero commitment
+        let y = RistrettoSecretKey::random(&mut rng);
+        let r_a = RistrettoSecretKey::random(&mut rng);
+        let r_x = RistrettoSecretKey::random(&mut rng);
+        let r_y = RistrettoSecretKey::random(&mut rng);
+        let ephemeral_commitment = factory.commit(&r_x, &r_a);
+        let ephemeral_pubkey = RistrettoPublicKey::from_secret_key(&r_y);
+        let mut challenge = [0u8; RistrettoSecretKey::WIDE_REDUCTION_LEN];
+        rng.fill_bytes(&mut challenge);
+        let u_a = r_a;
+        let u_x = r_x;
+        let u_y = r_y + RistrettoSecretKey::from_uniform_bytes(&challenge).unwrap() * &y;
+        let sig = RistrettoComAndPubSig::new(ephemeral_commitment, ephemeral_pubkey, u_a, u_x, u_y);
+
+        assert!(
+            !sig.verify_challenge(
+                &HomomorphicCommitment::<RistrettoPublicKey>::default(),
+                &RistrettoPublicKey::from_secret_key(&y),
+                &challenge,
+                &factory,
+                &mut rng
+            )
+        );
+    }
+
+    #[test]
+    fn zero_public_key() {
+        let mut rng = rand::thread_rng();
+        let factory = PedersenCommitmentFactory::default();
+
+        // Manually generate a signature on a zero public key
+        let a = RistrettoSecretKey::random(&mut rng);
+        let x = RistrettoSecretKey::random(&mut rng);
+        let r_a = RistrettoSecretKey::random(&mut rng);
+        let r_x = RistrettoSecretKey::random(&mut rng);
+        let r_y = RistrettoSecretKey::random(&mut rng);
+        let ephemeral_commitment = factory.commit(&r_x, &r_a);
+        let ephemeral_pubkey = RistrettoPublicKey::from_secret_key(&r_y);
+        let mut challenge = [0u8; RistrettoSecretKey::WIDE_REDUCTION_LEN];
+        rng.fill_bytes(&mut challenge);
+        let challenge_scalar = RistrettoSecretKey::from_uniform_bytes(&challenge).unwrap();
+        let u_a = r_a + &challenge_scalar * &a;
+        let u_x = r_x + &challenge_scalar * &x;
+        let u_y = r_y;
+        let sig = RistrettoComAndPubSig::new(ephemeral_commitment, ephemeral_pubkey, u_a, u_x, u_y);
+
+        assert!(
+            !sig.verify_challenge(
+                &factory.commit(&x, &a),
+                &RistrettoPublicKey::default(),
+                &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,28 @@ 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();
+
+        // Manually generate a signature on a zero commitment
+        let nonce_a = RistrettoSecretKey::random(&mut rng);
+        let nonce_x = RistrettoSecretKey::random(&mut rng);
+        let ephemeral_commitment = factory.commit(&nonce_x, &nonce_a);
+        let mut challenge = [0u8; RistrettoSecretKey::WIDE_REDUCTION_LEN];
+        rng.fill_bytes(&mut challenge);
+        let v = nonce_a;
+        let u = nonce_x;
+        let sig = RistrettoComSig::new(ephemeral_commitment, u, v);
+
+        assert!(
+            !sig.verify_challenge(
+                &HomomorphicCommitment::<RistrettoPublicKey>::default(),
+                &challenge,
+                &factory
+            )
+        );
+    }
 }

src/ristretto/ristretto_sig.rs

@@ -263,4 +263,22 @@ 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();
+
+        // Manually generate a signature on a zero key
+        let r = RistrettoSecretKey::random(&mut rng);
+        let sig_r = RistrettoPublicKey::from_secret_key(&r);
+        let sig_s = r;
+        let sig = RistrettoSchnorr::new(sig_r, sig_s);
+
+        assert!(
+            !sig.verify(
+                &RistrettoPublicKey::default(),
+                "The signature should fail to verify against any 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