ec/suite_b: Remove redundant assertion.

src/ec/suite_b/ecdsa/digest_scalar.rs

@@ -69,7 +69,7 @@ fn digest_scalar_(n: &Modulus<N>, digest: &[u8]) -> Scalar {
 mod tests {
     use super::digest_bytes_scalar;
     use crate::testutil as test;
-    use crate::{cpu, digest, ec::suite_b::ops::*, limb};
+    use crate::{cpu, digest, ec::suite_b::ops::*};
 
     #[test]
     fn test() {
@@ -99,12 +99,8 @@ mod tests {
                 assert_eq!(output.len(), ops.scalar_ops.scalar_bytes_len());
                 assert_eq!(output.len(), n.bytes_len());
 
-                let expected = scalar_parse_big_endian_variable(
-                    n,
-                    limb::AllowZero::Yes,
-                    untrusted::Input::from(&output),
-                )
-                .unwrap();
+                let expected =
+                    scalar_parse_big_endian_variable(n, untrusted::Input::from(&output)).unwrap();
 
                 let actual = digest_bytes_scalar(n, &input);
                 assert_eq!(

src/ec/suite_b/ecdsa/signing.rs

@@ -26,14 +26,21 @@ use crate::{
     io::der,
     limb, pkcs8, rand, signature,
 };
+use core::borrow::Borrow;
+
 /// An ECDSA signing algorithm.
 pub struct EcdsaSigningAlgorithm {
     curve: &'static ec::Curve,
     private_scalar_ops: &'static PrivateScalarOps,
     private_key_ops: &'static PrivateKeyOps,
     digest_alg: &'static digest::Algorithm,
     pkcs8_template: &'static pkcs8::Template,
-    format_rs: fn(ops: &'static ScalarOps, r: &Scalar, s: &Scalar, out: &mut [u8]) -> usize,
+    format_rs: fn(
+        ops: &'static ScalarOps,
+        r: NonZeroScalarRef<'_>,
+        s: NonZeroScalarRef<'_>,
+        out: &mut [u8],
+    ) -> usize,
     id: AlgorithmID,
 }
 
@@ -242,23 +249,28 @@ impl EcdsaKeyPair {
         let n = &scalar_ops.scalar_modulus(cpu);
 
         for _ in 0..100 {
-            // XXX: iteration conut?
+            // XXX: iteration count?
             // Step 1.
             let k = private_key::random_scalar(self.alg.private_key_ops, n, rng)?;
-            let k_inv = ops.scalar_inv_to_mont(&k, cpu);
+            let k = match n.nonzero_scalar_leak_nonzero(&k) {
+                Some(k) => k,
+                None => continue,
+            };
+            let k_inv = ops.scalar_inv_to_mont(k, cpu);
 
             // Step 2.
-            let r = private_key_ops.point_mul_base(&k, cpu);
+            let r = private_key_ops.point_mul_base(k.borrow(), cpu);
 
             // Step 3.
             let r = {
                 let (x, _) = private_key::affine_from_jacobian(private_key_ops, q, &r)?;
                 let x = q.elem_unencoded(&x);
                 n.elem_reduced_to_scalar(&x)
             };
-            if n.is_zero(&r) {
-                continue;
-            }
+            let r = match n.nonzero_scalar_leak_nonzero(&r) {
+                Some(r) => r,
+                None => continue,
+            };
 
             // Step 4 is done by the caller.
 
@@ -267,17 +279,18 @@ impl EcdsaKeyPair {
 
             // Step 6.
             let s = {
-                let mut e_plus_dr = scalar_ops.scalar_product(&self.d, &r, cpu);
+                let mut e_plus_dr = scalar_ops.scalar_product(&self.d, r.borrow(), cpu);
                 n.add_assign(&mut e_plus_dr, &e);
                 scalar_ops.scalar_product(&k_inv, &e_plus_dr, cpu)
             };
-            if n.is_zero(&s) {
-                continue;
-            }
+            let s = match n.nonzero_scalar_leak_nonzero(&s) {
+                Some(s) => s,
+                None => continue,
+            };
 
             // Step 7 with encoding.
             return Ok(signature::Signature::new(|sig_bytes| {
-                (self.alg.format_rs)(scalar_ops, &r, &s, sig_bytes)
+                (self.alg.format_rs)(scalar_ops, r, s, sig_bytes)
             }));
         }
 
@@ -382,25 +395,33 @@ impl AsRef<[u8]> for PublicKey {
     }
 }
 
-fn format_rs_fixed(ops: &'static ScalarOps, r: &Scalar, s: &Scalar, out: &mut [u8]) -> usize {
+fn format_rs_fixed(
+    ops: &'static ScalarOps,
+    r: NonZeroScalarRef<'_>,
+    s: NonZeroScalarRef<'_>,
+    out: &mut [u8],
+) -> usize {
     let scalar_len = ops.scalar_bytes_len();
 
     let (r_out, rest) = out.split_at_mut(scalar_len);
-    limb::big_endian_from_limbs(ops.leak_limbs(r), r_out);
+    limb::big_endian_from_limbs(ops.leak_limbs(r.borrow()), r_out);
 
     let (s_out, _) = rest.split_at_mut(scalar_len);
-    limb::big_endian_from_limbs(ops.leak_limbs(s), s_out);
+    limb::big_endian_from_limbs(ops.leak_limbs(s.borrow()), s_out);
 
     2 * scalar_len
 }
 
-fn format_rs_asn1(ops: &'static ScalarOps, r: &Scalar, s: &Scalar, out: &mut [u8]) -> usize {
-    // This assumes `a` is not zero since neither `r` or `s` is allowed to be
-    // zero.
-    fn format_integer_tlv(ops: &ScalarOps, a: &Scalar, out: &mut [u8]) -> usize {
+fn format_rs_asn1(
+    ops: &'static ScalarOps,
+    r: NonZeroScalarRef<'_>,
+    s: NonZeroScalarRef<'_>,
+    out: &mut [u8],
+) -> usize {
+    fn format_integer_tlv(ops: &ScalarOps, a: NonZeroScalarRef<'_>, out: &mut [u8]) -> usize {
         let mut fixed = [0u8; ec::SCALAR_MAX_BYTES + 1];
         let fixed = &mut fixed[..(ops.scalar_bytes_len() + 1)];
-        limb::big_endian_from_limbs(ops.leak_limbs(a), &mut fixed[1..]);
+        limb::big_endian_from_limbs(ops.leak_limbs(a.borrow()), &mut fixed[1..]);
 
         // Since `a_fixed_out` is an extra byte long, it is guaranteed to start
         // with a zero.

src/ec/suite_b/ecdsa/verification.rs

@@ -21,8 +21,9 @@ use crate::{
     ec::suite_b::{ops::*, public_key::*, verify_jacobian_point_is_on_the_curve},
     error,
     io::der,
-    limb, sealed, signature,
+    sealed, signature,
 };
+use core::borrow::Borrow;
 
 /// An ECDSA verification algorithm.
 pub struct EcdsaVerificationAlgorithm {
@@ -114,17 +115,23 @@ impl EcdsaVerificationAlgorithm {
 
         // NSA Guide Step 1: "If r and s are not both integers in the interval
         // [1, n − 1], output INVALID."
-        let r = scalar_parse_big_endian_variable(n, limb::AllowZero::No, r)?;
-        let s = scalar_parse_big_endian_variable(n, limb::AllowZero::No, s)?;
+        let r = scalar_parse_big_endian_variable(n, r)?;
+        let r = n
+            .nonzero_scalar_leak_nonzero(&r)
+            .ok_or(error::Unspecified)?;
+        let s = scalar_parse_big_endian_variable(n, s)?;
+        let s = n
+            .nonzero_scalar_leak_nonzero(&s)
+            .ok_or(error::Unspecified)?;
 
         // NSA Guide Step 4: "Compute w = s**−1 mod n, using the routine in
         // Appendix B.1."
-        let w = self.ops.scalar_inv_to_mont_vartime(&s, cpu);
+        let w = self.ops.scalar_inv_to_mont_vartime(s, cpu);
 
         // NSA Guide Step 5: "Compute u1 = (e * w) mod n, and compute
         // u2 = (r * w) mod n."
         let u1 = scalar_ops.scalar_product(&e, &w, cpu);
-        let u2 = scalar_ops.scalar_product(&r, &w, cpu);
+        let u2 = scalar_ops.scalar_product(r.borrow(), &w, cpu);
 
         // NSA Guide Step 6: "Compute the elliptic curve point
         // R = (xR, yR) = u1*G + u2*Q, using EC scalar multiplication and EC
@@ -152,7 +159,7 @@ impl EcdsaVerificationAlgorithm {
             let x = q.elem_unencoded(x);
             q.elems_are_equal(&r_jacobian, &x).leak()
         }
-        let mut r = self.ops.scalar_as_elem(&r);
+        let mut r = self.ops.scalar_as_elem(r.borrow());
         if sig_r_equals_x(q, &r, &x, &z2) {
             return Ok(());
         }

src/ec/suite_b/ops.rs

@@ -20,7 +20,7 @@ use crate::{
     error::{self, LenMismatchError},
     limb::*,
 };
-use core::marker::PhantomData;
+use core::{borrow::Borrow, marker::PhantomData};
 
 use elem::{mul_mont, unary_op, unary_op_assign, unary_op_from_binary_op_assign};
 
@@ -416,7 +416,7 @@ pub struct PublicScalarOps {
         p_xy: &(Elem<R>, Elem<R>),
         cpu: cpu::Features,
     ) -> Point,
-    scalar_inv_to_mont_vartime: fn(s: &Scalar<Unencoded>, cpu: cpu::Features) -> Scalar<R>,
+    scalar_inv_to_mont_vartime: fn(s: NonZeroScalarRef<'_>, cpu: cpu::Features) -> Scalar<R>,
     pub(super) q_minus_n: PublicElem<Unencoded>,
 }
 
@@ -446,7 +446,7 @@ impl Modulus<Q> {
 impl PublicScalarOps {
     pub(super) fn scalar_inv_to_mont_vartime(
         &self,
-        s: &Scalar<Unencoded>,
+        s: NonZeroScalarRef<'_>,
         cpu: cpu::Features,
     ) -> Scalar<R> {
         (self.scalar_inv_to_mont_vartime)(s, cpu)
@@ -468,9 +468,12 @@ impl PrivateScalarOps {
     }
 
     /// Returns the modular inverse of `a` (mod `n`). Panics if `a` is zero.
-    pub(super) fn scalar_inv_to_mont(&self, a: &Scalar, cpu: cpu::Features) -> Scalar<R> {
-        assert!(!self.scalar_ops.common.is_zero(a));
-        let a = self.to_mont(a, cpu);
+    pub(super) fn scalar_inv_to_mont(
+        &self,
+        a: NonZeroScalarRef<'_>,
+        cpu: cpu::Features,
+    ) -> Scalar<R> {
+        let a = self.to_mont(a.borrow(), cpu);
         (self.scalar_inv_to_mont)(a, cpu)
     }
 }
@@ -502,8 +505,33 @@ impl Modulus<N> {
             encoding: PhantomData,
         }
     }
+
+    pub fn nonzero_scalar_leak_nonzero<'s, E: Encoding>(
+        &self,
+        scalar: &'s Scalar<E>,
+    ) -> Option<NonZeroScalarRef<'s, E>> {
+        if limbs_are_zero(&scalar.limbs[..self.num_limbs.into()]).leak() {
+            return cold_none();
+        }
+        Some(NonZeroScalarRef(scalar))
+    }
 }
 
+impl<E: Encoding> Borrow<Scalar<E>> for NonZeroScalarRef<'_, E> {
+    fn borrow(&self) -> &Scalar<E> {
+        self.0
+    }
+}
+
+#[cold]
+#[inline(never)]
+fn cold_none<T>() -> Option<T> {
+    None
+}
+
+#[derive(Clone, Copy)]
+pub struct NonZeroScalarRef<'a, E: Encoding = Unencoded>(&'a Scalar<E>);
+
 // Returns (`a` squared `squarings` times) * `b`.
 fn elem_sqr_mul(
     ops: &CommonOps,
@@ -554,14 +582,13 @@ pub(super) fn scalar_parse_big_endian_fixed_consttime(
 #[inline]
 pub(super) fn scalar_parse_big_endian_variable(
     n: &Modulus<N>,
-    allow_zero: AllowZero,
     bytes: untrusted::Input,
 ) -> Result<Scalar, error::Unspecified> {
     let num_limbs = n.num_limbs.into();
     let mut r = Scalar::zero();
     parse_big_endian_in_range_and_pad_consttime(
         bytes,
-        allow_zero,
+        AllowZero::Yes,
         &n.limbs[..num_limbs],
         &mut r.limbs[..num_limbs],
     )?;
@@ -616,12 +643,6 @@ mod tests {
     use crate::testutil as test;
     use alloc::{format, vec, vec::Vec};
 
-    const ZERO_SCALAR: Scalar = Scalar {
-        limbs: [0; elem::NumLimbs::MAX],
-        m: PhantomData,
-        encoding: PhantomData,
-    };
-
     trait Convert<E: Encoding> {
         fn convert(self, q: &Modulus<Q>) -> Elem<E>;
     }
@@ -956,18 +977,6 @@ mod tests {
         })
     }
 
-    #[test]
-    #[should_panic(expected = "!self.scalar_ops.common.is_zero(a)")]
-    fn p256_scalar_inv_to_mont_zero_panic_test() {
-        let _ = p256::PRIVATE_SCALAR_OPS.scalar_inv_to_mont(&ZERO_SCALAR, cpu::features());
-    }
-
-    #[test]
-    #[should_panic(expected = "!self.scalar_ops.common.is_zero(a)")]
-    fn p384_scalar_inv_to_mont_zero_panic_test() {
-        let _ = p384::PRIVATE_SCALAR_OPS.scalar_inv_to_mont(&ZERO_SCALAR, cpu::features());
-    }
-
     #[test]
     fn p256_point_sum_test() {
         point_sum_test(
@@ -1381,7 +1390,7 @@ mod tests {
     fn consume_scalar(n: &Modulus<N>, test_case: &mut test::TestCase, name: &str) -> Scalar {
         let bytes = test_case.consume_bytes(name);
         let bytes = untrusted::Input::from(&bytes);
-        scalar_parse_big_endian_variable(n, AllowZero::Yes, bytes).unwrap()
+        scalar_parse_big_endian_variable(n, bytes).unwrap()
     }
 
     fn consume_scalar_mont(
@@ -1391,7 +1400,7 @@ mod tests {
     ) -> Scalar<R> {
         let bytes = test_case.consume_bytes(name);
         let bytes = untrusted::Input::from(&bytes);
-        let s = scalar_parse_big_endian_variable(n, AllowZero::Yes, bytes).unwrap();
+        let s = scalar_parse_big_endian_variable(n, bytes).unwrap();
         // “Transmute” it to a `Scalar<R>`.
         Scalar {
             limbs: s.limbs,