From 4bdb6e43fa285df88d57ce3b79f3711097328d28 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Fri, 15 May 2015 15:29:21 -0700 Subject: [PATCH] Remove remaining calls to the old lock functions. |SSL_CTX| and |X509_STORE| have grown their own locks. Several static locks have been added to hack around not being able to use a |CRYPTO_once_t| in public headers. Lastly, support for calling |SSL_CTX_set_generate_session_id| concurrently with active connections has been removed. No other property of an |SSL_CTX| works like that. Change-Id: Iff5fe3ee3fdd6ea9c9daee96f850b107ad8a6bca Reviewed-on: https://boringssl-review.googlesource.com/4775 Reviewed-by: Adam Langley --- crypto/x509/by_dir.c | 23 +++++++++++-------- crypto/x509/x509_lu.c | 46 +++++++++++++++++++------------------- crypto/x509/x_crl.c | 24 +++++++++++++++----- crypto/x509/x_pubkey.c | 16 ++++++++++--- crypto/x509v3/pcy_cache.c | 27 ++++++++++++++++------ crypto/x509v3/v3_purp.c | 27 +++++++++++++++++----- include/openssl/ssl.h | 3 +++ include/openssl/x509_vfy.h | 1 + ssl/ssl_cert.c | 35 +++++++++-------------------- ssl/ssl_lib.c | 21 ++++++++--------- ssl/ssl_sess.c | 33 ++++++++++++++------------- 11 files changed, 154 insertions(+), 102 deletions(-) diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c index 098c1bd9..34bb1e4b 100644 --- a/crypto/x509/by_dir.c +++ b/crypto/x509/by_dir.c @@ -66,6 +66,8 @@ #include #include +#include "../internal.h" + typedef struct lookup_dir_hashes_st { @@ -262,6 +264,10 @@ static int add_cert_dir(BY_DIR *ctx, const char *dir, int type) return 1; } +/* g_ent_hashes_lock protects the |hashes| member of all |BY_DIR_ENTRY| + * objects. */ +static struct CRYPTO_STATIC_MUTEX g_ent_hashes_lock = CRYPTO_STATIC_MUTEX_INIT; + static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, X509_OBJECT *ret) { @@ -337,7 +343,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, if (type == X509_LU_CRL && ent->hashes) { htmp.hash = h; - CRYPTO_r_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_STATIC_MUTEX_lock_read(&g_ent_hashes_lock); if (sk_BY_DIR_HASH_find(ent->hashes, &idx, &htmp)) { hent = sk_BY_DIR_HASH_value(ent->hashes, idx); @@ -348,7 +354,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, hent = NULL; k=0; } - CRYPTO_r_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_STATIC_MUTEX_unlock(&g_ent_hashes_lock); } else { @@ -418,19 +424,19 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, /* we have added it to the cache so now pull * it out again */ - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&xl->store_ctx->objs_lock); tmp = NULL; if (sk_X509_OBJECT_find(xl->store_ctx->objs, &idx, &stmp)) { tmp=sk_X509_OBJECT_value(xl->store_ctx->objs,idx); } - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&xl->store_ctx->objs_lock); /* If a CRL, update the last file suffix added for this */ if (type == X509_LU_CRL) { - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_STATIC_MUTEX_lock_write(&g_ent_hashes_lock); /* Look for entry again in case another thread added * an entry first. */ @@ -445,7 +451,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, hent = OPENSSL_malloc(sizeof(BY_DIR_HASH)); if (hent == NULL) { - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_STATIC_MUTEX_unlock(&g_ent_hashes_lock); ok = 0; goto finish; } @@ -453,7 +459,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, hent->suffix = k; if (!sk_BY_DIR_HASH_push(ent->hashes, hent)) { - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_STATIC_MUTEX_unlock(&g_ent_hashes_lock); OPENSSL_free(hent); ok = 0; goto finish; @@ -462,8 +468,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name, else if (hent->suffix < k) hent->suffix = k; - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); - + CRYPTO_STATIC_MUTEX_unlock(&g_ent_hashes_lock); } if (tmp != NULL) diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index fcae59d4..a662305e 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -188,6 +188,7 @@ X509_STORE *X509_STORE_new(void) return NULL; memset(ret, 0, sizeof(*ret)); ret->objs = sk_X509_OBJECT_new(x509_object_cmp); + CRYPTO_MUTEX_init(&ret->objs_lock); ret->cache = 1; ret->get_cert_methods = sk_X509_LOOKUP_new_null(); @@ -240,9 +241,8 @@ void X509_STORE_free(X509_STORE *vfy) if (!CRYPTO_refcount_dec_and_test_zero(&vfy->references)) { return; } -#ifdef REF_PRINT - REF_PRINT("X509_STORE",vfy); -#endif + + CRYPTO_MUTEX_cleanup(&vfy->objs_lock); sk=vfy->get_cert_methods; for (j=0; jobjs_lock); tmp=X509_OBJECT_retrieve_by_subject(ctx->objs,type,name); - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->objs_lock); if (tmp == NULL || type == X509_LU_CRL) { @@ -351,7 +351,7 @@ int X509_STORE_add_cert(X509_STORE *ctx, X509 *x) obj->type=X509_LU_X509; obj->data.x509=x; - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&ctx->objs_lock); X509_OBJECT_up_ref_count(obj); @@ -364,7 +364,7 @@ int X509_STORE_add_cert(X509_STORE *ctx, X509 *x) } else sk_X509_OBJECT_push(ctx->objs, obj); - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->objs_lock); return ret; } @@ -384,7 +384,7 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x) obj->type=X509_LU_CRL; obj->data.crl=x; - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&ctx->objs_lock); X509_OBJECT_up_ref_count(obj); @@ -397,7 +397,7 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x) } else sk_X509_OBJECT_push(ctx->objs, obj); - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->objs_lock); return ret; } @@ -498,7 +498,7 @@ STACK_OF(X509)* X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *nm) X509 *x; X509_OBJECT *obj; sk = sk_X509_new_null(); - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&ctx->ctx->objs_lock); idx = x509_object_idx_cnt(ctx->ctx->objs, X509_LU_X509, nm, &cnt); if (idx < 0) { @@ -506,18 +506,18 @@ STACK_OF(X509)* X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *nm) * objects to cache */ X509_OBJECT xobj; - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); if (!X509_STORE_get_by_subject(ctx, X509_LU_X509, nm, &xobj)) { sk_X509_free(sk); return NULL; } X509_OBJECT_free_contents(&xobj); - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&ctx->ctx->objs_lock); idx = x509_object_idx_cnt(ctx->ctx->objs,X509_LU_X509,nm, &cnt); if (idx < 0) { - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); sk_X509_free(sk); return NULL; } @@ -528,13 +528,13 @@ STACK_OF(X509)* X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *nm) x = obj->data.x509; if (!sk_X509_push(sk, X509_up_ref(x))) { - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); X509_free(x); sk_X509_pop_free(sk, X509_free); return NULL; } } - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); return sk; } @@ -546,24 +546,24 @@ STACK_OF(X509_CRL)* X509_STORE_get1_crls(X509_STORE_CTX *ctx, X509_NAME *nm) X509_CRL *x; X509_OBJECT *obj, xobj; sk = sk_X509_CRL_new_null(); - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&ctx->ctx->objs_lock); /* Check cache first */ idx = x509_object_idx_cnt(ctx->ctx->objs, X509_LU_CRL, nm, &cnt); /* Always do lookup to possibly add new CRLs to cache */ - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); if (!X509_STORE_get_by_subject(ctx, X509_LU_CRL, nm, &xobj)) { sk_X509_CRL_free(sk); return NULL; } X509_OBJECT_free_contents(&xobj); - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&ctx->ctx->objs_lock); idx = x509_object_idx_cnt(ctx->ctx->objs,X509_LU_CRL, nm, &cnt); if (idx < 0) { - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); sk_X509_CRL_free(sk); return NULL; } @@ -575,13 +575,13 @@ STACK_OF(X509_CRL)* X509_STORE_get1_crls(X509_STORE_CTX *ctx, X509_NAME *nm) CRYPTO_refcount_inc(&x->references); if (!sk_X509_CRL_push(sk, x)) { - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); X509_CRL_free(x); sk_X509_CRL_pop_free(sk, X509_CRL_free); return NULL; } } - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); return sk; } @@ -662,7 +662,7 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) /* Else find index of first cert accepted by 'check_issued' */ ret = 0; - CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_lock_write(&ctx->ctx->objs_lock); idx = X509_OBJECT_idx_by_subject(ctx->ctx->objs, X509_LU_X509, xn); if (idx != -1) /* should be true as we've had at least one match */ { @@ -684,7 +684,7 @@ int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) } } } - CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + CRYPTO_MUTEX_unlock(&ctx->ctx->objs_lock); return ret; } diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c index aa92fa98..9af13c01 100644 --- a/crypto/x509/x_crl.c +++ b/crypto/x509/x_crl.c @@ -65,6 +65,9 @@ #include #include +#include "../internal.h" + + /* Method to handle CRL access. * In general a CRL could be very large (several Mb) and can consume large * amounts of resources if stored in memory by multiple processes. @@ -463,6 +466,8 @@ static int crl_revoked_issuer_match(X509_CRL *crl, X509_NAME *nm, } +static struct CRYPTO_STATIC_MUTEX g_crl_sort_lock = CRYPTO_STATIC_MUTEX_INIT; + static int def_crl_lookup(X509_CRL *crl, X509_REVOKED **ret, ASN1_INTEGER *serial, X509_NAME *issuer) { @@ -471,13 +476,22 @@ static int def_crl_lookup(X509_CRL *crl, rtmp.serialNumber = serial; /* Sort revoked into serial number order if not already sorted. * Do this under a lock to avoid race condition. - */ - if (!sk_X509_REVOKED_is_sorted(crl->crl->revoked)) + */ + + CRYPTO_STATIC_MUTEX_lock_read(&g_crl_sort_lock); + const int is_sorted = sk_X509_REVOKED_is_sorted(crl->crl->revoked); + CRYPTO_STATIC_MUTEX_unlock(&g_crl_sort_lock); + + if (!is_sorted) { - CRYPTO_w_lock(CRYPTO_LOCK_X509_CRL); - sk_X509_REVOKED_sort(crl->crl->revoked); - CRYPTO_w_unlock(CRYPTO_LOCK_X509_CRL); + CRYPTO_STATIC_MUTEX_lock_write(&g_crl_sort_lock); + if (!sk_X509_REVOKED_is_sorted(crl->crl->revoked)) + { + sk_X509_REVOKED_sort(crl->crl->revoked); + } + CRYPTO_STATIC_MUTEX_unlock(&g_crl_sort_lock); } + if (!sk_X509_REVOKED_find(crl->crl->revoked, &idx, &rtmp)) return 0; /* Need to look for matching name */ diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c index d6512ae9..c2e08639 100644 --- a/crypto/x509/x_pubkey.c +++ b/crypto/x509/x_pubkey.c @@ -64,6 +64,7 @@ #include #include "../evp/internal.h" +#include "../internal.h" /* Minor tweak to operation: free up EVP_PKEY */ @@ -126,16 +127,25 @@ error: return 0; } +/* g_pubkey_lock is used to protect the initialisation of the |pkey| member of + * |X509_PUBKEY| objects. Really |X509_PUBKEY| should have a |CRYPTO_once_t| + * inside it for this, but |CRYPTO_once_t| is private and |X509_PUBKEY| is + * not. */ +static struct CRYPTO_STATIC_MUTEX g_pubkey_lock = CRYPTO_STATIC_MUTEX_INIT; + EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) { EVP_PKEY *ret=NULL; if (key == NULL) goto error; + CRYPTO_STATIC_MUTEX_lock_read(&g_pubkey_lock); if (key->pkey != NULL) { + CRYPTO_STATIC_MUTEX_unlock(&g_pubkey_lock); return EVP_PKEY_up_ref(key->pkey); } + CRYPTO_STATIC_MUTEX_unlock(&g_pubkey_lock); if (key->public_key == NULL) goto error; @@ -166,17 +176,17 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) } /* Check to see if another thread set key->pkey first */ - CRYPTO_w_lock(CRYPTO_LOCK_EVP_PKEY); + CRYPTO_STATIC_MUTEX_lock_write(&g_pubkey_lock); if (key->pkey) { - CRYPTO_w_unlock(CRYPTO_LOCK_EVP_PKEY); + CRYPTO_STATIC_MUTEX_unlock(&g_pubkey_lock); EVP_PKEY_free(ret); ret = key->pkey; } else { key->pkey = ret; - CRYPTO_w_unlock(CRYPTO_LOCK_EVP_PKEY); + CRYPTO_STATIC_MUTEX_unlock(&g_pubkey_lock); } return EVP_PKEY_up_ref(ret); diff --git a/crypto/x509v3/pcy_cache.c b/crypto/x509v3/pcy_cache.c index 5d59c00b..08f20aa2 100644 --- a/crypto/x509v3/pcy_cache.c +++ b/crypto/x509v3/pcy_cache.c @@ -60,6 +60,7 @@ #include #include "pcy_int.h" +#include "../internal.h" static int policy_data_cmp(const X509_POLICY_DATA **a, @@ -243,18 +244,30 @@ void policy_cache_free(X509_POLICY_CACHE *cache) OPENSSL_free(cache); } +/* g_x509_policy_cache_lock is used to protect against concurrent calls to + * |policy_cache_new|. Ideally this would be done with a |CRYPTO_once_t| + * in the |X509| structure, but |CRYPTO_once_t| isn't public. */ +static struct CRYPTO_STATIC_MUTEX g_x509_policy_cache_lock = + CRYPTO_STATIC_MUTEX_INIT; + const X509_POLICY_CACHE *policy_cache_set(X509 *x) { + X509_POLICY_CACHE *cache; - if (x->policy_cache == NULL) - { - CRYPTO_w_lock(CRYPTO_LOCK_X509); - policy_cache_new(x); - CRYPTO_w_unlock(CRYPTO_LOCK_X509); - } + CRYPTO_STATIC_MUTEX_lock_read(&g_x509_policy_cache_lock); + cache = x->policy_cache; + CRYPTO_STATIC_MUTEX_unlock(&g_x509_policy_cache_lock); + + if (cache != NULL) + return cache; - return x->policy_cache; + CRYPTO_STATIC_MUTEX_lock_write(&g_x509_policy_cache_lock); + if (x->policy_cache == NULL) + policy_cache_new(x); + cache = x->policy_cache; + CRYPTO_STATIC_MUTEX_unlock(&g_x509_policy_cache_lock); + return cache; } X509_POLICY_DATA *policy_cache_find_data(const X509_POLICY_CACHE *cache, diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c index 3f175c95..8ae8a064 100644 --- a/crypto/x509v3/v3_purp.c +++ b/crypto/x509v3/v3_purp.c @@ -67,6 +67,8 @@ #include #include +#include "../internal.h" + static void x509v3_cache_extensions(X509 *x); @@ -114,9 +116,7 @@ int X509_check_purpose(X509 *x, int id, int ca) int idx; const X509_PURPOSE *pt; if(!(x->ex_flags & EXFLAG_SET)) { - CRYPTO_w_lock(CRYPTO_LOCK_X509); x509v3_cache_extensions(x); - CRYPTO_w_unlock(CRYPTO_LOCK_X509); } if(id == -1) return 1; idx = X509_PURPOSE_get_by_id(id); @@ -367,6 +367,15 @@ static void setup_crldp(X509 *x) setup_dp(x, sk_DIST_POINT_value(x->crldp, i)); } +/* g_x509_cache_extensions_lock is used to protect against concurrent calls to + * |x509v3_cache_extensions|. Ideally this would be done with a |CRYPTO_once_t| + * in the |X509| structure, but |CRYPTO_once_t| isn't public. + * + * Note: it's not entirely clear whether this lock is needed. Not all paths to + * this function took a lock in OpenSSL. */ +static struct CRYPTO_STATIC_MUTEX g_x509_cache_extensions_lock = + CRYPTO_STATIC_MUTEX_INIT; + static void x509v3_cache_extensions(X509 *x) { BASIC_CONSTRAINTS *bs; @@ -377,7 +386,15 @@ static void x509v3_cache_extensions(X509 *x) X509_EXTENSION *ex; size_t i; int j; - if(x->ex_flags & EXFLAG_SET) return; + + CRYPTO_STATIC_MUTEX_lock_write(&g_x509_cache_extensions_lock); + + if(x->ex_flags & EXFLAG_SET) + { + CRYPTO_STATIC_MUTEX_unlock(&g_x509_cache_extensions_lock); + return; + } + X509_digest(x, EVP_sha1(), x->sha1_hash, NULL); /* V1 should mean no extensions ... */ if(!X509_get_version(x)) x->ex_flags |= EXFLAG_V1; @@ -501,6 +518,8 @@ static void x509v3_cache_extensions(X509 *x) } } x->ex_flags |= EXFLAG_SET; + + CRYPTO_STATIC_MUTEX_unlock(&g_x509_cache_extensions_lock); } /* CA checks common to all purposes @@ -544,9 +563,7 @@ static int check_ca(const X509 *x) int X509_check_ca(X509 *x) { if(!(x->ex_flags & EXFLAG_SET)) { - CRYPTO_w_lock(CRYPTO_LOCK_X509); x509v3_cache_extensions(x); - CRYPTO_w_unlock(CRYPTO_LOCK_X509); } return check_ca(x); diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 6d1c29e7..d6a18264 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -796,6 +796,9 @@ struct ssl_cipher_preference_list_st { struct ssl_ctx_st { const SSL_PROTOCOL_METHOD *method; + /* lock is used to protect various operations on this object. */ + CRYPTO_MUTEX lock; + /* max_version is the maximum acceptable protocol version. If zero, the * maximum supported version, currently (D)TLS 1.2, is used. */ uint16_t max_version; diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h index 32ba1341..146e047a 100644 --- a/include/openssl/x509_vfy.h +++ b/include/openssl/x509_vfy.h @@ -184,6 +184,7 @@ struct x509_store_st /* The following is a cache of trusted certs */ int cache; /* if true, stash any hits */ STACK_OF(X509_OBJECT) *objs; /* Cache of all objects */ + CRYPTO_MUTEX objs_lock; /* These are external lookup methods */ STACK_OF(X509_LOOKUP) *get_cert_methods; diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 6cb37853..f1fd6751 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -132,30 +132,18 @@ #include "internal.h" -int SSL_get_ex_data_X509_STORE_CTX_idx(void) { - static int ssl_x509_store_ctx_idx = -1; - int got_write_lock = 0; - - CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX); - - if (ssl_x509_store_ctx_idx < 0) { - CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX); - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); - got_write_lock = 1; - - if (ssl_x509_store_ctx_idx < 0) { - ssl_x509_store_ctx_idx = X509_STORE_CTX_get_ex_new_index( - 0, "SSL for verify callback", NULL, NULL, NULL); - } - } +static CRYPTO_once_t g_x509_store_ex_data_index_once; +static int g_x509_store_ex_data_index; - if (got_write_lock) { - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); - } else { - CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX); - } +static void ssl_x509_store_ex_data_index_init(void) { + g_x509_store_ex_data_index = X509_STORE_CTX_get_ex_new_index( + 0, "SSL for verify callback", NULL, NULL, NULL); +} - return ssl_x509_store_ctx_idx; +int SSL_get_ex_data_X509_STORE_CTX_idx(void) { + CRYPTO_once(&g_x509_store_ex_data_index_once, + ssl_x509_store_ex_data_index_init); + return g_x509_store_ex_data_index; } CERT *ssl_cert_new(void) { @@ -732,8 +720,6 @@ int SSL_add_dir_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack, const char *filename; int ret = 0; - CRYPTO_w_lock(CRYPTO_LOCK_READDIR); - /* Note that a side effect is that the CAs will be sorted by name */ while ((filename = OPENSSL_DIR_read(&d, dir))) { char buf[1024]; @@ -764,7 +750,6 @@ err: if (d) { OPENSSL_DIR_end(&d); } - CRYPTO_w_unlock(CRYPTO_LOCK_READDIR); return ret; } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index c4f4a29d..d0c52b6a 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -396,9 +396,7 @@ int SSL_set_session_id_context(SSL *ssl, const uint8_t *sid_ctx, } int SSL_CTX_set_generate_session_id(SSL_CTX *ctx, GEN_SESSION_CB cb) { - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); ctx->generate_session_id = cb; - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); return 1; } @@ -424,9 +422,9 @@ int SSL_has_matching_session_id(const SSL *ssl, const uint8_t *id, r.session_id_length = id_len; memcpy(r.session_id, id, id_len); - CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_lock_read(&ssl->ctx->lock); p = lh_SSL_SESSION_retrieve(ssl->ctx->sessions, &r); - CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_unlock(&ssl->ctx->lock); return p != NULL; } @@ -1650,6 +1648,8 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth) { ret->method = meth->method; + CRYPTO_MUTEX_init(&ret->lock); + ret->cert_store = NULL; ret->session_cache_mode = SSL_SESS_CACHE_SERVER; ret->session_cache_size = SSL_SESSION_CACHE_MAX_SIZE_DEFAULT; @@ -1774,6 +1774,7 @@ void SSL_CTX_free(SSL_CTX *ctx) { CRYPTO_free_ex_data(&g_ex_data_class_ssl_ctx, ctx, &ctx->ex_data); + CRYPTO_MUTEX_cleanup(&ctx->lock); lh_SSL_SESSION_free(ctx->sessions); X509_STORE_free(ctx->cert_store); ssl_cipher_preference_list_free(ctx->cipher_list); @@ -1996,13 +1997,13 @@ void ssl_update_cache(SSL *s, int mode) { (ctx->session_cache_mode & mode) == mode) { /* Automatically flush the internal session cache every 255 connections. */ int flush_cache = 0; - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_lock_write(&ctx->lock); ctx->handshakes_since_cache_flush++; if (ctx->handshakes_since_cache_flush >= 255) { flush_cache = 1; ctx->handshakes_since_cache_flush = 0; } - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_unlock(&ctx->lock); if (flush_cache) { SSL_CTX_flush_sessions(ctx, (unsigned long)time(NULL)); @@ -2624,9 +2625,9 @@ int ssl_ctx_log_rsa_client_key_exchange(SSL_CTX *ctx, return 0; } - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_lock_write(&ctx->lock); ret = BIO_write(bio, out, out_len) >= 0 && BIO_flush(bio); - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_unlock(&ctx->lock); OPENSSL_free(out); return ret; @@ -2664,9 +2665,9 @@ int ssl_ctx_log_master_secret(SSL_CTX *ctx, const uint8_t *client_random, return 0; } - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_lock_write(&ctx->lock); ret = BIO_write(bio, out, out_len) >= 0 && BIO_flush(bio); - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_unlock(&ctx->lock); OPENSSL_free(out); return ret; diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index b358b5eb..9ab45850 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -295,13 +295,11 @@ int ssl_get_new_session(SSL *s, int session) { } /* Choose which callback will set the session ID */ - CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX); if (s->generate_session_id) { cb = s->generate_session_id; } else if (s->initial_ctx->generate_session_id) { cb = s->initial_ctx->generate_session_id; } - CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX); /* Choose a session ID */ tmp = ss->session_id_length; @@ -419,10 +417,14 @@ int ssl_get_prev_session(SSL *s, const struct ssl_early_callback_ctx *ctx) { return 0; } memcpy(data.session_id, ctx->session_id, ctx->session_id_len); - CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX); - ret = SSL_SESSION_up_ref(lh_SSL_SESSION_retrieve(s->initial_ctx->sessions, - &data)); - CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX); + + CRYPTO_MUTEX_lock_read(&s->initial_ctx->lock); + ret = lh_SSL_SESSION_retrieve(s->initial_ctx->sessions, &data); + CRYPTO_MUTEX_unlock(&s->initial_ctx->lock); + + if (ret != NULL) { + SSL_SESSION_up_ref(ret); + } } if (try_session_cache && ret == NULL && @@ -524,8 +526,9 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) { SSL_SESSION_up_ref(c); /* if session c is in already in cache, we take back the increment later */ - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_lock_write(&ctx->lock); if (!lh_SSL_SESSION_insert(ctx->sessions, &s, c)) { + CRYPTO_MUTEX_unlock(&ctx->lock); return 0; } @@ -566,7 +569,7 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) { } } - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_unlock(&ctx->lock); return ret; } @@ -580,7 +583,7 @@ static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lock) { if (c != NULL && c->session_id_length != 0) { if (lock) { - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_lock_write(&ctx->lock); } r = lh_SSL_SESSION_retrieve(ctx->sessions, c); if (r == c) { @@ -590,7 +593,7 @@ static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lock) { } if (lock) { - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_unlock(&ctx->lock); } if (ret) { @@ -741,18 +744,18 @@ static void timeout_doall_arg(SSL_SESSION *sess, void *void_param) { } } -void SSL_CTX_flush_sessions(SSL_CTX *s, long t) { +void SSL_CTX_flush_sessions(SSL_CTX *ctx, long t) { TIMEOUT_PARAM tp; - tp.ctx = s; - tp.cache = s->sessions; + tp.ctx = ctx; + tp.cache = ctx->sessions; if (tp.cache == NULL) { return; } tp.time = t; - CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_lock_write(&ctx->lock); lh_SSL_SESSION_doall_arg(tp.cache, timeout_doall_arg, &tp); - CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); + CRYPTO_MUTEX_unlock(&ctx->lock); } int ssl_clear_bad_session(SSL *s) {