diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 29054bad7b..4e21057d0f 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2887,6 +2887,16 @@ keep_going: /* We will come back to here until there is #ifdef USE_SSL + /* + * Enable the libcrypto callbacks before checking if SSL needs + * to be done. This is done before sending the startup packet + * as depending on the type of authentication done, like MD5 + * or SCRAM that use cryptohashes, the callbacks would be + * required even without a SSL connection + */ + if (pqsecure_initialize(conn, false, true) < 0) + goto error_return; + /* * If SSL is enabled and we haven't already got encryption of * some sort running, request SSL instead of sending the @@ -2998,8 +3008,14 @@ 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 if libpq took care of + * doing that, 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..9c2222c1d1 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,22 +684,28 @@ 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 */ #endif /* ENABLE_THREAD_SAFETY */ - if (!ssl_lib_initialized) + if (!ssl_lib_initialized && do_ssl) { if (pq_init_ssl_lib) { @@ -740,10 +746,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 @@ -1402,46 +1408,63 @@ 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; + 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. + */ - SSL_shutdown(conn->ssl); - SSL_free(conn->ssl); - conn->ssl = NULL; - conn->ssl_in_use = false; - } + SSL_shutdown(conn->ssl); + SSL_free(conn->ssl); + conn->ssl = NULL; + conn->ssl_in_use = false; - if (conn->peer) - { - X509_free(conn->peer); - conn->peer = NULL; - } + 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 + } + else + { + /* + * In the non-SSL case, just remove the crypto callbacks if the + * connection has then loaded. This code path has no dependency + * on any pending SSL calls. + */ + if (conn->crypto_loaded) + destroy_needed = true; + } /* - * 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. + * This will remove our crypto locking hooks if this is the last + * connection using libcrypto which means we must wait to call it until + * after all the potential 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(); + 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 adf149a76f..2f052f61f8 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -486,6 +486,11 @@ 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 locking is handled + * internally in OpenSSL >= 1.1.0. */ #endif /* USE_OPENSSL */ #endif /* USE_SSL */ @@ -667,7 +672,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); @@ -696,11 +701,13 @@ extern void pgtls_init_library(bool do_ssl, int do_crypto); * Initialize SSL library. * * The conn parameter is only used to be able to pass back an error - * message - no connection-local setup is made here. + * message - no connection-local setup is made here. do_ssl controls + * if SSL is initialized, and do_crypto does the same for the crypto + * part. * * 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.