From 86f49768e8f01551f326d57db8fd76d404945894 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 17 Oct 2023 11:12:20 -0700 Subject: [PATCH] P-256 ECDSA verification: Clarify multiplication. Move more of the logic for the nistz256 multiplication into Rust. --- build.rs | 2 +- crypto/fipsmodule/ec/p256-nistz.c | 11 ++------ src/ec/suite_b/ops.rs | 10 +++++-- src/ec/suite_b/ops/p256.rs | 46 ++++++++++++++++++------------- 4 files changed, 38 insertions(+), 31 deletions(-) diff --git a/build.rs b/build.rs index a1e1f41d81..3cbae9d926 100644 --- a/build.rs +++ b/build.rs @@ -958,7 +958,7 @@ fn prefix_all_symbols(pp: char, prefix_prefix: &str, prefix: &str) -> String { "p256_point_double", "p256_point_mul", "p256_point_mul_base", - "p256_points_mul_public", + "p256_point_mul_base_vartime", "p256_scalar_mul_mont", "p256_scalar_sqr_rep_mont", "p256_sqr_mont", diff --git a/crypto/fipsmodule/ec/p256-nistz.c b/crypto/fipsmodule/ec/p256-nistz.c index f0fc61424d..c40b1085db 100644 --- a/crypto/fipsmodule/ec/p256-nistz.c +++ b/crypto/fipsmodule/ec/p256-nistz.c @@ -284,11 +284,8 @@ void p256_point_mul_base(P256_POINT *r, const Limb scalar[P256_LIMBS]) { limbs_copy(r->Z, p.Z, P256_LIMBS); } -void p256_points_mul_public(P256_POINT *r, - const Limb g_scalar[P256_LIMBS], - const Limb p_scalar[P256_LIMBS], - const Limb p_x[P256_LIMBS], - const Limb p_y[P256_LIMBS]) { +void p256_point_mul_base_vartime(P256_POINT *r, + const Limb g_scalar[P256_LIMBS]) { alignas(32) P256_POINT p; uint8_t p_str[33]; OPENSSL_memcpy(p_str, g_scalar, 32); @@ -336,10 +333,6 @@ void p256_points_mul_public(P256_POINT *r, ecp_nistz256_point_add_affine(&p, &p, &t); } - alignas(32) P256_POINT tmp; - ecp_nistz256_windowed_mul(&tmp, p_scalar, p_x, p_y); - ecp_nistz256_point_add(&p, &p, &tmp); - OPENSSL_memcpy(r->X, p.X, P256_LIMBS * sizeof(BN_ULONG)); OPENSSL_memcpy(r->Y, p.Y, P256_LIMBS * sizeof(BN_ULONG)); OPENSSL_memcpy(r->Z, p.Z, P256_LIMBS * sizeof(BN_ULONG)); diff --git a/src/ec/suite_b/ops.rs b/src/ec/suite_b/ops.rs index 83683a0f73..5aa241390e 100644 --- a/src/ec/suite_b/ops.rs +++ b/src/ec/suite_b/ops.rs @@ -979,6 +979,7 @@ mod tests { fn p256_point_mul_base_test() { point_mul_base_tests( &p256::PRIVATE_KEY_OPS, + |s| p256::PRIVATE_KEY_OPS.point_mul_base(s), test_file!("ops/p256_point_mul_base_tests.txt"), ); } @@ -987,16 +988,21 @@ mod tests { fn p384_point_mul_base_test() { point_mul_base_tests( &p384::PRIVATE_KEY_OPS, + |s| p384::PRIVATE_KEY_OPS.point_mul_base(s), test_file!("ops/p384_point_mul_base_tests.txt"), ); } - fn point_mul_base_tests(ops: &PrivateKeyOps, test_file: test::File) { + pub(super) fn point_mul_base_tests( + ops: &PrivateKeyOps, + f: impl Fn(&Scalar) -> Point, + test_file: test::File, + ) { test::run(test_file, |section, test_case| { assert_eq!(section, ""); let g_scalar = consume_scalar(ops.common, test_case, "g_scalar"); let expected_result = consume_point(ops, test_case, "r"); - let actual_result = ops.point_mul_base(&g_scalar); + let actual_result = f(&g_scalar); assert_point_actual_equals_expected(ops, &actual_result, &expected_result); Ok(()) }) diff --git a/src/ec/suite_b/ops/p256.rs b/src/ec/suite_b/ops/p256.rs index 566dbfe2be..adbed60936 100644 --- a/src/ec/suite_b/ops/p256.rs +++ b/src/ec/suite_b/ops/p256.rs @@ -127,30 +127,24 @@ pub static PUBLIC_SCALAR_OPS: PublicScalarOps = PublicScalarOps { }; #[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] -fn twin_mul_nistz256( - g_scalar: &Scalar, - p_scalar: &Scalar, - (p_x, p_y): &(Elem, Elem), -) -> Point { +fn twin_mul_nistz256(g_scalar: &Scalar, p_scalar: &Scalar, p_xy: &(Elem, Elem)) -> Point { + let scaled_g = point_mul_base_vartime(g_scalar); + let scaled_p = PRIVATE_KEY_OPS.point_mul(p_scalar, p_xy); + PRIVATE_KEY_OPS.common.point_sum(&scaled_g, &scaled_p) +} + +#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] +fn point_mul_base_vartime(g_scalar: &Scalar) -> Point { prefixed_extern! { - fn p256_points_mul_public(r: *mut Limb, // [3][COMMON_OPS.num_limbs] - g_scalar: *const Limb, // [COMMON_OPS.num_limbs] - p_scalar: *const Limb, // [COMMON_OPS.num_limbs] - p_x: *const Limb, // [COMMON_OPS.num_limbs] - p_y: *const Limb, // [COMMON_OPS.num_limbs] + fn p256_point_mul_base_vartime(r: *mut Limb, // [3][COMMON_OPS.num_limbs] + g_scalar: *const Limb, // [COMMON_OPS.num_limbs] ); } - let mut r = Point::new_at_infinity(); + let mut scaled_g = Point::new_at_infinity(); unsafe { - p256_points_mul_public( - r.xyz.as_mut_ptr(), - g_scalar.limbs.as_ptr(), - p_scalar.limbs.as_ptr(), - p_x.limbs.as_ptr(), - p_y.limbs.as_ptr(), - ); + p256_point_mul_base_vartime(scaled_g.xyz.as_mut_ptr(), g_scalar.limbs.as_ptr()); } - r + scaled_g } pub static PRIVATE_SCALAR_OPS: PrivateScalarOps = PrivateScalarOps { @@ -316,3 +310,17 @@ prefixed_extern! { rep: Limb, ); } + +#[cfg(test)] +mod tests { + #[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] + #[test] + fn p256_point_mul_base_vartime_test() { + use super::{super::tests::point_mul_base_tests, *}; + point_mul_base_tests( + &PRIVATE_KEY_OPS, + point_mul_base_vartime, + test_file!("p256_point_mul_base_tests.txt"), + ); + } +}