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

Andrew Chernow wrote:
>
>  > Maybe better, have it return a zero/nonzero error code; where one of the
>  > possibilities for failure is "you passed a bit I didn't understand".
>
> Why not just return those bit(s) instead of an arbitrary code?  How about:
>
> -1 = error (if it ever does anything that can fail)
>  0 = success (all bits known)
>  >0 = unknown bits (remaining known bits *have* been set)
>

I attached a patch that implements the above, using PQinitSecure as the
function name.

--
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.278
diff -C6 -r1.278 libpq.sgml
*** doc/src/sgml/libpq.sgml    11 Feb 2009 04:08:47 -0000    1.278
--- doc/src/sgml/libpq.sgml    18 Feb 2009 15:22:04 -0000
***************
*** 6174,6185 ****
--- 6174,6215 ----
     <application>libpq</application> that the <acronym>SSL</> library
     has already been initialized by your application.
     <!-- 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.
+
+    <variablelist>
+     <varlistentry>
+      <term>
+       <function>PQinitSecure</function>
+       <indexterm>
+        <primary>PQinitSecure</primary>
+       </indexterm>
+      </term>
+
+      <listitem>
+       <para>
+        Allows applications to select which secure components to initialize.
+        <synopsis>
+         int PQinitSecure(int flags);
+        </synopsis>
+       </para>
+
+       <para>
+        The flags argument can be any of the following: PG_SECURE_SSL,
+        PG_SECURE_CRYPTO.  PG_SECURE_SSL will initialize the SSL portion of
+        the OpenSSL library.  PG_SECURE_CRYPTO will initialize the crypto
+        portion of the OpenSSL library.  The function returns the bits it
+        did not understand or zero indicating it understood all bits in flags.
+        If an error occurs, such as calling this function without SSL
+        support enabled, -1 is returned.
+       </para>
+      </listitem>
+     </varlistentry>
+    </variablelist>
    </para>

    <table id="libpq-ssl-file-usage">
     <title>Libpq/Client SSL File Usage</title>
     <tgroup cols="3">
      <thead>
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    18 Feb 2009 15:22:04 -0000
***************
*** 149,154 ****
--- 149,155 ----
  PQinstanceData            147
  PQsetInstanceData         148
  PQresultInstanceData      149
  PQresultSetInstanceData   150
  PQfireResultCreateEvents  151
  PQconninfoParse           152
+ PQinitSecure              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    18 Feb 2009 15:22:04 -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,205 ----
  #ifdef USE_SSL
      pq_initssllib = do_init;
  #endif
  }

  /*
+  *    Exported function to allow application to tell us which secure
+  *  components to initialize.
+  */
+ int
+ PQinitSecure(int flags)
+ {
+     int code = -1;
+
+ #ifdef USE_SSL
+     pq_initssllib = flags & PG_SECURE_SSL ? true : false;
+     pq_initcryptolib = flags & PG_SECURE_CRYPTO ? true : false;;
+     code = flags & ~(PG_SECURE_SSL | PG_SECURE_CRYPTO);
+ #endif
+
+     return code;
+ }
+
+ /*
   *    Initialize global context
   */
  int
  pqsecure_initialize(PGconn *conn)
  {
      int            r = 0;
***************
*** 820,831 ****
--- 839,852 ----
   * 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)
--- 858,879 ----
          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);
          }
      }
--- 895,907 ----
                      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;
  }
--- 954,982 ----
  {
  #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    18 Feb 2009 15:22:04 -0000
***************
*** 33,44 ****
--- 33,50 ----
   */
  #define PG_COPYRES_ATTRS          0x01
  #define PG_COPYRES_TUPLES         0x02        /* Implies PG_COPYRES_ATTRS */
  #define PG_COPYRES_EVENTS         0x04
  #define PG_COPYRES_NOTICEHOOKS    0x08

+ /*
+  * Flags for PQinitSecure
+  */
+ #define PG_SECURE_SSL    0x01
+ #define PG_SECURE_CRYPTO 0x02
+
  /* Application-visible enum types */

  typedef enum
  {
      /*
       * Although it is okay to add to this list, values which become unused
***************
*** 299,310 ****
--- 305,319 ----
   * 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 int PQinitSecure(int flags);
+
  /* 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);