From a318884f78c5622f958105f6b7ce2a3dda3bac4c Mon Sep 17 00:00:00 2001 From: jsing Date: Wed, 8 Mar 2023 05:35:51 +0000 Subject: [PATCH] Stop trying to use EC_GFp_nist_method(). Currently, if compiled without OPENSSL_BN_ASM_MONT, EC_GROUP_new_curve_GFp() tries to use EC_GFp_nist_method(), falling back to EC_GFp_mont_method() if it is not a NIST curve (if OPENSSL_BN_ASM_MONT is defined we use EC_GFp_mont_method() unconditionally). Now that we have a reasonable non-assembly Montgomery implementation, the performance of EC_GFp_nist_method() is either similar or slower than EC_GFp_mont_method() (the exception being P-521, however if you're using that you're not doing it for performance reasons anyway). The EC_GFp_nist_method() uses rather scary BN NIST code (which would probably already be removed, if not for the BN and EC public APIs), it uses code paths that are currently less constant time, and there is additional overhead in checking to see if the curve is actually supported. Stop trying to use EC_GFp_nist_method() and unconditionally use EC_GFp_mont_method() in all cases. While here, factor out the common setup code and call it from both EC_GROUP_new_curve_GFp() and EC_GROUP_new_curve_GF2m(). ok beck@ tb@ --- lib/libcrypto/ec/ec_cvt.c | 96 ++++++++------------------------------- 1 file changed, 20 insertions(+), 76 deletions(-) diff --git a/lib/libcrypto/ec/ec_cvt.c b/lib/libcrypto/ec/ec_cvt.c index 365ca1aa7e3..fff9ab99cbd 100644 --- a/lib/libcrypto/ec/ec_cvt.c +++ b/lib/libcrypto/ec/ec_cvt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ec_cvt.c,v 1.8 2022/11/26 16:08:52 tb Exp $ */ +/* $OpenBSD: ec_cvt.c,v 1.9 2023/03/08 05:35:51 jsing Exp $ */ /* * Originally written by Bodo Moeller for the OpenSSL project. */ @@ -74,74 +74,31 @@ #include #include "ec_local.h" -EC_GROUP * -EC_GROUP_new_curve_GFp(const BIGNUM *p, const BIGNUM *a, const BIGNUM *b, - BN_CTX *ctx) +static EC_GROUP * +ec_group_new_curve(const EC_METHOD *method, const BIGNUM *p, const BIGNUM *a, + const BIGNUM *b, BN_CTX *ctx) { - const EC_METHOD *meth; - EC_GROUP *ret; - -#if defined(OPENSSL_BN_ASM_MONT) - /* - * This might appear controversial, but the fact is that generic - * prime method was observed to deliver better performance even - * for NIST primes on a range of platforms, e.g.: 60%-15% - * improvement on IA-64, ~25% on ARM, 30%-90% on P4, 20%-25% - * in 32-bit build and 35%--12% in 64-bit build on Core2... - * Coefficients are relative to optimized bn_nist.c for most - * intensive ECDSA verify and ECDH operations for 192- and 521- - * bit keys respectively. Choice of these boundary values is - * arguable, because the dependency of improvement coefficient - * from key length is not a "monotone" curve. For example while - * 571-bit result is 23% on ARM, 384-bit one is -1%. But it's - * generally faster, sometimes "respectfully" faster, sometimes - * "tolerably" slower... What effectively happens is that loop - * with bn_mul_add_words is put against bn_mul_mont, and the - * latter "wins" on short vectors. Correct solution should be - * implementing dedicated NxN multiplication subroutines for - * small N. But till it materializes, let's stick to generic - * prime method... - * - */ - meth = EC_GFp_mont_method(); -#else - meth = EC_GFp_nist_method(); -#endif - - ret = EC_GROUP_new(meth); - if (ret == NULL) - return NULL; - - if (!EC_GROUP_set_curve(ret, p, a, b, ctx)) { - unsigned long err; - - err = ERR_peek_last_error(); + EC_GROUP *group; - if (!(ERR_GET_LIB(err) == ERR_LIB_EC && - ((ERR_GET_REASON(err) == EC_R_NOT_A_NIST_PRIME) || - (ERR_GET_REASON(err) == EC_R_NOT_A_SUPPORTED_NIST_PRIME)))) { - /* real error */ + if ((group = EC_GROUP_new(method)) == NULL) + goto err; - EC_GROUP_clear_free(ret); - return NULL; - } - /* not an actual error, we just cannot use EC_GFp_nist_method */ + if (!EC_GROUP_set_curve(group, p, a, b, ctx)) + goto err; - ERR_clear_error(); + return group; - EC_GROUP_clear_free(ret); - meth = EC_GFp_mont_method(); + err: + EC_GROUP_clear_free(group); - ret = EC_GROUP_new(meth); - if (ret == NULL) - return NULL; + return NULL; +} - if (!EC_GROUP_set_curve(ret, p, a, b, ctx)) { - EC_GROUP_clear_free(ret); - return NULL; - } - } - return ret; +EC_GROUP * +EC_GROUP_new_curve_GFp(const BIGNUM *p, const BIGNUM *a, const BIGNUM *b, + BN_CTX *ctx) +{ + return ec_group_new_curve(EC_GFp_mont_method(), p, a, b, ctx); } #ifndef OPENSSL_NO_EC2M @@ -149,19 +106,6 @@ EC_GROUP * EC_GROUP_new_curve_GF2m(const BIGNUM *p, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) { - const EC_METHOD *meth; - EC_GROUP *ret; - - meth = EC_GF2m_simple_method(); - - ret = EC_GROUP_new(meth); - if (ret == NULL) - return NULL; - - if (!EC_GROUP_set_curve(ret, p, a, b, ctx)) { - EC_GROUP_clear_free(ret); - return NULL; - } - return ret; + return ec_group_new_curve(EC_GF2m_simple_method(), p, a, b, ctx); } #endif -- 2.20.1