Michael Paquier <michael@xxxxxxxxxxx> 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
diff -u cryptohash_openssl.c.orig cryptohash_openssl.c
--- cryptohash_openssl.c.orig 2022-01-06 11:15:59.268256281 -0500
+++ cryptohash_openssl.c 2022-01-06 11:22:28.602734304 -0500
@@ -21,6 +21,7 @@
#include "postgres_fe.h"
#endif
+#include <openssl/err.h>
#include <openssl/evp.h>
#include "common/cryptohash.h"
@@ -309,7 +310,14 @@
case PG_CRYPTOHASH_ERROR_DEST_LEN:
return _("destination buffer too small");
case PG_CRYPTOHASH_ERROR_OPENSSL:
- return _("OpenSSL failure");
+ {
+ unsigned long ecode = ERR_get_error();
+ const char *errreason = ERR_reason_error_string(ecode);
+
+ if (errreason)
+ return errreason;
+ return _("OpenSSL failure");
+ }
}
/* assume that the default is out-of-memory, anyway */
