diff --git a/src/ec/suite_b/ecdsa/digest_scalar.rs b/src/ec/suite_b/ecdsa/digest_scalar.rs index f35dc66ce4..439131d46c 100644 --- a/src/ec/suite_b/ecdsa/digest_scalar.rs +++ b/src/ec/suite_b/ecdsa/digest_scalar.rs @@ -69,7 +69,7 @@ fn digest_scalar_(n: &Modulus, 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!( diff --git a/src/ec/suite_b/ecdsa/signing.rs b/src/ec/suite_b/ecdsa/signing.rs index 3a492c4459..f931a4a9aa 100644 --- a/src/ec/suite_b/ecdsa/signing.rs +++ b/src/ec/suite_b/ecdsa/signing.rs @@ -26,6 +26,8 @@ use crate::{ io::der, limb, pkcs8, rand, signature, }; +use core::borrow::Borrow; + /// An ECDSA signing algorithm. pub struct EcdsaSigningAlgorithm { curve: &'static ec::Curve, @@ -33,7 +35,12 @@ pub struct EcdsaSigningAlgorithm { 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,13 +249,17 @@ 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 = { @@ -256,9 +267,10 @@ impl EcdsaKeyPair { 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. diff --git a/src/ec/suite_b/ecdsa/verification.rs b/src/ec/suite_b/ecdsa/verification.rs index 16b65df8dc..b0782cdce4 100644 --- a/src/ec/suite_b/ecdsa/verification.rs +++ b/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(()); } diff --git a/src/ec/suite_b/ops.rs b/src/ec/suite_b/ops.rs index 7c2ed7f208..7c58a14f8c 100644 --- a/src/ec/suite_b/ops.rs +++ b/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, Elem), cpu: cpu::Features, ) -> Point, - scalar_inv_to_mont_vartime: fn(s: &Scalar, cpu: cpu::Features) -> Scalar, + scalar_inv_to_mont_vartime: fn(s: NonZeroScalarRef<'_>, cpu: cpu::Features) -> Scalar, pub(super) q_minus_n: PublicElem, } @@ -446,7 +446,7 @@ impl Modulus { impl PublicScalarOps { pub(super) fn scalar_inv_to_mont_vartime( &self, - s: &Scalar, + s: NonZeroScalarRef<'_>, cpu: cpu::Features, ) -> Scalar { (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 { - 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 { + let a = self.to_mont(a.borrow(), cpu); (self.scalar_inv_to_mont)(a, cpu) } } @@ -502,8 +505,33 @@ impl Modulus { encoding: PhantomData, } } + + pub fn nonzero_scalar_leak_nonzero<'s, E: Encoding>( + &self, + scalar: &'s Scalar, + ) -> Option> { + if limbs_are_zero(&scalar.limbs[..self.num_limbs.into()]).leak() { + return cold_none(); + } + Some(NonZeroScalarRef(scalar)) + } } +impl Borrow> for NonZeroScalarRef<'_, E> { + fn borrow(&self) -> &Scalar { + self.0 + } +} + +#[cold] +#[inline(never)] +fn cold_none() -> Option { + None +} + +#[derive(Clone, Copy)] +pub struct NonZeroScalarRef<'a, E: Encoding = Unencoded>(&'a Scalar); + // 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, - allow_zero: AllowZero, bytes: untrusted::Input, ) -> Result { 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 { fn convert(self, q: &Modulus) -> Elem; } @@ -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, 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 { 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`. Scalar { limbs: s.limbs,