On Tue, Jan 04, 2022 at 12:54:35PM -0500, Tom Lane wrote: > I reproduced this on Fedora 35 with FIPS mode enabled. The problem > is that OpenSSL treats MD5 as a disallowed cipher type under FIPS > mode, so this call in pg_cryptohash_init fails: Is that 3.0.0 or 1.1.1? I can see the following, telling that Fedora 35 uses OpenSSL 1.1.1: https://packages.fedoraproject.org/pkgs/openssl/openssl/ And there is FIPS 2.0 for 1.0.1 and 1.0.2 (funnily, I recall that 1.0.2+FIPS allows MD5 to work with the EVP routines), but the module of FIPS 3.0 does not work with 1.1.1 AFAIK, so I may be confused. Or perhaps this is OpenSSL 1.1.1 with a separate module? The upstream code does nothing special with EVP_DigestInit_ex() in 1.1.1 (see digest.c), contrary to 3.0.0 where there is specific knowledge of FIPS. > status = EVP_DigestInit_ex(ctx->evpctx, EVP_md5(), NULL); > > and then we come back to this in md5_text(): > > /* get the hash result */ > if (pg_md5_hash(VARDATA_ANY(in_text), len, hexsum) == false) > ereport(ERROR, > (errcode(ERRCODE_OUT_OF_MEMORY), > errmsg("out of memory"))); > > So there's nothing actually misbehaving, but our error reportage sucks: > the hash functions have no way to report a specific failure code, > and the caller(s) think the only possible failure mode is OOM. Indeed, this error is a pilot error with the cryptohash integration of 14. In ~13, the custom MD5 implementation would only fail on OOM, but more failure modes are possible now. > I suppose we could get around the error by using our own MD5 code > even in OpenSSL-enabled builds, but that'd violate both the spirit > and the letter of FIPS certification. I don't think we should go back to an integration of our custom MD5 if we have external libraries that provide support for it. That's against the set of FIPS policies. > I think the right response is > to upgrade the error-reporting API in this area, so that the message > could look more like "MD5 is disallowed in FIPS mode". Hmm. I am not sure how much we should try to make the backend, or even the frontend, FIPS-aware (remember for example 0182438 where we avoided this kind of complexity), and not all SSL libraries we would potentially add support for may care about such error states. The cleanest approach may be to extend the APIs to store an **errstr so as implementations are free to assign the error string they want to send back for the error reporting, rather than using more error codes. If we want to improve things in this area with FIPS (aka allow check-world to pass in this case), we would need more in terms of alternate test output, and extra tweaks for the authentication tests, as well. Perhaps the best thing to do in the long term would be to drop MD5, but we are not there yet IMO even if password_encryption default has changed, and upgrade scenarios get hairy. At the end, I agree that we should improve the error message in these two cases. However, I would stick to simplicity by not assuming that those two code paths fail only on OOM, and reword things in md5_text() and md5_bytea() with a simple "could not compute MD5 hash". Any code paths calling the routines of md5_common.c just do that as well for ages when the computation fails, and that's what we care about here. -- Michael
Attachment:
signature.asc
Description: PGP signature
