Skip to content

Commit

Permalink
Don't reject small-order verification keys (#137)
Browse files Browse the repository at this point in the history
* Don't reject small-order verification keys

Fixes #127.

* Added missing changelog entries
  • Loading branch information
str4d authored Nov 18, 2021
1 parent 2f240d8 commit a32ae3f
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 8 deletions.
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
13 changes: 7 additions & 6 deletions src/verification_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,13 @@ impl<T: SigType> TryFrom<VerificationKeyBytes<T>> for VerificationKey<T> {
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 <bool>::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)
}
Expand Down
13 changes: 11 additions & 2 deletions tests/smallorder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(<bool>::from(identity.is_small_order()), true);
let bytes = identity.to_bytes();
let pk_bytes = VerificationKeyBytes::<SpendAuth>::from(bytes);
assert!(VerificationKey::<SpendAuth>::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!(<bool>::from(order4.is_small_order()), true);
let bytes = order4.to_bytes();
let pk_bytes = VerificationKeyBytes::<SpendAuth>::from(bytes);
assert!(VerificationKey::<SpendAuth>::try_from(pk_bytes).is_err());
assert!(VerificationKey::<SpendAuth>::try_from(pk_bytes).is_ok());
}

0 comments on commit a32ae3f

Please sign in to comment.