From 3d2acf48ce3186c63dda91b4704e2c16c9ce1672 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 15 May 2019 10:35:14 +0100 Subject: [PATCH] Constant-time field square root WARNING: THIS IS NOT FULLY CONSTANT TIME YET! This will be fixed once we migrate to the jubjub and bls12_381 crates. --- bellman/src/groth16/tests/dummy_engine.rs | 83 ++++------ ff/ff_derive/src/lib.rs | 180 ++++++++++------------ ff/src/lib.rs | 12 +- pairing/src/bls12_381/ec.rs | 31 +++- pairing/src/bls12_381/fq.rs | 44 +----- pairing/src/bls12_381/fq2.rs | 26 +--- pairing/src/bls12_381/fr.rs | 31 +--- pairing/src/bls12_381/tests/mod.rs | 18 ++- pairing/src/tests/field.rs | 4 +- zcash_primitives/src/jubjub/edwards.rs | 9 +- zcash_primitives/src/jubjub/fs.rs | 71 +++------ zcash_primitives/src/jubjub/montgomery.rs | 33 ++-- zcash_primitives/src/jubjub/tests.rs | 16 +- zcash_proofs/src/circuit/ecc.rs | 10 +- 14 files changed, 223 insertions(+), 345 deletions(-) diff --git a/bellman/src/groth16/tests/dummy_engine.rs b/bellman/src/groth16/tests/dummy_engine.rs index 87c22d6..d904004 100644 --- a/bellman/src/groth16/tests/dummy_engine.rs +++ b/bellman/src/groth16/tests/dummy_engine.rs @@ -1,7 +1,4 @@ -use ff::{ - Field, LegendreSymbol, PrimeField, PrimeFieldDecodingError, PrimeFieldRepr, ScalarEngine, - SqrtField, -}; +use ff::{Field, PrimeField, PrimeFieldDecodingError, PrimeFieldRepr, ScalarEngine, SqrtField}; use group::{CurveAffine, CurveProjective, EncodedPoint, GroupDecodingError}; use pairing::{Engine, PairingCurveAffine}; @@ -10,7 +7,7 @@ use std::cmp::Ordering; use std::fmt; use std::num::Wrapping; use std::ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign}; -use subtle::{Choice, ConditionallySelectable, CtOption}; +use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption}; const MODULUS_R: Wrapping = Wrapping(64513); @@ -23,6 +20,12 @@ impl Default for Fr { } } +impl ConstantTimeEq for Fr { + fn ct_eq(&self, other: &Fr) -> Choice { + (self.0).0.ct_eq(&(other.0).0) + } +} + impl fmt::Display for Fr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { write!(f, "{}", (self.0).0) @@ -179,57 +182,39 @@ impl Field for Fr { } impl SqrtField for Fr { - fn legendre(&self) -> LegendreSymbol { - // s = self^((r - 1) // 2) - let s = self.pow([32256]); - if s == ::zero() { - LegendreSymbol::Zero - } else if s == ::one() { - LegendreSymbol::QuadraticResidue - } else { - LegendreSymbol::QuadraticNonResidue - } - } - - fn sqrt(&self) -> Option { + fn sqrt(&self) -> CtOption { // Tonelli-Shank's algorithm for q mod 16 = 1 // https://eprint.iacr.org/2012/685.pdf (page 12, algorithm 5) - match self.legendre() { - LegendreSymbol::Zero => Some(*self), - LegendreSymbol::QuadraticNonResidue => None, - LegendreSymbol::QuadraticResidue => { - let mut c = Fr::root_of_unity(); - // r = self^((t + 1) // 2) - let mut r = self.pow([32]); - // t = self^t - let mut t = self.pow([63]); - let mut m = Fr::S; + let mut c = Fr::root_of_unity(); + // r = self^((t + 1) // 2) + let mut r = self.pow([32]); + // t = self^t + let mut t = self.pow([63]); + let mut m = Fr::S; - while t != ::one() { - let mut i = 1; - { - let mut t2i = t.square(); - loop { - if t2i == ::one() { - break; - } - t2i = t2i.square(); - i += 1; - } + while t != ::one() { + let mut i = 1; + { + let mut t2i = t.square(); + loop { + if t2i == ::one() { + break; } - - for _ in 0..(m - i - 1) { - c = c.square(); - } - MulAssign::mul_assign(&mut r, &c); - c = c.square(); - MulAssign::mul_assign(&mut t, &c); - m = i; + t2i = t2i.square(); + i += 1; } - - Some(r) } + + for _ in 0..(m - i - 1) { + c = c.square(); + } + MulAssign::mul_assign(&mut r, &c); + c = c.square(); + MulAssign::mul_assign(&mut t, &c); + m = i; } + + CtOption::new(r, (r * r).ct_eq(self)) } } diff --git a/ff/ff_derive/src/lib.rs b/ff/ff_derive/src/lib.rs index 6a1ecec..e3f8ca3 100644 --- a/ff/ff_derive/src/lib.rs +++ b/ff/ff_derive/src/lib.rs @@ -413,105 +413,82 @@ fn prime_field_constants_and_sqrt( ); let generator = biguint_to_u64_vec((generator.clone() * &r) % &modulus, limbs); - let mod_minus_1_over_2 = - biguint_to_u64_vec((&modulus - BigUint::from_str("1").unwrap()) >> 1, limbs); - let legendre_impl = quote! { - fn legendre(&self) -> ::ff::LegendreSymbol { - // s = self^((modulus - 1) // 2) - let s = self.pow(#mod_minus_1_over_2); - if s == Self::zero() { - ::ff::LegendreSymbol::Zero - } else if s == Self::one() { - ::ff::LegendreSymbol::QuadraticResidue - } else { - ::ff::LegendreSymbol::QuadraticNonResidue + let sqrt_impl = if (&modulus % BigUint::from_str("4").unwrap()) + == BigUint::from_str("3").unwrap() + { + let mod_plus_1_over_4 = + biguint_to_u64_vec((&modulus + BigUint::from_str("1").unwrap()) >> 2, limbs); + + quote! { + impl ::ff::SqrtField for #name { + fn sqrt(&self) -> ::subtle::CtOption { + use ::subtle::ConstantTimeEq; + + // Because r = 3 (mod 4) + // sqrt can be done with only one exponentiation, + // via the computation of self^((r + 1) // 4) (mod r) + let sqrt = self.pow(#mod_plus_1_over_4); + + ::subtle::CtOption::new( + sqrt, + (sqrt * &sqrt).ct_eq(self), // Only return Some if it's the square root. + ) + } } } + } else if (&modulus % BigUint::from_str("16").unwrap()) == BigUint::from_str("1").unwrap() { + let t_minus_1_over_2 = biguint_to_u64_vec((&t - BigUint::one()) >> 1, limbs); + + quote! { + impl ::ff::SqrtField for #name { + fn sqrt(&self) -> ::subtle::CtOption { + // Tonelli-Shank's algorithm for q mod 16 = 1 + // https://eprint.iacr.org/2012/685.pdf (page 12, algorithm 5) + use ::subtle::{ConditionallySelectable, ConstantTimeEq}; + + // w = self^((t - 1) // 2) + let w = self.pow(#t_minus_1_over_2); + + let mut v = S; + let mut x = *self * &w; + let mut b = x * &w; + + // Initialize z as the 2^S root of unity. + let mut z = #name(ROOT_OF_UNITY); + + for max_v in (1..=S).rev() { + let mut k = 1; + let mut tmp = b.square(); + let mut j_less_than_v: ::subtle::Choice = 1.into(); + + for j in 2..max_v { + let tmp_is_one = tmp.ct_eq(&#name::one()); + let squared = #name::conditional_select(&tmp, &z, tmp_is_one).square(); + tmp = #name::conditional_select(&squared, &tmp, tmp_is_one); + let new_z = #name::conditional_select(&z, &squared, tmp_is_one); + j_less_than_v &= !j.ct_eq(&v); + k = u32::conditional_select(&j, &k, tmp_is_one); + z = #name::conditional_select(&z, &new_z, j_less_than_v); + } + + let result = x * &z; + x = #name::conditional_select(&result, &x, b.ct_eq(&#name::one())); + z = z.square(); + b = b * &z; + v = k; + } + + ::subtle::CtOption::new( + x, + (x * &x).ct_eq(self), // Only return Some if it's the square root. + ) + } + } + } + } else { + quote! {} }; - let sqrt_impl = - if (&modulus % BigUint::from_str("4").unwrap()) == BigUint::from_str("3").unwrap() { - let mod_minus_3_over_4 = - biguint_to_u64_vec((&modulus - BigUint::from_str("3").unwrap()) >> 2, limbs); - - // Compute -R as (m - r) - let rneg = biguint_to_u64_vec(&modulus - &r, limbs); - - quote! { - impl ::ff::SqrtField for #name { - #legendre_impl - - fn sqrt(&self) -> Option { - // Shank's algorithm for q mod 4 = 3 - // https://eprint.iacr.org/2012/685.pdf (page 9, algorithm 2) - - let mut a1 = self.pow(#mod_minus_3_over_4); - - let mut a0 = a1.square(); - a0.mul_assign(self); - - if a0.0 == #repr(#rneg) { - None - } else { - a1.mul_assign(self); - Some(a1) - } - } - } - } - } else if (&modulus % BigUint::from_str("16").unwrap()) == BigUint::from_str("1").unwrap() { - let t_plus_1_over_2 = biguint_to_u64_vec((&t + BigUint::one()) >> 1, limbs); - let t = biguint_to_u64_vec(t.clone(), limbs); - - quote! { - impl ::ff::SqrtField for #name { - #legendre_impl - - fn sqrt(&self) -> Option { - // Tonelli-Shank's algorithm for q mod 16 = 1 - // https://eprint.iacr.org/2012/685.pdf (page 12, algorithm 5) - - match self.legendre() { - ::ff::LegendreSymbol::Zero => Some(*self), - ::ff::LegendreSymbol::QuadraticNonResidue => None, - ::ff::LegendreSymbol::QuadraticResidue => { - let mut c = #name(ROOT_OF_UNITY); - let mut r = self.pow(#t_plus_1_over_2); - let mut t = self.pow(#t); - let mut m = S; - - while t != Self::one() { - let mut i = 1; - { - let mut t2i = t.square(); - loop { - if t2i == Self::one() { - break; - } - t2i = t2i.square(); - i += 1; - } - } - - for _ in 0..(m - i - 1) { - c = c.square(); - } - r.mul_assign(&c); - c = c.square(); - t.mul_assign(&c); - m = i; - } - - Some(r) - } - } - } - } - } - } else { - quote! {} - }; - // Compute R^2 mod m let r2 = biguint_to_u64_vec((&r * &r) % &modulus, limbs); @@ -771,6 +748,13 @@ fn prime_field_impl( let multiply_impl = mul_impl(quote! {self}, quote! {other}, limbs); let montgomery_impl = mont_impl(limbs); + // (self.0).0[0].ct_eq(&(other.0).0[0]) & (self.0).0[1].ct_eq(&(other.0).0[1]) & ... + let mut ct_eq_impl = proc_macro2::TokenStream::new(); + ct_eq_impl.append_separated( + (0..limbs).map(|i| quote! { (self.0).0[#i].ct_eq(&(other.0).0[#i]) }), + proc_macro2::Punct::new('&', proc_macro2::Spacing::Alone), + ); + // (self.0).0[0], (self.0).0[1], ..., 0, 0, 0, 0, ... let mut into_repr_params = proc_macro2::TokenStream::new(); into_repr_params.append_separated( @@ -797,6 +781,12 @@ fn prime_field_impl( } } + impl ::subtle::ConstantTimeEq for #name { + fn ct_eq(&self, other: &#name) -> ::subtle::Choice { + #ct_eq_impl + } + } + impl ::std::cmp::PartialEq for #name { fn eq(&self, other: &#name) -> bool { self.0 == other.0 diff --git a/ff/src/lib.rs b/ff/src/lib.rs index 675d0b3..d4602ba 100644 --- a/ff/src/lib.rs +++ b/ff/src/lib.rs @@ -94,12 +94,9 @@ pub trait Field: /// This trait represents an element of a field that has a square root operation described for it. pub trait SqrtField: Field { - /// Returns the Legendre symbol of the field element. - fn legendre(&self) -> LegendreSymbol; - /// Returns the square root of the field element, if it is /// quadratic residue. - fn sqrt(&self) -> Option; + fn sqrt(&self) -> CtOption; } /// This trait represents a wrapper around a biginteger which can encode any element of a particular @@ -199,13 +196,6 @@ pub trait PrimeFieldRepr: } } -#[derive(Debug, PartialEq)] -pub enum LegendreSymbol { - Zero = 0, - QuadraticResidue = 1, - QuadraticNonResidue = -1, -} - /// An error that may occur when trying to interpret a `PrimeFieldRepr` as a /// `PrimeField` element. #[derive(Debug)] diff --git a/pairing/src/bls12_381/ec.rs b/pairing/src/bls12_381/ec.rs index e80fe17..5132289 100644 --- a/pairing/src/bls12_381/ec.rs +++ b/pairing/src/bls12_381/ec.rs @@ -97,7 +97,7 @@ macro_rules! curve_impl { /// /// If and only if `greatest` is set will the lexicographically /// largest y-coordinate be selected. - fn get_point_from_x(x: $basefield, greatest: bool) -> Option<$affine> { + fn get_point_from_x(x: $basefield, greatest: bool) -> CtOption<$affine> { // Compute x^3 + b let mut x3b = x.square(); x3b.mul_assign(&x); @@ -199,8 +199,9 @@ macro_rules! curve_impl { let x = $basefield::random(rng); let greatest = rng.next_u32() % 2 != 0; - if let Some(p) = $affine::get_point_from_x(x, greatest) { - let p = p.scale_by_cofactor(); + let p = $affine::get_point_from_x(x, greatest); + if p.is_some().into() { + let p = p.unwrap().scale_by_cofactor(); if !p.is_zero() { return p; @@ -603,6 +604,7 @@ pub mod g1 { use rand_core::RngCore; use std::fmt; use std::ops::{AddAssign, MulAssign, Neg, SubAssign}; + use subtle::CtOption; curve_impl!( "G1", @@ -807,7 +809,12 @@ pub mod g1 { let x = Fq::from_repr(x) .map_err(|e| GroupDecodingError::CoordinateDecodingError("x coordinate", e))?; - G1Affine::get_point_from_x(x, greatest).ok_or(GroupDecodingError::NotOnCurve) + let ret = G1Affine::get_point_from_x(x, greatest); + if ret.is_some().into() { + Ok(ret.unwrap()) + } else { + Err(GroupDecodingError::NotOnCurve) + } } } fn from_affine(affine: G1Affine) -> Self { @@ -919,7 +926,9 @@ pub mod g1 { rhs.mul_assign(&x); rhs.add_assign(&G1Affine::get_coeff_b()); - if let Some(y) = rhs.sqrt() { + let y = rhs.sqrt(); + if y.is_some().into() { + let y = y.unwrap(); let yrepr = y.into_repr(); let negy = y.neg(); let negyrepr = negy.into_repr(); @@ -1270,6 +1279,7 @@ pub mod g2 { use rand_core::RngCore; use std::fmt; use std::ops::{AddAssign, MulAssign, Neg, SubAssign}; + use subtle::CtOption; curve_impl!( "G2", @@ -1498,7 +1508,12 @@ pub mod g2 { })?, }; - G2Affine::get_point_from_x(x, greatest).ok_or(GroupDecodingError::NotOnCurve) + let ret = G2Affine::get_point_from_x(x, greatest); + if ret.is_some().into() { + Ok(ret.unwrap()) + } else { + Err(GroupDecodingError::NotOnCurve) + } } } fn from_affine(affine: G2Affine) -> Self { @@ -1623,7 +1638,9 @@ pub mod g2 { rhs.mul_assign(&x); rhs.add_assign(&G2Affine::get_coeff_b()); - if let Some(y) = rhs.sqrt() { + let y = rhs.sqrt(); + if y.is_some().into() { + let y = y.unwrap(); let negy = y.neg(); let p = G2Affine { diff --git a/pairing/src/bls12_381/fq.rs b/pairing/src/bls12_381/fq.rs index bc4a7b3..5090083 100644 --- a/pairing/src/bls12_381/fq.rs +++ b/pairing/src/bls12_381/fq.rs @@ -2074,8 +2074,9 @@ fn test_fq_sqrt() { // Ensure sqrt(a)^2 = a for random a let a = Fq::random(&mut rng); - if let Some(tmp) = a.sqrt() { - assert_eq!(a, tmp.square()); + let tmp = a.sqrt(); + if tmp.is_some().into() { + assert_eq!(a, tmp.unwrap().square()); } } } @@ -2205,7 +2206,7 @@ fn test_fq_root_of_unity() { Fq::root_of_unity() ); assert_eq!(Fq::root_of_unity().pow([1 << Fq::S]), Fq::one()); - assert!(Fq::multiplicative_generator().sqrt().is_none()); + assert!(bool::from(Fq::multiplicative_generator().sqrt().is_none())); } #[test] @@ -2231,40 +2232,3 @@ fn test_fq_ordering() { fn fq_repr_tests() { crate::tests::repr::random_repr_tests::(); } - -#[test] -fn test_fq_legendre() { - use ff::LegendreSymbol::*; - use ff::SqrtField; - - assert_eq!(QuadraticResidue, Fq::one().legendre()); - assert_eq!(Zero, Fq::zero().legendre()); - - assert_eq!( - QuadraticNonResidue, - Fq::from_repr(FqRepr::from(2)).unwrap().legendre() - ); - assert_eq!( - QuadraticResidue, - Fq::from_repr(FqRepr::from(4)).unwrap().legendre() - ); - - let e = FqRepr([ - 0x52a112f249778642, - 0xd0bedb989b7991f, - 0xdad3b6681aa63c05, - 0xf2efc0bb4721b283, - 0x6057a98f18c24733, - 0x1022c2fd122889e4, - ]); - assert_eq!(QuadraticNonResidue, Fq::from_repr(e).unwrap().legendre()); - let e = FqRepr([ - 0x6dae594e53a96c74, - 0x19b16ca9ba64b37b, - 0x5c764661a59bfc68, - 0xaa346e9b31c60a, - 0x346059f9d87a9fa9, - 0x1d61ac6bfd5c88b, - ]); - assert_eq!(QuadraticResidue, Fq::from_repr(e).unwrap().legendre()); -} diff --git a/pairing/src/bls12_381/fq2.rs b/pairing/src/bls12_381/fq2.rs index de0939a..8f7cbb2 100644 --- a/pairing/src/bls12_381/fq2.rs +++ b/pairing/src/bls12_381/fq2.rs @@ -244,15 +244,13 @@ impl Field for Fq2 { } impl SqrtField for Fq2 { - fn legendre(&self) -> ::ff::LegendreSymbol { - self.norm().legendre() - } - - fn sqrt(&self) -> Option { + /// WARNING: THIS IS NOT ACTUALLY CONSTANT TIME YET! + /// THIS WILL BE REPLACED BY THE bls12_381 CRATE, WHICH IS CONSTANT TIME! + fn sqrt(&self) -> CtOption { // Algorithm 9, https://eprint.iacr.org/2012/685.pdf if self.is_zero() { - Some(Self::zero()) + CtOption::new(Self::zero(), Choice::from(1)) } else { // a1 = self^((q - 3) / 4) let mut a1 = self.pow([ @@ -275,7 +273,7 @@ impl SqrtField for Fq2 { }; if a0 == neg1 { - None + CtOption::new(Self::zero(), Choice::from(0)) } else { a1.mul_assign(self); @@ -298,7 +296,7 @@ impl SqrtField for Fq2 { a1.mul_assign(&alpha); } - Some(a1) + CtOption::new(a1, Choice::from(1)) } } } @@ -993,18 +991,6 @@ fn test_fq2_sqrt() { ); } -#[test] -fn test_fq2_legendre() { - use ff::LegendreSymbol::*; - - assert_eq!(Zero, Fq2::zero().legendre()); - // i^2 = -1 - let mut m1 = Fq2::one().neg(); - assert_eq!(QuadraticResidue, m1.legendre()); - m1.mul_by_nonresidue(); - assert_eq!(QuadraticNonResidue, m1.legendre()); -} - #[cfg(test)] use rand_core::SeedableRng; #[cfg(test)] diff --git a/pairing/src/bls12_381/fr.rs b/pairing/src/bls12_381/fr.rs index 226c1aa..b2fa4e1 100644 --- a/pairing/src/bls12_381/fr.rs +++ b/pairing/src/bls12_381/fr.rs @@ -278,30 +278,6 @@ fn test_fr_repr_sub_noborrow() { ); } -#[test] -fn test_fr_legendre() { - use ff::LegendreSymbol::*; - use ff::SqrtField; - - assert_eq!(QuadraticResidue, Fr::one().legendre()); - assert_eq!(Zero, Fr::zero().legendre()); - - let e = FrRepr([ - 0x0dbc5349cd5664da, - 0x8ac5b6296e3ae29d, - 0x127cb819feceaa3b, - 0x3a6b21fb03867191, - ]); - assert_eq!(QuadraticResidue, Fr::from_repr(e).unwrap().legendre()); - let e = FrRepr([ - 0x96341aefd047c045, - 0x9b5f4254500a4d65, - 0x1ee08223b68ac240, - 0x31d9cd545c0ec7c6, - ]); - assert_eq!(QuadraticNonResidue, Fr::from_repr(e).unwrap().legendre()); -} - #[test] fn test_fr_repr_add_nocarry() { let mut rng = XorShiftRng::from_seed([ @@ -833,8 +809,9 @@ fn test_fr_sqrt() { // Ensure sqrt(a)^2 = a for random a let a = Fr::random(&mut rng); - if let Some(tmp) = a.sqrt() { - assert_eq!(a, tmp.square()); + let tmp = a.sqrt(); + if tmp.is_some().into() { + assert_eq!(a, tmp.unwrap().square()); } } } @@ -996,7 +973,7 @@ fn test_fr_root_of_unity() { Fr::root_of_unity() ); assert_eq!(Fr::root_of_unity().pow([1 << Fr::S]), Fr::one()); - assert!(Fr::multiplicative_generator().sqrt().is_none()); + assert!(bool::from(Fr::multiplicative_generator().sqrt().is_none())); } #[test] diff --git a/pairing/src/bls12_381/tests/mod.rs b/pairing/src/bls12_381/tests/mod.rs index 2fd4deb..9c5b2c9 100644 --- a/pairing/src/bls12_381/tests/mod.rs +++ b/pairing/src/bls12_381/tests/mod.rs @@ -193,7 +193,10 @@ fn test_g1_uncompressed_invalid_vectors() { x3b.mul_assign(&x); x3b.add_assign(&Fq::from_repr(FqRepr::from(4)).unwrap()); // TODO: perhaps expose coeff_b through API? - if let Some(y) = x3b.sqrt() { + let y = x3b.sqrt(); + if y.is_some().into() { + let y = y.unwrap(); + // We know this is on the curve, but it's likely not going to be in the correct subgroup. x.into_repr().write_be(&mut o.as_mut()[0..]).unwrap(); y.into_repr().write_be(&mut o.as_mut()[48..]).unwrap(); @@ -332,7 +335,10 @@ fn test_g2_uncompressed_invalid_vectors() { c1: Fq::from_repr(FqRepr::from(4)).unwrap(), }); // TODO: perhaps expose coeff_b through API? - if let Some(y) = x3b.sqrt() { + let y = x3b.sqrt(); + if y.is_some().into() { + let y = y.unwrap(); + // We know this is on the curve, but it's likely not going to be in the correct subgroup. x.c1.into_repr().write_be(&mut o.as_mut()[0..]).unwrap(); x.c0.into_repr().write_be(&mut o.as_mut()[48..]).unwrap(); @@ -424,7 +430,7 @@ fn test_g1_compressed_invalid_vectors() { x3b.mul_assign(&x); x3b.add_assign(&Fq::from_repr(FqRepr::from(4)).unwrap()); // TODO: perhaps expose coeff_b through API? - if let Some(_) = x3b.sqrt() { + if x3b.sqrt().is_some().into() { x.add_assign(&Fq::one()); } else { x.into_repr().write_be(&mut o.as_mut()[0..]).unwrap(); @@ -448,7 +454,7 @@ fn test_g1_compressed_invalid_vectors() { x3b.mul_assign(&x); x3b.add_assign(&Fq::from_repr(FqRepr::from(4)).unwrap()); // TODO: perhaps expose coeff_b through API? - if let Some(_) = x3b.sqrt() { + if x3b.sqrt().is_some().into() { // We know this is on the curve, but it's likely not going to be in the correct subgroup. x.into_repr().write_be(&mut o.as_mut()[0..]).unwrap(); o.as_mut()[0] |= 0b1000_0000; @@ -556,7 +562,7 @@ fn test_g2_compressed_invalid_vectors() { c1: Fq::from_repr(FqRepr::from(4)).unwrap(), }); // TODO: perhaps expose coeff_b through API? - if let Some(_) = x3b.sqrt() { + if x3b.sqrt().is_some().into() { x.add_assign(&Fq2::one()); } else { x.c1.into_repr().write_be(&mut o.as_mut()[0..]).unwrap(); @@ -587,7 +593,7 @@ fn test_g2_compressed_invalid_vectors() { c1: Fq::from_repr(FqRepr::from(4)).unwrap(), }); // TODO: perhaps expose coeff_b through API? - if let Some(_) = x3b.sqrt() { + if x3b.sqrt().is_some().into() { // We know this is on the curve, but it's likely not going to be in the correct subgroup. x.c1.into_repr().write_be(&mut o.as_mut()[0..]).unwrap(); x.c0.into_repr().write_be(&mut o.as_mut()[48..]).unwrap(); diff --git a/pairing/src/tests/field.rs b/pairing/src/tests/field.rs index 0374122..a073604 100644 --- a/pairing/src/tests/field.rs +++ b/pairing/src/tests/field.rs @@ -1,4 +1,4 @@ -use ff::{Field, LegendreSymbol, PrimeField, SqrtField}; +use ff::{Field, PrimeField, SqrtField}; use rand_core::{RngCore, SeedableRng}; use rand_xorshift::XorShiftRng; @@ -32,7 +32,6 @@ pub fn random_sqrt_tests() { for _ in 0..10000 { let a = F::random(&mut rng); let b = a.square(); - assert_eq!(b.legendre(), LegendreSymbol::QuadraticResidue); let b = b.sqrt().unwrap(); let negb = b.neg(); @@ -43,7 +42,6 @@ pub fn random_sqrt_tests() { let mut c = F::one(); for _ in 0..10000 { let mut b = c.square(); - assert_eq!(b.legendre(), LegendreSymbol::QuadraticResidue); b = b.sqrt().unwrap(); diff --git a/zcash_primitives/src/jubjub/edwards.rs b/zcash_primitives/src/jubjub/edwards.rs index 94f1ceb..1b3ebc0 100644 --- a/zcash_primitives/src/jubjub/edwards.rs +++ b/zcash_primitives/src/jubjub/edwards.rs @@ -1,6 +1,6 @@ use ff::{BitIterator, Field, PrimeField, PrimeFieldRepr, SqrtField}; use std::ops::{AddAssign, MulAssign, Neg, SubAssign}; -use subtle::{Choice, CtOption}; +use subtle::CtOption; use super::{montgomery, JubjubEngine, JubjubParams, PrimeOrder, Unknown}; @@ -126,7 +126,7 @@ impl Point { // tmp1 = (y^2 - 1) / (dy^2 + 1) tmp1.mul_assign(&tmp2); - match tmp1.sqrt().map(|mut x| { + tmp1.sqrt().map(|mut x| { if x.into_repr().is_odd() != sign { x = x.neg(); } @@ -141,10 +141,7 @@ impl Point { z: E::Fr::one(), _marker: PhantomData, } - }) { - Some(p) => CtOption::new(p, Choice::from(1)), - None => CtOption::new(Point::zero(), Choice::from(0)), - } + }) }) } diff --git a/zcash_primitives/src/jubjub/fs.rs b/zcash_primitives/src/jubjub/fs.rs index d2b61cb..a493e7a 100644 --- a/zcash_primitives/src/jubjub/fs.rs +++ b/zcash_primitives/src/jubjub/fs.rs @@ -1,12 +1,11 @@ use byteorder::{ByteOrder, LittleEndian}; use ff::{ - adc, mac_with_carry, sbb, BitIterator, Field, - LegendreSymbol::{self, *}, - PrimeField, PrimeFieldDecodingError, PrimeFieldRepr, SqrtField, + adc, mac_with_carry, sbb, BitIterator, Field, PrimeField, PrimeFieldDecodingError, + PrimeFieldRepr, SqrtField, }; use rand_core::RngCore; use std::ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign}; -use subtle::{Choice, ConditionallySelectable, CtOption}; +use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption}; use super::ToUniform; @@ -264,6 +263,15 @@ impl Default for Fs { } } +impl ConstantTimeEq for Fs { + fn ct_eq(&self, other: &Fs) -> Choice { + (self.0).0[0].ct_eq(&(other.0).0[0]) + & (self.0).0[1].ct_eq(&(other.0).0[1]) + & (self.0).0[2].ct_eq(&(other.0).0[2]) + & (self.0).0[3].ct_eq(&(other.0).0[3]) + } +} + impl ::std::fmt::Display for Fs { fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { write!(f, "Fs({})", self.into_repr()) @@ -731,24 +739,7 @@ impl ToUniform for Fs { } impl SqrtField for Fs { - fn legendre(&self) -> LegendreSymbol { - // s = self^((s - 1) // 2) - let s = self.pow([ - 0x684b872f6b7b965b, - 0x53341049e6640841, - 0x83339d80809a1d80, - 0x73eda753299d7d4, - ]); - if s == Self::zero() { - Zero - } else if s == Self::one() { - QuadraticResidue - } else { - QuadraticNonResidue - } - } - - fn sqrt(&self) -> Option { + fn sqrt(&self) -> CtOption { // Shank's algorithm for s mod 4 = 3 // https://eprint.iacr.org/2012/685.pdf (page 9, algorithm 2) @@ -761,13 +752,9 @@ impl SqrtField for Fs { ]); let mut a0 = a1.square(); a0.mul_assign(self); + a1.mul_assign(self); - if a0 == NEGATIVE_ONE { - None - } else { - a1.mul_assign(self); - Some(a1) - } + CtOption::new(a1, !a0.ct_eq(&NEGATIVE_ONE)) } } @@ -1025,27 +1012,6 @@ fn test_fs_repr_sub_noborrow() { } } -#[test] -fn test_fs_legendre() { - assert_eq!(QuadraticResidue, Fs::one().legendre()); - assert_eq!(Zero, Fs::zero().legendre()); - - let e = FsRepr([ - 0x8385eec23df1f88e, - 0x9a01fb412b2dba16, - 0x4c928edcdd6c22f, - 0x9f2df7ef69ecef9, - ]); - assert_eq!(QuadraticResidue, Fs::from_repr(e).unwrap().legendre()); - let e = FsRepr([ - 0xe8ed9f299da78568, - 0x35efdebc88b2209, - 0xc82125cb1f916dbe, - 0x6813d2b38c39bd0, - ]); - assert_eq!(QuadraticNonResidue, Fs::from_repr(e).unwrap().legendre()); -} - #[test] fn test_fr_repr_add_nocarry() { let mut rng = XorShiftRng::from_seed([ @@ -1569,8 +1535,9 @@ fn test_fs_sqrt() { // Ensure sqrt(a)^2 = a for random a let a = Fs::random(&mut rng); - if let Some(tmp) = a.sqrt() { - assert_eq!(a, tmp.square()); + let tmp = a.sqrt(); + if tmp.is_some().into() { + assert_eq!(a, tmp.unwrap().square()); } } } @@ -1730,5 +1697,5 @@ fn test_fs_root_of_unity() { Fs::root_of_unity() ); assert_eq!(Fs::root_of_unity().pow([1 << Fs::S]), Fs::one()); - assert!(Fs::multiplicative_generator().sqrt().is_none()); + assert!(bool::from(Fs::multiplicative_generator().sqrt().is_none())); } diff --git a/zcash_primitives/src/jubjub/montgomery.rs b/zcash_primitives/src/jubjub/montgomery.rs index b9ca82e..9cad803 100644 --- a/zcash_primitives/src/jubjub/montgomery.rs +++ b/zcash_primitives/src/jubjub/montgomery.rs @@ -1,5 +1,6 @@ use ff::{BitIterator, Field, PrimeField, PrimeFieldRepr, SqrtField}; use std::ops::{AddAssign, MulAssign, Neg, SubAssign}; +use subtle::CtOption; use super::{edwards, JubjubEngine, JubjubParams, PrimeOrder, Unknown}; @@ -47,7 +48,7 @@ impl PartialEq for Point { } impl Point { - pub fn get_for_x(x: E::Fr, sign: bool, params: &E::Params) -> Option { + pub fn get_for_x(x: E::Fr, sign: bool, params: &E::Params) -> CtOption { // Given an x on the curve, y = sqrt(x^3 + A*x^2 + x) let mut x2 = x.square(); @@ -58,21 +59,18 @@ impl Point { x2.mul_assign(&x); rhs.add_assign(&x2); - match rhs.sqrt() { - Some(mut y) => { - if y.into_repr().is_odd() != sign { - y = y.neg(); - } - - Some(Point { - x, - y, - infinity: false, - _marker: PhantomData, - }) + rhs.sqrt().map(|mut y| { + if y.into_repr().is_odd() != sign { + y = y.neg(); } - None => None, - } + + Point { + x, + y, + infinity: false, + _marker: PhantomData, + } + }) } /// This guarantees the point is in the prime order subgroup @@ -88,8 +86,9 @@ impl Point { let x = E::Fr::random(rng); let sign = rng.next_u32() % 2 != 0; - if let Some(p) = Self::get_for_x(x, sign, params) { - return p; + let p = Self::get_for_x(x, sign, params); + if p.is_some().into() { + return p.unwrap(); } } } diff --git a/zcash_primitives/src/jubjub/tests.rs b/zcash_primitives/src/jubjub/tests.rs index 511a5eb..84b3a96 100644 --- a/zcash_primitives/src/jubjub/tests.rs +++ b/zcash_primitives/src/jubjub/tests.rs @@ -1,6 +1,6 @@ use super::{edwards, montgomery, JubjubEngine, JubjubParams, PrimeOrder}; -use ff::{Field, LegendreSymbol, PrimeField, PrimeFieldRepr, SqrtField}; +use ff::{Field, PrimeField, PrimeFieldRepr, SqrtField}; use std::ops::{AddAssign, MulAssign, Neg, SubAssign}; use rand_core::{RngCore, SeedableRng}; @@ -319,8 +319,8 @@ fn test_jubjub_params(params: &E::Params) { // The twisted Edwards addition law is complete when d is nonsquare // and a is square. - assert!(params.edwards_d().legendre() == LegendreSymbol::QuadraticNonResidue); - assert!(a.legendre() == LegendreSymbol::QuadraticResidue); + assert!(bool::from(params.edwards_d().sqrt().is_none())); + assert!(bool::from(a.sqrt().is_some())); } { @@ -330,30 +330,30 @@ fn test_jubjub_params(params: &E::Params) { let mut tmp = *params.edwards_d(); // 1 / d is nonsquare - assert!(tmp.invert().unwrap().legendre() == LegendreSymbol::QuadraticNonResidue); + assert!(bool::from(tmp.invert().unwrap().sqrt().is_none())); // tmp = -d tmp = tmp.neg(); // -d is nonsquare - assert!(tmp.legendre() == LegendreSymbol::QuadraticNonResidue); + assert!(bool::from(tmp.sqrt().is_none())); // 1 / -d is nonsquare - assert!(tmp.invert().unwrap().legendre() == LegendreSymbol::QuadraticNonResidue); + assert!(bool::from(tmp.invert().unwrap().sqrt().is_none())); } { // Check that A^2 - 4 is nonsquare: let mut tmp = params.montgomery_a().square(); tmp.sub_assign(&E::Fr::from_str("4").unwrap()); - assert!(tmp.legendre() == LegendreSymbol::QuadraticNonResidue); + assert!(bool::from(tmp.sqrt().is_none())); } { // Check that A - 2 is nonsquare: let mut tmp = params.montgomery_a().clone(); tmp.sub_assign(&E::Fr::from_str("2").unwrap()); - assert!(tmp.legendre() == LegendreSymbol::QuadraticNonResidue); + assert!(bool::from(tmp.sqrt().is_none())); } { diff --git a/zcash_proofs/src/circuit/ecc.rs b/zcash_proofs/src/circuit/ecc.rs index 82e6761..05baf8b 100644 --- a/zcash_proofs/src/circuit/ecc.rs +++ b/zcash_proofs/src/circuit/ecc.rs @@ -1025,8 +1025,9 @@ mod test { let x = Fr::random(rng); let s: bool = rng.next_u32() % 2 != 0; - if let Some(p) = montgomery::Point::::get_for_x(x, s, params) { - break p; + let p = montgomery::Point::::get_for_x(x, s, params); + if p.is_some().into() { + break p.unwrap(); } }; @@ -1034,8 +1035,9 @@ mod test { let x = Fr::random(rng); let s: bool = rng.next_u32() % 2 != 0; - if let Some(p) = montgomery::Point::::get_for_x(x, s, params) { - break p; + let p = montgomery::Point::::get_for_x(x, s, params); + if p.is_some().into() { + break p.unwrap(); } };