Properly unregister OpenSSL callbacks when libpq is done with

it's connection. This is required for applications that unload
the libpq library (such as PHP) in which case we'd otherwise
have pointers to these functions when they no longer exist.

This needs a bit more testing before we can consider a backpatch,
so not doing that yet.

In passing, remove unused functions in backend/libpq.

Bruce Momjian and Magnus Hagander, per report and analysis
by Russell Smith.

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index fd7e703c526ab19b86b27ca3dd42bc4c8d634c2c..db41179b78e0685ec91a81f10a7c80e00c531be9 100644 (file)
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.86 2008/11/20 09:29:36 mha Exp $
+ *   $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.87 2008/12/03 20:04:26 mha Exp $
  *
  *   Since the server static private key ($DataDir/server.key)
  *   will normally be stored unencrypted so that the database
@@ -88,7 +88,6 @@ static DH  *tmp_dh_cb(SSL *s, int is_export, int keylength);
 static int verify_cb(int, X509_STORE_CTX *);
 static void info_cb(const SSL *ssl, int type, int args);
 static void initialize_SSL(void);
-static void destroy_SSL(void);
 static int open_server_SSL(Port *);
 static void close_SSL(Port *);
 static const char *SSLerrmessage(void);
@@ -192,17 +191,6 @@ secure_initialize(void)
    return 0;
 }
 
-/*
- * Destroy global context
- */
-void
-secure_destroy(void)
-{
-#ifdef USE_SSL
-   destroy_SSL();
-#endif
-}
-
 /*
  * Indicate if we have loaded the root CA store to verify certificates
  */
@@ -843,19 +831,6 @@ initialize_SSL(void)
    }
 }
 
-/*
- * Destroy global SSL context.
- */
-static void
-destroy_SSL(void)
-{
-   if (SSL_context)
-   {
-       SSL_CTX_free(SSL_context);
-       SSL_context = NULL;
-   }
-}
-
 /*
  * Attempt to negotiate SSL connection.
  */

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 5d1747821bf29006b7d2a23cc11b739450402340..8c4c463bea2b47e20297f60564867dcdd16da2dc 100644 (file)
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.110 2008/12/02 10:39:30 mha Exp $
+ *   $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.111 2008/12/03 20:04:26 mha Exp $
  *
  * NOTES
  *
@@ -44,6 +44,7 @@
 #endif
 #include <arpa/inet.h>
 #endif
+
 #include <sys/stat.h>
 
 #ifdef ENABLE_THREAD_SAFETY
@@ -89,20 +90,32 @@ static bool verify_peer_name_matches_certificate(PGconn *);
 static int verify_cb(int ok, X509_STORE_CTX *ctx);
 static int client_cert_cb(SSL *, X509 **, EVP_PKEY **);
 static int init_ssl_system(PGconn *conn);
+static void destroy_ssl_system(void);
 static int initialize_SSL(PGconn *);
-static void destroy_SSL(void);
+static void destroySSL(void);
 static PostgresPollingStatusType open_client_SSL(PGconn *);
 static void close_SSL(PGconn *);
 static char *SSLerrmessage(void);
 static void SSLerrfree(char *buf);
-#endif
 
-#ifdef USE_SSL
 static bool pq_initssllib = true;
-
 static SSL_CTX *SSL_context = NULL;
+
+#ifdef ENABLE_THREAD_SAFETY
+static int ssl_open_connections = 0;
+
+#ifndef WIN32
+static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
+#else
+static pthread_mutex_t ssl_config_mutex = NULL;
+static long win32_ssl_create_mutex = 0;
 #endif
 
+#endif /* ENABLE_THREAD_SAFETY */
+
+#endif /* SSL */
+
+
 /*
  * Macros to handle disabling and then restoring the state of SIGPIPE handling.
  * Note that DISABLE_SIGPIPE() must appear at the start of a block.
@@ -186,7 +199,7 @@ void
 pqsecure_destroy(void)
 {
 #ifdef USE_SSL
-   destroy_SSL();
+   destroySSL();
 #endif
 }
 
@@ -734,6 +747,9 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 }
 
 #ifdef ENABLE_THREAD_SAFETY
+/*
+ * Callback functions for OpenSSL internal locking
+ */
 
 static unsigned long
 pq_threadidcallback(void)
