feat!: differentiate scalar parsing from byte arrays (#194)

Currently, creating a scalar `RistrettoSecretKey` [from a byte
array](https://github.com/tari-project/tari-crypto/blob/053119f2110aaf3089c7b9df96f50b8cc8d3217a/src/ristretto/ristretto_keys.rs#L90-L100)
performs modular reduction on 32 bytes. For cases where the input is
intended to be canonical, this is suboptimal. For cases where the input
is produced from a hashing operation, wide reduction should be used to
mitigate bias.

This work renames `SecretKey::from_bytes` to
`SecretKey::from_canonical_bytes` to support an underlying `ByteArray`
trait update. In the case of `RistrettoSecretKey`, it uses the curve
library's canonical parser and returns an error if the provided byte
slice is not a canonical scalar encoding.

It also adds a new `SecretKey::from_uniform_bytes` function that uses
wide reduction. For constructions like signatures and KDFs that use
hashing operations to produce scalar values, this function is used and
the underlying hashers are updated to produce 64-byte output in the case
of `RistrettoSecretKey`.

It updates the Schnorr signature API to support raw signing and
verification using challenge byte slices that are either canonical
encodings or uniform. It renames several existing functions for clarity.

It corrects a few typos that were discovered along the way.

Closes #189.

BREAKING CHANGE: This changes the way that scalars are produced from
byte arrays, modifies the `SecretKey` trait and corresponding
`RistrettoSecretKey` implementation, and updates the Schnorr signature
API.

Cargo.toml

@@ -11,7 +11,7 @@ version = "0.18.0"
 edition = "2018"
 
 [dependencies]
-tari_utilities = { version = "0.5", default-features = false, features = ["zero"] }
+tari_utilities = { version = "0.6", default-features = false, features = ["zero"] }
 blake2 = { version = "0.10", default-features = false  }
 borsh = { version = "0.10" , optional = true , default-features = false}
 bulletproofs_plus = { package = "tari_bulletproofs_plus", version = "0.3", optional = true }
@@ -27,7 +27,7 @@ snafu = { version = "0.7", default-features = false}
 zeroize = {version = "1" , default-features = false}
 
 [dev-dependencies]
-tari_utilities = { version = "0.5", features = ["std"] }
+tari_utilities = { version = "0.6", features = ["std"] }
 serde = { version = "1.0"}
 bincode = { version = "1.1" }
 criterion = { version = "0.5", default-features = false }

benches/signatures.rs

@@ -46,7 +46,7 @@ fn sign_message(c: &mut Criterion) {
         b.iter_batched(
             gen_keypair,
             |d| {
-                let _sig = RistrettoSchnorr::sign_message(&d.k, d.m, &mut OsRng).unwrap();
+                let _sig = RistrettoSchnorr::sign(&d.k, d.m, &mut OsRng).unwrap();
             },
             BatchSize::SmallInput,
         );
@@ -60,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, &mut OsRng).unwrap();
+                let s = RistrettoSchnorr::sign(&d.k, d.m, &mut OsRng).unwrap();
                 (d, s)
             },
-            |(d, s)| assert!(s.verify_message(&d.p, d.m)),
+            |(d, s)| assert!(s.verify(&d.p, d.m)),
             BatchSize::SmallInput,
         );
     });

src/commitment.rs

