From 97374a3e243ad8fcc707018f861390e7220b7ceb Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 2 Mar 2023 10:04:18 -0500 Subject: [PATCH] 3.8.6 Correct transcript to scalar derivation Replaces the externally passed in Digest with C::H since C is available. --- Cargo.lock | 6 ++- crypto/ciphersuite/Cargo.toml | 1 + crypto/ciphersuite/src/lib.rs | 5 ++- crypto/schnorr/Cargo.toml | 1 - crypto/schnorr/src/aggregate.rs | 71 ++++++++++++++++++++------------- crypto/schnorr/src/tests/mod.rs | 6 +-- 6 files changed, 54 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b740c7f0..a4913d97 100644 --- a/Cargo.lock +++ b/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", ] diff --git a/crypto/ciphersuite/Cargo.toml b/crypto/ciphersuite/Cargo.toml index f3f12f78..4497299d 100644 --- a/crypto/ciphersuite/Cargo.toml +++ b/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 } diff --git a/crypto/ciphersuite/src/lib.rs b/crypto/ciphersuite/src/lib.rs index 076da606..ab5a0a4b 100644 --- a/crypto/ciphersuite/src/lib.rs +++ b/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 + 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]; diff --git a/crypto/schnorr/Cargo.toml b/crypto/schnorr/Cargo.toml index 9606206d..2a46c47a 100644 --- a/crypto/schnorr/Cargo.toml +++ b/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" diff --git a/crypto/schnorr/src/aggregate.rs b/crypto/schnorr/src/aggregate.rs index c528d9b3..bcd36910 100644 --- a/crypto/schnorr/src/aggregate.rs +++ b/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( - digest: &mut DigestTranscript, -) -> 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(digest: &mut DigestTranscript) -> 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 SchnorrAggregate { /// The DST used here must prevent a collision with whatever hash function produced the /// challenges. #[must_use] - pub fn verify( - &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::::new(dst); + let mut digest = DigestTranscript::::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 SchnorrAggregate { 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 SchnorrAggregate { #[allow(non_snake_case)] #[derive(Clone, Debug, Zeroize)] -pub struct SchnorrAggregator { - digest: DigestTranscript, +pub struct SchnorrAggregator { + digest: DigestTranscript, sigs: Vec>, } -impl SchnorrAggregator { +impl SchnorrAggregator { /// 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::::new(dst), sigs: vec![] }; + let mut res = Self { digest: DigestTranscript::::new(dst), sigs: vec![] }; res.digest.domain_separate(b"signatures"); res } @@ -152,7 +169,7 @@ impl SchnorrAggregator { 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) } diff --git a/crypto/schnorr/src/tests/mod.rs b/crypto/schnorr/src/tests/mod.rs index c2791870..1b4fe212 100644 --- a/crypto/schnorr/src/tests/mod.rs +++ b/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() { // Create 5 signatures let mut keys = vec![]; let mut challenges = vec![]; - let mut aggregator = SchnorrAggregator::::new(DST); + let mut aggregator = SchnorrAggregator::::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() { let aggregate = aggregator.complete().unwrap(); let aggregate = SchnorrAggregate::::read::<&[u8]>(&mut aggregate.serialize().as_ref()).unwrap(); - assert!(aggregate.verify::( + assert!(aggregate.verify( DST, keys .iter()