From a505a6cd275ac1defd93f553f6fbc8da61cbd23b Mon Sep 17 00:00:00 2001 From: "zihao.jiang" Date: Mon, 26 Dec 2016 22:21:36 +0800 Subject: [PATCH] Winpr/openssl: Fix digests initialization in multi-thread SSL functions like OpenSSL_add_all_digests should be invoked at very beginning as they are not MT safe. If not we might meet double free exception as following: #0 0x00007f23ddd71c37 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007f23ddd75028 in abort () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x00007f23dddae2a4 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #3 0x00007f23dddba55e in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #4 0x00007f23dc6ecfcd in CRYPTO_free () from /lib/x86_64-linux-gnu/libcrypto.so.1.0.0 #5 0x00007f23dc6ef8d1 in OBJ_NAME_add () from /lib/x86_64-linux-gnu/libcrypto.so.1.0.0 #6 0x00007f23dc77dcd8 in EVP_add_digest () from /lib/x86_64-linux-gnu/libcrypto.so.1.0.0 #7 0x00007f23dc782321 in OpenSSL_add_all_digests () from /lib/x86_64-linux-gnu/libcrypto.so.1.0.0 #8 0x00007f23c781da28 in winpr_openssl_get_evp_md (md=4) at /home/zihao/workspace/zihao_FreeRDP/winpr/libwinpr/crypto/hash.c:52 #9 0x00007f23c781dccb in winpr_Digest_Init (ctx=0x7f22d064d470, md=) at /home/zihao/workspace/zihao_FreeRDP/winpr/libwinpr/crypto/hash.c:344 #10 0x00007f23d486139b in security_salted_mac_signature (rdp=0x7f23859f5a20, data=0x7f238542d4fb "\004\204\022\004", length=4743, encryption=, output=0x7 at /home/zihao/workspace/zihao_FreeRDP/libfreerdp/core/security.c:378 #11 0x00007f23d488d73f in fastpath_send_update_pdu (fastpath=, updateCode=4 '\004', s=0x7f23859f5f40, skipCompression=true) at /home/zihao/workspace/zihao_FreeRDP/libfreerdp/core/fastpath.c:1076 #12 0x00007f23d4891c4f in update_send_surface_frame_bits (context=0x7f23859f5540, cmd=0x7f22b2ffcc80, first=true, last=true, frameId=6) at /home/zihao/workspace/zihao_FreeRDP/libfreerdp/core/update.c:1041 Related reports: https://rt.openssl.org/Ticket/Display.html?id=2216&user=guest&pass=guest --- libfreerdp/common/test/CMakeLists.txt | 2 +- libfreerdp/common/test/TestCommonAssistance.c | 3 +++ libfreerdp/core/freerdp.c | 2 ++ winpr/libwinpr/crypto/cipher.c | 2 -- winpr/libwinpr/crypto/hash.c | 2 -- winpr/libwinpr/crypto/test/TestCryptoCipher.c | 3 +++ winpr/libwinpr/crypto/test/TestCryptoHash.c | 3 +++ winpr/libwinpr/crypto/test/TestCryptoProtectMemory.c | 2 ++ winpr/libwinpr/utils/ssl.c | 2 ++ 9 files changed, 16 insertions(+), 5 deletions(-) diff --git a/libfreerdp/common/test/CMakeLists.txt b/libfreerdp/common/test/CMakeLists.txt index 3247101cc..006af97d4 100644 --- a/libfreerdp/common/test/CMakeLists.txt +++ b/libfreerdp/common/test/CMakeLists.txt @@ -13,7 +13,7 @@ create_test_sourcelist(${MODULE_PREFIX}_SRCS add_executable(${MODULE_NAME} ${${MODULE_PREFIX}_SRCS}) -target_link_libraries(${MODULE_NAME} freerdp) +target_link_libraries(${MODULE_NAME} freerdp winpr) set_target_properties(${MODULE_NAME} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${TESTING_OUTPUT_DIRECTORY}") diff --git a/libfreerdp/common/test/TestCommonAssistance.c b/libfreerdp/common/test/TestCommonAssistance.c index 7c8d977f1..0e7b79244 100644 --- a/libfreerdp/common/test/TestCommonAssistance.c +++ b/libfreerdp/common/test/TestCommonAssistance.c @@ -1,6 +1,7 @@ #include #include +#include #include @@ -180,6 +181,8 @@ int test_msrsc_incident_file_type2() int TestCommonAssistance(int argc, char* argv[]) { + winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT); + if (test_msrsc_incident_file_type1() != 0) { printf("test_msrsc_incident_file_type1 failed\n"); diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index ee974ac2d..794337382 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -868,6 +869,7 @@ freerdp* freerdp_new() if (!instance) return NULL; + winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT); instance->ContextSize = sizeof(rdpContext); instance->SendChannelData = freerdp_send_channel_data; instance->ReceiveChannelData = freerdp_channels_data; diff --git a/winpr/libwinpr/crypto/cipher.c b/winpr/libwinpr/crypto/cipher.c index c60e07d17..72e13af6a 100644 --- a/winpr/libwinpr/crypto/cipher.c +++ b/winpr/libwinpr/crypto/cipher.c @@ -109,8 +109,6 @@ const EVP_CIPHER* winpr_openssl_get_evp_cipher(int cipher) { const EVP_CIPHER* evp = NULL; - OpenSSL_add_all_ciphers(); - switch (cipher) { case WINPR_CIPHER_NULL: diff --git a/winpr/libwinpr/crypto/hash.c b/winpr/libwinpr/crypto/hash.c index 199ca0bc8..949d687c4 100644 --- a/winpr/libwinpr/crypto/hash.c +++ b/winpr/libwinpr/crypto/hash.c @@ -49,8 +49,6 @@ const EVP_MD* winpr_openssl_get_evp_md(int md) { const EVP_MD* evp = NULL; - OpenSSL_add_all_digests(); - switch (md) { case WINPR_MD_MD2: diff --git a/winpr/libwinpr/crypto/test/TestCryptoCipher.c b/winpr/libwinpr/crypto/test/TestCryptoCipher.c index 1f99a3f93..c61d1b693 100644 --- a/winpr/libwinpr/crypto/test/TestCryptoCipher.c +++ b/winpr/libwinpr/crypto/test/TestCryptoCipher.c @@ -2,6 +2,7 @@ #include #include #include +#include @@ -216,6 +217,8 @@ static BOOL test_crypto_cipher_key() int TestCryptoCipher(int argc, char* argv[]) { + winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT); + if (!test_crypto_cipher_aes_128_cbc()) return -1; diff --git a/winpr/libwinpr/crypto/test/TestCryptoHash.c b/winpr/libwinpr/crypto/test/TestCryptoHash.c index a5101c81f..be944cc8f 100644 --- a/winpr/libwinpr/crypto/test/TestCryptoHash.c +++ b/winpr/libwinpr/crypto/test/TestCryptoHash.c @@ -2,6 +2,7 @@ #include #include #include +#include static const char* TEST_MD5_DATA = "test"; static const BYTE* TEST_MD5_HASH = (BYTE*) "\x09\x8f\x6b\xcd\x46\x21\xd3\x73\xca\xde\x4e\x83\x26\x27\xb4\xf6"; @@ -267,6 +268,8 @@ out: int TestCryptoHash(int argc, char* argv[]) { + winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT); + if (!test_crypto_hash_md5()) return -1; diff --git a/winpr/libwinpr/crypto/test/TestCryptoProtectMemory.c b/winpr/libwinpr/crypto/test/TestCryptoProtectMemory.c index 798130492..3e5690e70 100644 --- a/winpr/libwinpr/crypto/test/TestCryptoProtectMemory.c +++ b/winpr/libwinpr/crypto/test/TestCryptoProtectMemory.c @@ -2,6 +2,7 @@ #include #include #include +#include #include static const char* SECRET_PASSWORD_TEST = "MySecretPassword123!"; @@ -24,6 +25,7 @@ int TestCryptoProtectMemory(int argc, char* argv[]) } CopyMemory(pCipherText, pPlainText, cbPlainText); ZeroMemory(&pCipherText[cbPlainText], (cbCipherText - cbPlainText)); + winpr_InitializeSSL(WINPR_SSL_INIT_DEFAULT); if (!CryptProtectMemory(pCipherText, cbCipherText, CRYPTPROTECTMEMORY_SAME_PROCESS)) { diff --git a/winpr/libwinpr/utils/ssl.c b/winpr/libwinpr/utils/ssl.c index a2e0f2987..bb8d8aec4 100644 --- a/winpr/libwinpr/utils/ssl.c +++ b/winpr/libwinpr/utils/ssl.c @@ -257,6 +257,8 @@ static BOOL CALLBACK _winpr_openssl_initialize(PINIT_ONCE once, PVOID param, PVO SSL_load_error_strings(); /* SSL_library_init() always returns "1" */ SSL_library_init(); + OpenSSL_add_all_digests(); + OpenSSL_add_all_ciphers(); g_winpr_openssl_initialized_by_winpr = TRUE; return TRUE; }