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_group.cpp b/src/lib/pubkey/ec_group/ec_group.cpp index e7a7cbc265e..54ddc9bd4b1 100644 --- a/src/lib/pubkey/ec_group/ec_group.cpp +++ b/src/lib/pubkey/ec_group/ec_group.cpp @@ -761,19 +761,17 @@ std::optional EC_Group::Mul2Table::mul2_vartime(const EC_Scalar& } } -std::optional EC_Group::Mul2Table::mul2_vartime_x_mod_order(const EC_Scalar& x, const EC_Scalar& y) const { - auto s = m_tbl->mul2_vartime_x_mod_order(x._inner(), y._inner()); - if(s) { - return EC_Scalar::_from_inner(std::move(s)); - } else { - return {}; - } +bool EC_Group::Mul2Table::mul2_vartime_x_mod_order_eq(const EC_Scalar& v, + const EC_Scalar& x, + const EC_Scalar& y) const { + return m_tbl->mul2_vartime_x_mod_order_eq(v._inner(), x._inner(), y._inner()); } -std::optional EC_Group::Mul2Table::mul2_vartime_x_mod_order(const EC_Scalar& c, - const EC_Scalar& x, - const EC_Scalar& y) const { - return this->mul2_vartime_x_mod_order(c * x, c * y); +bool EC_Group::Mul2Table::mul2_vartime_x_mod_order_eq(const EC_Scalar& v, + const EC_Scalar& c, + const EC_Scalar& x, + const EC_Scalar& y) const { + return this->mul2_vartime_x_mod_order_eq(v, c * x, c * y); } } // namespace Botan diff --git a/src/lib/pubkey/ec_group/ec_group.h b/src/lib/pubkey/ec_group/ec_group.h index a5dc7b1888f..46ade067628 100644 --- a/src/lib/pubkey/ec_group/ec_group.h +++ b/src/lib/pubkey/ec_group/ec_group.h @@ -249,28 +249,29 @@ class BOTAN_PUBLIC_API(2, 0) EC_Group final { std::optional mul2_vartime(const EC_Scalar& x, const EC_Scalar& y) const; /** - * Return the x coordinate of g*x + h*y, reduced modulo the order + * Check if v equals the x coordinate of g*x + h*y reduced modulo the order * * Where g is the group generator and h is the value passed to the constructor * - * Returns nullopt if g*x + h*y was the point at infinity + * Returns false if unequal, including if g*x + h*y was the point at infinity * * @warning this function is variable time with respect to x and y */ - std::optional mul2_vartime_x_mod_order(const EC_Scalar& x, const EC_Scalar& y) const; + bool mul2_vartime_x_mod_order_eq(const EC_Scalar& v, const EC_Scalar& x, const EC_Scalar& y) const; /** - * Return the x coordinate of g*x*c + h*y*c, reduced modulo the order + * Check if v equals the x coordinate of g*x*c + h*y*c reduced modulo the order * * Where g is the group generator and h is the value passed to the constructor * - * Returns nullopt if g*x*c + h*y*c was the point at infinity + * Returns false if unequal, including if g*x*c + h*y*c was the point at infinity * - * @warning this function is variable time with respect to c, x and y + * @warning this function is variable time with respect to x and y */ - std::optional mul2_vartime_x_mod_order(const EC_Scalar& c, - const EC_Scalar& x, - const EC_Scalar& y) const; + bool mul2_vartime_x_mod_order_eq(const EC_Scalar& v, + const EC_Scalar& c, + const EC_Scalar& x, + const EC_Scalar& y) const; ~Mul2Table(); diff --git a/src/lib/pubkey/ec_group/ec_inner_bn.cpp b/src/lib/pubkey/ec_group/ec_inner_bn.cpp index 710fec32063..271fe93ae51 100644 --- a/src/lib/pubkey/ec_group/ec_inner_bn.cpp +++ b/src/lib/pubkey/ec_group/ec_inner_bn.cpp @@ -163,18 +163,71 @@ std::unique_ptr EC_Mul2Table_Data_BN::mul2_vartime(const EC return std::make_unique(m_group, std::move(pt)); } -std::unique_ptr EC_Mul2Table_Data_BN::mul2_vartime_x_mod_order(const EC_Scalar_Data& x, - const EC_Scalar_Data& y) const { - BOTAN_ARG_CHECK(x.group() == m_group && y.group() == m_group, "Curve mismatch"); +bool EC_Mul2Table_Data_BN::mul2_vartime_x_mod_order_eq(const EC_Scalar_Data& v, + const EC_Scalar_Data& x, + const EC_Scalar_Data& y) const { + BOTAN_ARG_CHECK(x.group() == m_group && y.group() == m_group && v.group() == m_group, "Curve mismatch"); + 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 nullptr; + return false; + } + + /* + * 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 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; } - return std::make_unique(m_group, m_group->mod_order(pt.get_affine_x())); + + 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_bn.h b/src/lib/pubkey/ec_group/ec_inner_bn.h index a130ccf263a..b10227e11e4 100644 --- a/src/lib/pubkey/ec_group/ec_inner_bn.h +++ b/src/lib/pubkey/ec_group/ec_inner_bn.h @@ -92,8 +92,9 @@ class EC_Mul2Table_Data_BN final : public EC_Mul2Table_Data { std::unique_ptr mul2_vartime(const EC_Scalar_Data& x, const EC_Scalar_Data& y) const override; - std::unique_ptr mul2_vartime_x_mod_order(const EC_Scalar_Data& x, - const EC_Scalar_Data& y) const override; + bool mul2_vartime_x_mod_order_eq(const EC_Scalar_Data& v, + const EC_Scalar_Data& x, + const EC_Scalar_Data& y) const override; private: std::shared_ptr m_group; 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 3d4f1b2c68b..c80ebafd5ba 100644 --- a/src/lib/pubkey/ec_group/ec_inner_data.h +++ b/src/lib/pubkey/ec_group/ec_inner_data.h @@ -93,9 +93,12 @@ class EC_Mul2Table_Data { virtual std::unique_ptr mul2_vartime(const EC_Scalar_Data& x, const EC_Scalar_Data& y) const = 0; - // Returns nullptr if g*x + h*y was point at infinity - virtual std::unique_ptr mul2_vartime_x_mod_order(const EC_Scalar_Data& x, - const EC_Scalar_Data& y) const = 0; + // Check if v == (g*x + h*y).x % n + // + // Returns false if g*x + h*y was point at infinity + virtual bool mul2_vartime_x_mod_order_eq(const EC_Scalar_Data& v, + const EC_Scalar_Data& x, + const EC_Scalar_Data& y) const = 0; }; class EC_Group_Data final : public std::enable_shared_from_this { @@ -136,6 +139,8 @@ class EC_Group_Data final : public std::enable_shared_from_this { 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; } @@ -228,6 +233,7 @@ class EC_Group_Data final : public std::enable_shared_from_this { 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; }; diff --git a/src/lib/pubkey/ecdsa/ecdsa.cpp b/src/lib/pubkey/ecdsa/ecdsa.cpp index 972a7c98adb..4717703a459 100644 --- a/src/lib/pubkey/ecdsa/ecdsa.cpp +++ b/src/lib/pubkey/ecdsa/ecdsa.cpp @@ -219,9 +219,8 @@ bool ECDSA_Verification_Operation::verify(const uint8_t msg[], size_t msg_len, c const auto w = s.invert(); - if(const auto v = m_gy_mul.mul2_vartime_x_mod_order(w, m, r)) { - return (v == r); - } + // Check if r == x_coord(g*w*m + y*w*r) % n + return m_gy_mul.mul2_vartime_x_mod_order_eq(r, w, m, r); } } diff --git a/src/lib/pubkey/ecgdsa/ecgdsa.cpp b/src/lib/pubkey/ecgdsa/ecgdsa.cpp index 502e4d30b23..a89725290b3 100644 --- a/src/lib/pubkey/ecgdsa/ecgdsa.cpp +++ b/src/lib/pubkey/ecgdsa/ecgdsa.cpp @@ -109,9 +109,8 @@ bool ECGDSA_Verification_Operation::verify(const uint8_t msg[], size_t msg_len, const auto w = r.invert(); - if(const auto v = m_gy_mul.mul2_vartime_x_mod_order(w, m, s)) { - return (v == r); - } + // Check if r == x_coord(g*w*m + y*w*s) % n + return m_gy_mul.mul2_vartime_x_mod_order_eq(r, w, m, s); } } diff --git a/src/lib/pubkey/gost_3410/gost_3410.cpp b/src/lib/pubkey/gost_3410/gost_3410.cpp index f8740fc4cce..a4bb18ef079 100644 --- a/src/lib/pubkey/gost_3410/gost_3410.cpp +++ b/src/lib/pubkey/gost_3410/gost_3410.cpp @@ -229,9 +229,8 @@ bool GOST_3410_Verification_Operation::verify(const uint8_t msg[], const auto v = e.invert(); - if(const auto w = m_gy_mul.mul2_vartime_x_mod_order(v, s, r.negate())) { - return (w == r); - } + // Check if r == x_coord(g*v*s - y*v*r) % n + return m_gy_mul.mul2_vartime_x_mod_order_eq(r, v, s, r.negate()); } } diff --git a/src/lib/pubkey/sm2/sm2.cpp b/src/lib/pubkey/sm2/sm2.cpp index d3f04ca8063..7f06d23aa26 100644 --- a/src/lib/pubkey/sm2/sm2.cpp +++ b/src/lib/pubkey/sm2/sm2.cpp @@ -204,9 +204,8 @@ bool SM2_Verification_Operation::is_valid_signature(const uint8_t sig[], size_t if(r.is_nonzero() && s.is_nonzero()) { const auto t = r + s; if(t.is_nonzero()) { - if(const auto v = m_gy_mul.mul2_vartime_x_mod_order(s, t)) { - return (v.value() + e) == r; - } + // Check if r - e = x_coord(g*s + y*t) % n + return m_gy_mul.mul2_vartime_x_mod_order_eq(r - e, s, t); } } }