@@ -765,54 +781,74 @@ pq_lockingcallback(int mode, int n, const char *file, int line)
 #endif   /* ENABLE_THREAD_SAFETY */
 
 /*
- * Also see similar code in fe-connect.c, default_threadlock()
+ * Initialize SSL system. In threadsafe mode, this includes setting
+ * up OpenSSL callback functions to do thread locking.
+ *
+ * If the caller has told us (through PQinitSSL) that he's taking care
+ * of SSL, we expect that callbacks are already set, and won't try to
+ * override it.
+ *
+ * The conn parameter is only used to be able to pass back an error
+ * message - no connection local setup is made.
  */
 static int
 init_ssl_system(PGconn *conn)
 {
 #ifdef ENABLE_THREAD_SAFETY
-#ifndef WIN32
-   static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
-#else
-   static pthread_mutex_t init_mutex = NULL;
-   static long mutex_initlock = 0;
-
-   if (init_mutex == NULL)
+#ifdef WIN32
+   /* Also see similar code in fe-connect.c, default_threadlock() */
+   if (ssl_config_mutex == NULL)
    {
-       while (InterlockedExchange(&mutex_initlock, 1) == 1)
+       while (InterlockedExchange(&win32_ssl_create_mutex, 1) == 1)
             /* loop, another thread own the lock */ ;
-       if (init_mutex == NULL)
+       if (ssl_config_mutex == NULL)
        {
-           if (pthread_mutex_init(&init_mutex, NULL))
+           if (pthread_mutex_init(&ssl_config_mutex, NULL))
                return -1;
        }
-       InterlockedExchange(&mutex_initlock, 0);
+       InterlockedExchange(&win32_ssl_create_mutex, 0);
    }
 #endif
-   if (pthread_mutex_lock(&init_mutex))
+   if (pthread_mutex_lock(&ssl_config_mutex))
        return -1;
 
-   if (pq_initssllib && pq_lockarray == NULL)
+   if (pq_initssllib)
    {
-       int         i;
-
-       CRYPTO_set_id_callback(pq_threadidcallback);
-
-       pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
-       if (!pq_lockarray)
-       {
-           pthread_mutex_unlock(&init_mutex);
-           return -1;
-       }
-       for (i = 0; i < CRYPTO_num_locks(); i++)
+       /*
+        * If necessary, set up an array to hold locks for OpenSSL. OpenSSL will
+        * tell us how big to make this array.
+        */
+       if (pq_lockarray == NULL)
        {
-           if (pthread_mutex_init(&pq_lockarray[i], NULL))
+           int i;
+
+           pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
+           if (!pq_lockarray)
+           {
+               pthread_mutex_unlock(&ssl_config_mutex);
                return -1;
+           }
+           for (i = 0; i < CRYPTO_num_locks(); i++)
+           {
+               if (pthread_mutex_init(&pq_lockarray[i], NULL))
+               {
+                   free(pq_lockarray);
+                   pq_lockarray = NULL;
+                   pthread_mutex_unlock(&ssl_config_mutex);
+                   return -1;
+               }
+           }
        }
 
-       CRYPTO_set_locking_callback(pq_lockingcallback);
+       if (ssl_open_connections++ == 0)
+       {
+           /* These are only required for threaded SSL applications */
+           CRYPTO_set_id_callback(pq_threadidcallback);
+           CRYPTO_set_locking_callback(pq_lockingcallback);
+       }
    }
-#endif
+#endif /* ENABLE_THREAD_SAFETY */
+
    if (!SSL_context)
    {
        if (pq_initssllib)
@@ -833,19 +869,67 @@ init_ssl_system(PGconn *conn)
                              err);
            SSLerrfree(err);
 #ifdef ENABLE_THREAD_SAFETY
-           pthread_mutex_unlock(&init_mutex);
+           pthread_mutex_unlock(&ssl_config_mutex);
 #endif
            return -1;
        }
    }
+
 #ifdef ENABLE_THREAD_SAFETY
-   pthread_mutex_unlock(&init_mutex);
+   pthread_mutex_unlock(&ssl_config_mutex);
 #endif
    return 0;
 }
 
 /*
- * Initialize global SSL context.
+ * This function is needed because if the libpq library is unloaded
+ * from the application, the callback functions will no longer exist when
+ * SSL used by other parts of the system.  For this reason,
+ * we unregister the SSL callback functions when the last libpq
+ * connection is closed.
+ *
+ * Callbacks are only set when we're compiled in threadsafe mode, so
+ * we only need to remove them in this case.
+ */
+static void
+destroy_ssl_system(void)
+{
+#ifdef ENABLE_THREAD_SAFETY
+   /* Mutex is created in initialize_ssl_system() */
+   if (pthread_mutex_lock(&ssl_config_mutex))
+       return;
+
+   if (pq_initssllib)
+   {
+       if (ssl_open_connections > 0)
+           --ssl_open_connections;
+
+       if (ssl_open_connections == 0)
+       {
+           /* No connections left, unregister all callbacks */
+           CRYPTO_set_locking_callback(NULL);
+           CRYPTO_set_id_callback(NULL);
+
+           /*
+            * We don't free the lock array. If we get another connection
+            * from the same caller, we will just re-use it with the existing
+            * mutexes.
+            *
+            * This means we leak a little memory on repeated load/unload
+            * of the library.
+            */
+           free(pqlockarray);
+           pqlockarray = NULL;
+       }
+   }
+
+   pthread_mutex_unlock(&ssl_config_mutex);
+#endif
+   return;
+}
+
+/*
+ * Initialize SSL context.
  */
 static int
 initialize_SSL(PGconn *conn)
@@ -932,17 +1016,10 @@ initialize_SSL(PGconn *conn)
    return 0;
 }
 
-/*
- * Destroy global SSL context.
- */
 static void
-destroy_SSL(void)
+destroySSL(void)
 {
-   if (SSL_context)
-   {
-       SSL_CTX_free(SSL_context);
-       SSL_context = NULL;
-   }
+   destroy_ssl_system();
 }
 
 /*
@@ -1061,6 +1138,7 @@ close_SSL(PGconn *conn)
        SSL_shutdown(conn->ssl);
        SSL_free(conn->ssl);
        conn->ssl = NULL;
+       pqsecure_destroy();
        /* We have to assume we got EPIPE */
        REMEMBER_EPIPE(true);
        RESTORE_SIGPIPE();