From 821feeabb0b70a0d965c77d2a9af686013b660ab Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sat, 19 Nov 2016 22:49:03 -0800 Subject: [PATCH 1/3] GUACAMOLE-223: Always invoke SSL_free(). --- src/libguacd/socket-ssl.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libguacd/socket-ssl.c b/src/libguacd/socket-ssl.c index aa8664f3..cfd2f667 100644 --- a/src/libguacd/socket-ssl.c +++ b/src/libguacd/socket-ssl.c @@ -107,6 +107,7 @@ static int __guac_socket_ssl_free_handler(guac_socket* socket) { /* Shutdown SSL */ guac_socket_ssl_data* data = (guac_socket_ssl_data*) socket->data; SSL_shutdown(data->ssl); + SSL_free(data->ssl); /* Close file descriptor */ close(data->fd); @@ -117,23 +118,29 @@ static int __guac_socket_ssl_free_handler(guac_socket* socket) { guac_socket* guac_socket_open_secure(SSL_CTX* context, int fd) { + /* Create new SSL structure */ + SSL* ssl = SSL_new(context); + if (ssl == NULL) + return NULL; + /* Allocate socket and associated data */ guac_socket* socket = guac_socket_alloc(); guac_socket_ssl_data* data = malloc(sizeof(guac_socket_ssl_data)); /* Init SSL */ data->context = context; - data->ssl = SSL_new(context); + data->ssl = ssl; SSL_set_fd(data->ssl, fd); /* Accept SSL connection, handle errors */ - if (SSL_accept(data->ssl) <= 0) { + if (SSL_accept(ssl) <= 0) { guac_error = GUAC_STATUS_INTERNAL_ERROR; guac_error_message = "SSL accept failed"; free(data); guac_socket_free(socket); + SSL_free(ssl); return NULL; } From cf01d0d634ccc5b0639f46663d661a497480aa69 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sat, 19 Nov 2016 23:10:14 -0800 Subject: [PATCH 2/3] GUACAMOLE-223: Link against libcrypto as well as libssl. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index d7a4e4c1..f98670f0 100644 --- a/configure.ac +++ b/configure.ac @@ -225,7 +225,7 @@ then have_ssl=yes AC_CHECK_HEADER(openssl/ssl.h,, [have_ssl=no]) - AC_CHECK_LIB([ssl], [SSL_CTX_new], [SSL_LIBS="$SSL_LIBS -lssl"], [have_ssl=no]) + AC_CHECK_LIB([ssl], [SSL_CTX_new], [SSL_LIBS="$SSL_LIBS -lssl -lcrypto"], [have_ssl=no]) if test "x${have_ssl}" = "xno" then From 9218a79e62c3125f72b4d0ebd944d13cddc38c89 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sat, 19 Nov 2016 23:13:08 -0800 Subject: [PATCH 3/3] GUACAMOLE-223: Set required thread-related callbacks for OpenSSL. --- src/guacd/daemon.c | 106 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 105 insertions(+), 1 deletion(-) diff --git a/src/guacd/daemon.c b/src/guacd/daemon.c index 840c68f8..1ce1c2f9 100644 --- a/src/guacd/daemon.c +++ b/src/guacd/daemon.c @@ -152,6 +152,97 @@ static int daemonize() { } +#ifdef ENABLE_SSL +/** + * Array of mutexes, used by OpenSSL. + */ +static pthread_mutex_t* guacd_openssl_locks = NULL; + +/** + * Called by OpenSSL when locking or unlocking the Nth mutex. + * + * @param mode + * A bitmask denoting the action to be taken on the Nth lock, such as + * CRYPTO_LOCK or CRYPTO_UNLOCK. + * + * @param n + * The index of the lock to lock or unlock. + * + * @param file + * The filename of the function setting the lock, for debugging purposes. + * + * @param line + * The line number of the function setting the lock, for debugging + * purposes. + */ +static void guacd_openssl_locking_callback(int mode, int n, + const char* file, int line){ + + /* Lock given mutex upon request */ + if (mode & CRYPTO_LOCK) + pthread_mutex_lock(&(guacd_openssl_locks[n])); + + /* Unlock given mutex upon request */ + else if (mode & CRYPTO_UNLOCK) + pthread_mutex_unlock(&(guacd_openssl_locks[n])); + +} + +/** + * Called by OpenSSL when determining the current thread ID. + * + * @return + * An ID which uniquely identifies the current thread. + */ +static unsigned long guacd_openssl_id_callback() { + return (unsigned long) pthread_self(); +} + +/** + * Creates the given number of mutexes, such that OpenSSL will have at least + * this number of mutexes at its disposal. + * + * @param count + * The number of mutexes (locks) to create. + */ +static void guacd_openssl_init_locks(int count) { + + int i; + + /* Allocate required number of locks */ + guacd_openssl_locks = + malloc(sizeof(pthread_mutex_t) * count); + + /* Initialize each lock */ + for (i=0; i < count; i++) + pthread_mutex_init(&(guacd_openssl_locks[i]), NULL); + +} + +/** + * Frees the given number of mutexes. + * + * @param count + * The number of mutexes (locks) to free. + */ +static void guacd_openssl_free_locks(int count) { + + int i; + + /* SSL lock array was not initialized */ + if (guacd_openssl_locks == NULL) + return; + + /* Free all locks */ + for (i=0; i < count; i++) + pthread_mutex_destroy(&(guacd_openssl_locks[i])); + + /* Free lock array */ + free(guacd_openssl_locks); + +} +#endif + int main(int argc, char* argv[]) { /* Server */ @@ -266,8 +357,14 @@ int main(int argc, char* argv[]) { /* Init SSL if enabled */ if (config->key_file != NULL || config->cert_file != NULL) { - /* Init SSL */ guacd_log(GUAC_LOG_INFO, "Communication will require SSL/TLS."); + + /* Init threadsafety in OpenSSL */ + guacd_openssl_init_locks(CRYPTO_num_locks()); + CRYPTO_set_id_callback(guacd_openssl_id_callback); + CRYPTO_set_locking_callback(guacd_openssl_locking_callback); + + /* Init SSL */ SSL_library_init(); SSL_load_error_strings(); ssl_context = SSL_CTX_new(SSLv23_server_method()); @@ -391,6 +488,13 @@ int main(int argc, char* argv[]) { return 3; } +#ifdef ENABLE_SSL + if (ssl_context != NULL) { + guacd_openssl_free_locks(CRYPTO_num_locks()); + SSL_CTX_free(ssl_context); + } +#endif + return 0; }