From 28eed1cba0acdd0e9804324118cae95c08eaa9d8 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Fri, 27 Sep 2019 00:37:30 +0000 Subject: [PATCH] Add PublicKey::verify_strict() and Keypair::verify_strict() methods. --- benches/ed25519_benchmarks.rs | 12 +++++ src/ed25519.rs | 72 +++++++++++++++++++++++++ src/public.rs | 99 +++++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+) diff --git a/benches/ed25519_benchmarks.rs b/benches/ed25519_benchmarks.rs index e07eb61..0af2812 100644 --- a/benches/ed25519_benchmarks.rs +++ b/benches/ed25519_benchmarks.rs @@ -56,6 +56,17 @@ mod ed25519_benches { }); } + fn verify_strict(c: &mut Criterion) { + let mut csprng: ThreadRng = thread_rng(); + let keypair: Keypair = Keypair::generate(&mut csprng); + let msg: &[u8] = b""; + let sig: Signature = keypair.sign(msg); + + c.bench_function("Ed25519 strict signature verification", move |b| { + b.iter(| | keypair.verify_strict(msg, &sig)) + }); + } + fn verify_batch_signatures(c: &mut Criterion) { static BATCH_SIZES: [usize; 8] = [4, 8, 16, 32, 64, 96, 128, 256]; @@ -90,6 +101,7 @@ mod ed25519_benches { sign, sign_expanded_key, verify, + verify_strict, verify_batch_signatures, key_generation, } diff --git a/src/ed25519.rs b/src/ed25519.rs index a76b091..43da922 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -447,6 +447,78 @@ impl Keypair { { self.public.verify_prehashed(prehashed_message, context, signature) } + + /// Strictly verify a signature on a message with this keypair's public key. + /// + /// # On The (Multiple) Sources of Malleability in Ed25519 Signatures + /// + /// This version of verification is technically non-RFC8032 compliant. The + /// following explains why. + /// + /// 1. Scalar Malleability + /// + /// The authors of the RFC explicitly stated that verification of an ed25519 + /// signature must fail if the scalar `s` is not properly reduced mod \ell: + /// + /// > To verify a signature on a message M using public key A, with F + /// > being 0 for Ed25519ctx, 1 for Ed25519ph, and if Ed25519ctx or + /// > Ed25519ph is being used, C being the context, first split the + /// > signature into two 32-octet halves. Decode the first half as a + /// > point R, and the second half as an integer S, in the range + /// > 0 <= s < L. Decode the public key A as point A'. If any of the + /// > decodings fail (including S being out of range), the signature is + /// > invalid.) + /// + /// All `verify_*()` functions within ed25519-dalek perform this check. + /// + /// 2. Point malleability + /// + /// The authors of the RFC added in a malleability check to step #3 in + /// §5.1.7, for small torsion components in the `R` value of the signature, + /// *which is not strictly required*, as they state: + /// + /// > Check the group equation [8][S]B = [8]R + [8][k]A'. It's + /// > sufficient, but not required, to instead check [S]B = R + [k]A'. + /// + /// # History of Malleability Checks + /// + /// As originally defined (cf. the "Malleability" section in the README of + /// this repo), ed25519 signatures didn't consider *any* form of + /// malleability to be an issue. Later the scalar malleability was + /// considered important. Still later, particularly with interests in + /// cryptocurrency design and in unique identities (e.g. for Signal users, + /// Tor onion services, etc.), the group element malleability became a + /// concern. + /// + /// However, libraries had already been created to conform to the original + /// definition. One well-used library in particular even implemented the + /// group element malleability check, *but only for batch verification*! + /// Which meant that even using the same library, a single signature could + /// verify fine individually, but suddenly, when verifying it with a bunch + /// of other signatures, the whole batch would fail! + /// + /// # "Strict" Verification + /// + /// This method performs *both* of the above signature malleability checks. + /// + /// It must be done as a separate method because one doesn't simply get to + /// change the definition of a cryptographic primitive ten years + /// after-the-fact with zero consideration for backwards compatibility in + /// hardware and protocols which have it already have the older definition + /// baked in. + /// + /// # Return + /// + /// Returns `Ok(())` if the signature is valid, and `Err` otherwise. + #[allow(non_snake_case)] + pub fn verify_strict( + &self, + message: &[u8], + signature: &Signature, + ) -> Result<(), SignatureError> + { + self.public.verify_strict(message, signature) + } } #[cfg(feature = "serde")] diff --git a/src/public.rs b/src/public.rs index ae3bfa3..f25c88f 100644 --- a/src/public.rs +++ b/src/public.rs @@ -244,6 +244,105 @@ impl PublicKey { Err(SignatureError(InternalError::VerifyError)) } } + + /// Strictly verify a signature on a message with this keypair's public key. + /// + /// # On The (Multiple) Sources of Malleability in Ed25519 Signatures + /// + /// This version of verification is technically non-RFC8032 compliant. The + /// following explains why. + /// + /// 1. Scalar Malleability + /// + /// The authors of the RFC explicitly stated that verification of an ed25519 + /// signature must fail if the scalar `s` is not properly reduced mod \ell: + /// + /// > To verify a signature on a message M using public key A, with F + /// > being 0 for Ed25519ctx, 1 for Ed25519ph, and if Ed25519ctx or + /// > Ed25519ph is being used, C being the context, first split the + /// > signature into two 32-octet halves. Decode the first half as a + /// > point R, and the second half as an integer S, in the range + /// > 0 <= s < L. Decode the public key A as point A'. If any of the + /// > decodings fail (including S being out of range), the signature is + /// > invalid.) + /// + /// All `verify_*()` functions within ed25519-dalek perform this check. + /// + /// 2. Point malleability + /// + /// The authors of the RFC added in a malleability check to step #3 in + /// §5.1.7, for small torsion components in the `R` value of the signature, + /// *which is not strictly required*, as they state: + /// + /// > Check the group equation [8][S]B = [8]R + [8][k]A'. It's + /// > sufficient, but not required, to instead check [S]B = R + [k]A'. + /// + /// # History of Malleability Checks + /// + /// As originally defined (cf. the "Malleability" section in the README of + /// this repo), ed25519 signatures didn't consider *any* form of + /// malleability to be an issue. Later the scalar malleability was + /// considered important. Still later, particularly with interests in + /// cryptocurrency design and in unique identities (e.g. for Signal users, + /// Tor onion services, etc.), the group element malleability became a + /// concern. + /// + /// However, libraries had already been created to conform to the original + /// definition. One well-used library in particular even implemented the + /// group element malleability check, *but only for batch verification*! + /// Which meant that even using the same library, a single signature could + /// verify fine individually, but suddenly, when verifying it with a bunch + /// of other signatures, the whole batch would fail! + /// + /// # "Strict" Verification + /// + /// This method performs *both* of the above signature malleability checks. + /// + /// It must be done as a separate method because one doesn't simply get to + /// change the definition of a cryptographic primitive ten years + /// after-the-fact with zero consideration for backwards compatibility in + /// hardware and protocols which have it already have the older definition + /// baked in. + /// + /// # Return + /// + /// Returns `Ok(())` if the signature is valid, and `Err` otherwise. + #[allow(non_snake_case)] + pub fn verify_strict( + &self, + message: &[u8], + signature: &Signature, + ) -> Result<(), SignatureError> + { + let mut h: Sha512 = Sha512::new(); + let R: EdwardsPoint; + let k: Scalar; + let minus_A: EdwardsPoint = -self.1; + let signature_R: EdwardsPoint; + + match signature.R.decompress() { + None => return Err(SignatureError(InternalError::VerifyError)), + Some(x) => signature_R = x, + } + + // Logical OR is fine here as we're not trying to be constant time. + if signature_R.is_small_order() || self.1.is_small_order() { + return Err(SignatureError(InternalError::VerifyError)); + } + + h.input(signature.R.as_bytes()); + h.input(self.as_bytes()); + h.input(&message); + + k = Scalar::from_hash(h); + R = EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s); + + if R == signature_R { + Ok(()) + } else { + Err(SignatureError(InternalError::VerifyError)) + } + } } #[cfg(feature = "serde")]