diff --git a/Cargo.toml b/Cargo.toml index e5570e9..ebcdf71 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ serde_cbor = "0.6" digest = "0.7" generic-array = "0.9" clear_on_drop = "=0.2.3" -subtle = { version = "0.5", features = ["generic-impls"], default-features = false } +subtle = { version = "0.6", features = ["generic-impls"], default-features = false } stdsimd = { version = "0.0.4", optional = true } serde = { version = "1.0", optional = true } rand = { version = "0.4", optional = true } @@ -48,7 +48,7 @@ rand = { version = "0.4", optional = true } digest = "0.7" generic-array = "0.9" clear_on_drop = "=0.2.3" -subtle = { version = "0.5", features = ["generic-impls"], default-features = false } +subtle = { version = "0.6", features = ["generic-impls"], default-features = false } stdsimd = { version = "0.0.4", optional = true } serde = { version = "1.0", optional = true } # Allowing rand to be optional during builds causes a build failure when compiling for no_std targets diff --git a/src/backend/avx2/edwards.rs b/src/backend/avx2/edwards.rs index fa83c72..3ef8535 100644 --- a/src/backend/avx2/edwards.rs +++ b/src/backend/avx2/edwards.rs @@ -20,6 +20,7 @@ use core::borrow::Borrow; use stdsimd::simd::{u32x8, i32x8}; use subtle::ConditionallyAssignable; +use subtle::Choice; use edwards; use scalar::Scalar; @@ -52,7 +53,7 @@ impl From for edwards::EdwardsPoint { } impl ConditionallyAssignable for ExtendedPoint { - fn conditional_assign(&mut self, other: &ExtendedPoint, choice: u8) { + fn conditional_assign(&mut self, other: &ExtendedPoint, choice: Choice) { self.0.conditional_assign(&other.0, choice); } } @@ -115,7 +116,7 @@ impl Identity for CachedPoint { } impl ConditionallyAssignable for CachedPoint { - fn conditional_assign(&mut self, other: &CachedPoint, choice: u8) { + fn conditional_assign(&mut self, other: &CachedPoint, choice: Choice) { self.0.conditional_assign(&other.0, choice); } } diff --git a/src/backend/avx2/field.rs b/src/backend/avx2/field.rs index 8d3af8b..d8b7883 100644 --- a/src/backend/avx2/field.rs +++ b/src/backend/avx2/field.rs @@ -38,10 +38,11 @@ use backend::avx2::constants::{P_TIMES_2_LO, P_TIMES_2_HI, P_TIMES_16_LO, P_TIME pub(crate) struct FieldElement32x4(pub(crate) [u32x8; 5]); use subtle::ConditionallyAssignable; +use subtle::Choice; impl ConditionallyAssignable for FieldElement32x4 { - fn conditional_assign(&mut self, other: &FieldElement32x4, choice: u8) { - let mask = (-(choice as i32)) as u32; + fn conditional_assign(&mut self, other: &FieldElement32x4, choice: Choice) { + let mask = (-(choice.unwrap_u8() as i32)) as u32; let mask_vec = u32x8::splat(mask); for i in 0..5 { self.0[i] = self.0[i] ^ (mask_vec & (self.0[i] ^ other.0[i])); diff --git a/src/backend/u32/field.rs b/src/backend/u32/field.rs index fef4615..8c65a46 100644 --- a/src/backend/u32/field.rs +++ b/src/backend/u32/field.rs @@ -22,6 +22,7 @@ use core::ops::{Mul, MulAssign}; use core::ops::Neg; use subtle::ConditionallyAssignable; +use subtle::Choice; /// A `FieldElement32` represents an element of the field /// \\( \mathbb Z / (2\^{255} - 19)\\). @@ -219,10 +220,9 @@ impl<'a> Neg for &'a FieldElement32 { } impl ConditionallyAssignable for FieldElement32 { - fn conditional_assign(&mut self, f: &FieldElement32, choice: u8) { - let mask = (-(choice as i32)) as u32; + fn conditional_assign(&mut self, other: &FieldElement32, choice: Choice) { for i in 0..10 { - self.0[i] ^= mask & (self.0[i] ^ f.0[i]); + self.0[i].conditional_assign(&other.0[i], choice); } } } diff --git a/src/backend/u64/field.rs b/src/backend/u64/field.rs index 0859991..d685ff3 100644 --- a/src/backend/u64/field.rs +++ b/src/backend/u64/field.rs @@ -18,6 +18,7 @@ use core::ops::{Mul, MulAssign}; use core::ops::Neg; use subtle::ConditionallyAssignable; +use subtle::Choice; /// A `FieldElement64` represents an element of the field /// \\( \mathbb Z / (2\^{255} - 19)\\). @@ -209,10 +210,9 @@ impl<'a> Neg for &'a FieldElement64 { } impl ConditionallyAssignable for FieldElement64 { - fn conditional_assign(&mut self, f: &FieldElement64, choice: u8) { - let mask = (-(choice as i64)) as u64; + fn conditional_assign(&mut self, other: &FieldElement64, choice: Choice) { for i in 0..5 { - self.0[i] ^= mask & (self.0[i] ^ f.0[i]); + self.0[i].conditional_assign(&other.0[i], choice); } } } diff --git a/src/constants.rs b/src/constants.rs index f353d8d..b1207ea 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -165,7 +165,7 @@ mod test { fn test_sqrt_constants_sign() { let minus_one = FieldElement::minus_one(); let (was_nonzero_square, invsqrt_m1) = minus_one.invsqrt(); - assert_eq!(was_nonzero_square, 1u8); + 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? diff --git a/src/curve_models/mod.rs b/src/curve_models/mod.rs index f9b29d8..59426fa 100644 --- a/src/curve_models/mod.rs +++ b/src/curve_models/mod.rs @@ -126,11 +126,13 @@ use core::fmt::Debug; use core::ops::{Add, Sub, Neg}; +use subtle::ConditionallyAssignable; +use subtle::Choice; + use constants; use field::FieldElement; use edwards::EdwardsPoint; -use subtle::ConditionallyAssignable; use traits::ValidityCheck; pub mod window; @@ -275,7 +277,7 @@ impl ValidityCheck for ProjectivePoint { // ------------------------------------------------------------------------ impl ConditionallyAssignable for ProjectiveNielsPoint { - fn conditional_assign(&mut self, other: &ProjectiveNielsPoint, choice: u8) { + fn conditional_assign(&mut self, other: &ProjectiveNielsPoint, choice: Choice) { self.Y_plus_X.conditional_assign(&other.Y_plus_X, choice); self.Y_minus_X.conditional_assign(&other.Y_minus_X, choice); self.Z.conditional_assign(&other.Z, choice); @@ -284,7 +286,7 @@ impl ConditionallyAssignable for ProjectiveNielsPoint { } impl ConditionallyAssignable for AffineNielsPoint { - fn conditional_assign(&mut self, other: &AffineNielsPoint, choice: u8) { + fn conditional_assign(&mut self, other: &AffineNielsPoint, choice: Choice) { // PreComputedGroupElementCMove() self.y_plus_x.conditional_assign(&other.y_plus_x, choice); self.y_minus_x.conditional_assign(&other.y_minus_x, choice); diff --git a/src/curve_models/window.rs b/src/curve_models/window.rs index c247459..ada3706 100644 --- a/src/curve_models/window.rs +++ b/src/curve_models/window.rs @@ -14,9 +14,10 @@ use core::fmt::Debug; -use subtle; use subtle::ConditionallyNegatable; use subtle::ConditionallyAssignable; +use subtle::ConstantTimeEq; +use subtle::Choice; use traits::Identity; @@ -67,12 +68,12 @@ where T: Identity + ConditionallyAssignable + ConditionallyNegatable let mut t = T::identity(); for j in 1..9 { // Copy `points[j-1] == j*P` onto `t` in constant time if `|x| == j`. - t.conditional_assign(&self.0[j-1], - subtle::bytes_equal(xabs as u8, j as u8)); + let c = (xabs as u8).ct_eq(&(j as u8)); + t.conditional_assign(&self.0[j-1], c); } // Now t == |x| * P. - let neg_mask = (xmask & 1) as u8; + let neg_mask = Choice::from((xmask & 1) as u8); t.conditional_negate(neg_mask); // Now t == x * P. diff --git a/src/edwards.rs b/src/edwards.rs index 952399d..68e02e4 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -17,9 +17,9 @@ //! //! ## Equality Testing //! -//! The `EdwardsPoint` struct implements the `subtle::Equal` trait for -//! constant-time equality checking, and the Rust `Eq` trait for -//! variable-time equality checking. +//! The `EdwardsPoint` struct implements the `subtle::ConstantTimeEq` +//! trait for constant-time equality checking, and the Rust `Eq` trait +//! for variable-time equality checking. //! //! ## Cofactor-related functions //! @@ -101,11 +101,10 @@ use core::ops::{Mul, MulAssign}; use core::ops::Index; use core::borrow::Borrow; -use subtle::slices_equal; use subtle::ConditionallyAssignable; use subtle::ConditionallyNegatable; -// XXX subtle::Equal -use subtle::Equal; +use subtle::Choice; +use subtle::ConstantTimeEq; use constants; @@ -164,11 +163,12 @@ impl CompressedEdwardsY { let v = &(&YY * &constants::EDWARDS_D) + &Z; // v = dy²+1 let (is_nonzero_square, mut X) = FieldElement::sqrt_ratio(&u, &v); - if is_nonzero_square != 1u8 { return None; } + if is_nonzero_square.unwrap_u8() != 1u8 { return None; } // Flip the sign of X if it's not correct - let compressed_sign_bit = self.as_bytes()[31] >> 7; + let compressed_sign_bit = Choice::from(self.as_bytes()[31] >> 7); let current_sign_bit = X.is_negative(); + X.conditional_negate(current_sign_bit ^ compressed_sign_bit); Some(EdwardsPoint{ X: X, Y: Y, Z: Z, T: &X * &Y }) @@ -282,7 +282,7 @@ impl ValidityCheck for EdwardsPoint { // ------------------------------------------------------------------------ impl ConditionallyAssignable for EdwardsPoint { - fn conditional_assign(&mut self, other: &EdwardsPoint, choice: u8) { + fn conditional_assign(&mut self, other: &EdwardsPoint, choice: Choice) { self.X.conditional_assign(&other.X, choice); self.Y.conditional_assign(&other.Y, choice); self.Z.conditional_assign(&other.Z, choice); @@ -294,16 +294,15 @@ impl ConditionallyAssignable for EdwardsPoint { // Equality // ------------------------------------------------------------------------ -impl Equal for EdwardsPoint { - fn ct_eq(&self, other: &EdwardsPoint) -> u8 { - slices_equal(self.compress().as_bytes(), - other.compress().as_bytes()) +impl ConstantTimeEq for EdwardsPoint { + fn ct_eq(&self, other: &EdwardsPoint) -> Choice { + self.compress().as_bytes().ct_eq(other.compress().as_bytes()) } } impl PartialEq for EdwardsPoint { fn eq(&self, other: &EdwardsPoint) -> bool { - self.ct_eq(other) == 1u8 + self.ct_eq(other).unwrap_u8() == 1u8 } } @@ -374,8 +373,8 @@ impl EdwardsPoint { let y = &self.Y * &recip; let mut s: [u8; 32]; - s = y.to_bytes(); - s[31] ^= (x.is_negative() << 7) as u8; + s = y.to_bytes(); + s[31] ^= x.is_negative().unwrap_u8() << 7; CompressedEdwardsY(s) } } @@ -1203,7 +1202,7 @@ mod test { Z: FieldElement::from_bytes(&two_bytes), T: FieldElement::zero() }; - assert!(id1.ct_eq(&id2) == 1u8); + assert_eq!(id1.ct_eq(&id2).unwrap_u8(), 1u8); } /// Sanity check for conversion to precomputed points @@ -1290,9 +1289,9 @@ mod test { let mut p1 = AffineNielsPoint::identity(); let bp = constants::ED25519_BASEPOINT_POINT.to_affine_niels(); - p1.conditional_assign(&bp, 0); + p1.conditional_assign(&bp, Choice::from(0)); assert_eq!(p1, id); - p1.conditional_assign(&bp, 1); + p1.conditional_assign(&bp, Choice::from(1)); assert_eq!(p1, bp); } diff --git a/src/field.rs b/src/field.rs index 756423e..6b599f4 100644 --- a/src/field.rs +++ b/src/field.rs @@ -24,11 +24,10 @@ use core::cmp::{Eq, PartialEq}; -use subtle::slices_equal; -use subtle::byte_is_nonzero; use subtle::ConditionallyAssignable; use subtle::ConditionallyNegatable; -use subtle::Equal; +use subtle::Choice; +use subtle::ConstantTimeEq; use constants; use backend; @@ -54,37 +53,19 @@ pub use backend::u32::field::*; pub type FieldElement = backend::u32::field::FieldElement32; impl Eq for FieldElement {} + impl PartialEq for FieldElement { - /// Test equality between two `FieldElement`s. Since the - /// internal representation is not canonical, the field elements - /// are normalized to wire format before comparison. - /// - /// # Warning - /// - /// This comparison is *not* constant time. It could easily be - /// made to be, but the main use of an `Eq` implementation is for - /// branching, so it seems pointless to do so. fn eq(&self, other: &FieldElement) -> bool { - let self_bytes = self.to_bytes(); - let other_bytes = other.to_bytes(); - let mut are_equal: bool = true; - for i in 0..32 { - are_equal &= self_bytes[i] == other_bytes[i]; - } - are_equal + self.ct_eq(other).unwrap_u8() == 1u8 } } -impl Equal for FieldElement { +impl ConstantTimeEq for FieldElement { /// Test equality between two `FieldElement`s. Since the /// internal representation is not canonical, the field elements /// are normalized to wire format before comparison. - /// - /// # Returns - /// - /// `1u8` if the two `FieldElement`s are equal, and `0u8` otherwise. - fn ct_eq(&self, other: &FieldElement) -> u8 { - slices_equal(&self.to_bytes(), &other.to_bytes()) + fn ct_eq(&self, other: &FieldElement) -> Choice { + self.to_bytes().ct_eq(&other.to_bytes()) } } @@ -95,33 +76,22 @@ impl FieldElement { /// /// # Return /// - /// If negative, return `1u8`. Otherwise, return `0u8`. - pub fn is_negative(&self) -> u8 { + /// If negative, return `Choice(1)`. Otherwise, return `Choice(0)`. + pub fn is_negative(&self) -> Choice { let bytes = self.to_bytes(); - (bytes[0] & 1) as u8 + (bytes[0] & 1).into() } /// Determine if this `FieldElement` is zero. /// /// # Return /// - /// If zero, return `1u8`. Otherwise, return `0u8`. - pub fn is_zero(&self) -> u8 { - 1u8 & (!self.is_nonzero()) - } - - /// Determine if this `FieldElement` is non-zero. - /// - /// # Return - /// - /// If non-zero, return `1u8`. Otherwise, return `0u8`. - pub fn is_nonzero(&self) -> u8 { //FeIsNonZero + /// If zero, return `Choice(1)`. Otherwise, return `Choice(0)`. + pub fn is_zero(&self) -> Choice { + let zero = [0u8; 32]; let bytes = self.to_bytes(); - let mut x = 0u8; - for b in &bytes { - x |= *b; - } - byte_is_nonzero(x) + + bytes.ct_eq(&zero) } /// Compute (self^(2^250-1), self^11), used as a helper function @@ -275,7 +245,25 @@ impl FieldElement { /// - `(0u8, zero)` if `v` is zero; /// - `(0u8, garbage)` if `u/v` is nonsquare. /// - pub fn sqrt_ratio(u: &FieldElement, v: &FieldElement) -> (u8, FieldElement) { + /// # 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) { // Using the same trick as in ed25519 decoding, we merge the // inversion, the square root, and the square test as follows. // @@ -333,7 +321,7 @@ impl FieldElement { /// - `(0u8, zero)` if `self` is zero; /// - `(0u8, garbage)` if `self` is nonsquare. /// - pub fn invsqrt(&self) -> (u8, FieldElement) { + pub fn invsqrt(&self) -> (Choice, FieldElement) { FieldElement::sqrt_ratio(&FieldElement::one(), self) } @@ -487,11 +475,11 @@ mod test { let one = FieldElement::one(); let minus_one = FieldElement::minus_one(); let mut x = one; - x.conditional_negate(1u8); + x.conditional_negate(Choice::from(1)); assert_eq!(x, minus_one); - x.conditional_negate(0u8); + x.conditional_negate(Choice::from(0)); assert_eq!(x, minus_one); - x.conditional_negate(1u8); + x.conditional_negate(Choice::from(1)); assert_eq!(x, one); } diff --git a/src/montgomery.rs b/src/montgomery.rs index 0c953bc..8ba7670 100644 --- a/src/montgomery.rs +++ b/src/montgomery.rs @@ -60,8 +60,8 @@ use traits::{Identity, ValidityCheck}; use subtle::ConditionallyAssignable; use subtle::ConditionallySwappable; -use subtle::Equal; -use subtle::Mask; +use subtle::ConstantTimeEq; +use subtle::Choice; /// Holds the \\(u\\)-coordinate of a point on the Montgomery form of /// Curve25519 or its twist. @@ -69,8 +69,8 @@ use subtle::Mask; pub struct MontgomeryPoint(pub [u8; 32]); /// Equality of `MontgomeryPoint`s is defined mod p. -impl Equal for MontgomeryPoint { - fn ct_eq(&self, other: &MontgomeryPoint) -> u8 { +impl ConstantTimeEq for MontgomeryPoint { + fn ct_eq(&self, other: &MontgomeryPoint) -> Choice { let self_fe = FieldElement::from_bytes(&self.0); let other_fe = FieldElement::from_bytes(&other.0); @@ -80,7 +80,7 @@ impl Equal for MontgomeryPoint { impl PartialEq for MontgomeryPoint { fn eq(&self, other: &MontgomeryPoint) -> bool { - self.ct_eq(other) == 1u8 + self.ct_eq(other).unwrap_u8() == 1u8 } } @@ -157,7 +157,7 @@ impl Identity for ProjectivePoint { } impl ConditionallyAssignable for ProjectivePoint { - fn conditional_assign(&mut self, that: &ProjectivePoint, choice: Mask) { + fn conditional_assign(&mut self, that: &ProjectivePoint, choice: Choice) { self.U.conditional_assign(&that.U, choice); self.W.conditional_assign(&that.W, choice); } @@ -244,14 +244,14 @@ impl Mul for MontgomeryPoint { let bits: [i8; 256] = scalar.bits(); for i in (0..255).rev() { - let mask: u8 = (bits[i+1] ^ bits[i]) as u8; + let choice: u8 = (bits[i+1] ^ bits[i]) as u8; - debug_assert!(mask == 0 || mask == 1); + debug_assert!(choice == 0 || choice == 1); - x0.conditional_swap(&mut x1, mask); + x0.conditional_swap(&mut x1, choice.into()); differential_add_and_double(&mut x0, &mut x1, &affine_u); } - x0.conditional_swap(&mut x1, bits[0] as u8); + x0.conditional_swap(&mut x1, Choice::from(bits[0] as u8)); x0.to_affine() } diff --git a/src/ristretto.rs b/src/ristretto.rs index fa9ef68..3b75855 100644 --- a/src/ristretto.rs +++ b/src/ristretto.rs @@ -57,9 +57,10 @@ //! checking in the Ristretto group can be done in projective //! coordinates without requiring an inversion, so it is much faster. //! -//! The `RistrettoPoint` struct implements the `subtle::Equal` trait for -//! constant-time equality checking, and the Rust `Eq` trait for -//! variable-time equality checking. +//! The `RistrettoPoint` struct implements the +//! `subtle::ConstantTimeEq` trait for constant-time equality +//! checking, and the Rust `Eq` trait for variable-time equality +//! checking. //! //! ## Scalars //! @@ -479,10 +480,10 @@ use generic_array::typenum::U32; use constants; use field::FieldElement; -use subtle; use subtle::ConditionallyAssignable; use subtle::ConditionallyNegatable; -use subtle::Equal; +use subtle::ConstantTimeEq; +use subtle::Choice; use edwards; use edwards::EdwardsPoint; @@ -538,10 +539,10 @@ impl CompressedRistretto { let s = FieldElement::from_bytes(self.as_bytes()); let s_bytes_check = s.to_bytes(); let s_encoding_is_canonical = - subtle::slices_equal(&s_bytes_check[..], self.as_bytes()); + &s_bytes_check[..].ct_eq(self.as_bytes()); let s_is_negative = s.is_negative(); - if s_encoding_is_canonical == 0u8 || s_is_negative == 1u8 { + if s_encoding_is_canonical.unwrap_u8() == 0u8 || s_is_negative.unwrap_u8() == 1u8 { return None; } @@ -565,7 +566,7 @@ impl CompressedRistretto { let t = &x * &y; - if ok == 0u8 || t.is_negative() == 1u8 || y.is_zero() == 1u8 { + if ok.unwrap_u8() == 0u8 || t.is_negative().unwrap_u8() == 1u8 || y.is_zero().unwrap_u8() == 1u8 { return None; } else { return Some(RistrettoPoint(EdwardsPoint{X: x, Y: y, Z: one, T: t})); @@ -839,7 +840,7 @@ impl RistrettoPoint { 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, 1u8); + 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); @@ -944,17 +945,18 @@ impl Identity for RistrettoPoint { impl PartialEq for RistrettoPoint { fn eq(&self, other: &RistrettoPoint) -> bool { - self.ct_eq(other) == 1u8 + self.ct_eq(other).unwrap_u8() == 1u8 } } -impl Equal for RistrettoPoint { +impl ConstantTimeEq for RistrettoPoint { /// Test equality between two `RistrettoPoint`s. /// /// # Returns /// - /// `1u8` if the two `RistrettoPoint`s are equal, and `0u8` otherwise. - fn ct_eq(&self, other: &RistrettoPoint) -> u8 { + /// * `Choice(1)` if the two `RistrettoPoint`s are equal; + /// * `Choice(0)` otherwise. + fn ct_eq(&self, other: &RistrettoPoint) -> Choice { let X1Y2 = &self.0.X * &other.0.Y; let Y1X2 = &self.0.Y * &other.0.X; let X1X2 = &self.0.X * &other.0.X; @@ -1145,7 +1147,7 @@ impl RistrettoBasepointTable { // ------------------------------------------------------------------------ impl ConditionallyAssignable for RistrettoPoint { - /// Conditionally assign `other` to `self`, if `choice == 1u8`. + /// Conditionally assign `other` to `self`, if `choice == Choice(1)`. /// /// # Example /// @@ -1153,24 +1155,26 @@ impl ConditionallyAssignable for RistrettoPoint { /// # extern crate subtle; /// # extern crate curve25519_dalek; /// # - /// # use subtle::ConditionallyAssignable; + /// use subtle::ConditionallyAssignable; + /// use subtle::Choice; /// # /// # use curve25519_dalek::traits::Identity; /// # use curve25519_dalek::ristretto::RistrettoPoint; /// # use curve25519_dalek::constants; /// # fn main() { + /// /// let A = RistrettoPoint::identity(); /// let B = constants::RISTRETTO_BASEPOINT_POINT; /// /// let mut P = A; /// - /// P.conditional_assign(&B, 0u8); - /// assert!(P == A); - /// P.conditional_assign(&B, 1u8); - /// assert!(P == B); + /// P.conditional_assign(&B, Choice::from(0)); + /// assert_eq!(P, A); + /// P.conditional_assign(&B, Choice::from(1)); + /// assert_eq!(P, B); /// # } /// ``` - fn conditional_assign(&mut self, other: &RistrettoPoint, choice: u8) { + fn conditional_assign(&mut self, other: &RistrettoPoint, choice: Choice) { self.0.X.conditional_assign(&other.0.X, choice); self.0.Y.conditional_assign(&other.0.Y, choice); self.0.Z.conditional_assign(&other.0.Z, choice); diff --git a/src/scalar.rs b/src/scalar.rs index ae17169..4e7d7a7 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -26,9 +26,9 @@ use rand::Rng; use digest::Digest; use generic_array::typenum::U64; -use subtle::slices_equal; +use subtle::Choice; use subtle::ConditionallyAssignable; -use subtle::Equal; +use subtle::ConstantTimeEq; use backend; use constants; @@ -145,29 +145,14 @@ impl Debug for Scalar { impl Eq for Scalar {} impl PartialEq for Scalar { - /// Test equality between two `Scalar`s. - /// - /// # Warning - /// - /// This function is *not* guaranteed to be constant time and should only be - /// used for debugging purposes. - /// - /// # Returns - /// - /// True if they are equal, and false otherwise. fn eq(&self, other: &Self) -> bool { - slices_equal(&self.bytes, &other.bytes) == 1u8 + self.ct_eq(other).unwrap_u8() == 1u8 } } -impl Equal for Scalar { - /// Test equality between two `Scalar`s in constant time. - /// - /// # Returns - /// - /// `1u8` if they are equal, and `0u8` otherwise. - fn ct_eq(&self, other: &Self) -> u8 { - slices_equal(&self.bytes, &other.bytes) +impl ConstantTimeEq for Scalar { + fn ct_eq(&self, other: &Self) -> Choice { + self.bytes.ct_eq(&other.bytes) } } @@ -246,34 +231,9 @@ impl<'a> Neg for Scalar { } impl ConditionallyAssignable for Scalar { - /// Conditionally assign another Scalar to this one. - /// - /// ``` - /// # extern crate curve25519_dalek; - /// # extern crate subtle; - /// # use curve25519_dalek::scalar::Scalar; - /// # use subtle::ConditionallyAssignable; - /// # fn main() { - /// let a = Scalar::from_bits([0u8;32]); - /// let b = Scalar::from_bits([1u8;32]); - /// let mut t = a; - /// t.conditional_assign(&b, 0u8); - /// assert!(t[0] == a[0]); - /// t.conditional_assign(&b, 1u8); - /// assert!(t[0] == b[0]); - /// # } - /// ``` - /// - /// # Preconditions - /// - /// * `choice` in {0,1} - // XXX above test checks first byte because Scalar does not impl Eq - fn conditional_assign(&mut self, other: &Scalar, choice: u8) { - // if choice = 0u8, mask = (-0i8) as u8 = 00000000 - // if choice = 1u8, mask = (-1i8) as u8 = 11111111 - let mask = -(choice as i8) as u8; + fn conditional_assign(&mut self, other: &Scalar, choice: Choice) { for i in 0..32 { - self.bytes[i] ^= mask & (self.bytes[i] ^ other.bytes[i]); + self.bytes[i].conditional_assign(&other.bytes[i], choice); } } } diff --git a/src/traits.rs b/src/traits.rs index b0beeb9..d774833 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -32,9 +32,9 @@ pub trait IsIdentity { /// Implement generic identity equality testing for a point representations /// which have constant-time equality testing and a defined identity /// constructor. -impl IsIdentity for T where T: subtle::Equal + Identity { +impl IsIdentity for T where T: subtle::ConstantTimeEq + Identity { fn is_identity(&self) -> bool { - self.ct_eq(&T::identity()) == 1u8 + self.ct_eq(&T::identity()).unwrap_u8() == 1u8 } }