Make verify_batch deterministic

.github/workflows/rust.yml

@@ -30,7 +30,6 @@ jobs:
       - run: cargo test --target ${{ matrix.target }} --no-default-features --features alloc --lib
       - run: cargo test --target ${{ matrix.target }}
       - run: cargo test --target ${{ matrix.target }} --features batch
-      - run: cargo test --target ${{ matrix.target }} --features batch_deterministic
       - run: cargo test --target ${{ matrix.target }} --features serde
       - run: cargo test --target ${{ matrix.target }} --features pem
 
@@ -68,7 +67,6 @@ jobs:
     - uses: actions/checkout@v3
     - uses: dtolnay/rust-toolchain@stable
     - run: cargo build --benches --features batch
-    - run: cargo build --benches --features batch_deterministic
 
   rustfmt:
     name: Check formatting

Cargo.toml

@@ -24,10 +24,9 @@ rustdoc-args = ["--cfg", "docsrs"]
 features = ["nightly", "batch", "pkcs8"]
 
 [dependencies]
-curve25519-dalek = { version = "=4.0.0-pre.3", default-features = false, features = ["digest", "rand_core"] }
+curve25519-dalek = { version = "=4.0.0-pre.3", default-features = false, features = ["digest"] }
 ed25519 = { version = "=2.0.0-rc.0", default-features = false }
 merlin = { version = "3", default-features = false, optional = true }
-rand = { version = "0.8", default-features = false, optional = true }
 rand_core = { version = "0.6.4", default-features = false, optional = true }
 serde = { version = "1.0", default-features = false, optional = true }
 serde_bytes = { version = "0.11", optional = true }
@@ -44,23 +43,21 @@ rand = "0.8"
 rand_core = { version = "0.6.4", default-features = false }
 serde = { version = "1.0", features = ["derive"] }
 toml = { version = "0.5" }
+curve25519-dalek = { version = "=4.0.0-pre.3", default-features = false, features = ["digest", "rand_core"] }
 
 [[bench]]
 name = "ed25519_benchmarks"
 harness = false
 
 [features]
-default = ["std", "rand"]
-alloc = ["curve25519-dalek/alloc", "ed25519/alloc", "rand?/alloc", "serde?/alloc", "zeroize/alloc"]
-std = ["alloc", "ed25519/std", "rand?/std", "serde?/std", "sha2/std"]
+default = ["std", "rand_core"]
+alloc = ["curve25519-dalek/alloc", "ed25519/alloc", "serde?/alloc", "zeroize/alloc"]
+std = ["alloc", "ed25519/std", "serde?/std", "sha2/std"]
 
 asm = ["sha2/asm"]
-batch = ["alloc", "merlin", "rand"]
-# This feature enables deterministic batch verification.
-batch_deterministic = ["alloc", "merlin", "rand"]
+batch = ["alloc", "merlin", "rand_core"]
 # This features turns off stricter checking for scalar malleability in signatures
 legacy_compatibility = []
 pkcs8 = ["ed25519/pkcs8"]
 pem = ["alloc", "ed25519/pem", "pkcs8"]
-rand = ["dep:rand", "dep:rand_core"]
 serde = ["dep:serde", "serde_bytes", "ed25519/serde"]

README.md

@@ -170,55 +170,19 @@ transactions.
 The scalar component of a signature is not the only source of signature
 malleability, however.  Both the public key used for signature verification and
 the group element component of the signature are malleable, as they may contain
