Re: BUG #12799: libpq - SSL pqsecure_read() doesn't clean openssl error queue before reading

On Thu, Feb 18, 2016 at 2:38 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> +1 for changing both sides. I'm fairly sure that you could provoke
> problems of this ilk in the backend too, for example if client connection
> is using SSL and we also establish an outgoing SSL connection using
> postgres_fdw or dblink.

I didn't consider that, although I did note that Heikki was also in
favor of covering both sides. It should be fairly straightforward.

> BTW, do we have a reproducible test case?

Yes, but that seems almost unnecessary. Basically, we trust that the
per-thread error queue doesn't have anything in it, even though it
clearly can. That assumption could be violated because malloc()
returns NULL in SSLerrmessage(), for example. That's a case that does
not even involve any non-libpq use of OpenSSL. Clearly we need to be
more careful about clearing the queue generally, but especially
because everyone else will get this wrong.

The SSL_get_error() man page says:

"""
In addition to ssl and ret, SSL_get_error() inspects the current thread's
OpenSSL error queue. Thus, SSL_get_error() must be used in the same thread
that performed the TLS/SSL I/O operation, and no other OpenSSL function calls
should appear in between. The current thread's error queue must be empty
before the TLS/SSL I/O operation is attempted, or SSL_get_error() will not work
reliably.

"""

It is more or less that simple. We can't let this happen without
severe consequences. It's on us to coordinate how to prevent this
outcome, with no direction provided on how that should work out in the
real world.

There seem to be some really thin wrappers for OpenSSL for scripting
languages like Ruby and PHP around, that pass the buck on to users of
those languages. Naturally, they often get it wrong, because the
interface is so impractical.

--
Peter Geoghegan