From 6e04e42e061eee0d269d73b378a96ad91fc31ff8 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 2 Jul 2015 13:40:12 +0200 Subject: [PATCH] Openssl: add thread support and fix initialization [Problem] Openssl is used in multiple threads without proper thread support. Openssl initialization is scattered across several threads/files. [Solution] Lock and thread id callbacks registered. Openssl initialization refactored and fixed. [Verification] Run ckm-tests --output=text & ckm-tests-internal Change-Id: Iff26af6a0afd67001155aac040949bfde9cc6d31 --- src/manager/client-capi/ckmc-type.cpp | 5 +- src/manager/client/client-manager-impl.cpp | 2 +- src/manager/common/crypto-init.cpp | 131 ++++++++++++++++++++++++++-- src/manager/common/crypto-init.h | 5 +- src/manager/common/pkcs12-impl.cpp | 2 +- src/manager/crypto/sw-backend/internals.cpp | 28 ------ src/manager/crypto/sw-backend/internals.h | 6 -- src/manager/crypto/sw-backend/store.cpp | 2 - src/manager/main/key-manager-main.cpp | 16 +--- src/manager/main/service-thread.h | 5 ++ 10 files changed, 141 insertions(+), 61 deletions(-) diff --git a/src/manager/client-capi/ckmc-type.cpp b/src/manager/client-capi/ckmc-type.cpp index 94fad5e..c7458ab 100644 --- a/src/manager/client-capi/ckmc-type.cpp +++ b/src/manager/client-capi/ckmc-type.cpp @@ -34,6 +34,7 @@ #include #include #include +#include namespace { @@ -194,7 +195,7 @@ int ckmc_cert_new(unsigned char *raw_cert, size_t cert_size, ckmc_data_format_e KEY_MANAGER_CAPI int ckmc_load_cert_from_file(const char *file_path, ckmc_cert_s **cert) { - OpenSSL_add_all_algorithms(); + CKM::initOpenSslOnce(); FILE *fp = fopen(file_path, "r"); if(fp == NULL) @@ -394,7 +395,7 @@ int ckmc_load_from_pkcs12_file(const char *file_path, const char *passphrase, ck }; - OpenSSL_add_all_algorithms(); + CKM::initOpenSslOnce(); int ret = CKMC_ERROR_NONE; diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index 79774c1..17c2361 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -92,7 +92,7 @@ ManagerImpl::ManagerImpl() m_ocspConnection(SERVICE_SOCKET_OCSP), m_encryptionConnection(SERVICE_SOCKET_ENCRYPTION) { - initCryptoLib(); + initOpenSslOnce(); } diff --git a/src/manager/common/crypto-init.cpp b/src/manager/common/crypto-init.cpp index 47f99b7..9b1a7b7 100644 --- a/src/manager/common/crypto-init.cpp +++ b/src/manager/common/crypto-init.cpp @@ -20,25 +20,142 @@ */ #include "crypto-init.h" + #include -#include #include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include namespace CKM { +namespace { + +const char* DEV_HW_RANDOM_FILE = "/dev/hwrng"; +const char* DEV_URANDOM_FILE = "/dev/urandom"; +const size_t RANDOM_BUFFER_LEN = 32; + +std::mutex* g_mutexes = NULL; + +void lockingCallback(int mode, int type, const char*, int) +{ + if(!g_mutexes) { + LogError("Openssl mutexes do not exist"); + return; + } + + if (mode & CRYPTO_LOCK) + g_mutexes[type].lock(); + else if (mode & CRYPTO_UNLOCK) + g_mutexes[type].unlock(); +} + +unsigned long threadIdCallback() { + std::hash hasher; + return hasher(std::this_thread::get_id()); +} + +void opensslInstallLocks() +{ + g_mutexes = new std::mutex[CRYPTO_num_locks()]; + + CRYPTO_set_id_callback(threadIdCallback); + CRYPTO_set_locking_callback(lockingCallback); +} + +void opensslUninstallLocks() +{ + CRYPTO_set_id_callback(NULL); + CRYPTO_set_locking_callback(NULL); + + delete[] g_mutexes; + g_mutexes = NULL; +} + +} // namespace anonymous + + +void initOpenSsl() { + // Loads all error strings (crypto and ssl) + SSL_load_error_strings(); + + /* + * Initialize libcrypto (add all algorithms, digests & ciphers) + * It also does the stuff from SSL_library_init() except for ssl_load_ciphers() + */ + OpenSSL_add_all_algorithms(); // Can be optimized by using EVP_add_cipher instead + + /* + * Initialize libssl (OCSP uses it) + * SSL_library_init() == OpenSSL_add_ssl_algorithms() + * It always returns 1 + */ + SSL_library_init(); + + // load default configuration (/etc/ssl/openssl.cnf) + OPENSSL_config(NULL); + + // enable FIPS mode by default + if(0 == FIPS_mode_set(1)) { + LogError("Failed to set FIPS mode"); + } + + /* + * Initialize entropy + * entropy sources - /dev/random,/dev/urandom(Default) + */ + int ret = 0; + + std::ifstream ifile(DEV_HW_RANDOM_FILE); + if(ifile.is_open()) + ret= RAND_load_file(DEV_HW_RANDOM_FILE, RANDOM_BUFFER_LEN); + + if(ret != RANDOM_BUFFER_LEN ){ + LogWarning("Error in HW_RAND file load"); + ret = RAND_load_file(DEV_URANDOM_FILE, RANDOM_BUFFER_LEN); + + if(ret != RANDOM_BUFFER_LEN) + LogError("Error in U_RAND_file_load"); + } + + // Install locks for multithreading support + opensslInstallLocks(); +} + +void deinitOpenSsl() { + opensslUninstallLocks(); + CONF_modules_unload(1); + EVP_cleanup(); + ERR_free_strings(); + deinitOpenSslThread(); +} + +void deinitOpenSslThread() { + CRYPTO_cleanup_all_ex_data(); + ERR_remove_thread_state(NULL); +} namespace { std::mutex cryptoInitMutex; -void initOpenSSL(); +void initOpenSslAndDetach(); typedef void(*initFnPtr)(); // has to be atomic as storing function pointer is not an atomic operation on armv7l -std::atomic initFn (&initOpenSSL); +std::atomic initFn (&initOpenSslAndDetach); void initEmpty() {} -void initOpenSSL() { +void initOpenSslAndDetach() { // DCLP std::lock_guard lock(cryptoInitMutex); /* @@ -47,9 +164,7 @@ void initOpenSSL() { */ if(initFn.load(std::memory_order_relaxed) != &initEmpty) { - OpenSSL_add_all_ciphers(); - OpenSSL_add_all_algorithms(); - OpenSSL_add_all_digests(); + initOpenSsl(); /* * Synchronizes with load. Everything that happened before this store in this thread is @@ -62,7 +177,7 @@ void initOpenSSL() { } // namespace anonymous -void initCryptoLib() { +void initOpenSslOnce() { /* * Synchronizes with store. Everything that happened before store in another thread will be * visible in this thread after load. diff --git a/src/manager/common/crypto-init.h b/src/manager/common/crypto-init.h index 5845a12..e2419b1 100644 --- a/src/manager/common/crypto-init.h +++ b/src/manager/common/crypto-init.h @@ -24,7 +24,10 @@ namespace CKM { -COMMON_API void initCryptoLib(); +COMMON_API void initOpenSsl(); +COMMON_API void deinitOpenSsl(); +COMMON_API void deinitOpenSslThread(); +COMMON_API void initOpenSslOnce(); } // namespace CKM diff --git a/src/manager/common/pkcs12-impl.cpp b/src/manager/common/pkcs12-impl.cpp index 6c10ff1..6914698 100644 --- a/src/manager/common/pkcs12-impl.cpp +++ b/src/manager/common/pkcs12-impl.cpp @@ -69,7 +69,7 @@ PKCS12Impl::PKCS12Impl(const RawBuffer &buffer, const Password &password) } // needed if parsing is done before manager initialization - initCryptoLib(); + initOpenSslOnce(); if (!PKCS12_verify_mac(pkcs12, password.c_str(), password.size())) { LogDebug("Pkcs12 verify failed. Wrong password"); diff --git a/src/manager/crypto/sw-backend/internals.cpp b/src/manager/crypto/sw-backend/internals.cpp index ad646fb..658d6eb 100644 --- a/src/manager/crypto/sw-backend/internals.cpp +++ b/src/manager/crypto/sw-backend/internals.cpp @@ -19,7 +19,6 @@ * @version 1.0 */ #include -#include #include #include @@ -46,8 +45,6 @@ #define OPENSSL_SUCCESS 1 // DO NOTCHANGE THIS VALUE #define OPENSSL_FAIL 0 // DO NOTCHANGE THIS VALUE -#define DEV_HW_RANDOM_FILE "/dev/hwrng" -#define DEV_URANDOM_FILE "/dev/urandom" namespace CKM { namespace Crypto { @@ -284,31 +281,6 @@ InitCipherFn selectCipher(AlgoType type, size_t key_len = 32, bool encryption = } // anonymous namespace -int initialize() { - int hw_rand_ret = 0; - int u_rand_ret = 0; - - // try to initialize using ERR_load_crypto_strings and OpenSSL_add_all_algorithms - ERR_load_crypto_strings(); - OpenSSL_add_all_algorithms(); - - // initialize entropy - std::ifstream ifile(DEV_HW_RANDOM_FILE); - if(ifile.is_open()) { - u_rand_ret= RAND_load_file(DEV_HW_RANDOM_FILE, 32); - } - if(u_rand_ret != 32 ){ - LogError("Error in HW_RAND file load"); - hw_rand_ret = RAND_load_file(DEV_URANDOM_FILE, 32); - - if(hw_rand_ret != 32) { - ThrowErr(Exc::Crypto::InternalError, "Error in U_RAND_file_load"); - } - } - - return CKM_CRYPTO_INIT_SUCCESS; -} - const EVP_MD *getMdAlgo(const HashAlgorithm hashAlgo) { const EVP_MD *md_algo=NULL; switch(hashAlgo) { diff --git a/src/manager/crypto/sw-backend/internals.h b/src/manager/crypto/sw-backend/internals.h index df3b245..ebd14de 100644 --- a/src/manager/crypto/sw-backend/internals.h +++ b/src/manager/crypto/sw-backend/internals.h @@ -39,12 +39,6 @@ namespace Crypto { namespace SW { namespace Internals { -// During initialization, FIPS_MODE and the entropy source are set -// and system certificates are loaded to memory. -// FIPS_MODE - ON, OFF(Default) -// entropy source - /dev/random,/dev/urandom(Default) -int initialize(); - TokenPair createKeyPairRSA(CryptoBackend backendId, const int size); TokenPair createKeyPairDSA(CryptoBackend backendId, const int size); TokenPair createKeyPairECDSA(CryptoBackend backendId, ElipticCurve type1); diff --git a/src/manager/crypto/sw-backend/store.cpp b/src/manager/crypto/sw-backend/store.cpp index 4d6b1bf..77bcb7a 100644 --- a/src/manager/crypto/sw-backend/store.cpp +++ b/src/manager/crypto/sw-backend/store.cpp @@ -41,8 +41,6 @@ namespace SW { Store::Store(CryptoBackend backendId) : GStore(backendId) { - // initialize openssl internals - Internals::initialize(); } GKeyUPtr Store::getKey(const Token &token) { diff --git a/src/manager/main/key-manager-main.cpp b/src/manager/main/key-manager-main.cpp index d1e486e..67a6631 100644 --- a/src/manager/main/key-manager-main.cpp +++ b/src/manager/main/key-manager-main.cpp @@ -22,11 +22,6 @@ #include #include -#include -#include -#include -#include - #include #include @@ -35,6 +30,7 @@ #include #include #include +#include #include #include @@ -88,10 +84,7 @@ int main(void) { } LogInfo("Init external libraries SKMM and openssl"); - SSL_load_error_strings(); - SSL_library_init(); - OpenSSL_add_all_ciphers(); - OPENSSL_config(NULL); + CKM::initOpenSsl(); CKM::KeyProvider::initializeLibrary(); @@ -108,9 +101,8 @@ int main(void) { // Manager has been destroyed and we may close external libraries. LogInfo("Deinit SKMM and openssl"); CKM::KeyProvider::closeLibrary(); - // Deinit OPENSSL ? - EVP_cleanup(); - ERR_free_strings(); + + CKM::deinitOpenSsl(); } catch (const std::runtime_error& e) { diff --git a/src/manager/main/service-thread.h b/src/manager/main/service-thread.h index 2b0609b..32087b7 100644 --- a/src/manager/main/service-thread.h +++ b/src/manager/main/service-thread.h @@ -34,6 +34,8 @@ #include #include +#include + #include #include @@ -95,6 +97,9 @@ protected: static void ThreadLoopStatic(ServiceThread *ptr) { ptr->ThreadLoop(); + + // cleanup openssl in every thread + deinitOpenSslThread(); } void ThreadLoop(){ -- 2.7.4