Re: SSL renegotiation and other related woes

Heikki Linnakangas wrote:
>> Can we work-around that easily? I believe we can. The crucial part is
>> that we mustn't let SSL_write() to process any incoming application
>> data. We can achieve that if we always call SSL_read() to drain such
>> data, before calling SSL_write(). But that's not quite enough; if we're
>> in renegotiation, SSL_write() might still try to read more data from the
>> socket that has arrived after the SSL_read() call. Fortunately, we can
>> easily prevent that by hacking pqsecure_raw_read() to always return
>> EWOULDBLOCK, when it's called through SSL_write().
>>
>> The attached patch does that. I've been running your pg_receivexlog test
>> case with this for about 15 minutes without any errors now, with
>> ssl_renegotiation_limit=50kB, while before it errored out within seconds.
>
> Here is a simplified version of this patch. It seems to be enough to not
> let SSL_write() to read any data from the socket. No need to call
> SSL_read() first, and the server-side changes I made were only needed
> because of the other patch I had applied.
>
> Thoughts? Can you reproduce any errors with this?
>
>> In theory, I guess we should do similar hacks in the server, and always
>> call SSL_read() before SSL_write(), but it seems to work without it. Or
>> maybe not; OpenSSL server and client code is not symmetric, so perhaps
>> it works in server mode without that.
>
> Not included in this patch, but I believe we apply a similar patch to
> the server-side too.

It seems to me that there is a bug in the server part of your first patch
(not included in your second patch):

--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -576,11 +576,11 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
/*
* If SSL renegotiations are enabled and we're getting close to the
* limit, start one now; but avoid it if there's one already in
- * progress. Request the renegotiation 1kB before the limit has
+ * progress. Request the renegotiation 32kB before the limit has
* actually expired.
*/
if (ssl_renegotiation_limit && !in_ssl_renegotiation &&
- port->count > (ssl_renegotiation_limit - 1) * 1024L)
+ port->count > (ssl_renegotiation_limit - 32) * 1024L)
{
in_ssl_renegotiation = true;

Experiment shows that for values of ssl_renegotiation_limit less than 32,
no renegotiation takes place at all with this change applied.
port->count is an unsigned long.

Yours,
Laurenz Albe