Thread: PATCH SSL_pending() checks in libpq/fe-misc.c

PATCH SSL_pending() checks in libpq/fe-misc.c

From
Jack Bates
Date:
Hello:

I took a look at the SSL code in libpq/fe-misc.c and noticed what I 
think is a small problem.  A patch is included at the bottom of this 
email against anoncvs TopOfTree this evening.

The SSL library buffers input data internally.  Nowhere in libpq's code 
is this buffer being checked via SSL_pending(), which can lead to a 
condition where once in a while a socket appears to "hang" or "lag". This is because select() won't see bytes buffered
bythe library.  A 
 
condition like this is most likely to occur when the library's read 
buffer has been filled previously and another read is to be performed. If the end of the backend's transmission was
lessthan one SSL frame 
 
payload away from the last byte returned in the previous read, this will 
likely hang.  Trust me that I learned of this most painfully...  

I am looking deeper at how to enable non-blocking SSL sockets in libpq. As Tom Lane states, this is primarily a matter
ofchecking SSL error 
 
codes, particularly for SSL_WANT_READ and SSL_WANT_WRITE, and reacting 
appropriately.  I'll see about that as I have more free time.

Even though I'm doing this, I tend to agree with Tom that SSH tunnels 
are a really good way to make the whole SSL problem just go away.

My quick patch to perform the SSL_pending() checks:

===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v
retrieving revision 1.70
diff -r1.70 fe-misc.c
350a351>  * -or- if SSL is enabled and used, is it buffering bytes?
361a363,371> /* Check for SSL library buffering read bytes */> #ifdef USE_SSL>       if (conn->ssl &&
SSL_pending(conn->ssl)> 0)>       {>               /* short-circuit the select */>               return 1;>       }>
#endif>
784a795,797>  * If SSL enabled and used and forRead, buffered bytes short-circuit the>  * call to select().>  *
801a815,823>> /* Check for SSL library buffering read bytes */> #ifdef USE_SSL>       if (forRead && conn->ssl &&
SSL_pending(conn->ssl)> 0)>       {>               /* short-circuit the select */>               return 0;>       }>
#endif

_Of_course_ I am just fine with this patch being under a Berkeley-style 
license and included in PostgreSQL.

Cheers.

-- 

Jack Bates
Portland, OR, USA
http://www.floatingdoghead.net

Got privacy?
My PGP key: http://www.floatingdoghead.net/pubkey.txt




Re: PATCH SSL_pending() checks in libpq/fe-misc.c

From
Bruce Momjian
Date:
Would you send over a context diff, diff -c?


---------------------------------------------------------------------------

Jack Bates wrote:
> 
> Hello:
> 
> I took a look at the SSL code in libpq/fe-misc.c and noticed what I 
> think is a small problem.  A patch is included at the bottom of this 
> email against anoncvs TopOfTree this evening.
> 
> The SSL library buffers input data internally.  Nowhere in libpq's code 
> is this buffer being checked via SSL_pending(), which can lead to a 
> condition where once in a while a socket appears to "hang" or "lag". 
>  This is because select() won't see bytes buffered by the library.  A 
> condition like this is most likely to occur when the library's read 
> buffer has been filled previously and another read is to be performed. 
>  If the end of the backend's transmission was less than one SSL frame 
> payload away from the last byte returned in the previous read, this will 
> likely hang.  Trust me that I learned of this most painfully...  
> 
> I am looking deeper at how to enable non-blocking SSL sockets in libpq. 
>  As Tom Lane states, this is primarily a matter of checking SSL error 
> codes, particularly for SSL_WANT_READ and SSL_WANT_WRITE, and reacting 
> appropriately.  I'll see about that as I have more free time.
> 
> Even though I'm doing this, I tend to agree with Tom that SSH tunnels 
> are a really good way to make the whole SSL problem just go away.
> 
> My quick patch to perform the SSL_pending() checks:
> 
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-misc.c,v
> retrieving revision 1.70
> diff -r1.70 fe-misc.c
> 350a351
>  >  * -or- if SSL is enabled and used, is it buffering bytes?
> 361a363,371
>  > /* Check for SSL library buffering read bytes */
>  > #ifdef USE_SSL
>  >       if (conn->ssl && SSL_pending(conn->ssl) > 0)
>  >       {
>  >               /* short-circuit the select */
>  >               return 1;
>  >       }
>  > #endif
>  >
> 784a795,797
>  >  * If SSL enabled and used and forRead, buffered bytes short-circuit the
>  >  * call to select().
>  >  *
> 801a815,823
>  >
>  > /* Check for SSL library buffering read bytes */
>  > #ifdef USE_SSL
>  >       if (forRead && conn->ssl && SSL_pending(conn->ssl) > 0)
>  >       {
>  >               /* short-circuit the select */
>  >               return 0;
>  >       }
>  > #endif
> 
> _Of_course_ I am just fine with this patch being under a Berkeley-style 
> license and included in PostgreSQL.
> 
> Cheers.
> 
> -- 
> 
> Jack Bates
> Portland, OR, USA
> http://www.floatingdoghead.net
> 
> Got privacy?
> My PGP key: http://www.floatingdoghead.net/pubkey.txt
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026