diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index db71fea169..1c05fa44a8 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2882,6 +2882,13 @@ keep_going: /* We will come back to here until there is } #endif + /* + * Enable the crypto callbacks before checking if SSL needs + * to be done and before sending the startup packet. + */ + if (pqsecure_initialize(conn, false, true) < 0) + goto error_return; + #ifdef USE_SSL /* @@ -2999,8 +3006,12 @@ keep_going: /* We will come back to here until there is { /* mark byte consumed */ conn->inStart = conn->inCursor; - /* Set up global SSL state if required */ - if (pqsecure_initialize(conn) != 0) + /* + * Set up global SSL state if required. The crypto + * state has already been set, so there is no need + * to make that happen again. + */ + if (pqsecure_initialize(conn, true, false) != 0) goto error_return; } else if (SSLok == 'N') diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index 0fa10a23b4..8b6f860d22 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -85,7 +85,7 @@ static bool pq_init_crypto_lib = true; static bool ssl_lib_initialized = false; #ifdef ENABLE_THREAD_SAFETY -static long ssl_open_connections = 0; +static long crypto_open_connections = 0; #ifndef WIN32 static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -111,7 +111,7 @@ pgtls_init_library(bool do_ssl, int do_crypto) * Disallow changing the flags while we have open connections, else we'd * get completely confused. */ - if (ssl_open_connections != 0) + if (crypto_open_connections != 0) return; #endif @@ -635,7 +635,7 @@ pq_lockingcallback(int mode, int n, const char *file, int line) * override it. */ int -pgtls_init(PGconn *conn) +pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto) { #ifdef ENABLE_THREAD_SAFETY #ifdef WIN32 @@ -684,16 +684,21 @@ pgtls_init(PGconn *conn) } } - if (ssl_open_connections++ == 0) + if (do_crypto && !conn->crypto_loaded) { - /* - * These are only required for threaded libcrypto applications, - * but make sure we don't stomp on them if they're already set. - */ - if (CRYPTO_get_id_callback() == NULL) - CRYPTO_set_id_callback(pq_threadidcallback); - if (CRYPTO_get_locking_callback() == NULL) - CRYPTO_set_locking_callback(pq_lockingcallback); + if (crypto_open_connections++ == 0) + { + /* + * These are only required for threaded libcrypto applications, + * but make sure we don't stomp on them if they're already set. + */ + if (CRYPTO_get_id_callback() == NULL) + CRYPTO_set_id_callback(pq_threadidcallback); + if (CRYPTO_get_locking_callback() == NULL) + CRYPTO_set_locking_callback(pq_lockingcallback); + } + + conn->crypto_loaded = true; } } #endif /* HAVE_CRYPTO_LOCK */ @@ -701,17 +706,20 @@ pgtls_init(PGconn *conn) if (!ssl_lib_initialized) { - if (pq_init_ssl_lib) + if (do_ssl) { + if (pq_init_ssl_lib) + { #ifdef HAVE_OPENSSL_INIT_SSL - OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL); + OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL); #else - OPENSSL_config(NULL); - SSL_library_init(); - SSL_load_error_strings(); + OPENSSL_config(NULL); + SSL_library_init(); + SSL_load_error_strings(); #endif + } + ssl_lib_initialized = true; } - ssl_lib_initialized = true; } #ifdef ENABLE_THREAD_SAFETY @@ -740,10 +748,10 @@ destroy_ssl_system(void) if (pthread_mutex_lock(&ssl_config_mutex)) return; - if (pq_init_crypto_lib && ssl_open_connections > 0) - --ssl_open_connections; + if (pq_init_crypto_lib && crypto_open_connections > 0) + --crypto_open_connections; - if (pq_init_crypto_lib && ssl_open_connections == 0) + if (pq_init_crypto_lib && crypto_open_connections == 0) { /* * No connections left, unregister libcrypto callbacks, if no one @@ -1400,48 +1408,68 @@ open_client_SSL(PGconn *conn) void pgtls_close(PGconn *conn) { - bool destroy_needed = false; - - if (conn->ssl) + if (conn->ssl_in_use) { - /* - * We can't destroy everything SSL-related here due to the possible - * later calls to OpenSSL routines which may need our thread - * callbacks, so set a flag here and check at the end. - */ - destroy_needed = true; + bool destroy_needed = false; - SSL_shutdown(conn->ssl); - SSL_free(conn->ssl); - conn->ssl = NULL; - conn->ssl_in_use = false; - } + if (conn->ssl) + { + /* + * We can't destroy everything SSL-related here due to the possible + * later calls to OpenSSL routines which may need our thread + * callbacks, so set a flag here and check at the end. + */ - if (conn->peer) - { - X509_free(conn->peer); - conn->peer = NULL; - } + SSL_shutdown(conn->ssl); + SSL_free(conn->ssl); + conn->ssl = NULL; + conn->ssl_in_use = false; + + destroy_needed = true; + } + + if (conn->peer) + { + X509_free(conn->peer); + conn->peer = NULL; + } #ifdef USE_SSL_ENGINE - if (conn->engine) - { - ENGINE_finish(conn->engine); - ENGINE_free(conn->engine); - conn->engine = NULL; - } + if (conn->engine) + { + ENGINE_finish(conn->engine); + ENGINE_free(conn->engine); + conn->engine = NULL; + } #endif - /* - * This will remove our SSL locking hooks, if this is the last SSL - * connection, which means we must wait to call it until after all SSL - * calls have been made, otherwise we can end up with a race condition and - * possible deadlocks. - * - * See comments above destroy_ssl_system(). - */ - if (destroy_needed) - destroy_ssl_system(); + /* + * This will remove our crypto locking hooks, if this is the last SSL + * connection, which means we must wait to call it until after all SSL + * calls have been made, otherwise we can end up with a race condition + * and possible deadlocks. + * + * See comments above destroy_ssl_system(). + */ + if (destroy_needed && conn->crypto_loaded) + { + destroy_ssl_system(); + conn->crypto_loaded = false; + } + } + else + { + /* + * In the non-SSL case, just remove the crypto callbacks if they + * are set. This will be done once this is the last connection + * to handle. + */ + if (conn->crypto_loaded) + { + destroy_ssl_system(); + conn->crypto_loaded = false; + } + } } diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index c601071838..b15d8d137c 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -159,12 +159,12 @@ PQinitOpenSSL(int do_ssl, int do_crypto) * Initialize global SSL context */ int -pqsecure_initialize(PGconn *conn) +pqsecure_initialize(PGconn *conn, bool do_ssl, bool do_crypto) { int r = 0; #ifdef USE_SSL - r = pgtls_init(conn); + r = pgtls_init(conn, do_ssl, do_crypto); #endif return r; @@ -191,8 +191,7 @@ void pqsecure_close(PGconn *conn) { #ifdef USE_SSL - if (conn->ssl_in_use) - pgtls_close(conn); + pgtls_close(conn); #endif } diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index ce36aabd25..fd42885153 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -485,6 +485,12 @@ struct pg_conn void *engine; /* dummy field to keep struct the same if * OpenSSL version changes */ #endif + bool crypto_loaded; /* Track if libcrypto locking callbacks + * have been done for this connection. + * This can be removed once support for + * OpenSSL 1.0.2 is removed as this + * this locking is handled internally in + * OpenSSL >= 1.1.0. */ #endif /* USE_OPENSSL */ #endif /* USE_SSL */ @@ -682,7 +688,7 @@ extern int pqWriteReady(PGconn *conn); /* === in fe-secure.c === */ -extern int pqsecure_initialize(PGconn *); +extern int pqsecure_initialize(PGconn *, bool, bool); extern PostgresPollingStatusType pqsecure_open_client(PGconn *); extern void pqsecure_close(PGconn *); extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len); @@ -715,7 +721,7 @@ extern void pgtls_init_library(bool do_ssl, int do_crypto); * * Returns 0 if OK, -1 on failure (adding a message to conn->errorMessage). */ -extern int pgtls_init(PGconn *conn); +extern int pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto); /* * Begin or continue negotiating a secure session.