ECDSA verification: Refactor a*g + b*p multiplication

build.rs

@@ -958,6 +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_point_mul_base_vartime",
         "p256_scalar_mul_mont",
         "p256_scalar_sqr_rep_mont",
         "p256_sqr_mont",

crypto/fipsmodule/ec/p256-nistz.c

@@ -284,4 +284,59 @@ void p256_point_mul_base(P256_POINT *r, const Limb scalar[P256_LIMBS]) {
   limbs_copy(r->Z, p.Z, P256_LIMBS);
 }
 
+void p256_point_mul_base_vartime(Limb r[3][P256_LIMBS],
+                                 const Limb g_scalar[P256_LIMBS]) {
+  alignas(32) P256_POINT p;
+  uint8_t p_str[33];
+  OPENSSL_memcpy(p_str, g_scalar, 32);
+  p_str[32] = 0;
+
+  // First window
+  size_t index = 0;
+  size_t wvalue = calc_first_wvalue(&index, p_str);
+
+  // Convert |p| from affine to Jacobian coordinates. We set Z to zero if |p|
+  // is infinity and |ONE| otherwise. |p| was computed from the table, so it
+  // is infinity iff |wvalue >> 1| is zero.
+  if ((wvalue >> 1) != 0) {
+    OPENSSL_memcpy(p.X, &ecp_nistz256_precomputed[0][(wvalue >> 1) - 1].X,
+                   sizeof(p.X));
+    OPENSSL_memcpy(p.Y, &ecp_nistz256_precomputed[0][(wvalue >> 1) - 1].Y,
+                   sizeof(p.Y));
+    OPENSSL_memcpy(p.Z, ONE, sizeof(p.Z));
+  } else {
+    OPENSSL_memset(p.X, 0, sizeof(p.X));
+    OPENSSL_memset(p.Y, 0, sizeof(p.Y));
+    OPENSSL_memset(p.Z, 0, sizeof(p.Z));
+  }
+
+  if ((wvalue & 1) == 1) {
+    ecp_nistz256_neg(p.Y, p.Y);
+  }
+
+  for (int i = 1; i < 37; i++) {
+    wvalue = calc_wvalue(&index, p_str);
+    if ((wvalue >> 1) == 0) {
+      continue;
+    }
+
+    alignas(32) P256_POINT_AFFINE t;
+    OPENSSL_memcpy(&t, &ecp_nistz256_precomputed[i][(wvalue >> 1) - 1],
+                   sizeof(t));
+    if ((wvalue & 1) == 1) {
+      ecp_nistz256_neg(t.Y, t.Y);
+    }
+
+    // Note |ecp_nistz256_point_add_affine| does not work if |p| and |t| are
+    // the same non-infinity point, so it is important that we compute the
+    // |g_scalar| term before the |p_scalar| term.
+    ecp_nistz256_point_add_affine(&p, &p, &t);
+  }
+
+
+  limbs_copy(r[0], p.X, P256_LIMBS);
+  limbs_copy(r[1], p.Y, P256_LIMBS);
+  limbs_copy(r[2], p.Z, P256_LIMBS);
+}
+
 #endif /* defined(OPENSSL_USE_NISTZ256) */

src/ec/suite_b/ecdsa/verification.rs

@@ -123,7 +123,7 @@ impl EcdsaVerificationAlgorithm {
         // NSA Guide Step 6: "Compute the elliptic curve point
         // R = (xR, yR) = u1*G + u2*Q, using EC scalar multiplication and EC
         // addition. If R is equal to the point at infinity, output INVALID."
-        let product = twin_mul(self.ops.private_key_ops, &u1, &u2, &peer_pub_key);
+        let product = (self.ops.twin_mul)(&u1, &u2, &peer_pub_key);
 
         // Verify that the point we computed is on the curve; see
         // `verify_affine_point_is_on_the_curve_scaled` for details on why. It
@@ -158,7 +158,7 @@ impl EcdsaVerificationAlgorithm {
         }
         if self.ops.elem_less_than(&r, &self.ops.q_minus_n) {
             self.ops
-                .private_key_ops
+                .scalar_ops
                 .common
                 .elem_add(&mut r, &public_key_ops.common.n);
             if sig_r_equals_x(self.ops, &r, &x, &z2) {
@@ -193,18 +193,6 @@ fn split_rs_asn1<'a>(
     })
 }
 
-fn twin_mul(
-    ops: &PrivateKeyOps,
-    g_scalar: &Scalar,
-    p_scalar: &Scalar,
-    p_xy: &(Elem<R>, Elem<R>),
-) -> Point {
-    // XXX: Inefficient. TODO: implement interleaved wNAF multiplication.
-    let scaled_g = ops.point_mul_base(g_scalar);
-    let scaled_p = ops.point_mul(p_scalar, p_xy);
-    ops.common.point_sum(&scaled_g, &scaled_p)
-}
-
 /// Verification of fixed-length (PKCS#11 style) ECDSA signatures using the
 /// P-256 curve and SHA-256.
 ///

