diff --git a/src/constants.rs b/src/constants.rs index c990b09..51833ac 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -129,12 +129,13 @@ mod test { } } + /// Test that SQRT_M1 is the positive square root of -1 #[test] - /// Test that SQRT_M1 is a square root of -1 fn test_sqrt_minus_one() { let minus_one = FieldElement::minus_one(); let sqrt_m1_sq = &constants::SQRT_M1 * &constants::SQRT_M1; assert_eq!(minus_one, sqrt_m1_sq); + assert_eq!(constants::SQRT_M1.is_negative().unwrap_u8(), 0); } #[test] @@ -143,8 +144,6 @@ mod test { let (was_nonzero_square, invsqrt_m1) = minus_one.invsqrt(); assert_eq!(was_nonzero_square.unwrap_u8(), 1u8); let sign_test_sqrt = &invsqrt_m1 * &constants::SQRT_M1; - // XXX it seems we have flipped the sign relative to - // the invsqrt function? assert_eq!(sign_test_sqrt, minus_one); } diff --git a/src/edwards.rs b/src/edwards.rs index a09ce45..7d801b1 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -169,9 +169,9 @@ impl CompressedEdwardsY { let YY = Y.square(); let u = &YY - &Z; // u = y²-1 let v = &(&YY * &constants::EDWARDS_D) + &Z; // v = dy²+1 - let (is_nonzero_square, mut X) = FieldElement::sqrt_ratio(&u, &v); + let (is_valid_y_coord, mut X) = FieldElement::sqrt_ratio_i(&u, &v); - if is_nonzero_square.unwrap_u8() != 1u8 { return None; } + if is_valid_y_coord.unwrap_u8() != 1u8 { return None; } // Flip the sign of X if it's not correct let compressed_sign_bit = Choice::from(self.as_bytes()[31] >> 7); diff --git a/src/field.rs b/src/field.rs index e523df9..93d59d0 100644 --- a/src/field.rs +++ b/src/field.rs @@ -195,40 +195,19 @@ impl FieldElement { t21 } - /// Given `FieldElements` `u` and `v`, attempt to compute - /// `sqrt(u/v)` in constant time. + /// Given `FieldElements` `u` and `v`, compute either `sqrt(u/v)` + /// or `sqrt(i*u/v)` in constant time. /// - /// This function always returns the nonnegative square root, if it exists. - /// - /// It would be much better to use an `Option` type here, but - /// doing so forces the caller to branch, which we don't want to - /// do. This seems like the least bad solution. + /// This function always returns the nonnegative square root. /// /// # Return /// - /// - `(1u8, sqrt(u/v))` if `v` is nonzero and `u/v` is square; - /// - `(0u8, zero)` if `v` is zero; - /// - `(0u8, garbage)` if `u/v` is nonsquare. + /// - `(Choice(1), +sqrt(u/v)) ` if `v` is nonzero and `u/v` is square; + /// - `(Choice(1), zero) ` if `u` is zero; + /// - `(Choice(0), zero) ` if `v` is zero and `u` is nonzero; + /// - `(Choice(0), +sqrt(i*u/v))` if `u/v` is nonsquare (so `i*u/v` is square). /// - /// # Example - /// - /// ```ignore - /// let one = FieldElement::one(); - /// let two = &one + &one; - /// let four = &two * &two; - /// - /// // two is nonsquare mod p - /// let (two_is_square, two_sqrt) = FieldElement::sqrt_ratio(&two, &one); - /// assert_eq!(two_is_square.unwrap_u8(), 0u8); - /// - /// // four is square mod p - /// let (four_is_square, four_sqrt) = FieldElement::sqrt_ratio(&four, &one); - /// - /// assert_eq!(four_is_square.unwrap_u8(), 1u8); - /// assert_eq!(four_sqrt.is_negative().unwrap_u8 - /// ``` - /// - pub fn sqrt_ratio(u: &FieldElement, v: &FieldElement) -> (Choice, FieldElement) { + pub fn sqrt_ratio_i(u: &FieldElement, v: &FieldElement) -> (Choice, FieldElement) { // Using the same trick as in ed25519 decoding, we merge the // inversion, the square root, and the square test as follows. // @@ -258,11 +237,14 @@ impl FieldElement { let mut r = &(u * &v3) * &(u * &v7).pow_p58(); let check = v * &r.square(); - let correct_sign_sqrt = check.ct_eq( u); - let flipped_sign_sqrt = check.ct_eq(&(-u)); + let i = &constants::SQRT_M1; + + let correct_sign_sqrt = check.ct_eq( u); + let flipped_sign_sqrt = check.ct_eq( &(-u)); + let flipped_sign_sqrt_i = check.ct_eq(&(&(-u)*i)); let r_prime = &constants::SQRT_M1 * &r; - r.conditional_assign(&r_prime, flipped_sign_sqrt); + r.conditional_assign(&r_prime, flipped_sign_sqrt | flipped_sign_sqrt_i); // Choose the nonnegative square root. let r_is_negative = r.is_negative(); @@ -273,42 +255,20 @@ impl FieldElement { (was_nonzero_square, r) } - /// For `self` a nonzero square, compute 1/sqrt(self) in - /// constant time. + /// Attempt to compute `1/sqrt(self)` in constant time. /// - /// It would be much better to use an `Option` type here, but - /// doing so forces the caller to branch, which we don't want to - /// do. This seems like the least bad solution. + /// Convenience wrapper around `sqrt_ratio_i`. + /// + /// This function always returns the nonnegative square root. /// /// # Return /// - /// - `(1u8, 1/sqrt(self))` if `self` is a nonzero square; - /// - `(0u8, zero)` if `self` is zero; - /// - `(0u8, garbage)` if `self` is nonsquare. + /// - `(Choice(1), +sqrt(1/self)) ` if `self` is a nonzero square; + /// - `(Choice(0), zero) ` if `self` is zero; + /// - `(Choice(0), +sqrt(i*u/v)) ` if `self` is a nonzero nonsquare; /// pub fn invsqrt(&self) -> (Choice, FieldElement) { - FieldElement::sqrt_ratio(&FieldElement::one(), self) - } - - /// chi calculates `self^((p-1)/2)`. - /// - /// # Return - /// - /// * If this element is a non-zero square, returns `1`. - /// * If it is zero, returns `0`. - /// * If it is non-square, returns `-1`. - pub fn chi(&self) -> FieldElement { // extra25519.chi - // The bits of (p-1)/2 = 2^254 -10 are 0110111111...11. - // - // nonzero bits of exponent - let (t19, _) = self.pow22501(); // 249..0 - let t20 = t19.pow2k(4); // 253..4 - let t21 = self.square(); // 1 - let t22 = t21.square(); // 2 - let t23 = &t22 * &t21; // 2,1 - let t24 = &t20 * &t23; // 253..4,2,1 - - t24 + FieldElement::sqrt_ratio_i(&FieldElement::one(), self) } } @@ -392,6 +352,45 @@ mod test { } } + #[test] + fn sqrt_ratio_behavior() { + let zero = FieldElement::zero(); + let one = FieldElement::one(); + let i = constants::SQRT_M1; + let two = &one + &one; // 2 is nonsquare mod p. + let four = &two + &two; // 4 is square mod p. + + // 0/0 should return (1, 0) since u is 0 + let (choice, sqrt) = FieldElement::sqrt_ratio_i(&zero, &zero); + assert_eq!(choice.unwrap_u8(), 1); + assert_eq!(sqrt, zero); + assert_eq!(sqrt.is_negative().unwrap_u8(), 0); + + // 1/0 should return (0, 0) since v is 0, u is nonzero + let (choice, sqrt) = FieldElement::sqrt_ratio_i(&one, &zero); + assert_eq!(choice.unwrap_u8(), 0); + assert_eq!(sqrt, zero); + assert_eq!(sqrt.is_negative().unwrap_u8(), 0); + + // 2/1 is nonsquare, so we expect (0, sqrt(i*2)) + let (choice, sqrt) = FieldElement::sqrt_ratio_i(&two, &one); + assert_eq!(choice.unwrap_u8(), 0); + assert_eq!(sqrt.square(), &two * &i); + assert_eq!(sqrt.is_negative().unwrap_u8(), 0); + + // 4/1 is square, so we expect (1, sqrt(4)) + let (choice, sqrt) = FieldElement::sqrt_ratio_i(&four, &one); + assert_eq!(choice.unwrap_u8(), 1); + assert_eq!(sqrt.square(), four); + assert_eq!(sqrt.is_negative().unwrap_u8(), 0); + + // 1/4 is square, so we expect (1, 1/sqrt(4)) + let (choice, sqrt) = FieldElement::sqrt_ratio_i(&one, &four); + assert_eq!(choice.unwrap_u8(), 1); + assert_eq!(&sqrt.square() * &four, one); + assert_eq!(sqrt.is_negative().unwrap_u8(), 0); + } + #[test] fn a_p58_vs_ap58_constant() { let a = FieldElement::from_bytes(&A_BYTES); @@ -399,17 +398,6 @@ mod test { assert_eq!(ap58, a.pow_p58()); } - #[test] - fn chi_on_square_and_nonsquare() { - let a = FieldElement::from_bytes(&A_BYTES); - // a is square - assert_eq!(a.chi(), FieldElement::one()); - let mut two_bytes = [0u8; 32]; two_bytes[0] = 2; - let two = FieldElement::from_bytes(&two_bytes); - // 2 is nonsquare - assert_eq!(two.chi(), FieldElement::minus_one()); - } - #[test] fn equality() { let a = FieldElement::from_bytes(&A_BYTES); diff --git a/src/ristretto.rs b/src/ristretto.rs index 8c401e3..a96e21c 100644 --- a/src/ristretto.rs +++ b/src/ristretto.rs @@ -586,21 +586,14 @@ impl RistrettoPoint { let d_sq = d.square(); let N = -&( &(&d_sq - &one) * &(&r + &one) ); - let mut s = FieldElement::zero(); let mut c = -&one; + let (N_over_D_is_square, mut s) = FieldElement::sqrt_ratio_i(&N, &D); + let mut s_prime = &s * r_0; + let s_prime_is_pos = !s_prime.is_negative(); + s_prime.conditional_negate(s_prime_is_pos); - let (N_over_D_is_square, maybe_s) = FieldElement::sqrt_ratio(&N, &D); - // s = sqrt(N/D) if N/D is square - s.conditional_assign(&maybe_s, N_over_D_is_square); - - // XXX how exactly do we reuse the computation of sqrt(N/D) to find sqrt(rN/D) ? - let (rN_over_D_is_square, mut maybe_s) = FieldElement::sqrt_ratio(&(&r*&N), &D); - maybe_s.negate(); - - // s = -sqrt(rN/D) if rN/D is square (should happen exactly when N/D is nonsquare) - debug_assert_eq!((N_over_D_is_square ^ rN_over_D_is_square).unwrap_u8(), 1u8); - s.conditional_assign(&maybe_s, rN_over_D_is_square); - c.conditional_assign(&r, rN_over_D_is_square); + s.conditional_assign(&s_prime, !N_over_D_is_square); + c.conditional_assign(&r, !N_over_D_is_square); // T = (c * (r - one) * (d-one).square()) - D; let T = &(&c * &(&(&r - &one) * &((d - &one).square()))) - &D;