3.8.6 Correct transcript to scalar derivation

Replaces the externally passed in Digest with C::H since C is available.

Cargo.lock

@@ -912,6 +912,7 @@ dependencies = [
  "elliptic-curve",
  "ff",
  "ff-group-tests",
+ "flexible-transcript",
  "group",
  "hex",
  "k256",
@@ -7493,13 +7494,14 @@ dependencies = [
 name = "schnorr-signatures"
 version = "0.2.0"
 dependencies = [
- "blake2",
  "ciphersuite",
  "dalek-ff-group",
- "digest 0.10.6",
+ "flexible-transcript",
  "group",
+ "hex",
  "multiexp",
  "rand_core 0.6.4",
+ "sha2 0.10.6",
  "zeroize",
 ]
 

crypto/ciphersuite/Cargo.toml

@@ -19,6 +19,7 @@ zeroize = { version = "1.5", features = ["zeroize_derive"] }
 subtle = "2"
 
 digest = "0.10"
+transcript = { package = "flexible-transcript", path = "../transcript", version = "0.2" }
 sha2 = { version = "0.10", optional = true }
 sha3 = { version = "0.10", optional = true }
 

crypto/ciphersuite/src/lib.rs

@@ -11,7 +11,8 @@ use rand_core::{RngCore, CryptoRng};
 use zeroize::Zeroize;
 use subtle::ConstantTimeEq;
 
-use digest::{core_api::BlockSizeUser, Digest};
+use digest::{core_api::BlockSizeUser, Digest, HashMarker};
+use transcript::SecureDigest;
 
 use group::{
   ff::{Field, PrimeField, PrimeFieldBits},
@@ -49,7 +50,7 @@ pub trait Ciphersuite: Clone + Copy + PartialEq + Eq + Debug + Zeroize {
   type G: Group<Scalar = Self::F> + GroupOps + PrimeGroup + Zeroize + ConstantTimeEq;
   /// Hash algorithm used with this curve.
   // Requires BlockSizeUser so it can be used within Hkdf which requies that.
-  type H: Clone + BlockSizeUser + Digest;
+  type H: Clone + BlockSizeUser + Digest + HashMarker + SecureDigest;
 
   /// ID for this curve.
   const ID: &'static [u8];

crypto/schnorr/Cargo.toml

@@ -17,7 +17,6 @@ rand_core = "0.6"
 
 zeroize = { version = "1.5", features = ["zeroize_derive"] }
 
-digest = "0.10"
 transcript = { package = "flexible-transcript", path = "../transcript", version = "0.2" }
 
 group = "0.12"

crypto/schnorr/src/aggregate.rs

@@ -13,28 +13,49 @@ use ciphersuite::Ciphersuite;
 
 use crate::SchnorrSignature;
 
-// Performs a big-endian modular reduction of the hash value
-// This is used by the below aggregator to prevent mutability
-// Only an 128-bit scalar is needed to offer 128-bits of security against malleability per
-// https://cr.yp.to/badbatch/badbatch-20120919.pdf
-// Accordingly, while a 256-bit hash used here with a 256-bit ECC will have bias, it shouldn't be
-// an issue
-fn scalar_from_digest<D: Clone + SecureDigest, F: PrimeField>(
-  digest: &mut DigestTranscript<D>,
-) -> F {
-  let bytes = digest.challenge(b"aggregation_weight");
+// Returns a unbiased scalar weight to use on a signature in order to prevent malleability
+fn weight<D: Clone + SecureDigest, F: PrimeField>(digest: &mut DigestTranscript<D>) -> F {
+  let mut bytes = digest.challenge(b"aggregation_weight");
   debug_assert_eq!(bytes.len() % 8, 0);
+  // This should be guaranteed thanks to SecureDigest
+  debug_assert!(bytes.len() >= 32);
 
   let mut res = F::zero();
   let mut i = 0;
-  while i < bytes.len() {
-    if i != 0 {
-      for _ in 0 .. 8 {
+
+  // Derive a scalar from enough bits of entropy that bias is < 2^128
+  // This can't be const due to its usage of a generic
+  // Also due to the usize::try_from, yet that could be replaced with an `as`
+  // The + 7 forces it to round up
+  #[allow(non_snake_case)]
+  let BYTES: usize = usize::try_from(((F::NUM_BITS + 128) + 7) / 8).unwrap();
+
+  let mut remaining = BYTES;
+
+  // We load bits in as u64s
+  const WORD_LEN_IN_BITS: usize = 64;
+  const WORD_LEN_IN_BYTES: usize = WORD_LEN_IN_BITS / 8;
+
+  let mut first = true;
+  while i < remaining {
+    // Shift over the already loaded bits
+    if !first {
+      for _ in 0 .. WORD_LEN_IN_BITS {
         res += res;
       }
     }
-    res += F::from(u64::from_be_bytes(bytes[i .. (i + 8)].try_into().unwrap()));
-    i += 8;
+    first = false;
+
+    // Add the next 64 bits
+    res += F::from(u64::from_be_bytes(bytes[i .. (i + WORD_LEN_IN_BYTES)].try_into().unwrap()));
+    i += WORD_LEN_IN_BYTES;
+
+    // If we've exhausted this challenge, get another
+    if i == bytes.len() {
+      bytes = digest.challenge(b"aggregation_weight_continued");
+      remaining -= i;
+      i = 0;
+    }
   }
   res
 }
@@ -92,16 +113,12 @@ impl<C: Ciphersuite> SchnorrAggregate<C> {
   /// The DST used here must prevent a collision with whatever hash function produced the
   /// challenges.
   #[must_use]
-  pub fn verify<D: Clone + SecureDigest>(
-    &self,
-    dst: &'static [u8],
-    keys_and_challenges: &[(C::G, C::F)],
-  ) -> bool {
+  pub fn verify(&self, dst: &'static [u8], keys_and_challenges: &[(C::G, C::F)]) -> bool {
     if self.Rs.len() != keys_and_challenges.len() {
       return false;
     }
 
-    let mut digest = DigestTranscript::<D>::new(dst);
+    let mut digest = DigestTranscript::<C::H>::new(dst);
     digest.domain_separate(b"signatures");
     for (_, challenge) in keys_and_challenges {
       digest.append_message(b"challenge", challenge.to_repr());
@@ -109,7 +126,7 @@ impl<C: Ciphersuite> SchnorrAggregate<C> {
 
     let mut pairs = Vec::with_capacity((2 * keys_and_challenges.len()) + 1);
     for (i, (key, challenge)) in keys_and_challenges.iter().enumerate() {
-      let z = scalar_from_digest(&mut digest);
+      let z = weight(&mut digest);
       pairs.push((z, self.Rs[i]));
       pairs.push((z * challenge, *key));
     }
@@ -120,18 +137,18 @@ impl<C: Ciphersuite> SchnorrAggregate<C> {
 
 #[allow(non_snake_case)]
 #[derive(Clone, Debug, Zeroize)]
-pub struct SchnorrAggregator<D: Clone + SecureDigest, C: Ciphersuite> {
-  digest: DigestTranscript<D>,
+pub struct SchnorrAggregator<C: Ciphersuite> {
+  digest: DigestTranscript<C::H>,
   sigs: Vec<SchnorrSignature<C>>,
 }
 
-impl<D: Clone + SecureDigest, C: Ciphersuite> SchnorrAggregator<D, C> {
+impl<C: Ciphersuite> SchnorrAggregator<C> {
   /// Create a new aggregator.
   ///
   /// The DST used here must prevent a collision with whatever hash function produced the
   /// challenges.
   pub fn new(dst: &'static [u8]) -> Self {
-    let mut res = Self { digest: DigestTranscript::<D>::new(dst), sigs: vec![] };
+    let mut res = Self { digest: DigestTranscript::<C::H>::new(dst), sigs: vec![] };
     res.digest.domain_separate(b"signatures");
     res
   }
@@ -152,7 +169,7 @@ impl<D: Clone + SecureDigest, C: Ciphersuite> SchnorrAggregator<D, C> {
       SchnorrAggregate { Rs: Vec::with_capacity(self.sigs.len()), s: C::F::zero() };
     for i in 0 .. self.sigs.len() {
       aggregate.Rs.push(self.sigs[i].R);
-      aggregate.s += self.sigs[i].s * scalar_from_digest::<_, C::F>(&mut self.digest);
+      aggregate.s += self.sigs[i].s * weight::<_, C::F>(&mut self.digest);
     }
     Some(aggregate)
   }

crypto/schnorr/src/tests/mod.rs

@@ -3,8 +3,6 @@ use core::ops::Deref;
 use zeroize::Zeroizing;
 use rand_core::OsRng;
 
-use sha2::Sha256;
-
 use group::{ff::Field, Group};
 use multiexp::BatchVerifier;
 
@@ -84,7 +82,7 @@ pub(crate) fn aggregate<C: Ciphersuite>() {
   // Create 5 signatures
   let mut keys = vec![];
   let mut challenges = vec![];
-  let mut aggregator = SchnorrAggregator::<Sha256, C>::new(DST);
+  let mut aggregator = SchnorrAggregator::<C>::new(DST);
   for i in 0 .. 5 {
     keys.push(Zeroizing::new(C::random_nonzero_F(&mut OsRng)));
     // In practice, this MUST be a secure challenge binding to the nonce, key, and any message
@@ -102,7 +100,7 @@ pub(crate) fn aggregate<C: Ciphersuite>() {
   let aggregate = aggregator.complete().unwrap();
   let aggregate =
     SchnorrAggregate::<C>::read::<&[u8]>(&mut aggregate.serialize().as_ref()).unwrap();
-  assert!(aggregate.verify::<Sha256>(
+  assert!(aggregate.verify(
     DST,
     keys
       .iter()