From d188a53617de68a707fe9459d4f4245d9a57cd9c Mon Sep 17 00:00:00 2001 From: Alessandro Ghedini Date: Fri, 4 Mar 2016 15:43:46 +0000 Subject: [PATCH 1/1] Convert CRYPTO_LOCK_{DH,DSA,RSA} to new multi-threading API Reviewed-by: Matt Caswell Reviewed-by: Rich Salz --- crypto/bn/bn_mont.c | 10 +++++----- crypto/dh/dh_key.c | 4 ++-- crypto/dh/dh_lib.c | 25 ++++++++++++++++++++----- crypto/dsa/dsa_lib.c | 23 ++++++++++++++++++----- crypto/dsa/dsa_ossl.c | 4 ++-- crypto/rsa/rsa_lib.c | 22 +++++++++++++++++----- crypto/rsa/rsa_ossl.c | 38 +++++++++++--------------------------- include/openssl/bn.h | 2 +- include/openssl/crypto.h | 3 --- include/openssl/dh.h | 1 + include/openssl/dsa.h | 1 + include/openssl/rsa.h | 1 + 12 files changed, 79 insertions(+), 55 deletions(-) diff --git a/crypto/bn/bn_mont.c b/crypto/bn/bn_mont.c index e0a9a0910e..dfa395afd6 100644 --- a/crypto/bn/bn_mont.c +++ b/crypto/bn/bn_mont.c @@ -496,14 +496,14 @@ BN_MONT_CTX *BN_MONT_CTX_copy(BN_MONT_CTX *to, BN_MONT_CTX *from) return (to); } -BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, int lock, +BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, CRYPTO_RWLOCK *lock, const BIGNUM *mod, BN_CTX *ctx) { BN_MONT_CTX *ret; - CRYPTO_r_lock(lock); + CRYPTO_THREAD_read_lock(lock); ret = *pmont; - CRYPTO_r_unlock(lock); + CRYPTO_THREAD_unlock(lock); if (ret) return ret; @@ -524,12 +524,12 @@ BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, int lock, } /* The locked compare-and-set, after the local work is done. */ - CRYPTO_w_lock(lock); + CRYPTO_THREAD_write_lock(lock); if (*pmont) { BN_MONT_CTX_free(ret); ret = *pmont; } else *pmont = ret; - CRYPTO_w_unlock(lock); + CRYPTO_THREAD_unlock(lock); return ret; } diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c index 5ee38e3a87..558ec8c3cd 100644 --- a/crypto/dh/dh_key.c +++ b/crypto/dh/dh_key.c @@ -140,7 +140,7 @@ static int generate_key(DH *dh) if (dh->flags & DH_FLAG_CACHE_MONT_P) { mont = BN_MONT_CTX_set_locked(&dh->method_mont_p, - CRYPTO_LOCK_DH, dh->p, ctx); + dh->lock, dh->p, ctx); if (!mont) goto err; } @@ -222,7 +222,7 @@ static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh) if (dh->flags & DH_FLAG_CACHE_MONT_P) { mont = BN_MONT_CTX_set_locked(&dh->method_mont_p, - CRYPTO_LOCK_DH, dh->p, ctx); + dh->lock, dh->p, ctx); if ((dh->flags & DH_FLAG_NO_EXP_CONSTTIME) == 0) { /* XXX */ BN_set_flags(dh->priv_key, BN_FLG_CONSTTIME); diff --git a/crypto/dh/dh_lib.c b/crypto/dh/dh_lib.c index 58280d8734..d7aed6a282 100644 --- a/crypto/dh/dh_lib.c +++ b/crypto/dh/dh_lib.c @@ -108,7 +108,7 @@ DH *DH_new_method(ENGINE *engine) if (ret == NULL) { DHerr(DH_F_DH_NEW_METHOD, ERR_R_MALLOC_FAILURE); - return (NULL); + return NULL; } ret->meth = DH_get_default_method(); @@ -135,16 +135,25 @@ DH *DH_new_method(ENGINE *engine) ret->references = 1; ret->flags = ret->meth->flags; + CRYPTO_new_ex_data(CRYPTO_EX_INDEX_DH, ret, &ret->ex_data); - if ((ret->meth->init != NULL) && !ret->meth->init(ret)) { + + ret->lock = CRYPTO_THREAD_lock_new(); + if (ret->lock == NULL) { #ifndef OPENSSL_NO_ENGINE ENGINE_finish(ret->engine); #endif CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DH, ret, &ret->ex_data); OPENSSL_free(ret); + return NULL; + } + + if ((ret->meth->init != NULL) && !ret->meth->init(ret)) { + DH_free(ret); ret = NULL; } - return (ret); + + return ret; } void DH_free(DH *r) @@ -153,7 +162,8 @@ void DH_free(DH *r) if (r == NULL) return; - i = CRYPTO_add(&r->references, -1, CRYPTO_LOCK_DH); + + CRYPTO_atomic_add(&r->references, -1, &i, r->lock); REF_PRINT_COUNT("DH", r); if (i > 0) return; @@ -167,6 +177,8 @@ void DH_free(DH *r) CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DH, r, &r->ex_data); + CRYPTO_THREAD_lock_free(r->lock); + BN_clear_free(r->p); BN_clear_free(r->g); BN_clear_free(r->q); @@ -180,7 +192,10 @@ void DH_free(DH *r) int DH_up_ref(DH *r) { - int i = CRYPTO_add(&r->references, 1, CRYPTO_LOCK_DH); + int i; + + if (CRYPTO_atomic_add(&r->references, 1, &i, r->lock) <= 0) + return 0; REF_PRINT_COUNT("DH", r); REF_ASSERT_ISNT(i < 2); diff --git a/crypto/dsa/dsa_lib.c b/crypto/dsa/dsa_lib.c index f7795b27bd..3b99b3c2eb 100644 --- a/crypto/dsa/dsa_lib.c +++ b/crypto/dsa/dsa_lib.c @@ -115,7 +115,7 @@ DSA *DSA_new_method(ENGINE *engine) ret = OPENSSL_zalloc(sizeof(*ret)); if (ret == NULL) { DSAerr(DSA_F_DSA_NEW_METHOD, ERR_R_MALLOC_FAILURE); - return (NULL); + return NULL; } ret->meth = DSA_get_default_method(); #ifndef OPENSSL_NO_ENGINE @@ -141,17 +141,25 @@ DSA *DSA_new_method(ENGINE *engine) ret->references = 1; ret->flags = ret->meth->flags & ~DSA_FLAG_NON_FIPS_ALLOW; + CRYPTO_new_ex_data(CRYPTO_EX_INDEX_DSA, ret, &ret->ex_data); - if ((ret->meth->init != NULL) && !ret->meth->init(ret)) { + + ret->lock = CRYPTO_THREAD_lock_new(); + if (ret->lock == NULL) { #ifndef OPENSSL_NO_ENGINE ENGINE_finish(ret->engine); #endif CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DSA, ret, &ret->ex_data); OPENSSL_free(ret); + return NULL; + } + + if ((ret->meth->init != NULL) && !ret->meth->init(ret)) { + DSA_free(ret); ret = NULL; } - return (ret); + return ret; } void DSA_free(DSA *r) @@ -161,7 +169,7 @@ void DSA_free(DSA *r) if (r == NULL) return; - i = CRYPTO_add(&r->references, -1, CRYPTO_LOCK_DSA); + CRYPTO_atomic_add(&r->references, -1, &i, r->lock); REF_PRINT_COUNT("DSA", r); if (i > 0) return; @@ -175,6 +183,8 @@ void DSA_free(DSA *r) CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DSA, r, &r->ex_data); + CRYPTO_THREAD_lock_free(r->lock); + BN_clear_free(r->p); BN_clear_free(r->q); BN_clear_free(r->g); @@ -185,7 +195,10 @@ void DSA_free(DSA *r) int DSA_up_ref(DSA *r) { - int i = CRYPTO_add(&r->references, 1, CRYPTO_LOCK_DSA); + int i; + + if (CRYPTO_atomic_add(&r->references, 1, &i, r->lock) <= 0) + return 0; REF_PRINT_COUNT("DSA", r); REF_ASSERT_ISNT(i < 2); diff --git a/crypto/dsa/dsa_ossl.c b/crypto/dsa/dsa_ossl.c index 0874e89141..f8b4647f04 100644 --- a/crypto/dsa/dsa_ossl.c +++ b/crypto/dsa/dsa_ossl.c @@ -265,7 +265,7 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, if (dsa->flags & DSA_FLAG_CACHE_MONT_P) { if (!BN_MONT_CTX_set_locked(&dsa->method_mont_p, - CRYPTO_LOCK_DSA, dsa->p, ctx)) + dsa->lock, dsa->p, ctx)) goto err; } @@ -388,7 +388,7 @@ static int dsa_do_verify(const unsigned char *dgst, int dgst_len, if (dsa->flags & DSA_FLAG_CACHE_MONT_P) { mont = BN_MONT_CTX_set_locked(&dsa->method_mont_p, - CRYPTO_LOCK_DSA, dsa->p, ctx); + dsa->lock, dsa->p, ctx); if (!mont) goto err; } diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c index 8b5015703c..9cc88142b6 100644 --- a/crypto/rsa/rsa_lib.c +++ b/crypto/rsa/rsa_lib.c @@ -157,18 +157,25 @@ RSA *RSA_new_method(ENGINE *engine) ENGINE_finish(ret->engine); #endif OPENSSL_free(ret); - return (NULL); + return NULL; } - if ((ret->meth->init != NULL) && !ret->meth->init(ret)) { + ret->lock = CRYPTO_THREAD_lock_new(); + if (ret->lock == NULL) { #ifndef OPENSSL_NO_ENGINE ENGINE_finish(ret->engine); #endif CRYPTO_free_ex_data(CRYPTO_EX_INDEX_RSA, ret, &ret->ex_data); OPENSSL_free(ret); + return NULL; + } + + if ((ret->meth->init != NULL) && !ret->meth->init(ret)) { + RSA_free(ret); ret = NULL; } - return (ret); + + return ret; } void RSA_free(RSA *r) @@ -178,7 +185,7 @@ void RSA_free(RSA *r) if (r == NULL) return; - i = CRYPTO_add(&r->references, -1, CRYPTO_LOCK_RSA); + CRYPTO_atomic_add(&r->references, -1, &i, r->lock); REF_PRINT_COUNT("RSA", r); if (i > 0) return; @@ -192,6 +199,8 @@ void RSA_free(RSA *r) CRYPTO_free_ex_data(CRYPTO_EX_INDEX_RSA, r, &r->ex_data); + CRYPTO_THREAD_lock_free(r->lock); + BN_clear_free(r->n); BN_clear_free(r->e); BN_clear_free(r->d); @@ -208,7 +217,10 @@ void RSA_free(RSA *r) int RSA_up_ref(RSA *r) { - int i = CRYPTO_add(&r->references, 1, CRYPTO_LOCK_RSA); + int i; + + if (CRYPTO_atomic_add(&r->references, 1, &i, r->lock) <= 0) + return 0; REF_PRINT_COUNT("RSA", r); REF_ASSERT_ISNT(i < 2); diff --git a/crypto/rsa/rsa_ossl.c b/crypto/rsa/rsa_ossl.c index b6b7dacb28..925cf65333 100644 --- a/crypto/rsa/rsa_ossl.c +++ b/crypto/rsa/rsa_ossl.c @@ -220,7 +220,7 @@ static int rsa_ossl_public_encrypt(int flen, const unsigned char *from, if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) if (!BN_MONT_CTX_set_locked - (&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx)) + (&rsa->_method_mod_n, rsa->lock, rsa->n, ctx)) goto err; if (!rsa->meth->bn_mod_exp(ret, f, rsa->e, rsa->n, ctx, @@ -248,18 +248,12 @@ static int rsa_ossl_public_encrypt(int flen, const unsigned char *from, static BN_BLINDING *rsa_get_blinding(RSA *rsa, int *local, BN_CTX *ctx) { BN_BLINDING *ret; - int got_write_lock = 0; CRYPTO_THREADID cur; - CRYPTO_r_lock(CRYPTO_LOCK_RSA); + CRYPTO_THREAD_write_lock(rsa->lock); if (rsa->blinding == NULL) { - CRYPTO_r_unlock(CRYPTO_LOCK_RSA); - CRYPTO_w_lock(CRYPTO_LOCK_RSA); - got_write_lock = 1; - - if (rsa->blinding == NULL) - rsa->blinding = RSA_setup_blinding(rsa, ctx); + rsa->blinding = RSA_setup_blinding(rsa, ctx); } ret = rsa->blinding; @@ -282,23 +276,13 @@ static BN_BLINDING *rsa_get_blinding(RSA *rsa, int *local, BN_CTX *ctx) *local = 0; if (rsa->mt_blinding == NULL) { - if (!got_write_lock) { - CRYPTO_r_unlock(CRYPTO_LOCK_RSA); - CRYPTO_w_lock(CRYPTO_LOCK_RSA); - got_write_lock = 1; - } - - if (rsa->mt_blinding == NULL) - rsa->mt_blinding = RSA_setup_blinding(rsa, ctx); + rsa->mt_blinding = RSA_setup_blinding(rsa, ctx); } ret = rsa->mt_blinding; } err: - if (got_write_lock) - CRYPTO_w_unlock(CRYPTO_LOCK_RSA); - else - CRYPTO_r_unlock(CRYPTO_LOCK_RSA); + CRYPTO_THREAD_unlock(rsa->lock); return ret; } @@ -432,7 +416,7 @@ static int rsa_ossl_private_encrypt(int flen, const unsigned char *from, if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) if (!BN_MONT_CTX_set_locked - (&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx)) { + (&rsa->_method_mod_n, rsa->lock, rsa->n, ctx)) { BN_free(local_d); goto err; } @@ -566,7 +550,7 @@ static int rsa_ossl_private_decrypt(int flen, const unsigned char *from, if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) if (!BN_MONT_CTX_set_locked - (&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx)) { + (&rsa->_method_mod_n, rsa->lock, rsa->n, ctx)) { BN_free(local_d); goto err; } @@ -674,7 +658,7 @@ static int rsa_ossl_public_decrypt(int flen, const unsigned char *from, if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) if (!BN_MONT_CTX_set_locked - (&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx)) + (&rsa->_method_mod_n, rsa->lock, rsa->n, ctx)) goto err; if (!rsa->meth->bn_mod_exp(ret, f, rsa->e, rsa->n, ctx, @@ -751,9 +735,9 @@ static int rsa_ossl_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) if (rsa->flags & RSA_FLAG_CACHE_PRIVATE) { if (!BN_MONT_CTX_set_locked - (&rsa->_method_mod_p, CRYPTO_LOCK_RSA, p, ctx) + (&rsa->_method_mod_p, rsa->lock, p, ctx) || !BN_MONT_CTX_set_locked(&rsa->_method_mod_q, - CRYPTO_LOCK_RSA, q, ctx)) { + rsa->lock, q, ctx)) { BN_free(local_p); BN_free(local_q); goto err; @@ -769,7 +753,7 @@ static int rsa_ossl_mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) if (rsa->flags & RSA_FLAG_CACHE_PUBLIC) if (!BN_MONT_CTX_set_locked - (&rsa->_method_mod_n, CRYPTO_LOCK_RSA, rsa->n, ctx)) + (&rsa->_method_mod_n, rsa->lock, rsa->n, ctx)) goto err; /* compute I mod q */ diff --git a/include/openssl/bn.h b/include/openssl/bn.h index 9807b2c684..db01b7e3b4 100644 --- a/include/openssl/bn.h +++ b/include/openssl/bn.h @@ -416,7 +416,7 @@ int BN_from_montgomery(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont, void BN_MONT_CTX_free(BN_MONT_CTX *mont); int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx); BN_MONT_CTX *BN_MONT_CTX_copy(BN_MONT_CTX *to, BN_MONT_CTX *from); -BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, int lock, +BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, CRYPTO_RWLOCK *lock, const BIGNUM *mod, BN_CTX *ctx); /* BN_BLINDING flags */ diff --git a/include/openssl/crypto.h b/include/openssl/crypto.h index b3c669d57e..12052e14ce 100644 --- a/include/openssl/crypto.h +++ b/include/openssl/crypto.h @@ -172,8 +172,6 @@ extern "C" { # define CRYPTO_LOCK_X509_PKEY 5 # define CRYPTO_LOCK_X509_CRL 6 # define CRYPTO_LOCK_X509_REQ 7 -# define CRYPTO_LOCK_DSA 8 -# define CRYPTO_LOCK_RSA 9 # define CRYPTO_LOCK_EVP_PKEY 10 # define CRYPTO_LOCK_X509_STORE 11 # define CRYPTO_LOCK_SSL_CTX 12 @@ -188,7 +186,6 @@ extern "C" { # define CRYPTO_LOCK_BIO 21 # define CRYPTO_LOCK_READDIR 24 # define CRYPTO_LOCK_RSA_BLINDING 25 -# define CRYPTO_LOCK_DH 26 # define CRYPTO_LOCK_MALLOC2 27 # define CRYPTO_LOCK_DSO 28 # define CRYPTO_LOCK_DYNLOCK 29 diff --git a/include/openssl/dh.h b/include/openssl/dh.h index 74bc9891a8..50f8e51c08 100644 --- a/include/openssl/dh.h +++ b/include/openssl/dh.h @@ -156,6 +156,7 @@ struct dh_st { CRYPTO_EX_DATA ex_data; const DH_METHOD *meth; ENGINE *engine; + CRYPTO_RWLOCK *lock; }; # define DH_GENERATOR_2 2 diff --git a/include/openssl/dsa.h b/include/openssl/dsa.h index a338eaedca..f10e1c2504 100644 --- a/include/openssl/dsa.h +++ b/include/openssl/dsa.h @@ -172,6 +172,7 @@ struct dsa_st { const DSA_METHOD *meth; /* functional reference if 'meth' is ENGINE-provided */ ENGINE *engine; + CRYPTO_RWLOCK *lock; }; # define d2i_DSAparams_fp(fp,x) (DSA *)ASN1_d2i_fp((char *(*)())DSA_new, \ diff --git a/include/openssl/rsa.h b/include/openssl/rsa.h index 4f6d44f468..adad0f144c 100644 --- a/include/openssl/rsa.h +++ b/include/openssl/rsa.h @@ -158,6 +158,7 @@ struct rsa_st { char *bignum_data; BN_BLINDING *blinding; BN_BLINDING *mt_blinding; + CRYPTO_RWLOCK *lock; }; # ifndef OPENSSL_RSA_MAX_MODULUS_BITS -- 2.34.1