fix: reject identity keys and commitments in signatures (#217)

Signature verification currently allows identity elements:
- `SchnorrSignature` allows an identity public key
- `CommitmentSignature` allows an identity commitment
- `CommitmentAndPublicKeySignature` allows both

This doesn't strictly break soundness, but does remove message binding.
While it shouldn't be problematic for unforgeability, it's a case that
shouldn't arise from an honest signer and is easy to check for.

This PR fails signature verification if such an identity element is
provided, and adds tests for each case. It does _not_ return an error if
the corresponding identity elements are provided by the signer, since
this precludes useful partial signature operations.

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