From 27a947295fbb69db9ac9d04818c122b697ca92da Mon Sep 17 00:00:00 2001 From: Cayle Sharrock Date: Wed, 9 Nov 2022 16:48:21 +0000 Subject: [PATCH] feat!: improve signature api (#145) * 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. * 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. * 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. * fix clippy lints * fix: benchmarks * chore: change bench message to [u8] In response to review comment * 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` * 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. * fix: update wasm tests to use correct type alias * docs: add missing docs and export new type Export `RistrettoSchnorrWithDomain` and add the missing docs clippy was complaining about * fix: test arguments --- Cargo.toml | 2 +- benches/signatures.rs | 18 ++--- src/ffi/keys.rs | 54 ++++++------- src/hashing.rs | 1 + src/ristretto/mod.rs | 2 +- src/ristretto/ristretto_sig.rs | 141 ++++++++++++++++++++++++++------ src/ristretto/utils.rs | 6 +- src/signatures/schnorr.rs | 142 ++++++++++++++++++++++++++++++++- src/wasm/key_utils.rs | 38 ++++++--- src/wasm/keyring.rs | 9 +-- 10 files changed, 322 insertions(+), 91 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/benches/signatures.rs b/benches/signatures.rs index a39828f1..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| { @@ -30,18 +29,15 @@ fn native_keypair(c: &mut Criterion) { struct SigningData { k: RistrettoSecretKey, p: RistrettoPublicKey, - r: RistrettoSecretKey, - 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 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 +45,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).unwrap(); }, BatchSize::SmallInput, ); @@ -63,10 +59,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, d.m).unwrap(); (d, s) }, - |(d, s)| assert!(s.verify(&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 38f8f179..b038c441 100644 --- a/src/ffi/keys.rs +++ b/src/ffi/keys.rs @@ -51,41 +51,45 @@ 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)) { 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 = 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". @@ -94,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() { @@ -123,13 +127,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). @@ -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 )); } 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/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 103df3b0..69809bf5 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 @@ -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,69 @@ 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", +/// let msg = "Maskerade"; +/// let k = RistrettoSecretKey::from_hex( +/// "bd0b253a619310340a4fa2de54cdd212eac7d088ee1dc47e305c3f6cbd020908", /// ) /// .unwrap(); -/// let s = RistrettoSecretKey::from_hex( +/// # #[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; + +/// # 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(); -/// 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; +pub type RistrettoSchnorrWithDomain = 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, + hash_domain, keys::{PublicKey, SecretKey}, - ristretto::{RistrettoPublicKey, RistrettoSchnorr, RistrettoSecretKey}, + ristretto::{ + ristretto_sig::RistrettoSchnorrWithDomain, + RistrettoPublicKey, + RistrettoSchnorr, + RistrettoSecretKey, + }, + signatures::{SchnorrSigChallenge, SchnorrSignature}, }; #[test] @@ -112,10 +146,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 +158,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 +190,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 +202,71 @@ 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::<_, _, SchnorrSigChallenge>::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()), + "d8f6b29b641113c91175b8d44f265ff1167d58d5aa5ee03e6f1f521505b09d80" + ); + } + + #[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()); + 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()); + } + + #[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..d38bbbab 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, nonce.clone(), e.as_bytes())?; Ok(SignatureSet { nonce, public_nonce, diff --git a/src/signatures/schnorr.rs b/src/signatures/schnorr.rs index 53322979..e44a9888 100644 --- a/src/signatures/schnorr.rs +++ b/src/signatures/schnorr.rs @@ -7,14 +7,24 @@ use std::{ cmp::Ordering, + marker::PhantomData, 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, DomainSeparation}, + keys::{PublicKey, SecretKey}, +}; + +// Define the hashing domain for Schnorr signatures +hash_domain!(SchnorrSigChallenge, "com.tari.schnorr_signature", 1); /// An error occurred during construction of a SchnorrSignature #[derive(Clone, Debug, Error, PartialEq, Eq, Deserialize, Serialize)] @@ -32,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, } } @@ -57,8 +70,36 @@ 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" + )] + #[allow(clippy::needless_pass_by_value)] pub fn sign(secret: K, nonce: K, challenge: &[u8]) -> Result - where K: Add + Mul + Mul { + 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 `::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<'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, @@ -70,6 +111,85 @@ 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<'a, B>(secret: &'a K, message: B) -> Result + where + K: Add + Mul<&'a K, Output = K>, + 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); + 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 @@ -176,3 +296,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..b9d58523 100644 --- a/src/wasm/key_utils.rs +++ b/src/wasm/key_utils.rs @@ -138,6 +138,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 +147,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 +166,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 = 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) => { result.error = format!("Could not create signature. {e}"); @@ -209,8 +224,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 +738,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, msg.as_bytes()).unwrap(); (sig, pk, sk) } @@ -837,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_challenge(&pk, &hash(SAMPLE_CHALLENGE))); + assert!(RistrettoSchnorr::new(p_nonce, s).verify_message(&pk, SAMPLE_CHALLENGE)); } #[wasm_bindgen_test] @@ -889,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))); } } 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)); } }