Normalize p256

p256/Cargo.toml

@@ -29,6 +29,8 @@ ecdsa-core = { version = "=0.13.0-pre", package = "ecdsa", default-features = fa
 hex-literal = "0.3"
 proptest = "1.0"
 rand_core = { version = "0.6", features = ["getrandom"] }
+num-bigint = "0.4"
+num-traits = "0.2"
 
 [features]
 default = ["arithmetic", "ecdsa", "pkcs8", "std"]

p256/src/arithmetic/scalar.rs

@@ -13,23 +13,28 @@ use elliptic_curve::{
     group::ff::{Field, PrimeField},
     ops::Reduce,
     rand_core::RngCore,
-    subtle::{Choice, ConditionallySelectable, ConstantTimeEq, ConstantTimeLess, CtOption},
+    subtle::{
+        Choice, ConditionallySelectable, ConstantTimeEq, ConstantTimeGreater, ConstantTimeLess,
+        CtOption,
+    },
     zeroize::DefaultIsZeroes,
     Curve, ScalarArithmetic, ScalarCore,
 };
 
 #[cfg(feature = "bits")]
 use {crate::ScalarBits, elliptic_curve::group::ff::PrimeFieldBits};
 
+#[cfg(test)]
+use num_bigint::{BigUint, ToBigUint};
+
 /// Array containing 4 x 64-bit unsigned integers.
 // TODO(tarcieri): replace this entirely with `U256`
 type U64x4 = [u64; 4];
 
 /// Constant representing the modulus
 /// n = FFFFFFFF 00000000 FFFFFFFF FFFFFFFF BCE6FAAD A7179E84 F3B9CAC2 FC632551
 const MODULUS: U64x4 = u256_to_u64x4(NistP256::ORDER);
-
-const MODULUS_SHR1: Scalar = Scalar(NistP256::ORDER.shr_vartime(1));
+const FRAC_MODULUS_2: U256 = NistP256::ORDER.shr_vartime(1);
 
 /// MU = floor(2^512 / n)
 ///    = 115792089264276142090721624801893421302707618245269942344307673200490803338238
@@ -71,7 +76,7 @@ impl ScalarArithmetic for NistP256 {
 /// Please see the documentation for the relevant traits for more information.
 #[derive(Clone, Copy, Debug, Default)]
 #[cfg_attr(docsrs, doc(cfg(feature = "arithmetic")))]
-pub struct Scalar(pub(crate) U256);
+pub struct Scalar(U256);
 
 impl Field for Scalar {
     fn random(mut rng: impl RngCore) -> Self {
@@ -277,6 +282,27 @@ impl Scalar {
         CtOption::new(inverse, !self.is_zero())
     }
 
+    /// Is this scalar greater than or equal to n / 2?
+    pub fn is_high(&self) -> Choice {
+        // self.0.ct_gt(&U256::from_uint_array(FRAC_MODULUS_2))
+        self.0.ct_gt(&FRAC_MODULUS_2)
+    }
+
+    /// Returns the scalar modulus as a `BigUint` object.
+    #[cfg(test)]
+    pub fn modulus_as_biguint() -> BigUint {
+        Self::one().neg().to_biguint().unwrap() + 1.to_biguint().unwrap()
+    }
+
+    /// Determine if this `Scalar` is zero.
+    ///
+    /// # Returns
+    ///
+    /// If zero, return `Choice(1)`.  Otherwise, return `Choice(0)`.
+    pub fn is_zero(&self) -> Choice {
+        self.ct_eq(&Scalar::zero())
+    }
+
     /// Faster inversion using Stein's algorithm
     #[allow(non_snake_case)]
     pub fn invert_vartime(&self) -> CtOption<Self> {
@@ -297,7 +323,7 @@ impl Scalar {
                 A.shr1();
 
                 if was_odd {
-                    A += MODULUS_SHR1;
+                    A += Scalar(FRAC_MODULUS_2);
                     A += Self::one();
                 }
             }
@@ -310,7 +336,7 @@ impl Scalar {
                 C.shr1();
 
                 if was_odd {
-                    C += MODULUS_SHR1;
+                    C += Scalar(FRAC_MODULUS_2);
                     C += Self::one();
                 }
             }
@@ -765,8 +791,10 @@ pub(crate) const fn u256_to_u64x4(u256: U256) -> U64x4 {
 #[cfg(test)]
 mod tests {
     use super::Scalar;
+    use crate::arithmetic::util::{biguint_to_bytes, bytes_to_biguint};
     use crate::{FieldBytes, SecretKey};
     use elliptic_curve::group::ff::{Field, PrimeField};
+    use num_bigint::{BigUint, ToBigUint};
 
     #[test]
     fn from_to_bytes_roundtrip() {
@@ -850,4 +878,51 @@ mod tests {
         let scalar_bits = ScalarBits::from(&-Scalar::from(1));
         assert_eq!(minus_one, scalar_bits);
     }
+
+    impl From<&BigUint> for Scalar {
+        fn from(x: &BigUint) -> Self {
+            debug_assert!(x < &Scalar::modulus_as_biguint());
+            let bytes = biguint_to_bytes(x);
+            Self::from_repr(bytes.into()).unwrap()
+        }
+    }
+
+    impl From<BigUint> for Scalar {
+        fn from(x: BigUint) -> Self {
+            Self::from(&x)
+        }
+    }
+
+    impl ToBigUint for Scalar {
+        fn to_biguint(&self) -> Option<BigUint> {
+            Some(bytes_to_biguint(self.to_bytes().as_ref()))
+        }
+    }
+
+    #[test]
+    fn is_high() {
+        // 0 is not high
+        let high: bool = Scalar::zero().is_high().into();
+        assert!(!high);
+
+        // 1 is not high
+        let one = 1.to_biguint().unwrap();
+        let high: bool = Scalar::from(&one).is_high().into();
+        assert!(!high);
+
+        let m = Scalar::modulus_as_biguint();
+        let m_by_2 = &m >> 1;
+
+        // M / 2 is not high
+        let high: bool = Scalar::from(&m_by_2).is_high().into();
+        assert!(!high);
+
+        // M / 2 + 1 is high
+        let high: bool = Scalar::from(&m_by_2 + &one).is_high().into();
+        assert!(high);
+
+        // MODULUS - 1 is high
+        let high: bool = Scalar::from(&m - &one).is_high().into();
+        assert!(high);
+    }
 }

p256/src/arithmetic/util.rs

@@ -1,5 +1,9 @@
 //! Helper functions.
 // TODO(tarcieri): replace these with `crypto-bigint`
+#[cfg(test)]
+use num_bigint::{BigUint, ToBigUint};
+#[cfg(test)]
+use num_traits::cast::ToPrimitive;
 
 /// Computes `a + b + carry`, returning the result along with the new carry. 64-bit version.
 #[inline(always)]
@@ -21,3 +25,24 @@ pub const fn mac(a: u64, b: u64, c: u64, carry: u64) -> (u64, u64) {
     let ret = (a as u128) + ((b as u128) * (c as u128)) + (carry as u128);
     (ret as u64, (ret >> 64) as u64)
 }
+
+/// Converts a byte array (big-endian) to BigUint.
+#[cfg(test)]
+pub fn bytes_to_biguint(bytes: &[u8; 32]) -> BigUint {
+    bytes
+        .iter()
+        .enumerate()
+        .map(|(i, w)| w.to_biguint().unwrap() << ((31 - i) * 8))
+        .sum()
+}
+
+/// Converts a BigUint to a byte array (big-endian).
+#[cfg(test)]
+pub fn biguint_to_bytes(x: &BigUint) -> [u8; 32] {
+    let mask = BigUint::from(u8::MAX);
+    let mut bytes = [0u8; 32];
+    for i in 0..32 {
+        bytes[i] = ((x >> ((31 - i) * 8)) as BigUint & &mask).to_u8().unwrap();
+    }
+    bytes
+}

p256/src/ecdsa.rs

@@ -40,6 +40,8 @@
 //! ```
 
 pub use ecdsa_core::signature::{self, Error};
+#[cfg(feature = "ecdsa")]
+use ecdsa_core::NormalizeLow;
 
 use super::NistP256;
 
@@ -131,6 +133,17 @@ impl VerifyPrimitive<NistP256> for AffinePoint {
     }
 }
 
+#[cfg(feature = "ecdsa")]
+impl NormalizeLow for Scalar {
+    fn normalize_low(&self) -> Option<Self> {
+        if self.is_high().into() {
+            Some(-self)
+        } else {
+            None
+        }
+    }
+}
+
 #[cfg(all(test, feature = "ecdsa"))]
 mod tests {
     use crate::{