Re: SSL renegotiation and other related woes - Mailing list pgsql-hackers

On 02/12/2015 01:33 AM, Andres Freund wrote:
> On 2015-02-11 14:54:03 +0200, Heikki Linnakangas wrote:
>> Thoughts? Can you reproduce any errors with this?
>
> I'm on battery right now, so I can't really test much.
>
> Did you test having an actual standby instead of pg_receivexlog? I saw
> some slightly different errors there.
>
> Does this patch alone work for you or did you test it together with the
> earlier one to fix the renegotiation handling server side when
> nonblocking? Because that frequently seemed to error out for me, at
> least over actual network. A latch loop + checking for the states seemed
> to fix that for me.

This patch alone worked for me.

>> +     * NB: This relies on the calling code to call pqsecure_read(), completing
>> +     * the renegotiation handshake, if pqsecure_write() returns 0. Otherwise
>> +     * we'll never make progress.
>> +     */
>
> I think this should a) refer to the fact that pqSendSome does that in
> blocking scenarios b) expand the comment around the pqReadData to
> reference that fact.
>
> How are we going to deal with callers using libpq in nonblocking mode. A
> client doing a large COPY FROM STDIN to the server using a nonblocking
> connection (not a stupid thing to do) could hit this IIUC.

Hmm, that's problematic even without SSL. If you do a large COPY FROM 
STDIN, and the server sends a lot of NOTICEs, the NOTICEs will be 
accumulated in the TCP send and receive buffers until they fill up. 
After that, the server will block on the send, and will stop reading the 
tuple data the client sends, and you get a deadlock. In blocking mode, 
pqSendSome calls pqReadData to avoid that.

I think pqSendSome should call pqReadData() after getting EWOULDBLOCK, 
even in non-blocking mode. It won't completely prevent the problem: it's 
possible that the receive buffer fills up, but the application doesn't 
call pqSendSome() until the socket becomes writeable, which won't happen 
until the client drains the incoming data is drained and unblocks the 
server. But it greatly reduces the risk. In practice that will solve the 
problem for SSL renegotiations.

We should also document that the application should always wait for the 
socket readable condition, in addition to writeable, and call 
pqConsumeInput(). That fixes the issue for good.

> Could we perhaps call SSL_peek() when SSL_write() hits WANTS_READ? In
> combination with your fix I think that should fix the possibility of
> such a deadlock? Especially on the serverside where the socket most of
> the time is is in, although emulated, blocking mode that seems easier -
> we can just retry afterwards.

Hmm. Not sure what the semantics of SSL_peek() are. At least we'd need 
to call it with a large enough buffer that it would read all the 
incoming data from the OS buffer. I think it would be safer to just call 
SSL_read(). Both the server and libpq have an input buffer that you can 
easily read into at any time.

- Heikki