Re: scram-sha-256 broken with FIPS and OpenSSL 1.0.2

On Tue, Dec 01, 2020 at 10:10:45AM +0100, Daniel Gustafsson wrote:
> That sounds like it would work. Since the cryptohash implementation owns and
> controls the void *data contents it can store whatever it needs to properly
> free it.

That seems to work properly. I have injected some elog(ERROR) with
the attached in some of the SQL functions from cryptohashes.c and the
cleanup happens with the correct resowner references. It felt a bit
strange to use directly the term EVP in resowner.c once I did this
switch, so this is renamed to "CryptoHash" instead.

> Reading through the v6 patch I see nothing sticking out and all review comments
> addressed, +1 on applying that one and then we'll take if from there with the
> remaining ones in the patchset.

Thanks. 0001 has been applied and the buildfarm does not complain, so
it looks like we are good (I'll take care of any issues, like the one
Fujii-san has just reported). Attached are new patches for 0002, the
EVP switch. One thing I noticed is that we need to free the backup
manifest a bit earlier once we begin to use resource owner in
basebackup.c as there is a specific step that may do a double-free.
This would not happen when not using OpenSSL or on HEAD. It would be
easy to separate the resowner and cryptohash portions of the patch
here, but both are tightly linked, so I'd prefer to keep them
together.

Another thing to note is that this makes 0003 for pgcrypto
meaningless, because this would track down only states created by
cryptohash_openssl.c, and not directly EVP_MD_CTX. Consistency in the
backend code is for the best, and considering that pgcrypto has
similar linked lists for ciphers it feels a bit weird to switch only
half of it to use something in resowner.c. So, I am not sure if we
need to do anything here, and I am discarding this part.
--
Michael