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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers