Robert Haas wrote:
On Fri, Feb 13, 2009 at 9:17 AM, Andrew Chernow <a...@esilo.com> wrote:
Should I create a patch implementing the PQinitCrypto idea?

I think that would be helpful.  Seeing the code will give everyone a
better idea of exactly what the proposed change is and whether it's
acceptable.

...Robert



Patch attached.

One thing I noticed is the ssl_open_connections variable is ref counting connections when pq_initssllib is true. But, it now only affects crypto library init and cleanup calls. Point is, ref counting is only needed if pq_initcryptolib is true and it should be renamed to crypto_open_connections. I didn't do this in the patch. Its the same old name and the counter is incremented if pq_initssllib or pq_initcryptolib is true. Please advise.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/interfaces/libpq/exports.txt
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.22
diff -C6 -r1.22 exports.txt
*** src/interfaces/libpq/exports.txt    22 Sep 2008 13:55:14 -0000      1.22
--- src/interfaces/libpq/exports.txt    13 Feb 2009 17:01:22 -0000
***************
*** 149,154 ****
--- 149,155 ----
  PQinstanceData            147
  PQsetInstanceData         148
  PQresultInstanceData      149
  PQresultSetInstanceData   150
  PQfireResultCreateEvents  151
  PQconninfoParse           152
+ PQinitCrypto              153
\ No newline at end of file
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.119
diff -C6 -r1.119 fe-secure.c
*** src/interfaces/libpq/fe-secure.c    28 Jan 2009 15:06:47 -0000      1.119
--- src/interfaces/libpq/fe-secure.c    13 Feb 2009 17:01:22 -0000
***************
*** 96,107 ****
--- 96,108 ----
  static PostgresPollingStatusType open_client_SSL(PGconn *);
  static void close_SSL(PGconn *);
  static char *SSLerrmessage(void);
  static void SSLerrfree(char *buf);
  
  static bool pq_initssllib = true;
+ static bool pq_initcryptolib = true;
  static SSL_CTX *SSL_context = NULL;
  
  #ifdef ENABLE_THREAD_SAFETY
  static int ssl_open_connections = 0;
  
  #ifndef WIN32
***************
*** 175,186 ****
--- 176,199 ----
  #ifdef USE_SSL
        pq_initssllib = do_init;
  #endif
  }
  
  /*
+  *    Exported function to allow application to tell us it's already
+  *    initialized the OpenSSL Crypto library.
+  */
+ void
+ PQinitCrypto(int do_init)
+ {
+ #ifdef USE_SSL
+       pq_initcryptolib = do_init;
+ #endif
+ }
+ 
+ /*
   *    Initialize global context
   */
  int
  pqsecure_initialize(PGconn *conn)
  {
        int                     r = 0;
***************
*** 820,831 ****
--- 833,846 ----
   * message - no connection local setup is made.
   */
  static int
  init_ssl_system(PGconn *conn)
  {
  #ifdef ENABLE_THREAD_SAFETY
+       int num_ssl_conns = 0;
+ 
  #ifdef WIN32
        /* Also see similar code in fe-connect.c, default_threadlock() */
        if (ssl_config_mutex == NULL)
        {
                while (InterlockedExchange(&win32_ssl_create_mutex, 1) == 1)
                         /* loop, another thread own the lock */ ;
***************
*** 837,849 ****
                InterlockedExchange(&win32_ssl_create_mutex, 0);
        }
  #endif
        if (pthread_mutex_lock(&ssl_config_mutex))
                return -1;
  
!       if (pq_initssllib)
        {
                /*
                 * 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)
--- 852,873 ----
                InterlockedExchange(&win32_ssl_create_mutex, 0);
        }
  #endif
        if (pthread_mutex_lock(&ssl_config_mutex))
                return -1;
  
!       /*
!        * Increment connection count if we are responsible for
!        * intiializing the SSL or Crypto library.  Currently, only
!        * crypto needs this during setup and cleanup.  That may
!        * change in the future so we always update the counter.
!        */
!       if (pq_initssllib || pq_initcryptolib)
!               num_ssl_conns = ssl_open_connections++;
! 
!       if (pq_initcryptolib)
        {
                /*
                 * 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)
***************
*** 865,877 ****
                                        pthread_mutex_unlock(&ssl_config_mutex);
                                        return -1;
                                }
                        }
                }
  
!               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);
                }
        }
--- 889,901 ----
                                        pthread_mutex_unlock(&ssl_config_mutex);
                                        return -1;
                                }
                        }
                }
  
!               if (num_ssl_conns == 0)
                {
                        /* These are only required for threaded SSL 
applications */
                        CRYPTO_set_id_callback(pq_threadidcallback);
                        CRYPTO_set_locking_callback(pq_lockingcallback);
                }
        }
***************
*** 924,955 ****
  {
  #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.
!                        */
!               }
        }
  
        pthread_mutex_unlock(&ssl_config_mutex);
  #endif
        return;
  }
--- 948,976 ----
  {
  #ifdef ENABLE_THREAD_SAFETY
        /* Mutex is created in initialize_ssl_system() */
        if (pthread_mutex_lock(&ssl_config_mutex))
                return;
  
!       if ((pq_initssllib || pq_initcryptolib) && ssl_open_connections > 0)
!               --ssl_open_connections;
! 
!       if (pq_initcryptolib && 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.
!                */
        }
  
        pthread_mutex_unlock(&ssl_config_mutex);
  #endif
        return;
  }
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.145
diff -C6 -r1.145 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h     1 Jan 2009 17:24:03 -0000       1.145
--- src/interfaces/libpq/libpq-fe.h     13 Feb 2009 17:01:22 -0000
***************
*** 299,310 ****
--- 299,313 ----
   * unencrypted connections or if any other TLS library is in use. */
  extern void *PQgetssl(PGconn *conn);
  
  /* Tell libpq whether it needs to initialize OpenSSL */
  extern void PQinitSSL(int do_init);
  
+ /* Tell libpq whether it needs to initialize the OpenSSL Crypto library. */
+ extern void PQinitCrypto(int do_init)
+ 
  /* Set verbosity for PQerrorMessage and PQresultErrorMessage */
  extern PGVerbosity PQsetErrorVerbosity(PGconn *conn, PGVerbosity verbosity);
  
  /* Enable/disable tracing */
  extern void PQtrace(PGconn *conn, FILE *debug_port);
  extern void PQuntrace(PGconn *conn);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to