From: Neil Horman Date: Wed, 25 Jun 2025 18:26:24 +0000 (-0400) Subject: Remove need for BN_BLINDING lock X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=902568bbd98a61d03495498b56e95e67e42cc71b;p=thirdparty%2Fopenssl.git Remove need for BN_BLINDING lock Issue https://github.com/openssl/project/issues/1245 has identified that we encounter a significant amount of time waiting to acquire the BN_BLINDING_lock when running our handshake perf test with 10 threads using an rsa key. Specifically, with 10 threads we spend about 19327731 usecs just waiting. So it would be great if we could eliminate the need to get the write lock here. Currently, the need for the lock is based off the fact that each rsa key has only a single blinding pointer, for which exclusive access is needed, with an attempt to use a fallback mt_blinding pointer in the shared case. If a key is shared by many threads, then we find ourselves needing to maniuplate this lock quite frequently if we are doing lots of ssl connections. To address this, I've come up with this approach. It replaces the blinding pointer with a pointer to a sparse array. The sparse array is then indexed by thread id. This allows us to do two things: When getting the blinding, we only need to take the read lock in the common case when looking up this threads blinding structure. Only in the first lookup for any thread do we need to take the write side lock when updating the table, and only then for a very brief critical section (i.e. we don't need to hold the lock when allocating/setting the blinding up via RSA_setup_blinding This trades off some extra memory usage for the above significant reduction in execution time. it also allows us to simplify the blinding code quite a bit by eliminating the need to handle shared blindings because blindings are never shared anymore Fixes openssl/project#1245 Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/27913) --- diff --git a/crypto/rsa/rsa_crpt.c b/crypto/rsa/rsa_crpt.c index 21c922e6096..d6c7353406a 100644 --- a/crypto/rsa/rsa_crpt.c +++ b/crypto/rsa/rsa_crpt.c @@ -61,28 +61,16 @@ int RSA_flags(const RSA *r) void RSA_blinding_off(RSA *rsa) { - BN_BLINDING_free(rsa->blinding); - rsa->blinding = NULL; rsa->flags &= ~RSA_FLAG_BLINDING; rsa->flags |= RSA_FLAG_NO_BLINDING; } int RSA_blinding_on(RSA *rsa, BN_CTX *ctx) { - int ret = 0; - - if (rsa->blinding != NULL) - RSA_blinding_off(rsa); - - rsa->blinding = RSA_setup_blinding(rsa, ctx); - if (rsa->blinding == NULL) - goto err; rsa->flags |= RSA_FLAG_BLINDING; rsa->flags &= ~RSA_FLAG_NO_BLINDING; - ret = 1; - err: - return ret; + return 1; } static BIGNUM *rsa_get_public_exp(const BIGNUM *d, const BIGNUM *p, @@ -162,8 +150,6 @@ BN_BLINDING *RSA_setup_blinding(RSA *rsa, BN_CTX *in_ctx) goto err; } - BN_BLINDING_set_current_thread(ret); - err: BN_CTX_end(ctx); if (ctx != in_ctx) diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c index d9ceb808800..77bb330bbb6 100644 --- a/crypto/rsa/rsa_lib.c +++ b/crypto/rsa/rsa_lib.c @@ -25,6 +25,7 @@ #include "crypto/bn.h" #include "crypto/evp.h" #include "crypto/rsa.h" +#include "crypto/sparse_array.h" #include "crypto/security_bits.h" #include "rsa_local.h" @@ -92,6 +93,10 @@ static RSA *rsa_new_intern(ENGINE *engine, OSSL_LIB_CTX *libctx) return NULL; } + ret->blindings_sa = ossl_rsa_alloc_blinding(); + if (ret->blindings_sa == NULL) + goto err; + ret->libctx = libctx; ret->meth = RSA_get_default_method(); #if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE) @@ -181,8 +186,7 @@ void RSA_free(RSA *r) RSA_PSS_PARAMS_free(r->pss); sk_RSA_PRIME_INFO_pop_free(r->prime_infos, ossl_rsa_multip_info_free); #endif - BN_BLINDING_free(r->blinding); - BN_BLINDING_free(r->mt_blinding); + ossl_rsa_free_blinding(r); OPENSSL_free(r); } @@ -1380,4 +1384,5 @@ int EVP_PKEY_CTX_set_rsa_keygen_primes(EVP_PKEY_CTX *ctx, int primes) return evp_pkey_ctx_set_params_strict(ctx, params); } + #endif diff --git a/crypto/rsa/rsa_local.h b/crypto/rsa/rsa_local.h index db9eb2a1dfb..90122f97fc0 100644 --- a/crypto/rsa/rsa_local.h +++ b/crypto/rsa/rsa_local.h @@ -93,8 +93,7 @@ struct rsa_st { BN_MONT_CTX *_method_mod_n; BN_MONT_CTX *_method_mod_p; BN_MONT_CTX *_method_mod_q; - BN_BLINDING *blinding; - BN_BLINDING *mt_blinding; + void *blindings_sa; CRYPTO_RWLOCK *lock; int dirty_cnt; @@ -196,5 +195,7 @@ int ossl_rsa_fips186_4_gen_prob_primes(RSA *rsa, RSA_ACVP_TEST *test, int ossl_rsa_padding_add_PKCS1_type_2_ex(OSSL_LIB_CTX *libctx, unsigned char *to, int tlen, const unsigned char *from, int flen); +void ossl_rsa_free_blinding(RSA *rsa); +void *ossl_rsa_alloc_blinding(void); #endif /* OSSL_CRYPTO_RSA_LOCAL_H */ diff --git a/crypto/rsa/rsa_ossl.c b/crypto/rsa/rsa_ossl.c index 0c0c73c65c6..a1d4877342a 100644 --- a/crypto/rsa/rsa_ossl.c +++ b/crypto/rsa/rsa_ossl.c @@ -15,12 +15,15 @@ #include "internal/cryptlib.h" #include "crypto/bn.h" +#include "crypto/sparse_array.h" #include "rsa_local.h" #include "internal/constant_time.h" #include #include #include +DEFINE_SPARSE_ARRAY_OF(BN_BLINDING); + static int rsa_ossl_public_encrypt(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, int padding); static int rsa_ossl_private_encrypt(int flen, const unsigned char *from, @@ -207,86 +210,76 @@ static int rsa_ossl_public_encrypt(int flen, const unsigned char *from, return r; } -static BN_BLINDING *rsa_get_blinding(RSA *rsa, int *local, BN_CTX *ctx) +static void free_bn_blinding(ossl_uintmax_t idx, BN_BLINDING *b, void *arg) { - BN_BLINDING *ret; + BN_BLINDING_free(b); +} - if (!CRYPTO_THREAD_read_lock(rsa->lock)) - return NULL; +void ossl_rsa_free_blinding(RSA *rsa) +{ + SPARSE_ARRAY_OF(BN_BLINDING) *blindings = rsa->blindings_sa; - if (rsa->blinding == NULL) { - /* - * This dance with upgrading the lock from read to write will be - * slower in cases of a single use RSA object, but should be - * significantly better in multi-thread cases (e.g. servers). It's - * probably worth it. - */ - CRYPTO_THREAD_unlock(rsa->lock); - if (!CRYPTO_THREAD_write_lock(rsa->lock)) - return NULL; - if (rsa->blinding == NULL) - rsa->blinding = RSA_setup_blinding(rsa, ctx); - } + ossl_sa_BN_BLINDING_doall_arg(blindings, free_bn_blinding, NULL); + ossl_sa_BN_BLINDING_free(blindings); +} - ret = rsa->blinding; - if (ret == NULL) - goto err; +void *ossl_rsa_alloc_blinding(void) +{ + return ossl_sa_BN_BLINDING_new(); +} - if (BN_BLINDING_is_current_thread(ret)) { - /* rsa->blinding is ours! */ +static BN_BLINDING *ossl_rsa_get_thread_bn_blinding(RSA *rsa) +{ + SPARSE_ARRAY_OF(BN_BLINDING) *blindings = rsa->blindings_sa; + uintptr_t tid = (uintptr_t)CRYPTO_THREAD_get_current_id(); - *local = 1; - } else { - /* resort to rsa->mt_blinding instead */ + return ossl_sa_BN_BLINDING_get(blindings, tid); +} - /* - * instructs rsa_blinding_convert(), rsa_blinding_invert() that the - * BN_BLINDING is shared, meaning that accesses require locks, and - * that the blinding factor must be stored outside the BN_BLINDING - */ - *local = 0; - - if (rsa->mt_blinding == NULL) { - CRYPTO_THREAD_unlock(rsa->lock); - if (!CRYPTO_THREAD_write_lock(rsa->lock)) - return NULL; - if (rsa->mt_blinding == NULL) - rsa->mt_blinding = RSA_setup_blinding(rsa, ctx); - } - ret = rsa->mt_blinding; - } +static int ossl_rsa_set_thread_bn_blinding(RSA *rsa, BN_BLINDING *b) +{ + SPARSE_ARRAY_OF(BN_BLINDING) *blindings = rsa->blindings_sa; + uintptr_t tid = (uintptr_t)CRYPTO_THREAD_get_current_id(); - err: - CRYPTO_THREAD_unlock(rsa->lock); - return ret; + return ossl_sa_BN_BLINDING_set(blindings, tid, b); } -static int rsa_blinding_convert(BN_BLINDING *b, BIGNUM *f, BIGNUM *unblind, - BN_CTX *ctx) +static BN_BLINDING *rsa_get_blinding(RSA *rsa, BN_CTX *ctx) { - if (unblind == NULL) { - /* - * Local blinding: store the unblinding factor in BN_BLINDING. - */ - return BN_BLINDING_convert_ex(f, NULL, b, ctx); - } else { - /* - * Shared blinding: store the unblinding factor outside BN_BLINDING. - */ - int ret; + BN_BLINDING *ret; - if (!BN_BLINDING_lock(b)) - return 0; + if (!CRYPTO_THREAD_read_lock(rsa->lock)) + return NULL; - ret = BN_BLINDING_convert_ex(f, unblind, b, ctx); - BN_BLINDING_unlock(b); + ret = ossl_rsa_get_thread_bn_blinding(rsa); + CRYPTO_THREAD_unlock(rsa->lock); - return ret; + if (ret == NULL) { + ret = RSA_setup_blinding(rsa, ctx); + if (!CRYPTO_THREAD_write_lock(rsa->lock)) { + BN_BLINDING_free(ret); + ret = NULL; + } else { + if (!ossl_rsa_set_thread_bn_blinding(rsa, ret)) { + BN_BLINDING_free(ret); + ret = NULL; + } + } + CRYPTO_THREAD_unlock(rsa->lock); } + + return ret; } -static int rsa_blinding_invert(BN_BLINDING *b, BIGNUM *f, BIGNUM *unblind, - BN_CTX *ctx) +static int rsa_blinding_convert(BN_BLINDING *b, BIGNUM *f, BN_CTX *ctx) +{ + /* + * Local blinding: store the unblinding factor in BN_BLINDING. + */ + return BN_BLINDING_convert_ex(f, NULL, b, ctx); +} + +static int rsa_blinding_invert(BN_BLINDING *b, BIGNUM *f, BN_CTX *ctx) { /* * For local blinding, unblind is set to NULL, and BN_BLINDING_invert_ex @@ -297,7 +290,7 @@ static int rsa_blinding_invert(BN_BLINDING *b, BIGNUM *f, BIGNUM *unblind, * to access the blinding without a lock. */ BN_set_flags(f, BN_FLG_CONSTTIME); - return BN_BLINDING_invert_ex(f, unblind, b, ctx); + return BN_BLINDING_invert_ex(f, NULL, b, ctx); } /* signing */ @@ -308,13 +301,6 @@ static int rsa_ossl_private_encrypt(int flen, const unsigned char *from, int i, num = 0, r = -1; unsigned char *buf = NULL; BN_CTX *ctx = NULL; - int local_blinding = 0; - /* - * Used only if the blinding structure is shared. A non-NULL unblind - * instructs rsa_blinding_convert() and rsa_blinding_invert() to store - * the unblinding factor outside the blinding structure. - */ - BIGNUM *unblind = NULL; BN_BLINDING *blinding = NULL; if ((ctx = BN_CTX_new_ex(rsa->libctx)) == NULL) @@ -359,19 +345,13 @@ static int rsa_ossl_private_encrypt(int flen, const unsigned char *from, goto err; if (!(rsa->flags & RSA_FLAG_NO_BLINDING)) { - blinding = rsa_get_blinding(rsa, &local_blinding, ctx); + blinding = rsa_get_blinding(rsa, ctx); if (blinding == NULL) { ERR_raise(ERR_LIB_RSA, ERR_R_INTERNAL_ERROR); goto err; } - } - if (blinding != NULL) { - if (!local_blinding && ((unblind = BN_CTX_get(ctx)) == NULL)) { - ERR_raise(ERR_LIB_RSA, ERR_R_BN_LIB); - goto err; - } - if (!rsa_blinding_convert(blinding, f, unblind, ctx)) + if (!rsa_blinding_convert(blinding, f, ctx)) goto err; } @@ -405,7 +385,7 @@ static int rsa_ossl_private_encrypt(int flen, const unsigned char *from, } if (blinding) - if (!rsa_blinding_invert(blinding, ret, unblind, ctx)) + if (!rsa_blinding_invert(blinding, ret, ctx)) goto err; if (padding == RSA_X931_PADDING) { @@ -524,13 +504,6 @@ static int rsa_ossl_private_decrypt(int flen, const unsigned char *from, unsigned char *buf = NULL; unsigned char kdk[SHA256_DIGEST_LENGTH] = {0}; BN_CTX *ctx = NULL; - int local_blinding = 0; - /* - * Used only if the blinding structure is shared. A non-NULL unblind - * instructs rsa_blinding_convert() and rsa_blinding_invert() to store - * the unblinding factor outside the blinding structure. - */ - BIGNUM *unblind = NULL; BN_BLINDING *blinding = NULL; /* @@ -606,19 +579,13 @@ static int rsa_ossl_private_decrypt(int flen, const unsigned char *from, goto err; if (!(rsa->flags & RSA_FLAG_NO_BLINDING)) { - blinding = rsa_get_blinding(rsa, &local_blinding, ctx); + blinding = rsa_get_blinding(rsa, ctx); if (blinding == NULL) { ERR_raise(ERR_LIB_RSA, ERR_R_INTERNAL_ERROR); goto err; } - } - if (blinding != NULL) { - if (!local_blinding && ((unblind = BN_CTX_get(ctx)) == NULL)) { - ERR_raise(ERR_LIB_RSA, ERR_R_BN_LIB); - goto err; - } - if (!rsa_blinding_convert(blinding, f, unblind, ctx)) + if (!rsa_blinding_convert(blinding, f, ctx)) goto err; } @@ -652,7 +619,7 @@ static int rsa_ossl_private_decrypt(int flen, const unsigned char *from, } if (blinding) - if (!rsa_blinding_invert(blinding, ret, unblind, ctx)) + if (!rsa_blinding_invert(blinding, ret, ctx)) goto err; /*