From 50316ffa505570e5bfc80c133f434386fd7ff757 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 | 65 ++++++++++++++++++- src/ristretto/ristretto_com_sig.rs | 27 +++++++- src/ristretto/ristretto_sig.rs | 18 +++++ .../commitment_and_public_key_signature.rs | 5 ++ src/signatures/commitment_signature.rs | 5 ++ src/signatures/schnorr.rs | 8 ++- 6 files changed, 125 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..ef7e5e10 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(), + &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 + ) + ); + } } diff --git a/src/ristretto/ristretto_com_sig.rs b/src/ristretto/ristretto_com_sig.rs index 4d790b38..566072bc 100644 --- a/src/ristretto/ristretto_com_sig.rs +++ b/src/ristretto/ristretto_com_sig.rs @@ -107,10 +107,11 @@ pub type RistrettoComSig = CommitmentSignature::default(), + &challenge, + &factory + ) + ); + } } diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index 055821b4..04de1276 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/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" + ) + ); + } } 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