From ef6efa3ca26c8d6e8c446061e32ad5852f502f13 Mon Sep 17 00:00:00 2001
From: Jack Lloyd <jack@randombit.net>
Date: Sat, 13 Jul 2024 23:01:19 -0400
Subject: [PATCH] Avoid an inversion when checking if x_coord(g*x+h*y) % n == v

Given a projective coordinate, we previously performed an inversion
to extract the affine x. But instead we can project v using the
same value of z as the projective point.

This improves ECDSA verification performance by between 4% and 9%,
depending on the curve.
---
 src/lib/pubkey/ec_group/curve_gfp.h       |  1 +
 src/lib/pubkey/ec_group/ec_inner_bn.cpp   | 55 ++++++++++++++++++++++-
 src/lib/pubkey/ec_group/ec_inner_data.cpp |  1 +
 src/lib/pubkey/ec_group/ec_inner_data.h   |  3 ++
 4 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/src/lib/pubkey/ec_group/curve_gfp.h b/src/lib/pubkey/ec_group/curve_gfp.h
index 3eadee0526e..b47a797a56f 100644
--- a/src/lib/pubkey/ec_group/curve_gfp.h
+++ b/src/lib/pubkey/ec_group/curve_gfp.h
@@ -114,6 +114,7 @@ class BOTAN_UNSTABLE_API CurveGFp final {
       friend class EC_Group_Data;
       friend class EC_Point_Base_Point_Precompute;
       friend class EC_Point_Var_Point_Precompute;
+      friend class EC_Mul2Table_Data_BN;
 
       /**
       * Create an uninitialized CurveGFp
diff --git a/src/lib/pubkey/ec_group/ec_inner_bn.cpp b/src/lib/pubkey/ec_group/ec_inner_bn.cpp
index fd99aa6fdeb..271fe93ae51 100644
--- a/src/lib/pubkey/ec_group/ec_inner_bn.cpp
+++ b/src/lib/pubkey/ec_group/ec_inner_bn.cpp
@@ -171,12 +171,63 @@ bool EC_Mul2Table_Data_BN::mul2_vartime_x_mod_order_eq(const EC_Scalar_Data& v,
    const auto& bn_v = EC_Scalar_Data_BN::checked_ref(v);
    const auto& bn_x = EC_Scalar_Data_BN::checked_ref(x);
    const auto& bn_y = EC_Scalar_Data_BN::checked_ref(y);
-   auto pt = m_tbl.multi_exp(bn_x.value(), bn_y.value());
+   const auto pt = m_tbl.multi_exp(bn_x.value(), bn_y.value());
 
    if(pt.is_zero()) {
       return false;
    }
-   return m_group->mod_order(pt.get_affine_x()) == bn_v.value();
+
+   /*
+   * Note we're working with the projective coordinate directly here!
+   * Nominally we're doing this:
+   *
+   * return m_group->mod_order(pt.get_affine_x()) == bn_v.value();
+   *
+   * However by instead projecting r to an identical z as the x
+   * coordinate, we can compare without having to perform an
+   * expensive inversion in the field.
+   *
+   * That is, given (x*z2) and r, instead of checking if
+   *    (x*z2)*z2^-1 == r,
+   * we check if
+   *    (x*z2) == (r*z2)
+   */
+   auto& curve = m_group->curve();
+
+   secure_vector<word> ws;
+   BigInt vr = bn_v.value();
+   curve.to_rep(vr, ws);
+   BigInt z2, v_z2;
+   curve.sqr(z2, pt.get_z(), ws);
+   curve.mul(v_z2, vr, z2, ws);
+
+   /*
+   * Since (typically) the group order is slightly less than the size
+   * of the field elements, its possible the signer had to reduce the
+   * r component. If they did not reduce r, then this value is correct.
+   *
+   * Due to the Hasse bound, this case occurs almost always; the
+   * probability that a reduction was actually required is
+   * approximately 1 in 2^(n/2) where n is the bit length of the curve.
+   */
+   if(pt.get_x() == v_z2) {
+      return true;
+   }
+
+   if(m_group->order_is_less_than_p()) {
+      vr = bn_v.value() + m_group->order();
+      if(vr < m_group->p()) {
+         curve.to_rep(vr, ws);
+         curve.mul(v_z2, vr, z2, ws);
+
+         if(pt.get_x() == v_z2) {
+            return true;
+         }
+      }
+   }
+
+   // Reject:
+   return false;
 }
 
 }  // namespace Botan
diff --git a/src/lib/pubkey/ec_group/ec_inner_data.cpp b/src/lib/pubkey/ec_group/ec_inner_data.cpp
index 8c7c87b3078..e13d15ddde9 100644
--- a/src/lib/pubkey/ec_group/ec_inner_data.cpp
+++ b/src/lib/pubkey/ec_group/ec_inner_data.cpp
@@ -39,6 +39,7 @@ EC_Group_Data::EC_Group_Data(const BigInt& p,
       m_a_is_minus_3(a == p - 3),
       m_a_is_zero(a.is_zero()),
       m_has_cofactor(m_cofactor != 1),
+      m_order_is_less_than_p(m_order < p),
       m_source(source) {
    if(!m_oid.empty()) {
       DER_Encoder der(m_der_named_curve);
diff --git a/src/lib/pubkey/ec_group/ec_inner_data.h b/src/lib/pubkey/ec_group/ec_inner_data.h
index a53cfb9c8a4..c80ebafd5ba 100644
--- a/src/lib/pubkey/ec_group/ec_inner_data.h
+++ b/src/lib/pubkey/ec_group/ec_inner_data.h
@@ -139,6 +139,8 @@ class EC_Group_Data final : public std::enable_shared_from_this<EC_Group_Data> {
 
       const BigInt& cofactor() const { return m_cofactor; }
 
+      bool order_is_less_than_p() const { return m_order_is_less_than_p; }
+
       bool has_cofactor() const { return m_has_cofactor; }
 
       const BigInt& g_x() const { return m_g_x; }
@@ -231,6 +233,7 @@ class EC_Group_Data final : public std::enable_shared_from_this<EC_Group_Data> {
       bool m_a_is_minus_3;
       bool m_a_is_zero;
       bool m_has_cofactor;
+      bool m_order_is_less_than_p;
       EC_Group_Source m_source;
 };