From 24562ae2f49ef0604a790c9f0a3414582b8df084 Mon Sep 17 00:00:00 2001
From: Tony Arcieri <bascule@gmail.com>
Date: Thu, 2 Feb 2023 19:40:53 -0700
Subject: [PATCH] elliptic-curve: remove generic `invert_vartime`
 implementation

It's mathematically unsafe in that it relies on field element
representations outside the curve's modulus, which doesn't work in a
generic context.

The newly added `Invert::invert_vartime` method allows plugging in
generic variable-time inversions.
---
 elliptic-curve/src/scalar.rs         |  4 +-
 elliptic-curve/src/scalar/blinded.rs |  7 +--
 elliptic-curve/src/scalar/invert.rs  | 69 ----------------------------
 elliptic-curve/src/scalar/nonzero.rs | 13 ------
 4 files changed, 5 insertions(+), 88 deletions(-)
 delete mode 100644 elliptic-curve/src/scalar/invert.rs

diff --git a/elliptic-curve/src/scalar.rs b/elliptic-curve/src/scalar.rs
index 6ffd63bea..eb992493a 100644
--- a/elliptic-curve/src/scalar.rs
+++ b/elliptic-curve/src/scalar.rs
@@ -3,14 +3,12 @@
 #[cfg(feature = "arithmetic")]
 mod blinded;
 #[cfg(feature = "arithmetic")]
-mod invert;
-#[cfg(feature = "arithmetic")]
 mod nonzero;
 mod primitive;
 
 pub use self::primitive::ScalarPrimitive;
 #[cfg(feature = "arithmetic")]
-pub use self::{blinded::BlindedScalar, invert::invert_vartime, nonzero::NonZeroScalar};
+pub use self::{blinded::BlindedScalar, nonzero::NonZeroScalar};
 
 use crypto_bigint::Integer;
 use subtle::Choice;
diff --git a/elliptic-curve/src/scalar/blinded.rs b/elliptic-curve/src/scalar/blinded.rs
index 33303aef1..29cfea98c 100644
--- a/elliptic-curve/src/scalar/blinded.rs
+++ b/elliptic-curve/src/scalar/blinded.rs
@@ -1,6 +1,6 @@
 //! Random blinding support for [`Scalar`]
 
-use super::{invert_vartime, Scalar};
+use super::Scalar;
 use crate::{ops::Invert, CurveArithmetic};
 use group::ff::Field;
 use rand_core::CryptoRngCore;
@@ -57,8 +57,9 @@ where
     fn invert(&self) -> CtOption<Scalar<C>> {
         // prevent side channel analysis of scalar inversion by pre-and-post-multiplying
         // with the random masking scalar
-        let masked_scalar = self.scalar * self.mask;
-        invert_vartime::<C>(&masked_scalar).map(|s| s * self.mask)
+        (self.scalar * self.mask)
+            .invert_vartime()
+            .map(|s| s * self.mask)
     }
 }
 
diff --git a/elliptic-curve/src/scalar/invert.rs b/elliptic-curve/src/scalar/invert.rs
deleted file mode 100644
index 1087835ec..000000000
--- a/elliptic-curve/src/scalar/invert.rs
+++ /dev/null
@@ -1,69 +0,0 @@
-use super::FromUintUnchecked;
-use crate::{CurveArithmetic, Scalar};
-use ff::{Field, PrimeField};
-use subtle::CtOption;
-
-/// Fast variable-time inversion using Stein's algorithm.
-///
-/// Returns none if the scalar is zero.
-///
-/// <https://link.springer.com/article/10.1007/s13389-016-0135-4>
-///
-/// ⚠️ WARNING!
-///
-/// This generic implementation relies on special properties of the scalar
-/// field implementation and may not work correctly! Please ensure your use
-/// cases are well-tested!
-///
-/// USE AT YOUR OWN RISK!
-#[allow(non_snake_case)]
-pub fn invert_vartime<C>(scalar: &Scalar<C>) -> CtOption<Scalar<C>>
-where
-    C: CurveArithmetic,
-{
-    let order_div_2 = Scalar::<C>::from_uint_unchecked(C::ORDER >> 1);
-
-    let mut u = *scalar;
-    let mut v = Scalar::<C>::from_uint_unchecked(C::ORDER); // note: technically invalid
-    let mut A = Scalar::<C>::ONE;
-    let mut C = Scalar::<C>::ZERO;
-
-    while !bool::from(u.is_zero()) {
-        // u-loop
-        while bool::from(u.is_even()) {
-            u >>= 1;
-
-            let was_odd: bool = A.is_odd().into();
-            A >>= 1;
-
-            if was_odd {
-                A += order_div_2;
-                A += Scalar::<C>::ONE;
-            }
-        }
-
-        // v-loop
-        while bool::from(v.is_even()) {
-            v >>= 1;
-
-            let was_odd: bool = C.is_odd().into();
-            C >>= 1;
-
-            if was_odd {
-                C += order_div_2;
-                C += Scalar::<C>::ONE;
-            }
-        }
-
-        // sub-step
-        if u < v {
-            v -= &u;
-            C -= &A;
-        } else {
-            u -= &v;
-            A -= &C;
-        }
-    }
-
-    CtOption::new(C, !scalar.is_zero())
-}
diff --git a/elliptic-curve/src/scalar/nonzero.rs b/elliptic-curve/src/scalar/nonzero.rs
index e4769cb87..c1f326b83 100644
--- a/elliptic-curve/src/scalar/nonzero.rs
+++ b/elliptic-curve/src/scalar/nonzero.rs
@@ -67,19 +67,6 @@ where
     pub fn from_uint(uint: C::Uint) -> CtOption<Self> {
         ScalarPrimitive::new(uint).and_then(|scalar| Self::new(scalar.into()))
     }
-
-    /// Perform an inversion in variable-time.
-    ///
-    /// ⚠️ WARNING!
-    ///
-    /// This method should not be used with (unblinded) secret scalars, as its
-    /// variable-time operation can potentially leak secrets through
-    /// sidechannels.
-    pub fn invert_vartime(&self) -> Self {
-        Self {
-            scalar: super::invert_vartime::<C>(&self.scalar).unwrap(),
-        }
-    }
 }
 
 impl<C> AsRef<Scalar<C>> for NonZeroScalar<C>