[ZBX-10336] Fatal Crash with TLS and OpenSSL Created: 2016 Feb 02 Updated: 2017 May 30 Resolved: 2016 Feb 09 |
|
Status: | Closed |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | Agent (G), Proxy (P), Server (S) |
Affects Version/s: | 3.0.0beta2 |
Fix Version/s: | 3.0.0rc1, 3.0.0rc2 |
Type: | Incident report | Priority: | Major |
Reporter: | Kenneth Palmertree | Assignee: | Unassigned |
Resolution: | Fixed | Votes: | 0 |
Labels: | agent, crash | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Attachments: | zabbix_agentd_crash.log |
Description |
Zabbix Windows Agent crashes randomly when the windows agent is compiled with TLS support using OpenSSL 1.0.2f due to multi-thread crashing when calling OpenSSL library. |
Comments |
Comment by Kenneth Palmertree [ 2016 Feb 02 ] |
Zabbix Agent for windows will crash randomly when compiled with TLS support and OpenSSL. This can be prevented on a multi-threaded application by calling two callback functions in OpenSSL which are locking_function and threadid_func. More information can be found at https://www.openssl.org/docs/manmaster/crypto/threads.html. Our windows agent that we compiled with OpenSSL was randomly crashing a few times an hour. We fixed the issue with the following patch: diff -ruN zabbix-original/src/libs/zbxcrypto/tls.c zabbix/src/libs/zbxcrypto/tls.c --- zabbix-original/src/libs/zbxcrypto/tls.c 2016-02-01 22:38:28.255770461 -0500 +++ zabbix/src/libs/zbxcrypto/tls.c 2016-02-01 22:41:25.822032910 -0500 @@ -116,6 +116,81 @@ ZBX_THREAD_LOCAL char info_buf[256]; #endif +#if defined(HAVE_OPENSSL) +int THREAD_setup(void); +int THREAD_cleanup(void); + +#if defined(_WINDOWS) + #define MUTEX_TYPE HANDLE + #define MUTEX_SETUP(x) (x) = CreateMutex(NULL, FALSE, NULL) + #define MUTEX_CLEANUP(x) CloseHandle(x) + #define MUTEX_LOCK(x) WaitForSingleObject((x), INFINITE) + #define MUTEX_UNLOCK(x) ReleaseMutex(x) + #define THREAD_ID GetCurrentThreadId( ) +#elif _POSIX_THREADS + /* _POSIX_THREADS is normally defined in unistd.h if pthreads are available + on your platform. */ + #define MUTEX_TYPE pthread_mutex_t + #define MUTEX_SETUP(x) pthread_mutex_init(&(x), NULL) + #define MUTEX_CLEANUP(x) pthread_mutex_destroy(&(x)) + #define MUTEX_LOCK(x) pthread_mutex_lock(&(x)) + #define MUTEX_UNLOCK(x) pthread_mutex_unlock(&(x)) + #define THREAD_ID pthread_self( ) +#else + #error You must define mutex operations appropriate for your platform! +#endif + +/* This array will store all of the mutexes available to OpenSSL. */ +static MUTEX_TYPE mutex_buf[1000] = { NULL }; + +static void locking_function(int mode, int n, const char * file, int line) +{ + if (mode & CRYPTO_LOCK) + { + MUTEX_LOCK(mutex_buf[n]); + zabbix_log(LOG_LEVEL_TRACE, "In locking_function: LOCK thread_id=%d", mutex_buf[n]); + } + else + { + MUTEX_UNLOCK(mutex_buf[n]); + zabbix_log(LOG_LEVEL_TRACE, "In locking_function: UNLOCK thread_id=%d", mutex_buf[n]); + } +} + +static unsigned long id_function(void) +{ + zabbix_log(LOG_LEVEL_TRACE, "id_function: thread_id=%d", THREAD_ID); + return ((unsigned long)THREAD_ID); +} + +int THREAD_setup(void) +{ + int i; + + zabbix_log(LOG_LEVEL_TRACE, "In THREAD_setup: CRYPTO_num_locks=%d",CRYPTO_num_locks( )); + for (i = 0; i < CRYPTO_num_locks( ); i++) + MUTEX_SETUP(mutex_buf[i]); + CRYPTO_set_id_callback(id_function); + CRYPTO_set_locking_callback(locking_function); + return 1; +} + +int THREAD_cleanup(void) +{ + int i; + + zabbix_log(LOG_LEVEL_TRACE, "In THREAD_cleanup: CRYPTO_num_locks=%d",CRYPTO_num_locks( )); + if (!mutex_buf) + return 0; + CRYPTO_set_id_callback(NULL); + CRYPTO_set_locking_callback(NULL); + for (i = 0; i < CRYPTO_num_locks( ); i++) + MUTEX_CLEANUP(mutex_buf[i]); + free(mutex_buf); + return 1; +} +#endif + #if defined(HAVE_POLARSSL) /********************************************************************************** * * @@ -2444,6 +2519,7 @@ SSL_load_error_strings(); ERR_load_BIO_strings(); SSL_library_init(); /* always returns "1" */ + THREAD_setup(); init_done = 1; @@ -2474,6 +2550,7 @@ init_done = 0; RAND_cleanup(); /* erase PRNG state */ ERR_free_strings(); + THREAD_cleanup(); } #endif } |
Comment by Andris Mednis [ 2016 Feb 02 ] |
Problem reproduced (see attached zabbix_agentd_crash.log). |
Comment by Sandis Neilands (Inactive) [ 2016 Feb 02 ] |
On mbedTLS thread safety: https://tls.mbed.org/kb/development/thread-safety-and-multi-threading . |
Comment by Andris Mednis [ 2016 Feb 03 ] |
For OpenSSL fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10336 . |
Comment by Andris Mednis [ 2016 Feb 03 ] |
Available in version pre-3.0.0rc1 (trunk) rev. 58258. |
Comment by Sandis Neilands (Inactive) [ 2016 Feb 04 ] |
(1) Instead of OPENSSL_malloc() we can use our own zbx_malloc(). Also, we can use zbx_mutex_destroy() instead of CloseHandle(). RESOLVED in r58275 (development branch). andris Thanks! Reviewed and accepted. |
Comment by Andris Mednis [ 2016 Feb 05 ] |
No crashes observed with mbedTLS (PolarSSL) and GnuTLS so far. |
Comment by Andris Mednis [ 2016 Feb 08 ] |
Some coding style improvements proposed by asaveljevs in r58296. |
Comment by Sandis Neilands (Inactive) [ 2016 Feb 08 ] |
Successfully tested. |
Comment by Andris Mednis [ 2016 Feb 08 ] |
Available in version pre-3.0.0rc2 (trunk) rev. 58322. |
Comment by Andris Mednis [ 2016 Feb 08 ] |
No need to update documentation. |