From ef7c6cd9cca6cd0d05204b8f4ea67e2e2b658baf Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 4 May 2026 16:23:12 -0700 Subject: [PATCH 1/3] ec: Remove `{elem,scalar}_parse_big_endian_fixed_consttime`. Instead of having these wrappers that implicitly allow or disallow zero, just have each call site indicate which behavior it wants. --- src/ec/suite_b/ops/mod.rs | 28 +++++++--------------------- src/ec/suite_b/private_key.rs | 9 +++++++-- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/ec/suite_b/ops/mod.rs b/src/ec/suite_b/ops/mod.rs index 67c22c8f58..a40bd95dc4 100644 --- a/src/ec/suite_b/ops/mod.rs +++ b/src/ec/suite_b/ops/mod.rs @@ -351,7 +351,7 @@ impl PublicKeyOps { ) -> Result, error::Unspecified> { let _cpu = cpu::features(); let encoded_value = input.read_bytes(self.common.len())?; - let parsed = elem_parse_big_endian_fixed_consttime(q, encoded_value)?; + let parsed = parse_big_endian_fixed_consttime(q, encoded_value, AllowZero::Yes)?; let mut r = Elem::zero(); let rr = Elem::from(&self.common.q.rr); // Montgomery encode (elem_to_mont). @@ -534,22 +534,6 @@ fn elem_sqr_mul_acc( ops.elem_mul(acc, b, cpu) } -#[inline] -pub(super) fn elem_parse_big_endian_fixed_consttime( - q: &Modulus, - bytes: untrusted::Input, -) -> Result, error::Unspecified> { - parse_big_endian_fixed_consttime(q, bytes, AllowZero::Yes) -} - -#[inline] -pub(super) fn scalar_parse_big_endian_fixed_consttime( - n: &Modulus, - bytes: untrusted::Input, -) -> Result { - parse_big_endian_fixed_consttime(n, bytes, AllowZero::No) -} - #[inline] pub(super) fn scalar_parse_big_endian_variable( n: &Modulus, @@ -583,7 +567,7 @@ pub(super) fn scalar_parse_big_endian_partially_reduced_variable_consttime( Ok(r) } -fn parse_big_endian_fixed_consttime( +pub(super) fn parse_big_endian_fixed_consttime( m: &Modulus, bytes: untrusted::Input, allow_zero: AllowZero, @@ -1298,7 +1282,8 @@ mod tests { let num_limbs = q.num_limbs.into(); let bytes = test::from_hex(elems[i]).unwrap(); let bytes = untrusted::Input::from(&bytes); - let r: Elem = elem_parse_big_endian_fixed_consttime(q, bytes).unwrap(); + let r: Elem = + parse_big_endian_fixed_consttime(q, bytes, AllowZero::Yes).unwrap(); // XXX: “Transmute” this to `Elem` limbs. limbs_out[(i * num_limbs)..((i + 1) * num_limbs)].copy_from_slice(&r.limbs[..num_limbs]); } @@ -1318,7 +1303,7 @@ mod tests { let bytes = test::from_hex(elems[i]).unwrap(); let bytes = untrusted::Input::from(&bytes); let unencoded: Elem = - elem_parse_big_endian_fixed_consttime(q, bytes).unwrap(); + parse_big_endian_fixed_consttime(q, bytes, AllowZero::Yes).unwrap(); // XXX: “Transmute” this to `Elem` limbs. Elem { limbs: unencoded.limbs, @@ -1371,7 +1356,8 @@ mod tests { bytes.extend(&unpadded_bytes); let bytes = untrusted::Input::from(&bytes); - let r: Elem = elem_parse_big_endian_fixed_consttime(q, bytes).unwrap(); + let r: Elem = + parse_big_endian_fixed_consttime(q, bytes, AllowZero::Yes).unwrap(); // XXX: “Transmute” this to an `Elem`. Elem { limbs: r.limbs, diff --git a/src/ec/suite_b/private_key.rs b/src/ec/suite_b/private_key.rs index 19129296c3..70807192d4 100644 --- a/src/ec/suite_b/private_key.rs +++ b/src/ec/suite_b/private_key.rs @@ -16,7 +16,12 @@ //! ECDSA signing). use super::{ops::*, verify_affine_point_is_on_the_curve}; -use crate::{arithmetic::montgomery::R, cpu, ec, error, limb, rand}; +use crate::{ + arithmetic::montgomery::R, + cpu, ec, error, + limb::{self, AllowZero}, + rand, +}; /// Generates a random scalar in the range [1, n). pub(super) fn random_scalar( @@ -125,7 +130,7 @@ pub(super) fn scalar_from_big_endian_bytes( // way, we avoid needing to compute or store the value (n - 1), we avoid the // need to implement a function to add one to a scalar, and we avoid needing // to convert the scalar back into an array of bytes. - scalar_parse_big_endian_fixed_consttime(n, untrusted::Input::from(bytes)) + parse_big_endian_fixed_consttime(n, untrusted::Input::from(bytes), AllowZero::No) } pub(super) fn public_from_private( From 3a68e03a075fe1e002ebffc4e92c4ed0fd97fb8b Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 4 May 2026 16:33:14 -0700 Subject: [PATCH 2/3] ec: DRY `consume_scalar`/`consume_scalar_mont`. --- src/ec/suite_b/ops/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ec/suite_b/ops/mod.rs b/src/ec/suite_b/ops/mod.rs index a40bd95dc4..5d8ad17c81 100644 --- a/src/ec/suite_b/ops/mod.rs +++ b/src/ec/suite_b/ops/mod.rs @@ -1377,9 +1377,7 @@ mod tests { test_case: &mut test::TestCase, name: &str, ) -> 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 = consume_scalar(n, test_case, name); // “Transmute” it to a `Scalar`. Scalar { limbs: s.limbs, From 90da625a75adff0fc12ea7ff19695cb51bb1e010 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 4 May 2026 16:39:13 -0700 Subject: [PATCH 3/3] ec testing: Clarify `consume_elem`. Most tests aren't actually operating on Montgomery-encoded elements. Separate `consume_elem` and `consume_elem_mont` to make this clearer. No functional change. --- src/ec/suite_b/ops/mod.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ec/suite_b/ops/mod.rs b/src/ec/suite_b/ops/mod.rs index 5d8ad17c81..31120c4449 100644 --- a/src/ec/suite_b/ops/mod.rs +++ b/src/ec/suite_b/ops/mod.rs @@ -847,9 +847,9 @@ mod tests { test::run(test_file, |section, test_case| { assert_eq!(section, ""); - let mut a = consume_elem(q, test_case, "a"); - let b = consume_elem(q, test_case, "b"); - let r = consume_elem(q, test_case, "r"); + let mut a = consume_elem_mont(q, test_case, "a"); + let b = consume_elem_mont(q, test_case, "b"); + let r = consume_elem_mont(q, test_case, "r"); q.elem_mul(&mut a, &b); assert_limbs_are_equal(ops, &a.limbs, &r.limbs); @@ -1350,14 +1350,17 @@ mod tests { } } - fn consume_elem(q: &Modulus, test_case: &mut test::TestCase, name: &str) -> Elem { + fn consume_elem(q: &Modulus, test_case: &mut test::TestCase, name: &str) -> Elem { let unpadded_bytes = test_case.consume_bytes(name); let mut bytes = vec![0; q.bytes_len() - unpadded_bytes.len()]; bytes.extend(&unpadded_bytes); let bytes = untrusted::Input::from(&bytes); - let r: Elem = - parse_big_endian_fixed_consttime(q, bytes, AllowZero::Yes).unwrap(); + parse_big_endian_fixed_consttime(q, bytes, AllowZero::Yes).unwrap() + } + + fn consume_elem_mont(q: &Modulus, test_case: &mut test::TestCase, name: &str) -> Elem { + let r = consume_elem(q, test_case, name); // XXX: “Transmute” this to an `Elem`. Elem { limbs: r.limbs,