Make Scalar::is_canonical return Choice, and Scalar::from_canonical_bytes return CTOption

Author: elichai

src/lib.rs

@@ -270,7 +270,7 @@ extern crate zeroize;
 extern crate fiat_crypto;
 
 // Used for traits related to constant-time code.
-extern crate subtle;
+pub extern crate subtle;
 
 #[cfg(all(test, feature = "serde"))]
 extern crate bincode;

src/scalar.rs

@@ -35,28 +35,30 @@
 //!
 //! ```
 //! use curve25519_dalek::scalar::Scalar;
+//! use curve25519_dalek::subtle::CtOption;
 //!
 //! let one_as_bytes: [u8; 32] = Scalar::one().to_bytes();
-//! let a: Option<Scalar> = Scalar::from_canonical_bytes(one_as_bytes);
+//! let a: CtOption<Scalar> = Scalar::from_canonical_bytes(one_as_bytes);
 //!
-//! assert!(a.is_some());
+//! assert!(bool::from(a.is_some()));
 //! ```
 //!
 //! However, if we give it bytes representing a scalar larger than \\( \ell \\)
 //! (in this case, \\( \ell + 2 \\)), we'll get `None` back:
 //!
 //! ```
 //! use curve25519_dalek::scalar::Scalar;
+//! use curve25519_dalek::subtle::CtOption;
 //!
 //! let l_plus_two_bytes: [u8; 32] = [
 //!    0xef, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
 //!    0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
 //!    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 //!    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10,
 //! ];
-//! let a: Option<Scalar> = Scalar::from_canonical_bytes(l_plus_two_bytes);
+//! let a: CtOption<Scalar> = Scalar::from_canonical_bytes(l_plus_two_bytes);
 //!
-//! assert!(a.is_none());
+//! assert!(bool::from(a.is_none()));
 //! ```
 //!
 //! Another way to create a `Scalar` is by reducing a \\(256\\)-bit integer mod
@@ -132,7 +134,7 @@
 //! let two: Scalar = Scalar::one() + Scalar::one();
 //!
 //! assert!(a != two);              // the scalar is not reduced (mod l)…
-//! assert!(! a.is_canonical());    // …and therefore is not canonical.
+//! assert!(! bool::from(a.is_canonical()));    // …and therefore is not canonical.
 //! assert!(a.reduce() == two);     // if we were to reduce it manually, it would be.
 //! ```
 //!
@@ -157,7 +159,8 @@ use rand_core::{CryptoRng, RngCore};
 use digest::generic_array::typenum::U64;
 use digest::Digest;
 
-use subtle::{Choice, ConstantTimeGreater, ConditionallySelectable, ConstantTimeEq};
+use subtle::{CtOption, Choice};
+use subtle::{ConstantTimeGreater, ConditionallySelectable, ConstantTimeEq};
 
 use zeroize::Zeroize;
 
@@ -234,14 +237,9 @@ impl Scalar {
     /// - `Some(s)`, where `s` is the `Scalar` corresponding to `bytes`,
     ///   if `bytes` is a canonical byte representation;
     /// - `None` if `bytes` is not a canonical byte representation.
-    pub fn from_canonical_bytes(bytes: [u8; 32]) -> Option<Scalar> {
+    pub fn from_canonical_bytes(bytes: [u8; 32]) -> CtOption<Scalar> {
         let candidate = Scalar{bytes};
-
-        if candidate.is_canonical() {
-            Some(candidate)
-        } else {
-            None
-        }
+        CtOption::new(candidate, candidate.is_canonical())
     }
 
     /// Construct a `Scalar` from the low 255 bits of a 256-bit integer.
@@ -1106,7 +1104,7 @@ impl Scalar {
 
     /// Check whether this `Scalar` is the canonical representative mod \\(\ell\\).
     ///
-    /// This is intended for uses like input validation, where variable-time code is acceptable.
+    /// This is intended for uses like input validation.
     ///
     /// ```
     /// # extern crate curve25519_dalek;
@@ -1116,13 +1114,13 @@ impl Scalar {
     /// # fn main() {
     /// // 2^255 - 1, since `from_bits` clears the high bit
     /// let _2_255_minus_1 = Scalar::from_bits([0xff;32]);
-    /// assert!(!_2_255_minus_1.is_canonical());
+    /// assert!(!bool::from(_2_255_minus_1.is_canonical()));
     ///
     /// let reduced = _2_255_minus_1.reduce();
-    /// assert!(reduced.is_canonical());
+    /// assert!(bool::from(reduced.is_canonical()));
     /// # }
     /// ```
-    pub fn is_canonical(&self) -> bool {
+    pub fn is_canonical(&self) -> Choice {
         let mut over = Choice::from(0);
         let mut under = Choice::from(0);
         for (this, l) in self.unpack().0.iter().zip(&constants::L.0).rev() {
@@ -1131,7 +1129,7 @@ impl Scalar {
             under |= (!gt & !eq) &!over;
             over |= gt;
         }
-        under.into()
+        under
     }
 }
 
@@ -1670,16 +1668,16 @@ mod test {
         let non_canonical_full = [0xFF; 32];
         let non_canonical_255_minus_1 = [0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0b0111_1111];
 
-        assert!( Scalar::from_canonical_bytes(canonical_bytes).is_some() );
-        assert!( Scalar::from_canonical_bytes(canonical_l_minus_one).is_some() );
-        assert!( Scalar::from_canonical_bytes(canonical_zero).is_some() );
-        assert!( Scalar::from_canonical_bytes(canonical_255_minus_1).is_some() );
-        assert!( Scalar::from_canonical_bytes(non_canonical_bytes_because_unreduced).is_none() );
-        assert!( Scalar::from_canonical_bytes(non_canonical_bytes_because_highbit).is_none() );
-        assert!( Scalar::from_canonical_bytes(non_canonical_l).is_none() );
-        assert!( Scalar::from_canonical_bytes(non_canonical_l_plus_one).is_none() );
-        assert!( Scalar::from_canonical_bytes(non_canonical_full).is_none() );
-        assert!( Scalar::from_canonical_bytes(non_canonical_255_minus_1).is_none() );
+        assert!( bool::from(Scalar::from_canonical_bytes(canonical_bytes).is_some()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(canonical_l_minus_one).is_some()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(canonical_zero).is_some()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(canonical_255_minus_1).is_some()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(non_canonical_bytes_because_unreduced).is_none()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(non_canonical_bytes_because_highbit).is_none()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(non_canonical_l).is_none()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(non_canonical_l_plus_one).is_none()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(non_canonical_full).is_none()) );
+        assert!( bool::from(Scalar::from_canonical_bytes(non_canonical_255_minus_1).is_none()) );
     }
 
     #[test]