-a small torsion component as a consquence of the curve25519 group not being of
+a small torsion component as a consequence of the curve25519 group not being of
 prime order, but having a small cofactor of 8.
 
 If you wish to also eliminate this source of signature malleability, please
 review the
 [documentation for the `verify_strict()` function](https://doc.dalek.rs/ed25519_dalek/struct.PublicKey.html#method.verify_strict).
 
-# A Note on Randomness Generation
-
-The original paper's specification and the standarisation of RFC8032 do not
-specify precisely how randomness is to be generated, other than using a CSPRNG
-(Cryptographically Secure Random Number Generator).  Particularly in the case of
-signature verification, where the security proof _relies_ on the uniqueness of
-the blinding factors/nonces, it is paramount that these samples of randomness be
-unguessable to an adversary.  Because of this, a current growing belief among
-cryptographers is that it is safer to prefer _synthetic randomness_.
-
-To explain synthetic randomness, we should first explain how `ed25519-dalek`
-handles generation of _deterministic randomness_.  This mode is disabled by
-default due to a tiny-but-not-nonexistent chance that this mode will open users
-up to fault attacks, wherein an adversary who controls all of the inputs to
-batch verification (i.e. the public keys, signatures, and messages) can craft
-them in a specialised manner such as to induce a fault (e.g. causing a
-mistakenly flipped bit in RAM, overheating a processor, etc.).  In the
-deterministic mode, we seed the PRNG which generates our blinding factors/nonces
-by creating
-[a PRNG based on the Fiat-Shamir transform of the public inputs](https://merlin.cool/transcript/rng.html).
-This mode is potentially useful to protocols which require strong auditability
-guarantees, as well as those which do not have access to secure system-/chip-
-provided randomness.  This feature can be enabled via
-`--features='batch_deterministic'`.  Note that we _do not_ support deterministic
-signing, due to the numerous pitfalls therein, including a re-used nonce
-accidentally revealing the secret key.
-
-In the default mode, we do as above in the fully deterministic mode, but we
-ratchet the underlying keccak-f1600 function (used for the provided
-transcript-based PRNG) forward additionally based on some system-/chip- provided
-randomness.  This provides _synthetic randomness_, that is, randomness based on
-both deterministic and undeterinistic data.  The reason for doing this is to
-prevent badly seeded system RNGs from ruining the security of the signature
-verification scheme.
-
 # Features
 
 ## #![no_std]
 
-This library aims to be `#![no_std]` compliant.  If batch verification is
-required (`--features='batch'`), please enable either of the `std` or `alloc`
-features.
+This library aims is fully `#![no_std]` compliant. No features need to be
+enabled or disabled to suppose no-std.
 
 ## Nightly Compilers
 
@@ -264,11 +228,3 @@ with potentially many different public keys over potentially many different
 messages) is available via the `batch` feature.  It uses synthetic randomness, as
 noted above. Batch verification requires allocation, so this won't function in
 heapless settings.
-
-Batch verification is slightly faster with the `std` feature enabled, since it
-permits us to use `rand::thread_rng`.
-
-### Deterministic Batch Signature Verification
-
-The same notion of batch signature verification as above, but with purely
-deterministic randomness can be enabled via the `batch_deterministic` feature.

src/batch.rs

@@ -9,9 +9,6 @@
 
 //! Batch signature verification.
 
-#[cfg(all(feature = "batch", feature = "batch_deterministic"))]
-compile_error!("`batch` and `batch_deterministic` features are mutually exclusive");
-
 use alloc::vec::Vec;
 
 use core::convert::TryFrom;
@@ -27,7 +24,7 @@ pub use curve25519_dalek::digest::Digest;
 
 use merlin::Transcript;
 
-use rand::Rng;
+use rand_core::RngCore;
 
 use sha2::Sha512;
 
@@ -36,59 +33,10 @@ use crate::errors::SignatureError;
 use crate::signature::InternalSignature;
 use crate::VerifyingKey;
 
-/// Gets an RNG from the system, or the zero RNG if we're in deterministic mode. If available, we
-/// prefer `thread_rng`, since it's faster than `OsRng`.
-fn get_rng() -> impl rand_core::CryptoRngCore {
-    #[cfg(all(feature = "batch_deterministic", not(feature = "batch")))]
-    return ZeroRng;
-
-    #[cfg(all(feature = "batch", feature = "std"))]
-    return rand::thread_rng();
-
-    #[cfg(all(feature = "batch", not(feature = "std")))]
-    return rand::rngs::OsRng;
-}
-
-trait BatchTranscript {
-    fn append_scalars(&mut self, scalars: &Vec<Scalar>);
-    fn append_message_lengths(&mut self, message_lengths: &Vec<usize>);
-}
-
-impl BatchTranscript for Transcript {
-    /// Append some `scalars` to this batch verification sigma protocol transcript.
-    ///
-    /// For ed25519 batch verification, we include the following as scalars:
-    ///
-    /// * All of the computed `H(R||A||M)`s to the protocol transcript, and
-    /// * All of the `s` components of each signature.
-    ///
-    /// Each is also prefixed with their index in the vector.
-    fn append_scalars(&mut self, scalars: &Vec<Scalar>) {
-        for (i, scalar) in scalars.iter().enumerate() {
-            self.append_u64(b"", i as u64);
-            self.append_message(b"hram", scalar.as_bytes());
-        }
-    }
-
-    /// Append the lengths of the messages into the transcript.
-    ///
-    /// This is done out of an (potential over-)abundance of caution, to guard against the unlikely
-    /// event of collisions.  However, a nicer way to do this would be to append the message length
-    /// before the message, but this is messy w.r.t. the calculations of the `H(R||A||M)`s above.
-    fn append_message_lengths(&mut self, message_lengths: &Vec<usize>) {
-        for (i, len) in message_lengths.iter().enumerate() {
-            self.append_u64(b"", i as u64);
-            self.append_u64(b"mlen", *len as u64);
-        }
-    }
-}
-
 /// An implementation of `rand_core::RngCore` which does nothing, to provide purely deterministic
 /// transcript-based nonces, rather than synthetically random nonces.
-#[cfg(feature = "batch_deterministic")]
 struct ZeroRng;
 
-#[cfg(feature = "batch_deterministic")]
 impl rand_core::RngCore for ZeroRng {
     fn next_u32(&mut self) -> u32 {
         rand_core::impls::next_u32_via_fill(self)
@@ -114,9 +62,15 @@ impl rand_core::RngCore for ZeroRng {
     }
 }
 
-#[cfg(feature = "batch_deterministic")]
 impl rand_core::CryptoRng for ZeroRng {}
 
+// We write our own gen() function so we don't need to pull in the rand crate
+fn gen_u128<R: RngCore>(rng: &mut R) -> u128 {
+    let mut buf = [0u8; 16];
+    rng.fill_bytes(&mut buf);
+    u128::from_ne_bytes(buf)
+}
+
 /// Verify a batch of `signatures` on `messages` with their respective `verifying_keys`.
 ///
 /// # Inputs
@@ -131,32 +85,10 @@ impl rand_core::CryptoRng for ZeroRng {}
 ///   `SignatureError` containing a description of the internal error which
 ///   occured.
 ///
-/// # Notes on Nonce Generation & Malleability
-///
-/// ## On Synthetic Nonces
-///
-/// This library defaults to using what is called "synthetic" nonces, which
-/// means that a mixture of deterministic (per any unique set of inputs to this
-/// function) data and system randomness is used to seed the CSPRNG for nonce
-/// generation.  For more of the background theory on why many cryptographers
-/// currently believe this to be superior to either purely deterministic
-/// generation or purely relying on the system's randomness, see [this section
-/// of the Merlin design](https://merlin.cool/transcript/rng.html) by Henry de
-/// Valence, isis lovecruft, and Oleg Andreev, as well as Trevor Perrin's
-/// [designs for generalised
-/// EdDSA](https://moderncrypto.org/mail-archive/curves/2017/000925.html).
-///
 /// ## On Deterministic Nonces
 ///
-/// In order to be ammenable to protocols which require stricter third-party
-/// auditability trails, such as in some financial cryptographic settings, this
-/// library also supports a `--features=batch_deterministic` setting, where the
-/// nonces for batch signature verification are derived purely from the inputs
-/// to this function themselves.
-///
-/// **This is not recommended for use unless you have several cryptographers on
-///   staff who can advise you in its usage and all the horrible, terrible,
-///   awful ways it can go horribly, terribly, awfully wrong.**
+/// The nonces for batch signature verification are derived purely from the inputs to this function
+/// themselves.
 ///
 /// In any sigma protocol it is wise to include as much context pertaining
 /// to the public state in the protocol as possible, to avoid malleability
@@ -175,40 +107,30 @@ impl rand_core::CryptoRng for ZeroRng {}
 /// `M`s separately, saves us a bit of context hashing since the
 /// `H(R||A||M)`s need to be computed for the verification equation anyway.
 ///
-/// The latter prevents a malleability attack only found in deterministic batch
-/// signature verification (i.e. only when compiling `ed25519-dalek` with
-/// `--features batch_deterministic`) wherein an adversary, without access
+/// The latter prevents a malleability attack wherein an adversary, without access
 /// to the signing key(s), can take any valid signature, `(s,R)`, and swap
-/// `s` with `s' = -z1`.  This doesn't contitute a signature forgery, merely
+/// `s` with `s' = -z1`.  This doesn't constitute a signature forgery, merely
 /// a vulnerability, as the resulting signature will not pass single
 /// signature verification.  (Thanks to Github users @real_or_random and
 /// @jonasnick for pointing out this malleability issue.)
 ///
-/// For an additional way in which signatures can be made to probablistically
-/// falsely "pass" the synthethic batch verification equation *for the same
-/// inputs*, but *only some crafted inputs* will pass the deterministic batch
-/// single, and neither of these will ever pass single signature verification,
-/// see the documentation for [`VerifyingKey.validate()`].
-///
 /// # Examples
 ///
 /// ```
-/// use ed25519_dalek::verify_batch;
-/// use ed25519_dalek::SigningKey;
-/// use ed25519_dalek::VerifyingKey;
-/// use ed25519_dalek::Signer;
-/// use ed25519_dalek::Signature;
+/// use ed25519_dalek::{
+///     verify_batch, SigningKey, VerifyingKey, Signer, Signature,
+/// };
 /// use rand::rngs::OsRng;
 ///
 /// # fn main() {
 /// let mut csprng = OsRng;
 /// let signing_keys: Vec<_> = (0..64).map(|_| SigningKey::generate(&mut csprng)).collect();
 /// let msg: &[u8] = b"They're good dogs Brant";
-/// let messages: Vec<&[u8]> = (0..64).map(|_| msg).collect();
-/// let signatures:  Vec<Signature> = signing_keys.iter().map(|key| key.sign(&msg)).collect();
-/// let verifying_keys: Vec<VerifyingKey> = signing_keys.iter().map(|key| key.verifying_key()).collect();
+/// let messages: Vec<_> = (0..64).map(|_| msg).collect();
+/// let signatures:  Vec<_> = signing_keys.iter().map(|key| key.sign(&msg)).collect();
+/// let verifying_keys: Vec<_> = signing_keys.iter().map(|key| key.verifying_key()).collect();
 ///
-/// let result = verify_batch(&messages[..], &signatures[..], &verifying_keys[..]);
+/// let result = verify_batch(&messages, &signatures, &verifying_keys);
 /// assert!(result.is_ok());
 /// # }
 /// ```
@@ -234,43 +156,54 @@ pub fn verify_batch(
         .into());
     }
 
-    // Convert all signatures to `InternalSignature`
-    let signatures = signatures
-        .iter()
-        .map(InternalSignature::try_from)
-        .collect::<Result<Vec<_>, _>>()?;
+    // We immediately build a transcript of everything
+    let mut transcript: Transcript = Transcript::new(b"ed25519 batch verification");
 
-    // Compute H(R || A || M) for each (signature, public_key, message) triplet
-    let hrams: Vec<Scalar> = (0..signatures.len())
+    // We make one optimization in the transcript: since we will end up computing
+    // H(R || A || M) for each (signature, public_key, message) triplet, we will feed _that_ into
+    // our transcript rather than each R, A, M individually. This is secure so long as SHA512 is
+    // collision-resistant.
+    // It suffices to take `verifying_keys[i].as_bytes()` even though a `VerifyingKey` has two
+    // fields, and `as_bytes()` only returns the bytes of the first. This is because of our
+    // invariant on `VerifyingKey`, which is that the second field is always the (unique)
+    // decompression of the first. Thus, if one field changes, they both change.
+    let hrams: Vec<[u8; 64]> = (0..signatures.len())
         .map(|i| {
             let mut h: Sha512 = Sha512::default();
-            h.update(signatures[i].R.as_bytes());
+            h.update(signatures[i].r_bytes());
             h.update(verifying_keys[i].as_bytes());
             h.update(&messages[i]);
-            Scalar::from_hash(h)
+            h.finalize().try_into().unwrap()
         })
         .collect();
 
-    // Collect the message lengths and the scalar portions of the signatures, and add them into the
-    // transcript.
-    let message_lengths: Vec<usize> = messages.iter().map(|i| i.len()).collect();
-    let scalars: Vec<Scalar> = signatures.iter().map(|i| i.s).collect();
-
-    // Build a PRNG based on a transcript of the H(R || A || M)s seen thus far. This provides
-    // synthethic randomness in the default configuration, and purely deterministic in the case of
-    // compiling with the "batch_deterministic" feature.
-    let mut transcript: Transcript = Transcript::new(b"ed25519 batch verification");
+    // Covers verifying_keys, messages, and the R half of signatures
+    for hram in hrams.iter() {
+        transcript.append_message(b"hram", hram);
+    }
+    // Covers the s half of the signatures
+    for sig in signatures {
+        transcript.append_message(b"sig.s", sig.s_bytes());
+    }
 
-    transcript.append_scalars(&hrams);
-    transcript.append_message_lengths(&message_lengths);
-    transcript.append_scalars(&scalars);
+    // Finalize the transcript
+    let mut rng = transcript.build_rng().finalize(&mut ZeroRng);
 
-    let mut prng = transcript.build_rng().finalize(&mut get_rng());
+    // Convert all signatures to `InternalSignature`
+    let signatures = signatures
+        .iter()
+        .map(InternalSignature::try_from)
+        .collect::<Result<Vec<_>, _>>()?;
+    // Convert the H(R || A || M) values into scalars
+    let hrams: Vec<Scalar> = hrams
+        .iter()
+        .map(Scalar::from_bytes_mod_order_wide)
+        .collect();
 
     // Select a random 128-bit scalar for each signature.
     let zs: Vec<Scalar> = signatures
         .iter()
-        .map(|_| Scalar::from(prng.gen::<u128>()))
+        .map(|_| Scalar::from(gen_u128(&mut rng)))
         .collect();
 
     // Compute the basepoint coefficient, ∑ s[i]z[i] (mod l)