Commit cf15ac8aa96304a36699ae65398b7adac0d2ddde

Carlos Martín Nieto 2014-06-12T03:20:34

ssl: cargo-cult thread safety OpenSSL's tests init everything in the main thread, so let's do that.

diff --git a/src/global.c b/src/global.c
index fe41058..e9c940f 100644
--- a/src/global.c
+++ b/src/global.c
@@ -16,6 +16,11 @@ git_mutex git__mwindow_mutex;
 
 #define MAX_SHUTDOWN_CB 8
 
+#ifdef GIT_SSL
+# include <openssl/ssl.h>
+SSL_CTX *git__ssl_ctx;
+#endif
+
 git_mutex git__ssl_mutex;
 git_atomic git__ssl_init;
 
@@ -160,6 +165,15 @@ static pthread_key_t _tls_key;
 static pthread_once_t _once_init = PTHREAD_ONCE_INIT;
 int init_error = 0;
 
+static void init_ssl(void)
+{
+#ifdef GIT_SSL
+	SSL_load_error_strings();
+	OpenSSL_add_ssl_algorithms();
+	git__ssl_ctx = SSL_CTX_new(SSLv23_method());
+#endif
+}
+
 static void cb__free_status(void *st)
 {
 	git__free(st);
@@ -169,12 +183,18 @@ static void init_once(void)
 {
 	if ((init_error = git_mutex_init(&git__mwindow_mutex)) != 0)
 		return;
+	if ((init_error = git_mutex_init(&git__ssl_mutex)) != 0)
+		return;
 	pthread_key_create(&_tls_key, &cb__free_status);
 
+
 	/* Initialize any other subsystems that have global state */
 	if ((init_error = git_hash_global_init()) >= 0)
 		init_error = git_sysdir_global_init();
 
+	/* OpenSSL needs to be initialized from the main thread */
+	init_ssl();
+
 	GIT_MEMORY_BARRIER;
 }
 
diff --git a/src/global.h b/src/global.h
index 245f811..8904e2d 100644
--- a/src/global.h
+++ b/src/global.h
@@ -15,6 +15,11 @@ typedef struct {
 	git_error error_t;
 } git_global_st;
 
+#ifdef GIT_SSL
+# include <openssl/ssl.h>
+extern SSL_CTX *git__ssl_ctx;
+#endif
+
 git_global_st *git__global_state(void);
 
 extern git_mutex git__mwindow_mutex;
diff --git a/src/netops.c b/src/netops.c
index ba1a329..54804d4 100644
--- a/src/netops.c
+++ b/src/netops.c
@@ -163,7 +163,7 @@ void gitno_buffer_setup_callback(
 void gitno_buffer_setup(gitno_socket *socket, gitno_buffer *buf, char *data, size_t len)
 {
 #ifdef GIT_SSL
-	if (socket->ssl.ctx) {
+	if (socket->ssl.ssl) {
 		gitno_buffer_setup_callback(socket, buf, data, len, gitno__recv_ssl, NULL);
 		return;
 	}
@@ -208,7 +208,6 @@ static int gitno_ssl_teardown(gitno_ssl *ssl)
 		ret = 0;
 
 	SSL_free(ssl->ssl);
-	SSL_CTX_free(ssl->ctx);
 	return ret;
 }
 
@@ -428,30 +427,39 @@ static int init_ssl(void)
 	if (git__ssl_init.val)
 		return 0;
 
-
-	SSL_library_init();
-	SSL_load_error_strings();
+	SSL_CTX_set_mode(git__ssl_ctx, SSL_MODE_AUTO_RETRY);
+	SSL_CTX_set_verify(git__ssl_ctx, SSL_VERIFY_NONE, NULL);
+	if (!SSL_CTX_set_default_verify_paths(git__ssl_ctx)) {
+		unsigned long err = ERR_get_error();
+		giterr_set(GITERR_SSL, "failed to set verify paths: %s\n", ERR_error_string(err, NULL));
+		return -1;
+	}
 
 #ifdef GIT_THREADS
 	{
 		int num_locks, i;
 
-
-		CRYPTO_set_locking_callback(openssl_locking_function);
-
 		num_locks = CRYPTO_num_locks();
 		openssl_locks = git__calloc(num_locks, sizeof(git_mutex));
+		if (openssl_locks == NULL) {
+			git_mutex_unlock(&git__ssl_mutex);
+			return -1;
+		}
+		GITERR_CHECK_ALLOC(openssl_locks);
+
 		for (i = 0; i < num_locks; i++) {
-			if (git_mutex_init(&openssl_locks[i]) < 0) {
+			if (git_mutex_init(&openssl_locks[i]) != 0) {
 				git_mutex_unlock(&git__ssl_mutex);
 				giterr_set(GITERR_SSL, "failed to init lock %d", i);
 				return -1;
 			}
 		}
 	}
+
+	CRYPTO_set_locking_callback(openssl_locking_function);
 #endif
 
-	git_atomic_set(&git__ssl_init, 1);
+	git_atomic_inc(&git__ssl_init);
 	git_mutex_unlock(&git__ssl_mutex);
 
 	return 0;
@@ -464,16 +472,7 @@ static int ssl_setup(gitno_socket *socket, const char *host, int flags)
 	if (init_ssl() < 0)
 		return -1;
 
-	socket->ssl.ctx = SSL_CTX_new(SSLv23_method());
-	if (socket->ssl.ctx == NULL)
-		return ssl_set_error(&socket->ssl, 0);
-
-	SSL_CTX_set_mode(socket->ssl.ctx, SSL_MODE_AUTO_RETRY);
-	SSL_CTX_set_verify(socket->ssl.ctx, SSL_VERIFY_NONE, NULL);
-	if (!SSL_CTX_set_default_verify_paths(socket->ssl.ctx))
-		return ssl_set_error(&socket->ssl, 0);
-
-	socket->ssl.ssl = SSL_new(socket->ssl.ctx);
+	socket->ssl.ssl = SSL_new(git__ssl_ctx);
 	if (socket->ssl.ssl == NULL)
 		return ssl_set_error(&socket->ssl, 0);
 
@@ -610,7 +609,7 @@ int gitno_send(gitno_socket *socket, const char *msg, size_t len, int flags)
 	size_t off = 0;
 
 #ifdef GIT_SSL
-	if (socket->ssl.ctx)
+	if (socket->ssl.ssl)
 		return gitno_send_ssl(&socket->ssl, msg, len, flags);
 #endif
 
@@ -631,7 +630,7 @@ int gitno_send(gitno_socket *socket, const char *msg, size_t len, int flags)
 int gitno_close(gitno_socket *s)
 {
 #ifdef GIT_SSL
-	if (s->ssl.ctx &&
+	if (s->ssl.ssl &&
 		gitno_ssl_teardown(&s->ssl) < 0)
 		return -1;
 #endif
diff --git a/src/netops.h b/src/netops.h
index 8e3a252..dfb4ab7 100644
--- a/src/netops.h
+++ b/src/netops.h
@@ -16,7 +16,6 @@
 
 struct gitno_ssl {
 #ifdef GIT_SSL
-	SSL_CTX *ctx;
 	SSL *ssl;
 #else
 	size_t dummy;