Re: md5 issues Postgres14 on OL7

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> I have been looking at that, and finished with the attached. It is
> close to the end of the day, so this needs an extra lookup, but I have
> finished by using the idea of an extra routine, called
> pg_cryptohash_error(), able to grab the error saved in the private
> contexts, so as any callers from the backend or the frontend can feed
> on that. This way, it is possible to make the difference between
> several class of errors: OOMs, a too short destination buffer, OpenSSL
> internal error, etc.

I don't like the end result of this at all:

postgres=# select md5('foo');
ERROR: could not compute MD5 hash: OpenSSL failure

Maybe we've successfully laid off blame somewhere else, but this
doesn't move the user one inch towards understanding the cause.
I think we need to report the actual OpenSSL error reason.

I experimented with the attached, very rough delta on top of your
patch, and got

postgres=# select md5('foo');
ERROR: could not compute MD5 hash: disabled for FIPS

which seems far better, plus it'd work for other sorts of OpenSSL
failures.

There are two problems with my delta as it stands:

1. It draws a cast-away-const warning. We'd have to make the result
of pg_cryptohash_error be "const char *", which would be better
practice anyway, but that propagates into some other APIs and I didn't
take the trouble to chase it to the end.

2. It feels a bit bogus to be fetching ERR_get_error() at this point.
Maybe it's all right to assume that the OpenSSL error stack won't
change state before we get to pg_cryptohash_error, but I don't like
the idea much. I think it'd be better to capture ERR_get_error()
sooner and store it in an additional field in pg_cryptohash_ctx.

Also, I wonder if this shouldn't be unified with the SSLerrmessage()
support found in be-secure-openssl.c and fe-secure-openssl.c.

regards, tom lane