From 5736b6448ac17c6a359946b8da80a41c28e12f71 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 18 Dec 2023 14:15:30 -0600 Subject: [PATCH] Reject identity keys and commitments in signatures --- src/ristretto/ristretto_com_and_pub_sig.rs | 69 ++++++++++++++++++- src/ristretto/ristretto_com_sig.rs | 31 ++++++++- src/ristretto/ristretto_sig.rs | 17 +++++ .../commitment_and_public_key_signature.rs | 5 ++ src/signatures/commitment_signature.rs | 5 ++ src/signatures/schnorr.rs | 8 ++- 6 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/ristretto/ristretto_com_and_pub_sig.rs b/src/ristretto/ristretto_com_and_pub_sig.rs index d3fd04f1..8d328e69 100644 --- a/src/ristretto/ristretto_com_and_pub_sig.rs +++ b/src/ristretto/ristretto_com_and_pub_sig.rs @@ -82,10 +82,11 @@ pub type RistrettoComAndPubSig = CommitmentAndPublicKeySignature::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)); + } } diff --git a/src/ristretto/ristretto_com_sig.rs b/src/ristretto/ristretto_com_sig.rs index 4d790b38..682b9f64 100644 --- a/src/ristretto/ristretto_com_sig.rs +++ b/src/ristretto/ristretto_com_sig.rs @@ -107,10 +107,11 @@ pub type RistrettoComSig = CommitmentSignature::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)); + } } diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index 055821b4..8d77c667 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/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,)); + } } diff --git a/src/signatures/commitment_and_public_key_signature.rs b/src/signatures/commitment_and_public_key_signature.rs index cf526d46..f46e48a6 100644 --- a/src/signatures/commitment_and_public_key_signature.rs +++ b/src/signatures/commitment_and_public_key_signature.rs @@ -176,6 +176,11 @@ where C: HomomorphicCommitmentFactory

, 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; diff --git a/src/signatures/commitment_signature.rs b/src/signatures/commitment_signature.rs index 401d665c..412b3580 100644 --- a/src/signatures/commitment_signature.rs +++ b/src/signatures/commitment_signature.rs @@ -136,6 +136,11 @@ where for<'b> &'b HomomorphicCommitment

: Add<&'b HomomorphicCommitment

, Output = HomomorphicCommitment

>, C: HomomorphicCommitmentFactory

, { + // 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 diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index 535872ef..a5ae70a7 100644 --- a/src/signatures/schnorr.rs +++ b/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, { + // 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