Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

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.  And pg_strong_random.c stuff
would still remain around.

> 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.
--
Michael