RE: Bogus-looking SSL code in postmaster wait loop - Mailing list pgsql-hackers

From Magnus Hagander
Subject RE: Bogus-looking SSL code in postmaster wait loop
Date
Msg-id 215896B6B5E1CF11BC5600805FFEA82103D982D6@sirius.edu.sollentuna.se
Whole thread Raw
In response to Bogus-looking SSL code in postmaster wait loop  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
> The postmaster contains this code just before it waits for input:
> 
> #ifdef USE_SSL
>         for (curr = DLGetHead(PortList); curr; curr = DLGetSucc(curr))
>         {
>             if (((Port *) DLE_VAL(curr))->ssl &&
>                 SSL_pending(((Port *) DLE_VAL(curr))->ssl) > 0)
>             {
>                 no_select = true;
>                 break;
>             }
>         }
>         if (no_select)
>             FD_ZERO(&rmask);    /* So we don't accept() 
> anything below */
> #endif
> 
> I am not sure exactly what SSL_pending() is defined to mean, but as
> near as I can tell, whenever SSL_pending() returns true, the 
> postmaster
> will completely ignore every other input-ready condition.  This spells
> "denial of service" from where I sit: a nonresponsive SSL client will
> cause the postmaster to freeze up for all other clients.
> 
> Can anyone who knows about SSL defend or even explain the above code?
> I am strongly inclined to just dike it out.

SSL_pending() returns true when there is data in the SSL buffer of the
socket.
The problem is that since SSL uses block cipher, even if you read one just
byte from the socket (using ssl_read), OpenSSL will read a complete block
from the network, in order to be able to decrypt it. In this case, the rest
of that block will sit in the (SSL *)-structure. If you then select() the
socket, it will *not* return that there is more data available, because that
data has already been read from the network layer. Therefor, the select()
call would block *even though there is more data available* on that socket.

That would be an explanation, I think. I agree it's not an ideal situation.
Perhaps it could be replaced with a check that made it do something like
this around the actual select statement?


...
struct timeval tv;
tv.tv_sec = 0;
tv.tv_usec = 0;
if (select(nSockets, &rmask, &wmask, (fd_set *) NULL,no_select?&tv:(struct timeval *)NULL) < 0)
...

That way, select() would block *only* if there is nothing on the sockets
*and* nothing in the SSL buffers. If there is data in *either* sockets *or*
SSL buffers, it will go through to the loop that reads the data.

With this, completely removing the if(no_select) FD_ZERO part of the code.


This was a quick suggestion, so it may be flawed :-) Comments?


//Magnus


pgsql-hackers by date:

Previous
From: Lamar Owen
Date:
Subject: Re: 7.0.x RPMs
Next
From: Peter Eisentraut
Date:
Subject: Re: libpq needs -lsocket on UnixWare