Re: cryptohash: missing locking functions for OpenSSL <= 1.0.2?

On Thu, Feb 18, 2021 at 11:04:05AM +0900, Michael Paquier wrote:
> We have the code in place to properly initialize the crypto locking in
> libpq with ENABLE_THREAD_SAFETY, but the root of the issue is that the
> SSL and crypto initializations are grouped together. What we need to
> do here is to split those phases of the initialization so as non-SSL
> connections can use the crypto part properly, as pqsecure_initialize
> gets only called now when libpq negotiates SSL with the postmaster.
> It seems to me that we should make sure of a proper reset of the
> crypto part within pqDropConnection(), while the initialization needs
> to happen in PQconnectPoll().

So, I have tried a couple of things with a debug build of OpenSSL
1.0.2 at hand (two locks for the crypto and SSL initializations but
SSL_new() grabs some random bytes that need the same callback to be
set or the state of the threads is messed up, some global states to
control the load), and the simplest solution I have come up with is to
control in each pg_conn if the crypto callbacks have been initialized
or not so as we avoid multiple inits and/or drops of the state for a
single connection. I have arrived at this conclusion after hunting
down cases with pqDropConnection() which would could be called
multiple times, particularly if there are connection attempts to
multiple hosts.

The attached patch implements things this way, and initializes the
crypto callbacks before sending the startup packet, before deciding if
SSL needs to be requested or not. I have played with several
threading scenarios with this patch, with and without OpenSSL, and the
numbers match in terms of callback loading and unloading (the global
counter used in fe-secure-openssl.c gets to zero).
--
Michael