From a32ae3fc871bc72558ac2ce7eac933d1ad5f4a9c Mon Sep 17 00:00:00 2001 From: str4d Date: Thu, 18 Nov 2021 18:53:35 +0000 Subject: [PATCH] Don't reject small-order verification keys (#137) * Don't reject small-order verification keys Fixes ZcashFoundation/redjubjub#127. * Added missing changelog entries --- CHANGELOG.md | 28 ++++++++++++++++++++++++++++ src/verification_key.rs | 13 +++++++------ tests/smallorder.rs | 13 +++++++++++-- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f058807..87b3f53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,34 @@ Entries are listed in reverse chronological order. +## Unreleased + +* Fixed a bug where small-order verification keys (including the identity) were + handled inconsistently: the `VerificationKey` parsing logic rejected them, but + the identity `VerificationKey` could be produced from the zero `SigningKey`. + The behaviour is now to consistently accept all small-order verification keys, + matching the RedDSA specification. + + * Downstream users who currently rely on the inconsistent behaviour (for e.g. + consensus compatibility, either explicitly wanting to reject small-order + verification keys, or on the belief that this crate implemented the RedDSA + specification) should continue to use previous versions of this crate, until + they can either move the checks into their own code, or migrate their + consensus rules to match the RedDSA specification. + +## 0.4.0 + +* Upgrade `rand` to 0.8, `rand_core` to 0.6, and `rand_chacha` to 0.3, together + (#55) +* Migrate to `jubjub 0.6` (#59) +* Derive `Debug, PartialEq` (#67) +* Restrict the maximum number of FROST participants to 255 by using `u8` (#66) + +## 0.3.0 + +* Initial support for FROST (Flexible Round-Optimized Schnorr Threshold) + signatures. + ## 0.2.2 * Make `batch::Item: Clone + Debug` and add `batch::Item::verify_single` diff --git a/src/verification_key.rs b/src/verification_key.rs index b7ea8a2..17d32ef 100644 --- a/src/verification_key.rs +++ b/src/verification_key.rs @@ -99,12 +99,13 @@ impl TryFrom> for VerificationKey { let maybe_point = jubjub::AffinePoint::from_bytes(bytes.bytes); if maybe_point.is_some().into() { let point: jubjub::ExtendedPoint = maybe_point.unwrap().into(); - // This checks that the verification key is not of small order. - if ::from(point.is_small_order()) == false { - Ok(VerificationKey { point, bytes }) - } else { - Err(Error::MalformedVerificationKey) - } + // Note that small-order verification keys (including the identity) are not + // rejected here. Previously they were rejected, but this was a bug as the + // RedDSA specification allows them. Zcash Sapling rejects small-order points + // for the RedJubjub spend authorization key rk; this now occurs separately. + // Meanwhile, Zcash Orchard uses a prime-order group, so the only small-order + // point would be the identity, which is allowed in Orchard. + Ok(VerificationKey { point, bytes }) } else { Err(Error::MalformedVerificationKey) } diff --git a/tests/smallorder.rs b/tests/smallorder.rs index 3631779..019068e 100644 --- a/tests/smallorder.rs +++ b/tests/smallorder.rs @@ -5,11 +5,20 @@ use jubjub::{AffinePoint, Fq}; use redjubjub::*; #[test] -fn smallorder_publickey_fails() { +fn identity_publickey_passes() { + let identity = AffinePoint::identity(); + assert_eq!(::from(identity.is_small_order()), true); + let bytes = identity.to_bytes(); + let pk_bytes = VerificationKeyBytes::::from(bytes); + assert!(VerificationKey::::try_from(pk_bytes).is_ok()); +} + +#[test] +fn smallorder_publickey_passes() { // (1,0) is a point of order 4 on any Edwards curve let order4 = AffinePoint::from_raw_unchecked(Fq::one(), Fq::zero()); assert_eq!(::from(order4.is_small_order()), true); let bytes = order4.to_bytes(); let pk_bytes = VerificationKeyBytes::::from(bytes); - assert!(VerificationKey::::try_from(pk_bytes).is_err()); + assert!(VerificationKey::::try_from(pk_bytes).is_ok()); }