SSL BIO wrappers - Mailing list pgsql-hackers

From Magnus Hagander
Subject SSL BIO wrappers
Date
Msg-id 493E8C0A.5000004@hagander.net
Whole thread Raw
Responses Re: SSL BIO wrappers  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Attached patch replaces the SSL BIO wrapper code we have now, with one
that directly calls the send() and recv() functions instead. THis means
that they get passed through the rewrite macros to our internal
functions on Win32, and I think this will fix some of the strange errors
that seem to be platform specific there (there are some really hard to
reproduce bug reports around that).

They're of course tightly modeled around the code from OpenSSL - found
here: http://cvs.openssl.org/fileview?f=openssl/crypto/bio/bss_sock.c&v=1.15

(functions sock_read and sock_write)

So:

1) Thoughts in general?

2) Per my_sock_write - should we do the prepare read there as well, even
though it's a write? :-)

//Magnus
*** a/src/backend/libpq/be-secure.c
--- b/src/backend/libpq/be-secure.c
***************
*** 394,438 **** wloop:
  #ifdef USE_SSL

  /*
!  * Private substitute BIO: this wraps the SSL library's standard socket BIO
!  * so that we can enable and disable interrupts just while calling recv().
!  * We cannot have interrupts occurring while the bulk of openssl runs,
!  * because it uses malloc() and possibly other non-reentrant libc facilities.
   *
-  * As of openssl 0.9.7, we can use the reasonably clean method of interposing
-  * a wrapper around the standard socket BIO's sock_read() method.  This relies
-  * on the fact that sock_read() doesn't call anything non-reentrant, in fact
-  * not much of anything at all except recv().  If this ever changes we'd
-  * probably need to duplicate the code of sock_read() in order to push the
-  * interrupt enable/disable down yet another level.
   */

  static bool my_bio_initialized = false;
  static BIO_METHOD my_bio_methods;
- static int    (*std_sock_read) (BIO *h, char *buf, int size);

  static int
  my_sock_read(BIO *h, char *buf, int size)
  {
!     int            res;

      prepare_for_client_read();

!     res = std_sock_read(h, buf, size);

      client_read_ended();

      return res;
  }

  static BIO_METHOD *
  my_BIO_s_socket(void)
  {
      if (!my_bio_initialized)
      {
          memcpy(&my_bio_methods, BIO_s_socket(), sizeof(BIO_METHOD));
-         std_sock_read = my_bio_methods.bread;
          my_bio_methods.bread = my_sock_read;
          my_bio_initialized = true;
      }
      return &my_bio_methods;
--- 394,469 ----
  #ifdef USE_SSL

  /*
!  * Private substitute BIO: this does the sending and receiving using send() and
!  * recv() instead. This is so that we can enable and disable interrupts
!  * just while calling recv(). We cannot have interrupts occurring while
!  * the bulk of openssl runs, because it uses malloc() and possibly other
!  * non-reentrant libc facilities. We also need to call send() and recv()
!  * directly so it gets passed through the socket/signals layer on Win32.
!  *
!  * They are closely modelled on the original socket implementations in OpenSSL.
   *
   */

  static bool my_bio_initialized = false;
  static BIO_METHOD my_bio_methods;

  static int
  my_sock_read(BIO *h, char *buf, int size)
  {
!     int            res = 0;

      prepare_for_client_read();

!     if (buf != NULL)
!     {
!         res = recv(h->num, buf, size, 0);
!         BIO_clear_retry_flags(h);
!         if (res <= 0)
!         {
!             /* If we were interrupted, tell caller to retry */
!             if (errno == EINTR)
!             {
!                 BIO_set_retry_read(h);
!             }
!         }
!     }

      client_read_ended();

      return res;
  }

+ static int
+ my_sock_write(BIO *h, const char *buf, int size)
+ {
+     int            res = 0;
+
+     /*
+      * XXX: should we do a prepare_for_client_read here as well,
+      * even though it's not a read operation?
+      */
+
+     res = send(h->num, buf, size, 0);
+     if (res <= 0)
+     {
+         if (errno == EINTR)
+         {
+             BIO_set_retry_write(h);
+         }
+     }
+
+     return res;
+ }
+
  static BIO_METHOD *
  my_BIO_s_socket(void)
  {
      if (!my_bio_initialized)
      {
          memcpy(&my_bio_methods, BIO_s_socket(), sizeof(BIO_METHOD));
          my_bio_methods.bread = my_sock_read;
+         my_bio_methods.bwrite = my_sock_write;
          my_bio_initialized = true;
      }
      return &my_bio_methods;

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: new libpq SSL connection option
Next
From: Magnus Hagander
Date:
Subject: Re: new libpq SSL connection option