> On 8 Apr 2024, at 00:46, Michael Paquier <mich...@paquier.xyz> wrote: > > On Sat, Apr 06, 2024 at 07:47:43PM +0200, Daniel Gustafsson wrote: >> My apologies, I thought I did but clearly failed. My point was that this is >> a >> special/corner case where we try to find one of two different libraries (with >> different ideas about backwards compatability etc) for supporting a single >> thing. So instead I tested for the explicit versions like how we already >> test >> for the exact Perl version in config/perl.m4 (albeit that a library and a >> program are two different things of course). > > + # Function introduced in OpenSSL 1.1.1 and LibreSSL 3.3.2 > + AC_CHECK_FUNCS([EVP_PKEY_new_CMAC_key], [], [AC_MSG_ERROR([OpenSSL 1.1.1 > or later is required for SSL support])]) > > I can see why you want to do that, but we've always relied on > compilation failures while documenting the versions supported. I > don't disagree to your point of detecting that earlier, but it sounds > like this should be a separate patch separate from the one removing > support for OpenSSL 1.0.2 and 1.1.0, at least, because you are solving > two problems at once. > >> In bumping we want to move to 1.1.1 since that's the first version with the >> rewritten RNG which is fork-safe by design, something PostgreSQL clearly >> benefits from. There is no new API for this to gate on though. For LibreSSL >> we want 3.3.2 to a) ensure we have coverage in the BF and b) since it's the >> first version where the tests pass due to error message alignment with >> OpenSSL. >> The combination of these gets rid of lots of specialcased #ifdef soup. I >> wasn't however able to find a specific API call which is unique to the two >> version which we rely on. > > Based on the state of the patch, all the symbols cleaned up in > pg_config.h.in would be removed only by dropping support for > 1.0.2. The routines of protocol_openssl.c exist in 1.1.0. libpq > internal locking can be removed by dropping support for 1.0.2. So > a bunch of ifdefs are removed with 1.0.2 support, but that's much, > much less cleaned once 1.1.0 is removed.
If we are settling for removing 1.0.2 we need to make sure there is 1.1.0 buildfarm animals that actually run the sslcheck. 1.1.0 was never an LTS release so it's presence in distributions is far less widespread. I would prefer 1.1.1 but, either way, we have ample time to discuss that during the v18 cycle. > And pg_strong_random.c stuff would still remain around. It stays but as belts-and-suspenders safety against bugs, not as a requirement. >> Testing for the presence of an API provided and introduced by both libraries >> in >> the version we're interested in, but which we don't use, is the alternative >> but >> I thought that would be more frowned upon. EVP_PKEY_new_CMAC_key() was >> introduced in 1.1.1 and LibreSSL 3.3.2, so an AC_CHECK_FUNCS for that, as in >> the attached, achieves the version check but pollutes pg_config.h with a >> define >> which will never be used which seemed a bit ugly. > > Two lines in pg_config.h is a minimal cost. That does not look like a > big deal to me. > > - * Make sure processes do not share OpenSSL randomness state. This is > no > - * longer required in OpenSSL 1.1.1 and later versions, but until we > drop > - * support for version < 1.1.1 we need to do this. > + * Make sure processes do not share OpenSSL randomness state. This is > in > + * theory no longer be required in OpenSSL 1.1.1 and later versions, but > + * there is no harm in taking extra precautions. > > I was wondering if this should also document what you've mentioned, > aka that OpenSSL still found ways to break the randomness state and > that this is a cheap insurance against future mistakes that could > happen in this area. I'm not quite sure how stable Github links are over time, but I guess we could post the commit sha for the bug in OpenSSL (as well as the fix) which is a stable documentation of how it can be subtly broken. -- Daniel Gustafsson
