Re: PQinitSSL broken in some use casesf - Mailing list pgsql-hackers

From Andrew Chernow
Subject Re: PQinitSSL broken in some use casesf
Date
Msg-id 49D0CAC2.6080102@esilo.com
Whole thread Raw
In response to Re: PQinitSSL broken in some use casesf  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: PQinitSSL broken in some use casesf  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: PQinitSSL broken in some use casesf  (Robert Haas <robertmhaas@gmail.com>)
Re: PQinitSSL broken in some use casesf  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Tom Lane wrote:
>
> I personally would be happy with the two-argument function solution.
>

I modified my previous patch to use a two-argument function solution.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.280
diff -C6 -r1.280 libpq.sgml
*** doc/src/sgml/libpq.sgml    28 Mar 2009 01:36:11 -0000    1.280
--- doc/src/sgml/libpq.sgml    30 Mar 2009 13:29:48 -0000
***************
*** 6179,6190 ****
--- 6179,6219 ----
     <!-- If this URL changes replace it with a URL to www.archive.org. -->
     See <ulink
     url="http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html"></ulink>
     for details on the SSL API.
    </para>

+   <para>
+    <variablelist>
+     <varlistentry>
+      <term>
+       <function>PQinitSecure</function>
+       <indexterm>
+        <primary>PQinitSecure</primary>
+       </indexterm>
+      </term>
+
+      <listitem>
+       <para>
+        Allows applications to select which secure components to initialize.
+        <synopsis>
+         void PQinitSecure(int do_ssl, init do_crypto);
+        </synopsis>
+       </para>
+
+       <para>
+        When do_ssl is non-zero, OpenSSL's SSL library will be initialized.
+        When do_crypto is non-zero, OpenSSL's Crypto library will be initialized.
+        By default, both libraries are initialized.  When SSL support is not
+        compiled in, this function does nothing.
+       </para>
+      </listitem>
+     </varlistentry>
+    </variablelist>
+   </para>
+
    <table id="libpq-ssl-file-usage">
     <title>Libpq/Client SSL File Usage</title>
     <tgroup cols="3">
      <thead>
       <row>
        <entry>File</entry>
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    30 Mar 2009 13:29:48 -0000
***************
*** 149,154 ****
--- 149,155 ----
  PQinstanceData            147
  PQsetInstanceData         148
  PQresultInstanceData      149
  PQresultSetInstanceData   150
  PQfireResultCreateEvents  151
  PQconninfoParse           152
+ PQinitSecure              153
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.121
diff -C6 -r1.121 fe-secure.c
*** src/interfaces/libpq/fe-secure.c    28 Mar 2009 18:48:55 -0000    1.121
--- src/interfaces/libpq/fe-secure.c    30 Mar 2009 13:29:49 -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_init_ssl_lib = true;
+ static bool pq_init_crypto_lib = true;
  static SSL_CTX *SSL_context = NULL;

  #ifdef ENABLE_THREAD_SAFETY
  static int ssl_open_connections = 0;

  #ifndef WIN32
***************
*** 170,182 ****
   *    initialized OpenSSL.
   */
  void
  PQinitSSL(int do_init)
  {
  #ifdef USE_SSL
!     pq_init_ssl_lib = do_init;
  #endif
  }

  /*
   *    Initialize global context
   */
--- 171,196 ----
   *    initialized OpenSSL.
   */
  void
  PQinitSSL(int do_init)
  {
  #ifdef USE_SSL
!     PQinitSecure(do_init, do_init);
! #endif
! }
!
! /*
!  *    Exported function to allow application to tell us which secure
!  *    components to initialize.
!  */
! void
! PQinitSecure(int do_ssl, int do_crypto)
! {
! #ifdef USE_SSL
!     pq_init_ssl_lib = do_ssl;
!     pq_init_crypto_lib = do_crypto;
  #endif
  }

  /*
   *    Initialize global context
   */
***************
*** 820,831 ****
--- 834,847 ----
   * 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_init_ssl_lib)
      {
          /*
           * 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)
--- 853,874 ----
          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_init_ssl_lib || pq_init_crypto_lib)
!         num_ssl_conns = ssl_open_connections++;
!
!     if (pq_init_crypto_lib)
      {
          /*
           * 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)
          {
              /* This is actually libcrypto, not libssl. */
              /* These are only required for threaded SSL applications */
              CRYPTO_set_id_callback(pq_threadidcallback);
              CRYPTO_set_locking_callback(pq_lockingcallback);
          }
--- 890,902 ----
                      pthread_mutex_unlock(&ssl_config_mutex);
                      return -1;
                  }
              }
          }

!         if (num_ssl_conns == 0)
          {
              /* This is actually libcrypto, not libssl. */
              /* These are only required for threaded SSL applications */
              CRYPTO_set_id_callback(pq_threadidcallback);
              CRYPTO_set_locking_callback(pq_lockingcallback);
          }
***************
*** 925,957 ****
  {
  #ifdef ENABLE_THREAD_SAFETY
      /* Mutex is created in initialize_ssl_system() */
      if (pthread_mutex_lock(&ssl_config_mutex))
          return;

!     if (pq_init_ssl_lib)
      {
!         if (ssl_open_connections > 0)
!             --ssl_open_connections;

!         if (ssl_open_connections == 0)
!         {
!             /* This is actually libcrypto, not libssl. */
!             /* 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;
  }
--- 950,979 ----
  {
  #ifdef ENABLE_THREAD_SAFETY
      /* Mutex is created in initialize_ssl_system() */
      if (pthread_mutex_lock(&ssl_config_mutex))
          return;

!     if ((pq_init_ssl_lib || pq_init_crypto_lib) && ssl_open_connections > 0)
!         --ssl_open_connections;
!
!     if (pq_init_crypto_lib && ssl_open_connections == 0)
      {
!         /* This is actually libcrypto, not libssl. */
!         /* 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    30 Mar 2009 13:29:49 -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 which secure components to initialize. */
+ extern void PQinitSecure(int do_ssl, int do_crypto);
+
  /* 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);

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: More message encoding woes
Next
From: Marko Kreen
Date:
Subject: Re: 8.3.5: Crash in CountActiveBackends() - lockless race?