[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: File 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 .
On GnuTLS thread safety: http://gnutls.org/manual/html_node/Thread-safety.html .

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.

Generated at Sat Apr 20 19:16:06 EEST 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.