From df3064cff7f28210540614e64dfa0126c3cf78c3 Mon Sep 17 00:00:00 2001 From: subodh Date: Sat, 15 Nov 2014 17:17:53 -0800 Subject: [PATCH 1/1] Improve initialization of openssl Summary: SSLContext takes it upon itself to perform initialization of openssl along with cleaning it up. However in applications where there are multiple libraries using openssl, cleanup is not really desirable. Thus the application should own init and cleanup of openssl, rather than SSLContext. This is bad because openssl's init apis are not thread safe. This diff modifies SSLContext with 3 changes to make this possible in the context of apps: 1. Add macro SSLCONTEXT_NO_REFCOUNT, for apps to be able to disable refcounting 2. Move all steps of init into initializeOpenSSL. As a result, apps can choose when to call SSLContext::intitializeOpenSSL and if it has been called before, openssl will not be reinitialized. 3. Move randomize as a static method to make it a part of the initialization process. Test Plan: Unit tests. Reviewed By: ranjeeth@fb.com Subscribers: ranjeeth, folly-diffs@, ssl-diffs@, njormrod, seanc, trunkagent, dihde FB internal diff: D1686397 Signature: t1:1686397:1416270565:de805794d65af1c164ed334ff7ba93fe60b2b78a --- folly/io/async/SSLContext.cpp | 50 +++++++++++++++++++++++++++-------- folly/io/async/SSLContext.h | 28 ++++++++++++++------ 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/folly/io/async/SSLContext.cpp b/folly/io/async/SSLContext.cpp index 6fcbeafa..106974b0 100644 --- a/folly/io/async/SSLContext.cpp +++ b/folly/io/async/SSLContext.cpp @@ -34,24 +34,24 @@ struct CRYPTO_dynlock_value { namespace folly { -uint64_t SSLContext::count_ = 0; +bool SSLContext::initialized_ = false; std::mutex SSLContext::mutex_; #ifdef OPENSSL_NPN_NEGOTIATED int SSLContext::sNextProtocolsExDataIndex_ = -1; +#endif +#ifndef SSLCONTEXT_NO_REFCOUNT +uint64_t SSLContext::count_ = 0; #endif + // SSLContext implementation SSLContext::SSLContext(SSLVersion version) { { std::lock_guard g(mutex_); - if (!count_++) { - initializeOpenSSL(); - randomize(); -#ifdef OPENSSL_NPN_NEGOTIATED - sNextProtocolsExDataIndex_ = SSL_get_ex_new_index(0, - (void*)"Advertised next protocol index", nullptr, nullptr, nullptr); +#ifndef SSLCONTEXT_NO_REFCOUNT + count_++; #endif - } + initializeOpenSSLLocked(); } ctx_ = SSL_CTX_new(SSLv23_method()); @@ -94,10 +94,14 @@ SSLContext::~SSLContext() { deleteNextProtocolsStrings(); #endif - std::lock_guard g(mutex_); - if (!--count_) { - cleanupOpenSSL(); +#ifndef SSLCONTEXT_NO_REFCOUNT + { + std::lock_guard g(mutex_); + if (!--count_) { + cleanupOpenSSLLocked(); + } } +#endif } void SSLContext::ciphers(const std::string& ciphers) { @@ -580,6 +584,14 @@ void SSLContext::setSSLLockTypes(std::map inLockTypes) { } void SSLContext::initializeOpenSSL() { + std::lock_guard g(mutex_); + initializeOpenSSLLocked(); +} + +void SSLContext::initializeOpenSSLLocked() { + if (initialized_) { + return; + } SSL_library_init(); SSL_load_error_strings(); ERR_load_crypto_strings(); @@ -594,9 +606,24 @@ void SSLContext::initializeOpenSSL() { CRYPTO_set_dynlock_create_callback(dyn_create); CRYPTO_set_dynlock_lock_callback(dyn_lock); CRYPTO_set_dynlock_destroy_callback(dyn_destroy); + randomize(); +#ifdef OPENSSL_NPN_NEGOTIATED + sNextProtocolsExDataIndex_ = SSL_get_ex_new_index(0, + (void*)"Advertised next protocol index", nullptr, nullptr, nullptr); +#endif + initialized_ = true; } void SSLContext::cleanupOpenSSL() { + std::lock_guard g(mutex_); + cleanupOpenSSLLocked(); +} + +void SSLContext::cleanupOpenSSLLocked() { + if (!initialized_) { + return; + } + CRYPTO_set_id_callback(nullptr); CRYPTO_set_locking_callback(nullptr); CRYPTO_set_dynlock_create_callback(nullptr); @@ -607,6 +634,7 @@ void SSLContext::cleanupOpenSSL() { EVP_cleanup(); ERR_remove_state(0); locks.reset(); + initialized_ = false; } void SSLContext::setOptions(long options) { diff --git a/folly/io/async/SSLContext.h b/folly/io/async/SSLContext.h index 5819f68f..df3e9e04 100644 --- a/folly/io/async/SSLContext.h +++ b/folly/io/async/SSLContext.h @@ -198,10 +198,6 @@ class SSLContext { * Load a client CA list for validating clients */ virtual void loadClientCAList(const char* path); - /** - * Default randomize method. - */ - virtual void randomize(); /** * Override default OpenSSL password collector. * @@ -388,6 +384,18 @@ class SSLContext { */ static bool matchName(const char* host, const char* pattern, int size); + /** + * Functions for setting up and cleaning up openssl. + * They can be invoked during the start of the application. + */ + static void initializeOpenSSL(); + static void cleanupOpenSSL(); + + /** + * Default randomize method. + */ + static void randomize(); + protected: SSL_CTX* ctx_; @@ -403,7 +411,11 @@ class SSLContext { #endif static std::mutex mutex_; + static bool initialized_; + +#ifndef SSLCONTEXT_NO_REFCOUNT static uint64_t count_; +#endif #ifdef OPENSSL_NPN_NEGOTIATED /** @@ -421,10 +433,6 @@ class SSLContext { static int passwordCallback(char* password, int size, int, void* data); - static void initializeOpenSSL(); - static void cleanupOpenSSL(); - - #if OPENSSL_VERSION_NUMBER >= 0x1000105fL && !defined(OPENSSL_NO_TLSEXT) /** * The function that will be called directly from openssl @@ -444,6 +452,10 @@ class SSLContext { #endif std::string providedCiphersString_; + + // Functions are called when locked by the calling function. + static void initializeOpenSSLLocked(); + static void cleanupOpenSSLLocked(); }; typedef std::shared_ptr SSLContextPtr; -- 2.34.1