@@ -68,8 +68,8 @@ where P: PublicKey
 impl<P> ByteArray for HomomorphicCommitment<P>
 where P: PublicKey
 {
-    fn from_bytes(bytes: &[u8]) -> Result<Self, ByteArrayError> {
-        let p = P::from_bytes(bytes)?;
+    fn from_canonical_bytes(bytes: &[u8]) -> Result<Self, ByteArrayError> {
+        let p = P::from_canonical_bytes(bytes)?;
         Ok(Self(p))
     }
 

src/errors.rs

@@ -70,7 +70,7 @@ pub enum HashingError {
         reason: String,
     },
     /// The digest does not produce enough output
-    #[snafu(display("The digest does produce enough output.`{bytes}' bytes are required."))]
+    #[snafu(display("The digest does not produce enough output.`{bytes}' bytes are required."))]
     DigestTooShort {
         /// The number of bytes required
         bytes: usize,

src/hashing.rs

@@ -32,9 +32,16 @@ use alloc::string::String;
 use core::{marker::PhantomData, ops::Deref};
 
 use blake2::{Blake2b, Blake2bVar};
-use digest::{consts::U32, Digest, FixedOutput, FixedOutputReset, Output, OutputSizeUser, Update};
+use digest::{
+    consts::{U32, U64},
+    Digest,
+    FixedOutput,
+    FixedOutputReset,
+    Output,
+    OutputSizeUser,
+    Update,
+};
 use sha3::Sha3_256;
-use tari_utilities::ByteArray;
 
 use crate::{
     alloc::string::ToString,
@@ -209,10 +216,10 @@ impl<D: Digest> AsRef<[u8]> for DomainSeparatedHash<D> {
 /// Calculating a signature challenge
 ///
 /// ```
+/// # use blake2::Blake2b;
+/// # use digest::{consts::U32, Digest};
 /// # use tari_utilities::hex::{to_hex, Hex};
-/// use blake2::{Blake2b, Digest};
-/// use digest::consts::U32;
-/// use tari_crypto::{
+/// # use tari_crypto::{
 ///     hash_domain,
 ///     hashing::{DomainSeparatedHash, DomainSeparatedHasher, DomainSeparation},
 /// };
@@ -418,6 +425,8 @@ impl LengthExtensionAttackResistant for Sha3_256 {}
 
 impl LengthExtensionAttackResistant for Blake2b<U32> {}
 
+impl LengthExtensionAttackResistant for Blake2b<U64> {}
+
 //------------------------------------------------    HMAC  ------------------------------------------------------------
 /// A domain separation tag for use in MAC derivation algorithms.
 pub struct MacDomain;
@@ -517,62 +526,69 @@ impl<D: Digest> Deref for Mac<D> {
 /// `RistrettoKdf` is an implementation of [`DerivedKeyDomain`] that generates Ristretto keys.
 ///
 /// ```
+/// # use blake2::Blake2b;
+/// # use digest::{consts::U64, Digest};
 /// # use tari_utilities::ByteArray;
 /// # use tari_utilities::hex::Hex;
 /// # use tari_crypto::errors::HashingError;
 /// # use tari_crypto::hashing::{DerivedKeyDomain, MacDomain};
 /// # use tari_crypto::keys::SecretKey;
 /// # use tari_crypto::ristretto::ristretto_keys::RistrettoKdf;
 /// # use tari_crypto::ristretto::RistrettoSecretKey;
-/// # use digest::consts::U32;
-/// # use blake2::Blake2b;
 ///
 /// fn wallet_keys(
 ///     primary_key: &RistrettoSecretKey,
 ///     index: usize,
 /// ) -> Result<RistrettoSecretKey, HashingError> {
-///     RistrettoKdf::generate::<Blake2b<U32>>(
+///     RistrettoKdf::generate::<Blake2b<U64>>(
 ///         primary_key.as_bytes(),
 ///         &index.to_le_bytes(),
 ///         "wallet",
 ///     )
 /// }
 ///
 /// let key = RistrettoSecretKey::from_hex(
-///     "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c",
+///     "a8fb609c5ab7cc07548b076b6c25cc3237c4526fb7a6dcb83b26f457b172c20a",
 /// )
 /// .unwrap();
 /// let key_1 = wallet_keys(&key, 1).unwrap();
 /// assert_eq!(
 ///     key_1.to_hex(),
-///     "b778b8b5041fbde6c78be5bafd6d62633824bf303c97736d7337b3f6f70c4e0b"
+///     "08106b88a2ff4c52d1d8b458cf34802df8655ba989a7d91351e3504e087a2e0c"
 /// );
 /// let key_64 = wallet_keys(&key, 64).unwrap();
 /// assert_eq!(
 ///     key_64.to_hex(),
-///     "09e5204c93406ef3334ff5f7a4d5d84199ceb9119fafcb98928fa95e95f0ae05"
+///     "2c2206dadd2a21e71b6c52dd321572cde0f2b00e7116e1123fb580b09ed1b70e"
 /// );
 /// ```
 pub trait DerivedKeyDomain: DomainSeparation {
     /// The associated derived secret key type
     type DerivedKeyType: SecretKey;
 
-    /// Derive a key from the input key using a suitable domain separation tag and the given application label.
-    /// An error is returned if the supplied primary key isn't at least as long as the digest algorithm's output size.
+    /// Derive a key from the input key using a suitable domain separation tag and the given application label by wide
+    /// reduction. An error is returned if the supplied primary key isn't at least as long as the derived key.
     /// If the digest's output size is not sufficient to generate the derived key type, then an error will be thrown.
     fn generate<D>(primary_key: &[u8], data: &[u8], label: &'static str) -> Result<Self::DerivedKeyType, HashingError>
     where
         Self: Sized,
         D: Digest + Update,
     {
-        if primary_key.as_ref().len() < <D as Digest>::output_size() {
+        // Ensure the primary key is at least as long as the derived key
+        if primary_key.len() < <Self::DerivedKeyType as SecretKey>::KEY_LEN {
             return Err(HashingError::InputTooShort {});
         }
+
+        // Ensure the digest length is suitable for wide reduction
+        if <D as Digest>::output_size() != <Self::DerivedKeyType as SecretKey>::WIDE_REDUCTION_LEN {
+            return Err(HashingError::InputTooShort {});
+        }
+
         let hash = DomainSeparatedHasher::<D, Self>::new_with_label(label)
             .chain(primary_key)
             .chain(data)
             .finalize();
-        let derived_key = Self::DerivedKeyType::from_bytes(hash.as_ref())
+        let derived_key = Self::DerivedKeyType::from_uniform_bytes(hash.as_ref())
             .map_err(|e| HashingError::ConversionFromBytes { reason: e.to_string() })?;
         Ok(derived_key)
     }

src/keys.rs

@@ -9,7 +9,7 @@
 use core::ops::Add;
 
 use rand_core::{CryptoRng, RngCore};
-use tari_utilities::ByteArray;
+use tari_utilities::{ByteArray, ByteArrayError};
 use zeroize::{Zeroize, ZeroizeOnDrop};
 
 /// A trait specifying common behaviour for representing `SecretKey`s. Specific elliptic curve
@@ -32,13 +32,20 @@ pub trait SecretKey:
     /// The length of the byte encoding of a key, in bytes
     const KEY_LEN: usize;
 
+    /// The number of bytes used for construction by wide reduction
+    const WIDE_REDUCTION_LEN: usize;
+
     /// The length of the byte encoding of a key, in bytes
     fn key_length() -> usize {
         Self::KEY_LEN
     }
 
     /// Generates a random secret key
     fn random<R: RngCore + CryptoRng>(rng: &mut R) -> Self;
+
+    /// Generates a secret key from a slice of uniformly-distributed bytes using wide reduction
+    /// If the number of bytes is incorrect, this will fail
+    fn from_uniform_bytes(bytes: &[u8]) -> Result<Self, ByteArrayError>;
 }
 
 //----------------------------------------   Public Keys  ----------------------------------------//

src/ristretto/mod.rs

@@ -13,7 +13,6 @@ pub mod ristretto_keys;
 mod ristretto_sig;
 #[cfg(feature = "serde")]
 pub mod serialize;
-pub mod utils;
 
 pub use self::{
     ristretto_com_and_pub_sig::RistrettoComAndPubSig,

src/ristretto/pedersen/mod.rs

@@ -90,7 +90,7 @@ mod test {
         let (_, p) = RistrettoPublicKey::random_keypair(&mut rng);
         let c = PedersenCommitment::from_public_key(&p);
         assert_eq!(c.as_public_key(), &p);
-        let c2 = PedersenCommitment::from_bytes(c.as_bytes()).unwrap();
+        let c2 = PedersenCommitment::from_canonical_bytes(c.as_bytes()).unwrap();
         assert_eq!(c, c2);
     }
 

src/ristretto/ristretto_com_and_pub_sig.rs

@@ -30,9 +30,18 @@ use crate::{
 ///     "8063d85e151abee630e643e2b3dc47bfaeb8aa859c9d10d60847985f286aad19",
 /// )
 /// .unwrap();
-/// let u_a = RistrettoSecretKey::from_bytes(b"10000000000000000000000010000000").unwrap();
-/// let u_x = RistrettoSecretKey::from_bytes(b"a00000000000000000000000a0000000").unwrap();
-/// let u_y = RistrettoSecretKey::from_bytes(b"a00000000000000000000000a0000000").unwrap();
+/// let u_a = RistrettoSecretKey::from_hex(
+///     "a8fb609c5ab7cc07548b076b6c25cc3237c4526fb7a6dcb83b26f457b172c20a",
+/// )
+/// .unwrap();
+/// let u_x = RistrettoSecretKey::from_hex(
+///     "0e689df8ad4ad9d2fd5aaf8cb0a66d85cb0d4b7a380405514d453625813b0b0f",
+/// )
+/// .unwrap();
+/// let u_y = RistrettoSecretKey::from_hex(
+///     "f494050bd0d4ed0ec514cdce9430d0564df6b35d2a12b7daa0e99c7d94a06509",
+/// )
+/// .unwrap();
 /// let sig = RistrettoComAndPubSig::new(ephemeral_commitment, ephemeral_pubkey, u_a, u_x, u_y);
 /// ```
 ///
@@ -48,7 +57,7 @@ use crate::{
 /// # use tari_crypto::ristretto::pedersen::*;
 /// use tari_crypto::ristretto::pedersen::commitment_factory::PedersenCommitmentFactory;
 /// use tari_utilities::hex::Hex;
-/// use digest::consts::U32;
+/// use digest::consts::U64;
 ///
 /// let mut rng = rand::thread_rng();
 /// let a_val = RistrettoSecretKey::random(&mut rng);
@@ -57,7 +66,7 @@ use crate::{
 /// let a_nonce = RistrettoSecretKey::random(&mut rng);
 /// let x_nonce = RistrettoSecretKey::random(&mut rng);
 /// let y_nonce = RistrettoSecretKey::random(&mut rng);
-/// let e = Blake2b::<U32>::digest(b"Maskerade"); // In real life, this should be strong Fiat-Shamir!
+/// let e = Blake2b::<U64>::digest(b"Maskerade"); // In real life, this should be strong Fiat-Shamir!
 /// let factory = PedersenCommitmentFactory::default();
 /// let commitment = factory.commit(&x_val, &a_val);
 /// let pubkey = RistrettoPublicKey::from_secret_key(&y_val);
@@ -72,8 +81,8 @@ pub type RistrettoComAndPubSig = CommitmentAndPublicKeySignature<RistrettoPublic
 #[cfg(test)]
 mod test {
     use blake2::Blake2b;
-    use digest::{consts::U32, Digest};
-    use tari_utilities::{hex::from_hex, ByteArray};
+    use digest::{consts::U64, Digest};
+    use tari_utilities::ByteArray;
 
     use crate::{
         commitment::HomomorphicCommitmentFactory,
@@ -132,15 +141,15 @@ mod test {
         let ephemeral_commitment = factory.commit(&r_x, &r_a);
         let ephemeral_pubkey = RistrettoPublicKey::from_secret_key(&r_y);
 
-        // Challenge; doesn't use proper Fiat-Shamir, so it's for testing only!
-        let challenge = Blake2b::<U32>::new()
+        // Challenge; doesn't use domain-separated Fiat-Shamir, so it's for testing only!
+        let challenge = Blake2b::<U64>::new()
             .chain_update(commitment.as_bytes())
             .chain_update(pubkey.as_bytes())
             .chain_update(ephemeral_commitment.as_bytes())
             .chain_update(ephemeral_pubkey.as_bytes())
             .chain_update(b"Small Gods")
             .finalize();
-        let e_key = RistrettoSecretKey::from_bytes(&challenge).unwrap();
+        let e_key = RistrettoSecretKey::from_uniform_bytes(&challenge).unwrap();
 
         // Responses
         let u_a = &r_a + e_key.clone() * &a_value;
@@ -178,7 +187,7 @@ mod test {
         assert!(!sig.verify_challenge(&commitment, &evil_pubkey, &challenge, &factory, &mut rng));
 
         // A different challenge should fail
-        let evil_challenge = Blake2b::<U32>::digest(b"Guards! Guards!");
+        let evil_challenge = Blake2b::<U64>::digest(b"Guards! Guards!");
         assert!(!sig.verify_challenge(&commitment, &pubkey, &evil_challenge, &factory, &mut rng));
     }
 
@@ -205,7 +214,7 @@ mod test {
         let ephemeral_pubkey = RistrettoPublicKey::from_secret_key(&r_y);
 
         // Challenge; doesn't use proper Fiat-Shamir, so it's for testing only!
-        let challenge = Blake2b::<U32>::new()
+        let challenge = Blake2b::<U64>::new()
             .chain_update(commitment.as_bytes())
             .chain_update(pubkey.as_bytes())
             .chain_update(ephemeral_commitment.as_bytes())
@@ -292,7 +301,7 @@ mod test {
         let ephemeral_pubkey_bob = RistrettoPublicKey::from_secret_key(&r_y_bob);
 
         // The challenge is common to Alice and Bob; here we use an arbitrary hash
-        let challenge = Blake2b::<U32>::digest(b"Test challenge");
+        let challenge = Blake2b::<U64>::digest(b"Test challenge");
 
         // Alice's signature
         let sig_alice = RistrettoComAndPubSig::sign(
@@ -338,28 +347,6 @@ mod test {
         assert!(sig_sum.verify_challenge(&commitment_sum, &pubkey_sum, &challenge, &factory, &mut rng))
     }
 
-    /// 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]
-    fn challenge_from_invalid_scalar() {
-        let mut rng = rand::thread_rng();
-        let factory = PedersenCommitmentFactory::default();
-
-        let a_value = RistrettoSecretKey::random(&mut rng);
-        let x_value = RistrettoSecretKey::random(&mut rng);
-        let y_value = RistrettoSecretKey::random(&mut rng);
-
-        let message = from_hex("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap();
-
-        let r_a = RistrettoSecretKey::random(&mut rng);
-        let r_x = RistrettoSecretKey::random(&mut rng);
-        let r_y = RistrettoSecretKey::random(&mut rng);
-
-        assert!(
-            RistrettoComAndPubSig::sign(&a_value, &x_value, &y_value, &r_a, &r_x, &r_y, &message, &factory).is_ok()
-        );
-    }
-
     #[test]
     fn to_vec() {
         let sig = RistrettoComAndPubSig::default();