src/ec/suite_b/ops.rs

@@ -270,10 +270,7 @@ pub struct PublicScalarOps {
     pub scalar_ops: &'static ScalarOps,
     pub public_key_ops: &'static PublicKeyOps,
 
-    // XXX: `PublicScalarOps` shouldn't depend on `PrivateKeyOps`, but it does
-    // temporarily until `twin_mul` is rewritten.
-    pub private_key_ops: &'static PrivateKeyOps,
-
+    pub twin_mul: fn(g_scalar: &Scalar, p_scalar: &Scalar, p_xy: &(Elem<R>, Elem<R>)) -> Point,
     pub q_minus_n: Elem<Unencoded>,
 }
 
@@ -305,6 +302,19 @@ pub struct PrivateScalarOps {
     pub oneRR_mod_n: Scalar<RR>, // 1 * R**2 (mod n). TOOD: Use One<RR>.
 }
 
+// XXX: Inefficient and unnecessarily depends on `PrivateKeyOps`. TODO: implement interleaved wNAF
+// multiplication.
+fn twin_mul_inefficient(
+    ops: &PrivateKeyOps,
+    g_scalar: &Scalar,
+    p_scalar: &Scalar,
+    p_xy: &(Elem<R>, Elem<R>),
+) -> Point {
+    let scaled_g = ops.point_mul_base(g_scalar);
+    let scaled_p = ops.point_mul(p_scalar, p_xy);
+    ops.common.point_sum(&scaled_g, &scaled_p)
+}
+
 // This assumes n < q < 2*n.
 pub fn elem_reduced_to_scalar(ops: &CommonOps, elem: &Elem<Unencoded>) -> Scalar<Unencoded> {
     let num_limbs = ops.num_limbs;
@@ -969,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"),
         );
     }
@@ -977,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(())
         })

src/ec/suite_b/ops/p256.rs

@@ -114,10 +114,39 @@ pub static SCALAR_OPS: ScalarOps = ScalarOps {
 pub static PUBLIC_SCALAR_OPS: PublicScalarOps = PublicScalarOps {
     scalar_ops: &SCALAR_OPS,
     public_key_ops: &PUBLIC_KEY_OPS,
-    private_key_ops: &PRIVATE_KEY_OPS,
+
+    #[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))]
+    twin_mul: twin_mul_nistz256,
+
+    #[cfg(not(any(target_arch = "aarch64", target_arch = "x86_64")))]
+    twin_mul: |g_scalar, p_scalar, p_xy| {
+        twin_mul_inefficient(&PRIVATE_KEY_OPS, g_scalar, p_scalar, p_xy)
+    },
+
     q_minus_n: Elem::from_hex("4319055358e8617b0c46353d039cdaae"),
 };
 
+#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))]
+fn twin_mul_nistz256(g_scalar: &Scalar, p_scalar: &Scalar, p_xy: &(Elem<R>, Elem<R>)) -> 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_point_mul_base_vartime(r: *mut Limb,          // [3][COMMON_OPS.num_limbs]
+                                       g_scalar: *const Limb, // [COMMON_OPS.num_limbs]
+        );
+    }
+    let mut scaled_g = Point::new_at_infinity();
+    unsafe {
+        p256_point_mul_base_vartime(scaled_g.xyz.as_mut_ptr(), g_scalar.limbs.as_ptr());
+    }
+    scaled_g
+}
+
 pub static PRIVATE_SCALAR_OPS: PrivateScalarOps = PrivateScalarOps {
     scalar_ops: &SCALAR_OPS,
 
@@ -282,3 +311,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"),
+        );
+    }
+}

src/ec/suite_b/ops/p384.rs

@@ -122,7 +122,9 @@ pub static SCALAR_OPS: ScalarOps = ScalarOps {
 pub static PUBLIC_SCALAR_OPS: PublicScalarOps = PublicScalarOps {
     scalar_ops: &SCALAR_OPS,
     public_key_ops: &PUBLIC_KEY_OPS,
-    private_key_ops: &PRIVATE_KEY_OPS,
+    twin_mul: |g_scalar, p_scalar, p_xy| {
+        twin_mul_inefficient(&PRIVATE_KEY_OPS, g_scalar, p_scalar, p_xy)
+    },
 
     q_minus_n: Elem::from_hex("389cb27e0bc8d21fa7e5f24cb74f58851313e696333ad68c"),
 };