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

@@ -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]

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,26 +29,23 @@ 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) {
     c.bench_function("Create RistrettoSchnorr", move |b| {
         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,
         );
     });

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
             ));
         }

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;
 

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