From e1f6c26426186ede710f76c2f0f26adbbd336fa5 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Thu, 3 Nov 2022 12:07:11 +0000 Subject: [PATCH 01/11] feat: improve signature api There are some footguns in the `SchnorrSignature::sign` method that this PR seeks to mitigate. Firstly, it's not clear in the function docs that the nonce and pubkey are assumed to have been bound by the caller in the challenge. The docs are updated to make this clearer. Secondly, we deprecate the sign method and the utility module's sign method in favour of: - `sign_raw`, which is identical to the current sign method, but with a name that conveys the risk associated with using it. - `sign_message`, which does what many clients might have thought `sign` does, which correctly binds a nonce and public key to the message being signed in the challenge construction. - Matching `verify_*` methods. Tests and docs have been updated to reflect the changes. --- Cargo.toml | 2 +- src/hashing.rs | 1 + src/ristretto/ristretto_sig.rs | 76 ++++++++++++++++++++-------- src/ristretto/utils.rs | 6 ++- src/signatures/schnorr.rs | 92 +++++++++++++++++++++++++++++++++- 5 files changed, 152 insertions(+), 25 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5462b66a..0f4d2513 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ categories = ["cryptography"] homepage = "https://tari.com" readme = "README.md" license = "BSD-3-Clause" -version = "0.15.7" +version = "0.16.0" edition = "2018" [dependencies] diff --git a/src/hashing.rs b/src/hashing.rs index 1de63671..ff54cc19 100644 --- a/src/hashing.rs +++ b/src/hashing.rs @@ -520,6 +520,7 @@ pub trait DerivedKeyDomain: DomainSeparation { #[macro_export] macro_rules! hash_domain { ($name:ident, $domain:expr, $version: expr) => { + /// A hashing domain instance #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct $name; diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index 103df3b0..5bf1fedc 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/src/ristretto/ristretto_sig.rs @@ -53,9 +53,8 @@ use crate::{ /// /// #[allow(non_snake_case)] /// let (k, P) = get_keypair(); -/// let (r, R) = get_keypair(); -/// let e = Blake256::digest(b"Small Gods"); -/// let sig = RistrettoSchnorr::sign(k, r, &e); +/// let msg = "Small Gods"; +/// let sig = RistrettoSchnorr::sign_message(k, &msg); /// ``` /// /// # Verifying signatures @@ -72,34 +71,32 @@ use crate::{ /// # use tari_utilities::ByteArray; /// # use digest::Digest; /// -/// # #[allow(non_snake_case)] -/// let P = RistrettoPublicKey::from_hex( -/// "74896a30c89186b8194e25f8c1382f8d3081c5a182fb8f8a6d34f27fbefbfc70", -/// ) -/// .unwrap(); -/// let R = RistrettoPublicKey::from_hex( -/// "fa14cb581ce5717248444721242e6b195a482d503a853dea4acb513074d8d803", -/// ) -/// .unwrap(); -/// let s = RistrettoSecretKey::from_hex( +/// let msg = "Maskerade"; +/// let k = RistrettoSecretKey::from_hex( /// "bd0b253a619310340a4fa2de54cdd212eac7d088ee1dc47e305c3f6cbd020908", /// ) /// .unwrap(); -/// let sig = RistrettoSchnorr::new(R, s); -/// let e = Blake256::digest(b"Maskerade"); -/// assert!(sig.verify_challenge(&P, &e)); +/// # #[allow(non_snake_case)] +/// let P = RistrettoPublicKey::from_secret_key(&k); +/// let sig: SchnorrSignature = +/// SchnorrSignature::sign_message(k, msg).unwrap(); +/// assert!(sig.verify_message(&P, msg)); /// ``` pub type RistrettoSchnorr = SchnorrSignature; #[cfg(test)] mod test { use digest::Digest; - use tari_utilities::{hex::from_hex, ByteArray}; + use tari_utilities::{ + hex::{from_hex, to_hex, Hex}, + ByteArray, + }; use crate::{ hash::blake2::Blake256, keys::{PublicKey, SecretKey}, ristretto::{RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey}, + signatures::SchnorrSignature, }; #[test] @@ -112,10 +109,11 @@ mod test { /// Create a signature, and then verify it. Also checks that some invalid signatures fail to verify #[test] #[allow(non_snake_case)] - fn sign_and_verify_message() { + fn raw_sign_and_verify_challenge() { let mut rng = rand::thread_rng(); let (k, P) = RistrettoPublicKey::random_keypair(&mut rng); let (r, R) = RistrettoPublicKey::random_keypair(&mut rng); + // Use sign raw, and bind the nonce and public key manually let e = Blake256::new() .chain(P.as_bytes()) .chain(R.as_bytes()) @@ -123,7 +121,7 @@ mod test { .finalize(); let e_key = RistrettoSecretKey::from_bytes(&e).unwrap(); let s = &r + &e_key * &k; - let sig = RistrettoSchnorr::sign(k, r, &e).unwrap(); + let sig = RistrettoSchnorr::sign_raw(k, r, &e).unwrap(); let R_calc = sig.get_public_nonce(); assert_eq!(R, *R_calc); assert_eq!(sig.get_signature(), &s); @@ -155,9 +153,9 @@ mod test { .chain(b"Moving Pictures") .finalize(); // Calculate Alice's signature - let s1 = RistrettoSchnorr::sign(k1, r1, &e).unwrap(); + let s1 = RistrettoSchnorr::sign_raw(k1, r1, &e).unwrap(); // Calculate Bob's signature - let s2 = RistrettoSchnorr::sign(k2, r2, &e).unwrap(); + let s2 = RistrettoSchnorr::sign_raw(k2, r2, &e).unwrap(); // Now add the two signatures together let s_agg = &s1 + &s2; // Check that the multi-sig verifies @@ -167,11 +165,45 @@ mod test { /// Ristretto scalars have a max value 2^255. This test checks that hashed messages above this value can still be /// signed as a result of applying modulo arithmetic on the challenge value #[test] + #[allow(non_snake_case)] fn challenge_from_invalid_scalar() { let mut rng = rand::thread_rng(); let m = from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(); let k = RistrettoSecretKey::random(&mut rng); let r = RistrettoSecretKey::random(&mut rng); - assert!(RistrettoSchnorr::sign(k, r, &m).is_ok()); + assert!(RistrettoSchnorr::sign_raw(k, r, &m).is_ok()); + } + + #[test] + #[allow(non_snake_case)] + fn domain_separated_challenge() { + let P = + RistrettoPublicKey::from_hex("74896a30c89186b8194e25f8c1382f8d3081c5a182fb8f8a6d34f27fbefbfc70").unwrap(); + let R = + RistrettoPublicKey::from_hex("fa14cb581ce5717248444721242e6b195a482d503a853dea4acb513074d8d803").unwrap(); + let msg = "Moving Pictures"; + let hash = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&R, &P, msg); + let naiive = Blake256::new() + .chain(R.as_bytes()) + .chain(P.as_bytes()) + .chain(msg) + .finalize() + .to_vec(); + assert_ne!(hash.as_ref(), naiive.as_bytes()); + assert_eq!( + to_hex(hash.as_ref()), + "e64d66b31e2b1c81272f5574f41ab2c997114436c2d3706dca1cf947bed60198" + ); + } + + #[test] + #[allow(non_snake_case)] + fn sign_and_verify_message() { + let mut rng = rand::thread_rng(); + let (k, P) = RistrettoPublicKey::random_keypair(&mut rng); + let sig = RistrettoSchnorr::sign_message(k, "Queues are things that happen to other people").unwrap(); + assert!(sig.verify_message(&P, "Queues are things that happen to other people")); + assert!(!sig.verify_message(&P, "Qs are things that happen to other people")); + assert!(!sig.verify_message(&(&P + &P), "Queues are things that happen to other people")); } } diff --git a/src/ristretto/utils.rs b/src/ristretto/utils.rs index e66d77f2..6bea304a 100644 --- a/src/ristretto/utils.rs +++ b/src/ristretto/utils.rs @@ -29,6 +29,10 @@ pub struct SignatureSet { /// # Panics /// /// The function panics if it cannot generate a suitable signature +#[deprecated( + since = "0.16.0", + note = "Use SchnorrSignature::sign_message instead. This method will be removed in v1.0.0" +)] pub fn sign( private_key: &RistrettoSecretKey, message: &[u8], @@ -41,7 +45,7 @@ pub fn sign( .finalize() .to_vec(); let e = RistrettoSecretKey::from_bytes(&message).map_err(|_| SchnorrSignatureError::InvalidChallenge)?; - let s = RistrettoSchnorr::sign(private_key.clone(), nonce.clone(), e.as_bytes())?; + let s = RistrettoSchnorr::sign_raw(private_key.clone(), nonce.clone(), e.as_bytes())?; Ok(SignatureSet { nonce, public_nonce, diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index 53322979..bed9f887 100644 --- a/src/signatures/schnorr.rs +++ b/src/signatures/schnorr.rs @@ -10,11 +10,20 @@ use std::{ ops::{Add, Mul}, }; +use digest::Digest; use serde::{Deserialize, Serialize}; use tari_utilities::ByteArray; use thiserror::Error; -use crate::keys::{PublicKey, SecretKey}; +use crate::{ + hash::blake2::Blake256, + hash_domain, + hashing::{DomainSeparatedHash, DomainSeparatedHasher}, + keys::{PublicKey, SecretKey}, +}; + +// Define the hashing domain for Schnorr signatures +hash_domain!(SchnorrSigChallenge, "SchnorrSignature", 1); /// An error occurred during construction of a SchnorrSignature #[derive(Clone, Debug, Error, PartialEq, Eq, Deserialize, Serialize)] @@ -57,7 +66,31 @@ where /// Sign a challenge with the given `secret` and private `nonce`. Returns an SchnorrSignatureError if `::from_bytes(challenge)` returns an error. + /// + /// WARNING: The public key and nonce are NOT bound to the challenge. This method assumes that the challenge has + /// been constructed such that all commitments are already included in the challenge. + /// + /// Use [`sign_raw`] instead if this is what you want. (This method is a deprecated alias for `sign_raw`). + /// + /// If you want a simple API that binds the nonce and public key to the message, use [`sign_message`] instead. + #[deprecated( + since = "0.16.0", + note = "This method probably doesn't do what you think it does. Please use `sign_message` or `sign_raw` \ + instead, depending on your use case. This function will be removed in v1.0.0" + )] pub fn sign(secret: K, nonce: K, challenge: &[u8]) -> Result + where K: Add + Mul + Mul { + Self::sign_raw(secret, nonce, challenge) + } + + /// Sign a challenge with the given `secret` and private `nonce`. Returns an SchnorrSignatureError if `::from_bytes(challenge)` returns an error. + /// + /// WARNING: The public key and nonce are NOT bound to the challenge. This method assumes that the challenge has + /// been constructed such that all commitments are already included in the challenge. + /// + /// If you want a simple API that binds the nonce and public key to the message, use [`sign_message`] instead. + pub fn sign_raw(secret: K, nonce: K, challenge: &[u8]) -> Result where K: Add + Mul + Mul { // s = r + e.k let e = match K::from_bytes(challenge) { @@ -70,6 +103,63 @@ where Ok(Self::new(public_nonce, s)) } + /// Signs a message with the given secret key. + /// + /// This method correctly binds a nonce and the public key to the signature challenge, using domain-separated + /// hashing. The hasher is also opinionated in the sense that Blake2b 256-bit digest is always used. + /// + /// it is possible to customise the challenge by using [`construct_domain_separated_challenge`] and [`sign_raw`] + /// yourself, or even use [`sign_raw`] using a completely custom challenge. + pub fn sign_message(secret: K, message: B) -> Result + where + K: Add + Mul + Mul, + B: AsRef<[u8]>, + { + let nonce = K::random(&mut rand::thread_rng()); + let public_nonce = P::from_secret_key(&nonce); + let public_key = P::from_secret_key(&secret); + let challenge = Self::construct_domain_separated_challenge::<_, Blake256>(&public_nonce, &public_key, message); + Self::sign_raw(secret, nonce, challenge.as_ref()) + } + + /// Constructs an opinionated challenge hash for the given public nonce, public key and message. + /// + /// In general, the signature challenge is given by `H(R, P, m)`. Often, plain concatenation is used to construct + /// the challenge. In this implementation, the challenge is constructed by means of domain separated hashing + /// using the provided digest. + /// + /// This challenge is used in the [`sign_message`] and [`verify_message`] methods.If you wish to use a custom + /// challenge, you can use [`sign_raw`] instead. + pub fn construct_domain_separated_challenge( + public_nonce: &P, + public_key: &P, + message: B, + ) -> DomainSeparatedHash + where + B: AsRef<[u8]>, + D: Digest, + { + DomainSeparatedHasher::::new_with_label("challenge") + .chain(public_nonce.as_bytes()) + .chain(public_key.as_bytes()) + .chain(message.as_ref()) + .finalize() + } + + /// Verifies a signature created by the `sign_message` method. The function returns `true` if and only if the + /// message was signed by the secret key corresponding to the given public key, and that the challenge was + /// constructed using the domain-separation method defined in [`construct_domain_separated_challenge`]. + pub fn verify_message<'a, B>(&self, public_key: &'a P, message: B) -> bool + where + for<'b> &'b K: Mul<&'a P, Output = P>, + for<'b> &'b P: Add, + B: AsRef<[u8]>, + { + let challenge = + Self::construct_domain_separated_challenge::<_, Blake256>(&self.public_nonce, public_key, message); + self.verify_challenge(public_key, challenge.as_ref()) + } + /// Returns true if this signature is valid for a public key and challenge, otherwise false. This will always return /// false if `::from_bytes(challenge)` returns an error. pub fn verify_challenge<'a>(&self, public_key: &'a P, challenge: &[u8]) -> bool From ffe634db93f06baee0c118bc7aa2cd7344eaed80 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Thu, 3 Nov 2022 13:22:34 +0000 Subject: [PATCH 02/11] feat!: update wasm and ffi to use domain separated hashes The WASM and FFI library now use the domain-separated hashing algorithm to generate challenges for signatures. --- benches/signatures.rs | 10 ++++----- src/ffi/keys.rs | 15 +++++++------- src/ristretto/ristretto_sig.rs | 2 +- src/signatures/schnorr.rs | 16 ++++++++++++++- src/wasm/key_utils.rs | 37 +++++++++++++++++++++++----------- src/wasm/keyring.rs | 9 ++------- 6 files changed, 54 insertions(+), 35 deletions(-) diff --git a/benches/signatures.rs b/benches/signatures.rs index a39828f1..449d2792 100644 --- a/benches/signatures.rs +++ b/benches/signatures.rs @@ -30,7 +30,6 @@ fn native_keypair(c: &mut Criterion) { struct SigningData { k: RistrettoSecretKey, p: RistrettoPublicKey, - r: RistrettoSecretKey, m: RistrettoSecretKey, } @@ -39,9 +38,8 @@ fn gen_keypair() -> SigningData { let mut msg = [0u8; 32]; rng.fill_bytes(&mut msg); let (k, p) = RistrettoPublicKey::random_keypair(&mut rng); - let r = RistrettoSecretKey::random(&mut rng); let m = RistrettoSecretKey::from_bytes(&msg).unwrap(); - SigningData { k, p, r, m } + SigningData { k, p, m } } fn sign_message(c: &mut Criterion) { @@ -49,7 +47,7 @@ fn sign_message(c: &mut Criterion) { b.iter_batched( gen_keypair, |d| { - let _sig = RistrettoSchnorr::sign(d.k, d.r, &d.m.to_vec()).unwrap(); + let _sig = RistrettoSchnorr::sign_message(d.k, d.m.to_vec()).unwrap(); }, BatchSize::SmallInput, ); @@ -63,10 +61,10 @@ fn verify_message(c: &mut Criterion) { b.iter_batched( || { let d = gen_keypair(); - let s = RistrettoSchnorr::sign(d.k.clone(), d.r.clone(), &d.m.to_vec()).unwrap(); + let s = RistrettoSchnorr::sign_message(d.k.clone(), d.m.to_vec()).unwrap(); (d, s) }, - |(d, s)| assert!(s.verify(&d.p, &d.m)), + |(d, s)| assert!(s.verify_message(&d.p, d.m.as_bytes())), BatchSize::SmallInput, ); }); diff --git a/src/ffi/keys.rs b/src/ffi/keys.rs index 38f8f179..99b55a82 100644 --- a/src/ffi/keys.rs +++ b/src/ffi/keys.rs @@ -23,6 +23,7 @@ use crate::{ RistrettoSchnorr, RistrettoSecretKey, }, + signatures::SchnorrSignature, }; pub const KEY_LENGTH: usize = 32; @@ -70,13 +71,15 @@ pub unsafe extern "C" fn sign( Ok(k) => k, _ => return INVALID_SECRET_KEY_SER, }; + let pubkey = RistrettoPublicKey::from_secret_key(&k); let r = RistrettoSecretKey::random(&mut OsRng); + let pub_r = RistrettoPublicKey::from_secret_key(&r); let msg = match CStr::from_ptr(msg).to_str() { Ok(s) => s, _ => return STR_CONV_ERR, }; - let challenge = Blake256::digest(msg.as_bytes()); - let sig = match RistrettoSchnorr::sign(k, r, &challenge) { + let e = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&pub_r, &pubkey, msg.as_bytes()); + let sig = match RistrettoSchnorr::sign_raw(k, r, e.as_ref()) { Ok(sig) => sig, _ => return SIGNING_ERROR, }; @@ -123,13 +126,9 @@ pub unsafe extern "C" fn verify( Ok(s) => s, _ => return false, }; + let sig = RistrettoSchnorr::new(r_pub, sig); - let challenge = Blake256::digest(msg.as_bytes()); - let challenge = match RistrettoSecretKey::from_bytes(challenge.as_slice()) { - Ok(e) => e, - _ => return false, - }; - sig.verify(&pk, &challenge) + sig.verify_message(&pk, msg.as_bytes()) } /// Generate a Pedersen commitment (C) using the provided value and spending key (a, x). diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index 5bf1fedc..36ecb1f4 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/src/ristretto/ristretto_sig.rs @@ -192,7 +192,7 @@ mod test { assert_ne!(hash.as_ref(), naiive.as_bytes()); assert_eq!( to_hex(hash.as_ref()), - "e64d66b31e2b1c81272f5574f41ab2c997114436c2d3706dca1cf947bed60198" + "d8f6b29b641113c91175b8d44f265ff1167d58d5aa5ee03e6f1f521505b09d80" ); } diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index bed9f887..fc0b070c 100644 --- a/src/signatures/schnorr.rs +++ b/src/signatures/schnorr.rs @@ -23,7 +23,7 @@ use crate::{ }; // Define the hashing domain for Schnorr signatures -hash_domain!(SchnorrSigChallenge, "SchnorrSignature", 1); +hash_domain!(SchnorrSigChallenge, "com.tari.schnorr_signature", 1); /// An error occurred during construction of a SchnorrSignature #[derive(Clone, Debug, Error, PartialEq, Eq, Deserialize, Serialize)] @@ -266,3 +266,17 @@ where Some(self.cmp(other)) } } + +#[cfg(test)] +mod test { + use crate::{hashing::DomainSeparation, signatures::SchnorrSigChallenge}; + + #[test] + fn schnorr_hash_domain() { + assert_eq!(SchnorrSigChallenge::domain(), "com.tari.schnorr_signature"); + assert_eq!( + SchnorrSigChallenge::domain_separation_tag("test"), + "com.tari.schnorr_signature.v1.test" + ); + } +} diff --git a/src/wasm/key_utils.rs b/src/wasm/key_utils.rs index 60b57b48..b3415e7d 100644 --- a/src/wasm/key_utils.rs +++ b/src/wasm/key_utils.rs @@ -23,6 +23,7 @@ use crate::{ RistrettoSchnorr, RistrettoSecretKey, }, + signatures::SchnorrSignature, }; /// Result of calling [check_signature] and [check_comsig_signature] and [check_comandpubsig_signature] @@ -138,6 +139,7 @@ pub fn sign_challenge_with_nonce(private_key: &str, private_nonce: &str, challen return serde_wasm_bindgen::to_value(&result).unwrap(); }, }; + let pub_r = RistrettoPublicKey::from_secret_key(&r); let e = match from_hex(challenge_as_hex) { Ok(e) => e, @@ -146,7 +148,16 @@ pub fn sign_challenge_with_nonce(private_key: &str, private_nonce: &str, challen return serde_wasm_bindgen::to_value(&result).unwrap(); }, }; - sign_with_key(&k, &e, Some(&r), &mut result); + + let sig = match RistrettoSchnorr::sign_raw(k, r, &e) { + Ok(s) => s, + Err(e) => { + result.error = format!("Could not create signature. {e}"); + return JsValue::from_serde(&result).unwrap(); + }, + }; + result.public_nonce = Some(pub_r.to_hex()); + result.signature = Some(sig.get_signature().to_hex()); serde_wasm_bindgen::to_value(&result).unwrap() } @@ -156,18 +167,23 @@ pub(super) fn sign_message_with_key( r: Option<&RistrettoSecretKey>, result: &mut SignResult, ) { - let e = Blake256::digest(msg.as_bytes()); - sign_with_key(k, e.as_slice(), r, result) + sign_with_key(k, msg.as_bytes(), r, result) } #[allow(non_snake_case)] -pub(super) fn sign_with_key(k: &RistrettoSecretKey, e: &[u8], r: Option<&RistrettoSecretKey>, result: &mut SignResult) { +pub(super) fn sign_with_key( + k: &RistrettoSecretKey, + msg: &[u8], + r: Option<&RistrettoSecretKey>, + result: &mut SignResult, +) { let (r, R) = match r { Some(r) => (r.clone(), RistrettoPublicKey::from_secret_key(r)), None => RistrettoPublicKey::random_keypair(&mut OsRng), }; - - let sig = match RistrettoSchnorr::sign(k.clone(), r, e) { + let P = RistrettoPublicKey::from_secret_key(k); + let e = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&R, &P, msg); + let sig = match RistrettoSchnorr::sign_raw(k.clone(), r, e.as_ref()) { Ok(s) => s, Err(e) => { result.error = format!("Could not create signature. {e}"); @@ -209,8 +225,7 @@ pub fn check_signature(pub_nonce: &str, signature: &str, pub_key: &str, msg: &st }; let sig = RistrettoSchnorr::new(R, s); - let msg = Blake256::digest(msg.as_bytes()); - result.result = sig.verify_challenge(&P, msg.as_slice()); + result.result = sig.verify_message(&P, msg.as_bytes()); serde_wasm_bindgen::to_value(&result).unwrap() } @@ -724,9 +739,7 @@ mod test { fn create_signature(msg: &str) -> (RistrettoSchnorr, RistrettoPublicKey, RistrettoSecretKey) { let (sk, pk) = random_keypair(); - let (nonce, _) = random_keypair(); - let sig = SchnorrSignature::sign(sk.clone(), nonce, &hash(msg)).unwrap(); - + let sig = SchnorrSignature::sign_message(sk.clone(), msg.as_bytes()).unwrap(); (sig, pk, sk) } @@ -837,7 +850,7 @@ mod test { assert!(result.error.is_empty()); let p_nonce = RistrettoPublicKey::from_hex(&result.public_nonce.unwrap()).unwrap(); let s = RistrettoSecretKey::from_hex(&result.signature.unwrap()).unwrap(); - assert!(SchnorrSignature::new(p_nonce, s).verify_challenge(&pk, &hash(SAMPLE_CHALLENGE))); + assert!(SchnorrSignature::new(p_nonce, s).verify_message(&pk, SAMPLE_CHALLENGE)); } #[wasm_bindgen_test] diff --git a/src/wasm/keyring.rs b/src/wasm/keyring.rs index 23f31f63..ab3cbcb2 100644 --- a/src/wasm/keyring.rs +++ b/src/wasm/keyring.rs @@ -148,11 +148,10 @@ impl KeyRing { #[cfg(test)] mod test { - use blake2::{digest::Output, Digest}; use wasm_bindgen_test::*; use super::*; - use crate::{hash::blake2::Blake256, keys::SecretKey, ristretto::RistrettoSchnorr}; + use crate::{keys::SecretKey, ristretto::RistrettoSchnorr}; const SAMPLE_CHALLENGE: &str = "გამარჯობა"; @@ -163,10 +162,6 @@ mod test { kr } - fn hash>(preimage: T) -> Output { - Blake256::digest(preimage.as_ref()) - } - fn create_commitment(k: &RistrettoSecretKey, v: u64) -> PedersenCommitment { PedersenCommitmentFactory::default().commit_value(k, v) } @@ -240,7 +235,7 @@ mod test { let kr = new_keyring(); let sig = sign(&kr, "a").unwrap(); let pk = kr.expect_public_key("a"); - assert!(sig.verify_challenge(pk, &hash(SAMPLE_CHALLENGE))); + assert!(sig.verify_message(pk, SAMPLE_CHALLENGE)); } } From 1e25b291d2d3ba63fcdbea67ef7fda06a9a40211 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Thu, 3 Nov 2022 17:12:16 +0000 Subject: [PATCH 03/11] feat: allow passing of secret by reference The old API took ownership of the secret. This isn't necessary from an API point of view. It's more ergonomic to take a reference. The nonce still gives up ownerhip, since this should never be re-used. --- src/ffi/keys.rs | 2 +- src/ristretto/ristretto_sig.rs | 14 +++++++------- src/ristretto/utils.rs | 2 +- src/signatures/schnorr.rs | 15 +++++++++------ src/wasm/key_utils.rs | 6 +++--- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/ffi/keys.rs b/src/ffi/keys.rs index 99b55a82..affd5a8f 100644 --- a/src/ffi/keys.rs +++ b/src/ffi/keys.rs @@ -79,7 +79,7 @@ pub unsafe extern "C" fn sign( _ => return STR_CONV_ERR, }; let e = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&pub_r, &pubkey, msg.as_bytes()); - let sig = match RistrettoSchnorr::sign_raw(k, r, e.as_ref()) { + let sig = match RistrettoSchnorr::sign_raw(&k, r, e.as_ref()) { Ok(sig) => sig, _ => return SIGNING_ERROR, }; diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index 36ecb1f4..519f0a68 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/src/ristretto/ristretto_sig.rs @@ -54,7 +54,7 @@ use crate::{ /// #[allow(non_snake_case)] /// let (k, P) = get_keypair(); /// let msg = "Small Gods"; -/// let sig = RistrettoSchnorr::sign_message(k, &msg); +/// let sig = RistrettoSchnorr::sign_message(&k, &msg); /// ``` /// /// # Verifying signatures @@ -79,7 +79,7 @@ use crate::{ /// # #[allow(non_snake_case)] /// let P = RistrettoPublicKey::from_secret_key(&k); /// let sig: SchnorrSignature = -/// SchnorrSignature::sign_message(k, msg).unwrap(); +/// SchnorrSignature::sign_message(&k, msg).unwrap(); /// assert!(sig.verify_message(&P, msg)); /// ``` pub type RistrettoSchnorr = SchnorrSignature; @@ -121,7 +121,7 @@ mod test { .finalize(); let e_key = RistrettoSecretKey::from_bytes(&e).unwrap(); let s = &r + &e_key * &k; - let sig = RistrettoSchnorr::sign_raw(k, r, &e).unwrap(); + let sig = RistrettoSchnorr::sign_raw(&k, r, &e).unwrap(); let R_calc = sig.get_public_nonce(); assert_eq!(R, *R_calc); assert_eq!(sig.get_signature(), &s); @@ -153,9 +153,9 @@ mod test { .chain(b"Moving Pictures") .finalize(); // Calculate Alice's signature - let s1 = RistrettoSchnorr::sign_raw(k1, r1, &e).unwrap(); + let s1 = RistrettoSchnorr::sign_raw(&k1, r1, &e).unwrap(); // Calculate Bob's signature - let s2 = RistrettoSchnorr::sign_raw(k2, r2, &e).unwrap(); + let s2 = RistrettoSchnorr::sign_raw(&k2, r2, &e).unwrap(); // Now add the two signatures together let s_agg = &s1 + &s2; // Check that the multi-sig verifies @@ -171,7 +171,7 @@ mod test { let m = from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap(); let k = RistrettoSecretKey::random(&mut rng); let r = RistrettoSecretKey::random(&mut rng); - assert!(RistrettoSchnorr::sign_raw(k, r, &m).is_ok()); + assert!(RistrettoSchnorr::sign_raw(&k, r, &m).is_ok()); } #[test] @@ -201,7 +201,7 @@ mod test { fn sign_and_verify_message() { let mut rng = rand::thread_rng(); let (k, P) = RistrettoPublicKey::random_keypair(&mut rng); - let sig = RistrettoSchnorr::sign_message(k, "Queues are things that happen to other people").unwrap(); + let sig = RistrettoSchnorr::sign_message(&k, "Queues are things that happen to other people").unwrap(); assert!(sig.verify_message(&P, "Queues are things that happen to other people")); assert!(!sig.verify_message(&P, "Qs are things that happen to other people")); assert!(!sig.verify_message(&(&P + &P), "Queues are things that happen to other people")); diff --git a/src/ristretto/utils.rs b/src/ristretto/utils.rs index 6bea304a..9dd24557 100644 --- a/src/ristretto/utils.rs +++ b/src/ristretto/utils.rs @@ -45,7 +45,7 @@ pub fn sign( .finalize() .to_vec(); let e = RistrettoSecretKey::from_bytes(&message).map_err(|_| SchnorrSignatureError::InvalidChallenge)?; - let s = RistrettoSchnorr::sign_raw(private_key.clone(), nonce.clone(), e.as_bytes())?; + let s = RistrettoSchnorr::sign_raw(&private_key, nonce.clone(), e.as_bytes())?; Ok(SignatureSet { nonce, public_nonce, diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index fc0b070c..04665c0a 100644 --- a/src/signatures/schnorr.rs +++ b/src/signatures/schnorr.rs @@ -79,8 +79,11 @@ where instead, depending on your use case. This function will be removed in v1.0.0" )] pub fn sign(secret: K, nonce: K, challenge: &[u8]) -> Result - where K: Add + Mul + Mul { - Self::sign_raw(secret, nonce, challenge) + where + K: Add, + for<'a> K: Mul<&'a K, Output = K>, + { + Self::sign_raw(&secret, nonce, challenge) } /// Sign a challenge with the given `secret` and private `nonce`. Returns an SchnorrSignatureError if ` Result - where K: Add + Mul + Mul { + pub fn sign_raw<'a>(secret: &'a K, nonce: K, challenge: &[u8]) -> Result + where K: Add + Mul<&'a K, Output = K> { // s = r + e.k let e = match K::from_bytes(challenge) { Ok(e) => e, @@ -110,9 +113,9 @@ where /// /// it is possible to customise the challenge by using [`construct_domain_separated_challenge`] and [`sign_raw`] /// yourself, or even use [`sign_raw`] using a completely custom challenge. - pub fn sign_message(secret: K, message: B) -> Result + pub fn sign_message<'a, B>(secret: &'a K, message: B) -> Result where - K: Add + Mul + Mul, + K: Add + Mul<&'a K, Output = K>, B: AsRef<[u8]>, { let nonce = K::random(&mut rand::thread_rng()); diff --git a/src/wasm/key_utils.rs b/src/wasm/key_utils.rs index b3415e7d..567ae376 100644 --- a/src/wasm/key_utils.rs +++ b/src/wasm/key_utils.rs @@ -149,7 +149,7 @@ pub fn sign_challenge_with_nonce(private_key: &str, private_nonce: &str, challen }, }; - let sig = match RistrettoSchnorr::sign_raw(k, r, &e) { + let sig = match RistrettoSchnorr::sign_raw(&k, r, &e) { Ok(s) => s, Err(e) => { result.error = format!("Could not create signature. {e}"); @@ -183,7 +183,7 @@ pub(super) fn sign_with_key( }; let P = RistrettoPublicKey::from_secret_key(k); let e = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&R, &P, msg); - let sig = match RistrettoSchnorr::sign_raw(k.clone(), r, e.as_ref()) { + let sig = match RistrettoSchnorr::sign_raw(&k, r, e.as_ref()) { Ok(s) => s, Err(e) => { result.error = format!("Could not create signature. {e}"); @@ -739,7 +739,7 @@ mod test { fn create_signature(msg: &str) -> (RistrettoSchnorr, RistrettoPublicKey, RistrettoSecretKey) { let (sk, pk) = random_keypair(); - let sig = SchnorrSignature::sign_message(sk.clone(), msg.as_bytes()).unwrap(); + let sig = SchnorrSignature::sign_message(&sk, msg.as_bytes()).unwrap(); (sig, pk, sk) } From ebfe9b0a847c93ddc8fea71b469dc8e291c98603 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Thu, 3 Nov 2022 17:46:43 +0000 Subject: [PATCH 04/11] fix clippy lints --- src/ristretto/utils.rs | 2 +- src/signatures/schnorr.rs | 3 ++- src/wasm/key_utils.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ristretto/utils.rs b/src/ristretto/utils.rs index 9dd24557..d38bbbab 100644 --- a/src/ristretto/utils.rs +++ b/src/ristretto/utils.rs @@ -45,7 +45,7 @@ pub fn sign( .finalize() .to_vec(); let e = RistrettoSecretKey::from_bytes(&message).map_err(|_| SchnorrSignatureError::InvalidChallenge)?; - let s = RistrettoSchnorr::sign_raw(&private_key, nonce.clone(), e.as_bytes())?; + let s = RistrettoSchnorr::sign_raw(private_key, nonce.clone(), e.as_bytes())?; Ok(SignatureSet { nonce, public_nonce, diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index 04665c0a..edcc772c 100644 --- a/src/signatures/schnorr.rs +++ b/src/signatures/schnorr.rs @@ -78,6 +78,7 @@ where note = "This method probably doesn't do what you think it does. Please use `sign_message` or `sign_raw` \ instead, depending on your use case. This function will be removed in v1.0.0" )] + #[allow(clippy::needless_pass_by_value)] pub fn sign(secret: K, nonce: K, challenge: &[u8]) -> Result where K: Add, @@ -120,7 +121,7 @@ where { let nonce = K::random(&mut rand::thread_rng()); let public_nonce = P::from_secret_key(&nonce); - let public_key = P::from_secret_key(&secret); + let public_key = P::from_secret_key(secret); let challenge = Self::construct_domain_separated_challenge::<_, Blake256>(&public_nonce, &public_key, message); Self::sign_raw(secret, nonce, challenge.as_ref()) } diff --git a/src/wasm/key_utils.rs b/src/wasm/key_utils.rs index 567ae376..c650112e 100644 --- a/src/wasm/key_utils.rs +++ b/src/wasm/key_utils.rs @@ -183,7 +183,7 @@ pub(super) fn sign_with_key( }; let P = RistrettoPublicKey::from_secret_key(k); let e = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&R, &P, msg); - let sig = match RistrettoSchnorr::sign_raw(&k, r, e.as_ref()) { + let sig = match RistrettoSchnorr::sign_raw(k, r, e.as_ref()) { Ok(s) => s, Err(e) => { result.error = format!("Could not create signature. {e}"); From c5ae3ceb0d51511a8c1f918402944dc34297cce7 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Thu, 3 Nov 2022 17:55:08 +0000 Subject: [PATCH 05/11] fix: benchmarks --- benches/signatures.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benches/signatures.rs b/benches/signatures.rs index 449d2792..b10cbd55 100644 --- a/benches/signatures.rs +++ b/benches/signatures.rs @@ -47,7 +47,7 @@ fn sign_message(c: &mut Criterion) { b.iter_batched( gen_keypair, |d| { - let _sig = RistrettoSchnorr::sign_message(d.k, d.m.to_vec()).unwrap(); + let _sig = RistrettoSchnorr::sign_message(&d.k, d.m.to_vec()).unwrap(); }, BatchSize::SmallInput, ); @@ -61,7 +61,7 @@ fn verify_message(c: &mut Criterion) { b.iter_batched( || { let d = gen_keypair(); - let s = RistrettoSchnorr::sign_message(d.k.clone(), d.m.to_vec()).unwrap(); + let s = RistrettoSchnorr::sign_message(&d.k, d.m.to_vec()).unwrap(); (d, s) }, |(d, s)| assert!(s.verify_message(&d.p, d.m.as_bytes())), From c7addaeda63876e3f1fa68a966350abbbf28fe67 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Mon, 7 Nov 2022 11:31:49 +0000 Subject: [PATCH 06/11] chore: change bench message to [u8] In response to review comment --- benches/signatures.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/benches/signatures.rs b/benches/signatures.rs index b10cbd55..61cd07f8 100644 --- a/benches/signatures.rs +++ b/benches/signatures.rs @@ -30,15 +30,14 @@ fn native_keypair(c: &mut Criterion) { struct SigningData { k: RistrettoSecretKey, p: RistrettoPublicKey, - m: RistrettoSecretKey, + m: [u8; 32], } fn gen_keypair() -> SigningData { let mut rng = thread_rng(); - let mut msg = [0u8; 32]; - rng.fill_bytes(&mut msg); + let mut m = [0u8; 32]; + rng.fill_bytes(&mut m); let (k, p) = RistrettoPublicKey::random_keypair(&mut rng); - let m = RistrettoSecretKey::from_bytes(&msg).unwrap(); SigningData { k, p, m } } @@ -47,7 +46,7 @@ fn sign_message(c: &mut Criterion) { b.iter_batched( gen_keypair, |d| { - let _sig = RistrettoSchnorr::sign_message(&d.k, d.m.to_vec()).unwrap(); + let _sig = RistrettoSchnorr::sign_message(&d.k, &d.m).unwrap(); }, BatchSize::SmallInput, ); @@ -61,10 +60,10 @@ fn verify_message(c: &mut Criterion) { b.iter_batched( || { let d = gen_keypair(); - let s = RistrettoSchnorr::sign_message(&d.k, d.m.to_vec()).unwrap(); + let s = RistrettoSchnorr::sign_message(&d.k, &d.m).unwrap(); (d, s) }, - |(d, s)| assert!(s.verify_message(&d.p, d.m.as_bytes())), + |(d, s)| assert!(s.verify_message(&d.p, &d.m)), BatchSize::SmallInput, ); }); From d1e5abd0384e1867ce0b2e67dfae4632e558e33d Mon Sep 17 00:00:00 2001 From: CjS77 Date: Mon, 7 Nov 2022 12:30:46 +0000 Subject: [PATCH 07/11] feat: allow custom domains on schnorr sig This commit adds support for providing custom domain separation tags to SchnorrSignature. This is done by including a 3rd generic type to the SchnorrSignature struct definition. The provision is optional. By default, the domain hash separator uses SchnorrSigChallenge as the domain separation tag. The `RsitrettoSchnorr` type alias is updated to include the default domain separation tag, to keep backward compatibility. To provide a custom hasher, update usages of `RistrettoSchnorr` t `RistrettoSchnorrWithDomain` --- src/ristretto/ristretto_sig.rs | 41 +++++++++++++++++++++++++++++----- src/signatures/schnorr.rs | 34 ++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index 519f0a68..3622459d 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/src/ristretto/ristretto_sig.rs @@ -3,7 +3,7 @@ use crate::{ ristretto::{RistrettoPublicKey, RistrettoSecretKey}, - signatures::SchnorrSignature, + signatures::{SchnorrSigChallenge, SchnorrSignature}, }; /// # A Schnorr signature implementation on Ristretto @@ -82,7 +82,8 @@ use crate::{ /// SchnorrSignature::sign_message(&k, msg).unwrap(); /// assert!(sig.verify_message(&P, msg)); /// ``` -pub type RistrettoSchnorr = SchnorrSignature; +pub type RistrettoSchnorr = SchnorrSignature; +pub type RistrettoSchnorrWithDomain = SchnorrSignature; #[cfg(test)] mod test { @@ -94,9 +95,15 @@ mod test { use crate::{ hash::blake2::Blake256, + hash_domain, keys::{PublicKey, SecretKey}, - ristretto::{RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey}, - signatures::SchnorrSignature, + ristretto::{ + ristretto_sig::RistrettoSchnorrWithDomain, + RistrettoPublicKey, + RistrettoSchnorr, + RistrettoSecretKey, + }, + signatures::{SchnorrSigChallenge, SchnorrSignature}, }; #[test] @@ -182,7 +189,9 @@ mod test { let R = RistrettoPublicKey::from_hex("fa14cb581ce5717248444721242e6b195a482d503a853dea4acb513074d8d803").unwrap(); let msg = "Moving Pictures"; - let hash = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&R, &P, msg); + let hash = SchnorrSignature::<_, _, SchnorrSigChallenge>::construct_domain_separated_challenge::<_, Blake256>( + &R, &P, msg, + ); let naiive = Blake256::new() .chain(R.as_bytes()) .chain(P.as_bytes()) @@ -196,6 +205,28 @@ mod test { ); } + #[test] + #[allow(non_snake_case)] + fn custom_hash_domain() { + hash_domain!(TestDomain, "test.signature.com"); + let mut rng = rand::thread_rng(); + let (k, P) = RistrettoPublicKey::random_keypair(&mut rng); + let (r, _) = RistrettoPublicKey::random_keypair(&mut rng); + let msg = "Moving Pictures"; + // Using default domain + // NEVER re-use nonces in practice. This is done here explicitly to indicate that the domain separation + // prevents accidental signature duplication. + let sig1 = RistrettoSchnorr::sign_with_nonce_and_message(&k, r.clone(), msg).unwrap(); + // Using custom domain + let sig2 = RistrettoSchnorrWithDomain::::sign_with_nonce_and_message(&k, r, msg).unwrap(); + // The type system won't even let this compile :) + // assert_ne!(sig1, sig2); + // Prove that the nonces were reused. Again, NEVER do this + assert_eq!(sig1.get_public_nonce(), sig2.get_public_nonce()); + // But the signatures are different, for the same message, secret and nonce. + assert_ne!(sig1.get_signature(), sig2.get_signature()); + } + #[test] #[allow(non_snake_case)] fn sign_and_verify_message() { diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index edcc772c..e44a9888 100644 --- a/src/signatures/schnorr.rs +++ b/src/signatures/schnorr.rs @@ -7,6 +7,7 @@ use std::{ cmp::Ordering, + marker::PhantomData, ops::{Add, Mul}, }; @@ -18,7 +19,7 @@ use thiserror::Error; use crate::{ hash::blake2::Blake256, hash_domain, - hashing::{DomainSeparatedHash, DomainSeparatedHasher}, + hashing::{DomainSeparatedHash, DomainSeparatedHasher, DomainSeparation}, keys::{PublicKey, SecretKey}, }; @@ -41,21 +42,24 @@ pub enum SchnorrSignatureError { /// More details on Schnorr signatures can be found at [TLU](https://tlu.tarilabs.com/cryptography/introduction-schnorr-signatures). #[allow(non_snake_case)] #[derive(PartialEq, Eq, Copy, Debug, Clone, Serialize, Deserialize, Hash)] -pub struct SchnorrSignature { +pub struct SchnorrSignature { public_nonce: P, signature: K, + _phantom: PhantomData, } -impl SchnorrSignature +impl SchnorrSignature where P: PublicKey, K: SecretKey, + H: DomainSeparation, { /// Create a new `SchnorrSignature`. pub fn new(public_nonce: P, signature: K) -> Self { SchnorrSignature { public_nonce, signature, + _phantom: PhantomData, } } @@ -120,6 +124,28 @@ where B: AsRef<[u8]>, { let nonce = K::random(&mut rand::thread_rng()); + Self::sign_with_nonce_and_message(secret, nonce, message) + } + + /// Signs a message with the given secret key and provided nonce. + /// + /// This method correctly binds the nonce and the public key to the signature challenge, using domain-separated + /// hashing. The hasher is also opinionated in the sense that Blake2b 256-bit digest is always used. + /// + /// ** Important **: It is the caller's responsibility to ensure that the nonce is unique. This API tries to + /// prevent this by taking ownership of the nonce, which means that the caller has to explicitly clone the nonce + /// in order to re-use it, which is a small deterrent, but better than nothing. + /// + /// To delegate nonce handling to the callee, use [`Self::sign_message`] instead. + pub fn sign_with_nonce_and_message<'a, B>( + secret: &'a K, + nonce: K, + message: B, + ) -> Result + where + K: Add + Mul<&'a K, Output = K>, + B: AsRef<[u8]>, + { let public_nonce = P::from_secret_key(&nonce); let public_key = P::from_secret_key(secret); let challenge = Self::construct_domain_separated_challenge::<_, Blake256>(&public_nonce, &public_key, message); @@ -143,7 +169,7 @@ where B: AsRef<[u8]>, D: Digest, { - DomainSeparatedHasher::::new_with_label("challenge") + DomainSeparatedHasher::::new_with_label("challenge") .chain(public_nonce.as_bytes()) .chain(public_key.as_bytes()) .chain(message.as_ref()) From c8952b76be558368e1f0c672ae19e8c2e438d583 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Mon, 7 Nov 2022 13:01:11 +0000 Subject: [PATCH 08/11] fix: update ffi method docs and function sig There were some small inconsistencies between the docstrings and method sigs in the ffi module. The `verify` function also had the sig and nonce being mutable, which is incorrect, since they're input parameters in this case. --- src/ffi/keys.rs | 19 ++++++++++--------- src/ristretto/ristretto_sig.rs | 2 ++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/ffi/keys.rs b/src/ffi/keys.rs index affd5a8f..46d8de28 100644 --- a/src/ffi/keys.rs +++ b/src/ffi/keys.rs @@ -23,7 +23,6 @@ use crate::{ RistrettoSchnorr, RistrettoSecretKey, }, - signatures::SchnorrSignature, }; pub const KEY_LENGTH: usize = 32; @@ -52,19 +51,21 @@ pub unsafe extern "C" fn random_keypair(priv_key: *mut KeyArray, pub_key: *mut K OK } -/// Generate a Schnorr signature (s, R) using the provided private key and challenge (k, e). +/// Generate a Schnorr signature (s, R) using the provided private key and message (k, m). /// /// # Safety /// The caller MUST ensure that the string is null terminated e.g. "msg\0". /// If any args are null then the function returns -1 +/// +/// The public nonce and signature are returned in the provided mutable arrays. #[no_mangle] pub unsafe extern "C" fn sign( priv_key: *const KeyArray, msg: *const c_char, - nonce: *mut KeyArray, + public_nonce: *mut KeyArray, signature: *mut KeyArray, ) -> c_int { - if nonce.is_null() || signature.is_null() || priv_key.is_null() || msg.is_null() { + if public_nonce.is_null() || signature.is_null() || priv_key.is_null() || msg.is_null() { return NULL_POINTER; } let k = match RistrettoSecretKey::from_bytes(&(*priv_key)) { @@ -78,17 +79,17 @@ pub unsafe extern "C" fn sign( Ok(s) => s, _ => return STR_CONV_ERR, }; - let e = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&pub_r, &pubkey, msg.as_bytes()); + let e = RistrettoSchnorr::construct_domain_separated_challenge::<_, Blake256>(&pub_r, &pubkey, msg.as_bytes()); let sig = match RistrettoSchnorr::sign_raw(&k, r, e.as_ref()) { Ok(sig) => sig, _ => return SIGNING_ERROR, }; - (*nonce).copy_from_slice(sig.get_public_nonce().as_bytes()); + (*public_nonce).copy_from_slice(sig.get_public_nonce().as_bytes()); (*signature).copy_from_slice(sig.get_signature().as_bytes()); OK } -/// Verify that a Schnorr signature (s, R) is valid for the provided public key and challenge (P, e). +/// Verify that a Schnorr signature (s, R) is valid for the provided public key and message (P, m). /// /// # Safety /// The caller MUST ensure that the string is null terminated e.g. "msg\0". @@ -97,8 +98,8 @@ pub unsafe extern "C" fn sign( pub unsafe extern "C" fn verify( pub_key: *const KeyArray, msg: *const c_char, - pub_nonce: *mut KeyArray, - signature: *mut KeyArray, + pub_nonce: *const KeyArray, + signature: *const KeyArray, err_code: *mut c_int, ) -> bool { if pub_key.is_null() || msg.is_null() || pub_nonce.is_null() || signature.is_null() || err_code.is_null() { diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index 3622459d..a4c9023b 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/src/ristretto/ristretto_sig.rs @@ -223,6 +223,8 @@ mod test { // assert_ne!(sig1, sig2); // Prove that the nonces were reused. Again, NEVER do this assert_eq!(sig1.get_public_nonce(), sig2.get_public_nonce()); + assert!(sig1.verify_message(&P, msg)); + assert!(sig2.verify_message(&P, msg)); // But the signatures are different, for the same message, secret and nonce. assert_ne!(sig1.get_signature(), sig2.get_signature()); } From 63a466a5873799dd6ff40d0d7174029187c19daf Mon Sep 17 00:00:00 2001 From: CjS77 Date: Mon, 7 Nov 2022 13:09:12 +0000 Subject: [PATCH 09/11] fix: update wasm tests to use correct type alias --- src/wasm/key_utils.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/wasm/key_utils.rs b/src/wasm/key_utils.rs index c650112e..b9d58523 100644 --- a/src/wasm/key_utils.rs +++ b/src/wasm/key_utils.rs @@ -23,7 +23,6 @@ use crate::{ RistrettoSchnorr, RistrettoSecretKey, }, - signatures::SchnorrSignature, }; /// Result of calling [check_signature] and [check_comsig_signature] and [check_comandpubsig_signature] @@ -182,7 +181,7 @@ pub(super) fn sign_with_key( None => RistrettoPublicKey::random_keypair(&mut OsRng), }; let P = RistrettoPublicKey::from_secret_key(k); - let e = SchnorrSignature::construct_domain_separated_challenge::<_, Blake256>(&R, &P, msg); + let e = RistrettoSchnorr::construct_domain_separated_challenge::<_, Blake256>(&R, &P, msg); let sig = match RistrettoSchnorr::sign_raw(k, r, e.as_ref()) { Ok(s) => s, Err(e) => { @@ -850,7 +849,7 @@ mod test { assert!(result.error.is_empty()); let p_nonce = RistrettoPublicKey::from_hex(&result.public_nonce.unwrap()).unwrap(); let s = RistrettoSecretKey::from_hex(&result.signature.unwrap()).unwrap(); - assert!(SchnorrSignature::new(p_nonce, s).verify_message(&pk, SAMPLE_CHALLENGE)); + assert!(RistrettoSchnorr::new(p_nonce, s).verify_message(&pk, SAMPLE_CHALLENGE)); } #[wasm_bindgen_test] @@ -902,7 +901,7 @@ mod test { let p_nonce = RistrettoPublicKey::from_hex(&result.public_nonce.unwrap()).unwrap(); assert_eq!(p_nonce, expected_pr); let s = RistrettoSecretKey::from_hex(&result.signature.unwrap()).unwrap(); - assert!(SchnorrSignature::new(p_nonce, s).verify_challenge(&pk, &hash(SAMPLE_CHALLENGE))); + assert!(RistrettoSchnorr::new(p_nonce, s).verify_challenge(&pk, &hash(SAMPLE_CHALLENGE))); } } From 8947aee8bddd26b4365c363a3ff2bc9528a11c57 Mon Sep 17 00:00:00 2001 From: CjS77 Date: Mon, 7 Nov 2022 13:48:04 +0000 Subject: [PATCH 10/11] docs: add missing docs and export new type Export `RistrettoSchnorrWithDomain` and add the missing docs clippy was complaining about --- src/ristretto/mod.rs | 2 +- src/ristretto/ristretto_sig.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/ristretto/mod.rs b/src/ristretto/mod.rs index a934faa0..370acd49 100644 --- a/src/ristretto/mod.rs +++ b/src/ristretto/mod.rs @@ -21,7 +21,7 @@ pub use self::{ ristretto_com_and_pub_sig::RistrettoComAndPubSig, ristretto_com_sig::RistrettoComSig, ristretto_keys::{RistrettoPublicKey, RistrettoSecretKey}, - ristretto_sig::RistrettoSchnorr, + ristretto_sig::{RistrettoSchnorr, RistrettoSchnorrWithDomain}, }; // test modules diff --git a/src/ristretto/ristretto_sig.rs b/src/ristretto/ristretto_sig.rs index a4c9023b..69809bf5 100644 --- a/src/ristretto/ristretto_sig.rs +++ b/src/ristretto/ristretto_sig.rs @@ -83,6 +83,36 @@ use crate::{ /// assert!(sig.verify_message(&P, msg)); /// ``` pub type RistrettoSchnorr = SchnorrSignature; + +/// # A Schnorr signature implementation on Ristretto with a custom domain separation tag +/// +/// Usage is identical to [`RistrettoSchnorr`], except that you are able to specify the domain separation tag to use +/// when computing challenges for the signature. +/// +/// ## Example +/// /// ```edition2018 +/// # use tari_crypto::ristretto::*; +/// # use tari_crypto::keys::*; +/// # use tari_crypto::hash_domain; +/// # use tari_crypto::signatures::SchnorrSignature; +/// # use tari_crypto::hash::blake2::Blake256; +/// # use tari_utilities::hex::*; +/// # use tari_utilities::ByteArray; +/// # use digest::Digest; +/// +/// hash_domain!(MyCustomDomain, "com.example.custom"); +/// +/// let msg = "Maskerade"; +/// let k = RistrettoSecretKey::from_hex( +/// "bd0b253a619310340a4fa2de54cdd212eac7d088ee1dc47e305c3f6cbd020908", +/// ) +/// .unwrap(); +/// # #[allow(non_snake_case)] +/// let P = RistrettoPublicKey::from_secret_key(&k); +/// let sig: SchnorrSignature = +/// SchnorrSignature::sign_message(&k, msg).unwrap(); +/// assert!(sig.verify_message(&P, msg)); +/// ``` pub type RistrettoSchnorrWithDomain = SchnorrSignature; #[cfg(test)] From 90deb691bc611ce0943f15303785a69ae5b25e0e Mon Sep 17 00:00:00 2001 From: CjS77 Date: Mon, 7 Nov 2022 14:10:00 +0000 Subject: [PATCH 11/11] fix: test arguments --- benches/signatures.rs | 7 +++---- src/ffi/keys.rs | 24 +++++++++--------------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/benches/signatures.rs b/benches/signatures.rs index 61cd07f8..e8c4755f 100644 --- a/benches/signatures.rs +++ b/benches/signatures.rs @@ -9,7 +9,6 @@ use tari_crypto::{ keys::{PublicKey, SecretKey}, ristretto::{RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey}, }; -use tari_utilities::byte_array::ByteArray; fn generate_secret_key(c: &mut Criterion) { c.bench_function("Generate secret key", |b| { @@ -46,7 +45,7 @@ fn sign_message(c: &mut Criterion) { b.iter_batched( gen_keypair, |d| { - let _sig = RistrettoSchnorr::sign_message(&d.k, &d.m).unwrap(); + let _sig = RistrettoSchnorr::sign_message(&d.k, d.m).unwrap(); }, BatchSize::SmallInput, ); @@ -60,10 +59,10 @@ fn verify_message(c: &mut Criterion) { b.iter_batched( || { let d = gen_keypair(); - let s = RistrettoSchnorr::sign_message(&d.k, &d.m).unwrap(); + let s = RistrettoSchnorr::sign_message(&d.k, d.m).unwrap(); (d, s) }, - |(d, s)| assert!(s.verify_message(&d.p, &d.m)), + |(d, s)| assert!(s.verify_message(&d.p, d.m)), BatchSize::SmallInput, ); }); diff --git a/src/ffi/keys.rs b/src/ffi/keys.rs index 46d8de28..b038c441 100644 --- a/src/ffi/keys.rs +++ b/src/ffi/keys.rs @@ -491,36 +491,30 @@ mod test { assert!(!verify( null_mut(), msg.as_ptr() as *const c_char, - &mut pub_nonce, - &mut signature, - &mut err_code - ),); - assert!(!verify( - &pub_key, - null_mut(), - &mut pub_nonce, - &mut signature, + &pub_nonce, + &signature, &mut err_code ),); + assert!(!verify(&pub_key, null_mut(), &pub_nonce, &signature, &mut err_code),); assert!(!verify( &pub_key, msg.as_ptr() as *const c_char, null_mut(), - &mut signature, + &signature, &mut err_code ),); assert!(!verify( &pub_key, msg.as_ptr() as *const c_char, - &mut pub_nonce, + &pub_nonce, null_mut(), &mut err_code ),); assert!(!verify( &pub_key, msg.as_ptr() as *const c_char, - &mut pub_nonce, - &mut signature, + &pub_nonce, + &signature, null_mut() ),); } @@ -540,8 +534,8 @@ mod test { assert!(verify( &pub_key, msg.as_ptr() as *const c_char, - &mut pub_nonce, - &mut signature, + &pub_nonce, + &signature, &mut err_code )); }