Merge branch 'feature/subtle-choice_r1' into develop

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

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<ExtendedPoint> 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);
     }
 }

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) {
+    fn conditional_assign(&mut self, other: &FieldElement32x4, choice: Choice) {
-        let mask = (-(choice as i32)) as u32;
+        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]));

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) {
+    fn conditional_assign(&mut self, other: &FieldElement32, choice: Choice) {
-        let mask = (-(choice as i32)) as u32;
         for i in 0..10 {
-            self.0[i] ^= mask & (self.0[i] ^ f.0[i]);
+            self.0[i].conditional_assign(&other.0[i], choice);
         }
     }
 }

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) {
+    fn conditional_assign(&mut self, other: &FieldElement64, choice: Choice) {
-        let mask = (-(choice as i64)) as u64;
         for i in 0..5 {
-            self.0[i] ^= mask & (self.0[i] ^ f.0[i]);
+            self.0[i].conditional_assign(&other.0[i], choice);
         }
     }
 }

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?

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);

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],
+            let c = (xabs as u8).ct_eq(&(j as u8));
-                                 subtle::bytes_equal(xabs as u8, 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.
 

src/edwards.rs

@@ -17,9 +17,9 @@
 //!
 //! ## Equality Testing
 //!
-//! The `EdwardsPoint` struct implements the `subtle::Equal` trait for
+//! The `EdwardsPoint` struct implements the `subtle::ConstantTimeEq`
-//! constant-time equality checking, and the Rust `Eq` trait for
+//! trait for constant-time equality checking, and the Rust `Eq` trait
-//! variable-time equality checking.
+//! 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::Choice;
-use subtle::Equal;
+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 {
+impl ConstantTimeEq for EdwardsPoint {
-    fn ct_eq(&self, other: &EdwardsPoint) -> u8 {
+    fn ct_eq(&self, other: &EdwardsPoint) -> Choice {
-        slices_equal(self.compress().as_bytes(),
+        self.compress().as_bytes().ct_eq(other.compress().as_bytes())
-                     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 = y.to_bytes();
-        s[31] ^= (x.is_negative() << 7) as u8;
+        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);
     }
 

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();
+        self.ct_eq(other).unwrap_u8() == 1u8
-        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
     }
 }
 
-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.
-    ///
+    fn ct_eq(&self, other: &FieldElement) -> Choice {
-    /// # Returns
+        self.to_bytes().ct_eq(&other.to_bytes())
-    ///
-    /// `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())
     }
 }
 
@@ -95,33 +76,22 @@ impl FieldElement {
     ///
     /// # Return
     ///
-    /// If negative, return `1u8`.  Otherwise, return `0u8`.
+    /// If negative, return `Choice(1)`.  Otherwise, return `Choice(0)`.
-    pub fn is_negative(&self) -> u8 {
+    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`.
+    /// If zero, return `Choice(1)`.  Otherwise, return `Choice(0)`.
-    pub fn is_zero(&self) -> u8 {
+    pub fn is_zero(&self) -> Choice {
-        1u8 & (!self.is_nonzero())
+        let zero = [0u8; 32];
-    }
-
-    /// Determine if this `FieldElement` is non-zero.
-    ///
-    /// # Return
-    ///
-    /// If non-zero, return `1u8`.  Otherwise, return `0u8`.
-    pub fn is_nonzero(&self) -> u8 {  //FeIsNonZero
         let bytes = self.to_bytes();
-        let mut x = 0u8;
+
-        for b in &bytes {
+        bytes.ct_eq(&zero)
-            x |= *b;
-        }
-        byte_is_nonzero(x)
     }
 
     /// 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);
     }
 

src/montgomery.rs

@@ -60,8 +60,8 @@ use traits::{Identity, ValidityCheck};
 
 use subtle::ConditionallyAssignable;
 use subtle::ConditionallySwappable;
-use subtle::Equal;
+use subtle::ConstantTimeEq;
-use subtle::Mask;
+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 {
+impl ConstantTimeEq for MontgomeryPoint {
-    fn ct_eq(&self, other: &MontgomeryPoint) -> u8 {
+    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<Scalar> 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()
     }

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
+//! The `RistrettoPoint` struct implements the
-//! constant-time equality checking, and the Rust `Eq` trait for
+//! `subtle::ConstantTimeEq` trait for constant-time equality
-//! variable-time equality checking.
+//! 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.
+    /// * `Choice(1)` if the two `RistrettoPoint`s are equal;
-    fn ct_eq(&self, other: &RistrettoPoint) -> u8 {
+    /// * `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);
+    /// P.conditional_assign(&B, Choice::from(0));
-    /// assert!(P == A);
+    /// assert_eq!(P, A);
-    /// P.conditional_assign(&B, 1u8);
+    /// P.conditional_assign(&B, Choice::from(1));
-    /// assert!(P == B);
+    /// 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);

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 {
+impl ConstantTimeEq for Scalar {
-    /// Test equality between two `Scalar`s in constant time.
+    fn ct_eq(&self, other: &Self) -> Choice {
-    ///
+        self.bytes.ct_eq(&other.bytes)
-    /// # Returns
-    ///
-    /// `1u8` if they are equal, and `0u8` otherwise.
-    fn ct_eq(&self, other: &Self) -> u8 {
-        slices_equal(&self.bytes, &other.bytes)
     }
 }
 
@@ -246,34 +231,9 @@ impl<'a> Neg for Scalar {
 }
 
 impl ConditionallyAssignable for Scalar {
-    /// Conditionally assign another Scalar to this one.
+    fn conditional_assign(&mut self, other: &Scalar, choice: Choice) {
-    ///
-    /// ```
-    /// # 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;
         for i in 0..32 {
-            self.bytes[i] ^= mask & (self.bytes[i] ^ other.bytes[i]);
+            self.bytes[i].conditional_assign(&other.bytes[i], choice);
         }
     }
 }

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<T> IsIdentity for T where T: subtle::Equal + Identity {
+impl<T> 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
     }
 }