Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Remove obsolete unconstify()

  2. Only perform pg_strong_random init when required

  3. Remove support for OpenSSL older than 1.1.0

  4. Support SSL_R_VERSION_TOO_LOW when using LibreSSL

  5. Support disallowing SSL renegotiation when using LibreSSL

  6. Doc: Use past tense for things which happened in the past

  7. Remove support for OpenSSL 1.0.1

  8. Remove support for OpenSSL 0.9.8 and 1.0.0

  1. Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

    Michael Paquier <michael@paquier.xyz> — 2023-05-24T08:22:14Z

    Hi all,

$Subject says it all.  Based on an analysis of the code, I can note
the following when it comes to the removal of 1.0.1:
- A lot of code related to channel binding is now simplified as
X509_get_signature_nid() always exists, mostly around its CFLAGS.
- This removes the comments related to 1.0.1e introduced by 74242c2.

Then for 1.0.2, the following flags can be gone:
HAVE_ASN1_STRING_GET0_DATA
HAVE_BIO_GET_DATA
HAVE_BIO_METH_NEW
HAVE_HMAC_CTX_FREE
HAVE_HMAC_CTX_NEW

It would be nice to remove CRYPTO_lock(), but from what I can see the
function still exists in LibreSSL, meaning that libpq locking wouldn't
be thread-safe if these function's paths are removed.

Another related question is how much do we care about builds with
LibreSSL with MSVC?  This patch sets takes the simple path of assuming
that this has never been really tested, and if you look at the MSVC
scripts on HEAD we rely on a version number from OpenSSL, which is not
something LibreSSL copes nicely with already, as documented in
configure.ac.

OpenSSL 1.0.2 has been EOLd at the end of 2019, and 1.0.1 had its last
minor release in September 2019, so with Postgres 17 targetted in
September/October 2024, we would be five years behind that.

Last comes the buildfarm, and I suspect that a few animals are
building with 1.0.2.  Among what I have spotted:
- wobbegong and vulpes, on Fedora 27, though I could not find
references about the version used there.
- bonito, on Fedora 29.
- SUSE 12 SP{4,5} have 1.0.2 as their newer version.
- butterflyfish may not like that, if I recall correctly, as it should
have 1.0.2.

So, it seems to me that 1.0.1 would be a rather silent move for the
buildfarm, and 1.0.2 could lead to some noise.  Note as well that
1.1.0 support has been stopped by upstream at the same time as 1.0.1,
with a last minor release in September 2019, though that feels like a
huge jump at this stage.  On a code basis, removing 1.0.1 leads to the
most cleanup.  The removal of 1.0.2 would be nice, but the tweaks
needed for LibreSSL make it less appealing.

Attached are two patches to remove support for 1.0.1 and 1.0.2 for
now, kept separated for clarity, to be considered as something to do
at the beginning of the v17 cycle.  0001 is in a rather good shape
seen from here.

Now, for 0002 and the removal of 1.0.2, I am seeing two things once
OPENSSL_API_COMPAT is bumped from 0x10002000L to 0x10100000L:
- be-secure-openssl.c requires an extra openssl/bn.h, which is not a
big deal, from what I get.
- Much more interesting: OpenSSL_add_all_algorithms() has two macros
that get removed, see include/openssl/evp.h:
#  ifdef OPENSSL_LOAD_CONF
#   define OpenSSL_add_all_algorithms() \
    OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS \
                        | OPENSSL_INIT_ADD_ALL_DIGESTS \
                        | OPENSSL_INIT_LOAD_CONFIG, NULL)
#  else
#   define OpenSSL_add_all_algorithms() \
    OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS \
                        | OPENSSL_INIT_ADD_ALL_DIGESTS, NULL)
#  endif

The part where I am not quite sure of what to do is about
OPENSSL_LOAD_CONF.  We could call directly OPENSSL_init_crypto() and
add OPENSSL_INIT_LOAD_CONFIG if building with OPENSSL_LOAD_CONF, but
it feels a bit ugly to copy-paste this code from OpenSSL itself.
Note that patch 0002 still has OPENSSL_API_COMPAT at 0x10002000L.
OPENSSL_init_crypto() looks to be in LibreSSL, and it is new in
OpenSSL 1.1.0, so switching the minimum to OpenSSL 1.1.0 should not
require a new cflags on this one.

Thoughts?
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-05-24T09:36:56Z

    > On 24 May 2023, at 10:22, Michael Paquier <michael@paquier.xyz> wrote:

> The removal of 1.0.2 would be nice, but the tweaks
> needed for LibreSSL make it less appealing.

1.0.2 is also an LTS version available commercially for premium support
customers of OpenSSL (1.1.1 will become an LTS version as well), with 1.0.2zh
slated for release next week.  This raises the likelyhood of Postgres
installations using 1.0.2 in production still, and for some time to come.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2023-05-24T09:52:09Z

    On Wed, May 24, 2023 at 11:36:56AM +0200, Daniel Gustafsson wrote:
> 1.0.2 is also an LTS version available commercially for premium support
> customers of OpenSSL (1.1.1 will become an LTS version as well), with 1.0.2zh
> slated for release next week.  This raises the likelyhood of Postgres
> installations using 1.0.2 in production still, and for some time to come.

Good point.  Indeed, that makes it pretty clear that not dropping
1.0.2 would be the best option for the time being, so 0001 would be
enough.

I am wondering if we should worry about having a buildfarm member that
could test these binaries, though, in case they have compatibility
issues..  But it would be harder to debug without the code at hand, as
well.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-05-24T11:03:04Z

    > On 24 May 2023, at 11:52, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Wed, May 24, 2023 at 11:36:56AM +0200, Daniel Gustafsson wrote:
>> 1.0.2 is also an LTS version available commercially for premium support
>> customers of OpenSSL (1.1.1 will become an LTS version as well), with 1.0.2zh
>> slated for release next week.  This raises the likelyhood of Postgres
>> installations using 1.0.2 in production still, and for some time to come.
> 
> Good point.  Indeed, that makes it pretty clear that not dropping
> 1.0.2 would be the best option for the time being, so 0001 would be
> enough.

I think thats the right move re 1.0.2 support.  1.0.2 is also the version in
RHEL7 which is in ELS until 2026.

When we moved the goalposts to 1.0.1 (commit 7b283d0e1d1) we referred to RHEL6
using 1.0.1, and RHEL6 goes out of ELS in late June 2024 seems possible to drop
1.0.1 support during v17.  I haven't studied the patch yet but I'll have a look
at it.

> I am wondering if we should worry about having a buildfarm member that
> could test these binaries, though, in case they have compatibility
> issues..  But it would be harder to debug without the code at hand, as
> well.

It would be nice it the OpenSSL project could grant us an LTS license for a
buildfarm animal to ensure compatibility but I have no idea how realistic that
is (or how much the LTS version of 1.0.2 has diverged from the last available
public 1.0.2 version).

--
Daniel Gustafsson

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-24T14:15:41Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> It would be nice it the OpenSSL project could grant us an LTS license for a
> buildfarm animal to ensure compatibility but I have no idea how realistic that
> is (or how much the LTS version of 1.0.2 has diverged from the last available
> public 1.0.2 version).

Surely the answer must be "not much".  The entire point of an LTS
version is to not have to change dusty calling applications.

We had definitely better have some animals still using 1.0.2, but
I don't see much reason to think that the last public release
wouldn't be good enough.

			regards, tom lane

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

    Michael Paquier <michael@paquier.xyz> — 2023-05-24T22:18:15Z

    On Wed, May 24, 2023 at 01:03:04PM +0200, Daniel Gustafsson wrote:
> When we moved the goalposts to 1.0.1 (commit 7b283d0e1d1) we referred to RHEL6
> using 1.0.1, and RHEL6 goes out of ELS in late June 2024 seems possible to drop
> 1.0.1 support during v17.  I haven't studied the patch yet but I'll have a look
> at it.

Great, thanks for the help.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-05-25T08:16:27Z

    > On 25 May 2023, at 00:18, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Wed, May 24, 2023 at 01:03:04PM +0200, Daniel Gustafsson wrote:
>> When we moved the goalposts to 1.0.1 (commit 7b283d0e1d1) we referred to RHEL6
>> using 1.0.1, and RHEL6 goes out of ELS in late June 2024 seems possible to drop
>> 1.0.1 support during v17.  I haven't studied the patch yet but I'll have a look
>> at it.

Patch looks to be in pretty good shape, just a few minor comments:

-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
+#ifdef USE_OPENSSL
Since this is only calling the pgtls abstraction API and not directly into the
SSL implementation we should use USE_SSL here instead.  Same for the
corresponding hunks in the frontend code.


+	 * Note that if we don't support channel binding if we're not using SSL at
+	 * all, we would not have advertised the PLUS variant in the first place.
Seems like a word fell off when merging these sentences.  This should probably
be "..support channel binding, or if we're no.." or something similar.


-#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
-#define HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
+#ifdef USE_OPENSSL
 extern char *pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len);
 #endif
No need for any guard at all now is there?  All supported SSL implementations
have it, and I doubt we'd accept a new one that doesn't (or which can't
implement the function and error out).


-  # Functions introduced in OpenSSL 1.0.2. LibreSSL does not have
-  # SSL_CTX_set_cert_cb().
-  AC_CHECK_FUNCS([X509_get_signature_nid SSL_CTX_set_cert_cb])
+  # LibreSSL does not have SSL_CTX_set_cert_cb().
+  AC_CHECK_FUNCS([SSL_CTX_set_cert_cb])
The comment about when the function was introduced is still interesting and
should remain IMHO.

The changes to the Windows buildsystem worry me a bit, but they don't move the
goalposts in either direction wrt to LibreSSL on Windows so for the purpose of
this patch we don't need to do more.  Longer term we should either make
LibreSSL work on Windows builds, or explicitly not support it on Windows.

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-05-25T08:27:02Z

    > On 24 May 2023, at 16:15, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> We had definitely better have some animals still using 1.0.2, but
> I don't see much reason to think that the last public release
> wouldn't be good enough.

There are still RHEL7 animals like chub who use 1.0.2 so I'm not worried.  We
might want to consider displaying the OpenSSL version number during configure
(meson already does it) in all branches to make it easier to figure out which
versions we have coverage for?

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2023-05-25T23:47:16Z

    On Thu, May 25, 2023 at 10:27:02AM +0200, Daniel Gustafsson wrote:
> There are still RHEL7 animals like chub who use 1.0.2 so I'm not worried.  We
> might want to consider displaying the OpenSSL version number during configure
> (meson already does it) in all branches to make it easier to figure out which
> versions we have coverage for?

+1.
--
Michael

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

    Michael Paquier <michael@paquier.xyz> — 2023-05-26T02:08:52Z

    On Thu, May 25, 2023 at 10:16:27AM +0200, Daniel Gustafsson wrote:
> -#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
> +#ifdef USE_OPENSSL
> Since this is only calling the pgtls abstraction API and not directly into the
> SSL implementation we should use USE_SSL here instead.  Same for the
> corresponding hunks in the frontend code.

Makes sense, done.

> +	 * Note that if we don't support channel binding if we're not using SSL at
> +	 * all, we would not have advertised the PLUS variant in the first place.
> Seems like a word fell off when merging these sentences.  This should probably
> be "..support channel binding, or if we're no.." or something similar.

Indeed, something has been eaten when merging these lines.

> -#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
> -#define HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
> +#ifdef USE_OPENSSL
>  extern char *pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len);
>  #endif
> No need for any guard at all now is there?  All supported SSL implementations
> have it, and I doubt we'd accept a new one that doesn't (or which can't
> implement the function and error out).

Yup.  It looks that you are right.  A build without SSL is not
complaining once removed in libpq-int.h or libpq-be.h.

> -  # Functions introduced in OpenSSL 1.0.2. LibreSSL does not have
> -  # SSL_CTX_set_cert_cb().
> -  AC_CHECK_FUNCS([X509_get_signature_nid SSL_CTX_set_cert_cb])
> +  # LibreSSL does not have SSL_CTX_set_cert_cb().
> +  AC_CHECK_FUNCS([SSL_CTX_set_cert_cb])
> The comment about when the function was introduced is still interesting and
> should remain IMHO.

Okay.  Kept as well in meson.build.

> The changes to the Windows buildsystem worry me a bit, but they don't move the
> goalposts in either direction wrt to LibreSSL on Windows so for the purpose of
> this patch we don't need to do more.  Longer term we should either make
> LibreSSL work on Windows builds, or explicitly not support it on Windows.

Yes, I was wondering what to do about that in the long term.  With the
argument that the MSVC scripts may be gone over meson, it is not
really appealing to dig into that.

Something that was missing in 0001 is the bump of OPENSSL_API_COMPAT
in meson.build.  This was in 0002.

Please find attached an updated patch only for the removal of 1.0.1.
Thanks for the review. 
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-05-26T08:09:35Z

    > On 26 May 2023, at 04:08, Michael Paquier <michael@paquier.xyz> wrote:
> On Thu, May 25, 2023 at 10:16:27AM +0200, Daniel Gustafsson wrote:

>> The changes to the Windows buildsystem worry me a bit, but they don't move the
>> goalposts in either direction wrt to LibreSSL on Windows so for the purpose of
>> this patch we don't need to do more.  Longer term we should either make
>> LibreSSL work on Windows builds, or explicitly not support it on Windows.
> 
> Yes, I was wondering what to do about that in the long term.  With the
> argument that the MSVC scripts may be gone over meson, it is not
> really appealing to dig into that.

Yeah, let's revisit this during the v17 cycle when the future of these scripts
will become clearer.

> Something that was missing in 0001 is the bump of OPENSSL_API_COMPAT
> in meson.build.  This was in 0002.
> 
> Please find attached an updated patch only for the removal of 1.0.1.
> Thanks for the review. 

LGTM.

--
Daniel Gustafsson

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

    Jacob Champion <jchampion@timescale.com> — 2023-05-26T16:10:17Z

    On Thu, May 25, 2023 at 7:09 PM Michael Paquier <michael@paquier.xyz> wrote:
> Please find attached an updated patch only for the removal of 1.0.1.
> Thanks for the review.

Nice! Sorry about the new complications with LibreSSL. :(

> -  # Functions introduced in OpenSSL 1.0.2. LibreSSL does not have
> +  # Function introduced in OpenSSL 1.0.2. LibreSSL does not have
>    # SSL_CTX_set_cert_cb().
> -  AC_CHECK_FUNCS([X509_get_signature_nid SSL_CTX_set_cert_cb])
> +  AC_CHECK_FUNCS([SSL_CTX_set_cert_cb])

Can X509_get_signature_nid be moved to the required section up above?
As it is now, anyone configuring with -Dssl=auto can still pick up a
1.0.1 build, which Meson accepts, and then the build fails downstream.
If we require the function instead, Meson will ignore 1.0.1 (or, for
-Dssl=openssl, complain before we compile).

t/001_ssltests.pl has a reference to 1.0.1 that can probably be
entirely deleted:

    # ... (Nor for OpenSSL
    # 1.0.1, but that's old enough that accommodating it isn't worth the cost.)

Thanks,
--Jacob

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

    Michael Paquier <michael@paquier.xyz> — 2023-05-27T02:02:41Z

    On Fri, May 26, 2023 at 09:10:17AM -0700, Jacob Champion wrote:
> Can X509_get_signature_nid be moved to the required section up above?
> As it is now, anyone configuring with -Dssl=auto can still pick up a
> 1.0.1 build, which Meson accepts, and then the build fails downstream.
> If we require the function instead, Meson will ignore 1.0.1 (or, for
> -Dssl=openssl, complain before we compile).

Yes, I was wondering a bit if something more should be marked as
required, but just saw more value in removing all references to this
function.  Making the build fail straight when setting up things is OK
by me, but I am not convinced that X509_get_signature_nid() would be
the right choice for the job, as it is an OpenSSL artifact originally,
AFAIK.

The same problem exists with OpenSSL 1.0.0 on HEAD when building with
meson?  CRYPTO_new_ex_data() and SSL_new() exist there.

> t/001_ssltests.pl has a reference to 1.0.1 that can probably be
> entirely deleted:
> 
>     # ... (Nor for OpenSSL
>     # 1.0.1, but that's old enough that accommodating it isn't worth the cost.)

Not touching that is intentional.  It sounded useful to me as an
historic reference for LibreSSL ;)
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-06-02T08:35:43Z

    > On 27 May 2023, at 04:02, Michael Paquier <michael@paquier.xyz> wrote:

> Making the build fail straight when setting up things is OK
> by me, but I am not convinced that X509_get_signature_nid() would be
> the right choice for the job, as it is an OpenSSL artifact originally,
> AFAIK.

I think we should avoid the is-defined-in dance and just pull out the version
numbers for comparisons.  While it's true that LibreSSL doesn't play well with
OpenSSL versions, they do define their own which can be checked for to
distinguish the libraries.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2023-06-02T21:21:54Z

    On Fri, Jun 02, 2023 at 10:35:43AM +0200, Daniel Gustafsson wrote:
> I think we should avoid the is-defined-in dance and just pull out the version
> numbers for comparisons.  While it's true that LibreSSL doesn't play well with
> OpenSSL versions, they do define their own which can be checked for to
> distinguish the libraries.

Agreed.  How about tackling that in a separate patch?  At this stage,
I would like to not care about ./configure and do it only with meson,
but there is value in reporting the version number in ./configure to
see the version coverage in the buildfarm.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-06-02T21:23:19Z

    > On 2 Jun 2023, at 23:21, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Fri, Jun 02, 2023 at 10:35:43AM +0200, Daniel Gustafsson wrote:
>> I think we should avoid the is-defined-in dance and just pull out the version
>> numbers for comparisons.  While it's true that LibreSSL doesn't play well with
>> OpenSSL versions, they do define their own which can be checked for to
>> distinguish the libraries.
> 
> Agreed.  How about tackling that in a separate patch?

Absolutely, let's keep these goalposts in place and deal with that separately.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2023-07-03T04:26:44Z

    On Fri, Jun 02, 2023 at 11:23:19PM +0200, Daniel Gustafsson wrote:
> Absolutely, let's keep these goalposts in place and deal with that separately.

I have not gone back to this part yet, though I plan to do so.  As we
are at the beginning of the development cycle, I have applied the
patch to remove support for 1.0.1 for now on HEAD.  Let's see what the
buildfarm tells.
--
Michael

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

    Thomas Munro <thomas.munro@gmail.com> — 2023-07-03T18:40:49Z

    On Mon, Jul 3, 2023 at 4:26 PM Michael Paquier <michael@paquier.xyz> wrote:
> I have not gone back to this part yet, though I plan to do so.  As we
> are at the beginning of the development cycle, I have applied the
> patch to remove support for 1.0.1 for now on HEAD.  Let's see what the
> buildfarm tells.

curculio (OpenBSD 5.9) is failing with "undefined reference to
`X509_get_signature_nid'", but that's OK, Mikael already supplied a
modern OpenBSD system to replace it (schnauzer, which is green) and he
was planning to shut curculio down (see Direct I/O thread where that
came up because its GCC 4.2 compiler doesn't understand our stack
alignment directives; it will also break comprehensively when I push
the nearby all-supported-computers-have-locale_t patch from the
check_strxfrm_bug thread).

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-07-03T18:53:52Z

    > On 3 Jul 2023, at 20:40, Thomas Munro <thomas.munro@gmail.com> wrote:
> 
> On Mon, Jul 3, 2023 at 4:26 PM Michael Paquier <michael@paquier.xyz> wrote:
>> I have not gone back to this part yet, though I plan to do so.  As we
>> are at the beginning of the development cycle, I have applied the
>> patch to remove support for 1.0.1 for now on HEAD.  Let's see what the
>> buildfarm tells.
> 
> curculio (OpenBSD 5.9) is failing with "undefined reference to
> `X509_get_signature_nid'", but that's OK, Mikael already supplied a
> modern OpenBSD system to replace it

Thanks for the report!  OpenBSD 5.9 was released in 2016 and is thus well over
5 years EOL, so I agree that it doesn't warrant a code change from us to
support this.

--
Daniel Gustafsson

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

    Mikael Kjellström <mikael.kjellstrom@mksoft.nu> — 2023-07-03T20:23:02Z

    
On 2023-07-03 20:53, Daniel Gustafsson wrote:

>> curculio (OpenBSD 5.9) is failing with "undefined reference to
>> `X509_get_signature_nid'", but that's OK, Mikael already supplied a
>> modern OpenBSD system to replace it
> 
> Thanks for the report!  OpenBSD 5.9 was released in 2016 and is thus well over
> 5 years EOL, so I agree that it doesn't warrant a code change from us to
> support this.

I have retired curculio now.  So it will stop reporting in from now on.

/Mikael

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

    Michael Paquier <michael@paquier.xyz> — 2023-07-03T22:13:03Z

    On Mon, Jul 03, 2023 at 10:23:02PM +0200, Mikael Kjellström wrote:
> On 2023-07-03 20:53, Daniel Gustafsson wrote:
>>> curculio (OpenBSD 5.9) is failing with "undefined reference to
>>> `X509_get_signature_nid'", but that's OK, Mikael already supplied a
>>> modern OpenBSD system to replace it
>> 
>> Thanks for the report!  OpenBSD 5.9 was released in 2016 and is thus well over
>> 5 years EOL, so I agree that it doesn't warrant a code change from us to
>> support this.

OpenBSD 5.9 was EOL in 2017 as far as I know.

> I have retired curculio now.  So it will stop reporting in from now on.

Thanks Mikael!
--
Michael

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

    Michael Paquier <michael@paquier.xyz> — 2023-07-03T22:16:47Z

    On Tue, Jul 04, 2023 at 06:40:49AM +1200, Thomas Munro wrote:
> curculio (OpenBSD 5.9) is failing with "undefined reference to
> `X509_get_signature_nid'", but that's OK, Mikael already supplied a
> modern OpenBSD system to replace it (schnauzer, which is green) and he
> was planning to shut curculio down (see Direct I/O thread where that
> came up because its GCC 4.2 compiler doesn't understand our stack
> alignment directives; it will also break comprehensively when I push
> the nearby all-supported-computers-have-locale_t patch from the
> check_strxfrm_bug thread).

The second and third animals to fail are skate and snapper, both using
Debian 7 Wheezy.  As far as I know, it was an LTS supported until
2018.  The owner of both machines is added in CC.  I guess that we
this stuff could just remove --with-openssl from the configure
switches.
--
Michael

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

    Michael Paquier <michael@paquier.xyz> — 2023-07-04T06:03:01Z

    On Tue, Jul 04, 2023 at 07:16:47AM +0900, Michael Paquier wrote:
> The second and third animals to fail are skate and snapper, both using
> Debian 7 Wheezy.  As far as I know, it was an LTS supported until
> 2018.  The owner of both machines is added in CC.  I guess that we
> this stuff could just remove --with-openssl from the configure
> switches.

lapwing has reported a failure and runs a Debian 7, so adding Julien
in CC about the removal of --with-openssl or similar in this animal.
--
Michael

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

    Julien Rouhaud <rjuju123@gmail.com> — 2023-07-04T06:15:18Z

    Hi,

On Tue, Jul 04, 2023 at 03:03:01PM +0900, Michael Paquier wrote:
> On Tue, Jul 04, 2023 at 07:16:47AM +0900, Michael Paquier wrote:
> > The second and third animals to fail are skate and snapper, both using
> > Debian 7 Wheezy.  As far as I know, it was an LTS supported until
> > 2018.  The owner of both machines is added in CC.  I guess that we
> > this stuff could just remove --with-openssl from the configure
> > switches.
>
> lapwing has reported a failure and runs a Debian 7, so adding Julien
> in CC about the removal of --with-openssl or similar in this animal.

Thanks, I actually saw that and already took care of removing openssl support a
couple of hours ago, and also added a new note on the animal to remember when
it was removed.  It should come back to green at the next scheduled run.

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

    Michael Paquier <michael@paquier.xyz> — 2023-07-04T06:22:42Z

    On Tue, Jul 04, 2023 at 02:15:18PM +0800, Julien Rouhaud wrote:
> Thanks, I actually saw that and already took care of removing openssl support a
> couple of hours ago, and also added a new note on the animal to remember when
> it was removed.  It should come back to green at the next scheduled run.

Thanks!
--
Michael

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

    Thomas Munro <thomas.munro@gmail.com> — 2023-09-07T11:30:15Z

    On Wed, May 24, 2023 at 11:03 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 24 May 2023, at 11:52, Michael Paquier <michael@paquier.xyz> wrote:
> > On Wed, May 24, 2023 at 11:36:56AM +0200, Daniel Gustafsson wrote:
> >> 1.0.2 is also an LTS version available commercially for premium support
> >> customers of OpenSSL (1.1.1 will become an LTS version as well), with 1.0.2zh
> >> slated for release next week.  This raises the likelyhood of Postgres
> >> installations using 1.0.2 in production still, and for some time to come.
> >
> > Good point.  Indeed, that makes it pretty clear that not dropping
> > 1.0.2 would be the best option for the time being, so 0001 would be
> > enough.
>
> I think thats the right move re 1.0.2 support.  1.0.2 is also the version in
> RHEL7 which is in ELS until 2026.

I don't mind either way if we rip out OpenSSL 1.0.2 support now or
later, other than a general feeling that cryptography must be about
the worst possible category of software to keep supporting for years
after it has been declared EOL.

But.. I don't like the idea that our *next* release's library version
horizon is controlled by Red Hat's "ELS" phase.  The
yum.postgresql.org team aren't packaging 17 for RHEL7 AFAICS, which is
as it should be if you ask me, because the 10 year maintenance phase
ends before 17 will ship.  These hypothetical users that want to run
an OS even older than that and don't know how to get modern crypto
libraries on it but insist on a shiny new PostgreSQL release and build
it from source because there are no packages available... don't exist?

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

    Daniel Gustafsson <daniel@yesql.se> — 2023-09-07T11:44:11Z

    > On 7 Sep 2023, at 13:30, Thomas Munro <thomas.munro@gmail.com> wrote:

> I don't like the idea that our *next* release's library version
> horizon is controlled by Red Hat's "ELS" phase.

Agreed.  If we instead fence it by "only non-EOL version" then 1.1.1 is also on
the chopping block for v17 as it goes EOL in 4 days from now with 1.1.1w (which
contains a CVE, going out with a bang).  Not sure what the best strategy is,
but whichever we opt for I think the most important point is to document it
clearly.

> These hypothetical users that want to run
> an OS even older than that and don't know how to get modern crypto
> libraries on it but insist on a shiny new PostgreSQL release and build
> it from source because there are no packages available... don't exist?

Sadly I wouldn't be the least bit surprised if there are 1.0.2 users on modern
operating systems, especially given its LTS status (which OpenSSL hasn't even
capped but sells by "for as long as it remains commercially viable to do so"
basis).  That being said, my gut feeling is that 3.x has gotten pretty good
market penetration.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2023-09-07T23:48:12Z

    On Thu, Sep 07, 2023 at 01:44:11PM +0200, Daniel Gustafsson wrote:
> Sadly I wouldn't be the least bit surprised if there are 1.0.2 users on modern
> operating systems, especially given its LTS status (which OpenSSL hasn't even
> capped but sells by "for as long as it remains commercially viable to do so"
> basis).

Yes, I would not be surprised by that either.  TBH, I don't like much
the fact that we rely on OpenSSL to decide when we should cut it.  
Particularly since all the changes given to it after it got EOL'd are
close source at this point.

> That being said, my gut feeling is that 3.x has gotten pretty good
> market penetration.

Perhaps.
--
Michael

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

    Thomas Munro <thomas.munro@gmail.com> — 2023-09-08T00:19:28Z

    On Fri, Sep 8, 2023 at 11:48 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Thu, Sep 07, 2023 at 01:44:11PM +0200, Daniel Gustafsson wrote:
> > Sadly I wouldn't be the least bit surprised if there are 1.0.2 users on modern
> > operating systems, especially given its LTS status (which OpenSSL hasn't even
> > capped but sells by "for as long as it remains commercially viable to do so"
> > basis).
>
> Yes, I would not be surprised by that either.  TBH, I don't like much
> the fact that we rely on OpenSSL to decide when we should cut it.
> Particularly since all the changes given to it after it got EOL'd are
> close source at this point.

Right, just like there are people running ancient PostgreSQL and
buying support.  That's not relevant to PostgreSQL 17 IMHO, which
should target contemporary distributions.

BTW I'm not asking anyone to do anything here, I just didn't want to
allow the "RHEL ELS" and "closed source OpenSSL [sic]" theories
mentioned on this thread to go unchallenged.  Next time I'm trying to
clean up some other cruft in our tree, I don't want this thread to be
cited as evidence that that is our policy, because I don't buy it, it
doesn't make any sense.  Of course there is someone, somewhere selling
support for anything you can think of.  There are companies that
support VAXen.  There's a company in Irvine, California selling and
supporting modern drop-in replacements for PDP 11s for production use.

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

    Thomas Munro <thomas.munro@gmail.com> — 2024-03-30T20:48:31Z

    On Thu, Sep 7, 2023 at 11:44 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 7 Sep 2023, at 13:30, Thomas Munro <thomas.munro@gmail.com> wrote:
> > I don't like the idea that our *next* release's library version
> > horizon is controlled by Red Hat's "ELS" phase.
>
> Agreed.  If we instead fence it by "only non-EOL version" then 1.1.1 is also on
> the chopping block for v17 as it goes EOL in 4 days from now with 1.1.1w (which
> contains a CVE, going out with a bang).  Not sure what the best strategy is,
> but whichever we opt for I think the most important point is to document it
> clearly.

I was reminded of this thread by ambient security paranoia.  As it
stands, we require 1.0.2 (but we very much hope that package
maintainers and others in control of builds don't decide to use it).
Should we skip 1.1.1 and move to requiring 3 for v17?

Upstream says:

"The latest stable version is the 3.2 series supported until 23rd
November 2025. Also available is the 3.1 series supported until 14th
March 2025, and the 3.0 series which is a Long Term Support (LTS)
version and is supported until 7th September 2026. All older versions
(including 1.1.1, 1.1.0, 1.0.2, 1.0.0 and 0.9.8) are now out of
support and should not be used."

I understand that some distros eg RHEL8 will continue to ship and
presumably patch 1.1.1 until some date later than upstream's EOL, for
stability and the benefit of people that really need it for a bit
longer, but that's in parallel with their package for 3, right?  New
things should surely be able to require new things.  I think we'd have
to reject the argument that we should support it just because they
ship it until the year 2030, or that upstream will support anything
for $50,000/year.  I mean, they only do that because some old apps
need it, to which I say 40P01 DEADLOCK DETECTED.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-03-30T20:59:46Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> I was reminded of this thread by ambient security paranoia.  As it
> stands, we require 1.0.2 (but we very much hope that package
> maintainers and others in control of builds don't decide to use it).
> Should we skip 1.1.1 and move to requiring 3 for v17?

I'd be kind of sad if I couldn't test SSL stuff anymore on my
primary workstation, which has

$ rpm -q openssl
openssl-1.1.1k-12.el8_9.x86_64

I think it's probably true that <=1.0.2 is not in any distro that
we still need to pay attention to, but I reject the contention
that RHEL8 is not in that set.

			regards, tom lane

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

    Thomas Munro <thomas.munro@gmail.com> — 2024-03-30T21:27:44Z

    On Sun, Mar 31, 2024 at 9:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > I was reminded of this thread by ambient security paranoia.  As it
> > stands, we require 1.0.2 (but we very much hope that package
> > maintainers and others in control of builds don't decide to use it).
> > Should we skip 1.1.1 and move to requiring 3 for v17?
>
> I'd be kind of sad if I couldn't test SSL stuff anymore on my
> primary workstation, which has
>
> $ rpm -q openssl
> openssl-1.1.1k-12.el8_9.x86_64
>
> I think it's probably true that <=1.0.2 is not in any distro that
> we still need to pay attention to, but I reject the contention
> that RHEL8 is not in that set.

Hmm, OK so it doesn't have 3 available in parallel from base repos.
But it's also about to reach end of "full support" in 2 months[1], so
if we applied the policies we discussed in the LLVM-vacuuming thread
(to wit: build farm - EOL'd OSes), then...  One question I'm unclear
on is whether v17 will be packaged for RHEL8.

[1] https://access.redhat.com/product-life-cycles?product=Red%20Hat%20Enterprise%20Linux

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-02T18:55:41Z

    > On 30 Mar 2024, at 22:27, Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sun, Mar 31, 2024 at 9:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thanks a lot for bringing this up again Thomas, 1.0.2 has bitten me so many
times and I'd be thrilled to get rid of it.

>> I think it's probably true that <=1.0.2 is not in any distro that
>> we still need to pay attention to, but I reject the contention
>> that RHEL8 is not in that set.
> 
> Hmm, OK so it doesn't have 3 available in parallel from base repos.
> But it's also about to reach end of "full support" in 2 months[1], so
> if we applied the policies we discussed in the LLVM-vacuuming thread
> (to wit: build farm - EOL'd OSes), then...  One question I'm unclear
> on is whether v17 will be packaged for RHEL8.

While 1.1.1 is EOL in upstream, it won't buy us much to deprecate past it since
we don't really make use of 3.x specific functionality.  I wouldn't mind not
being on the hook to support an EOL version of OpenSSL for another 5 years, but
it also won't shift the needle too much.  For v18 I'd like to work on modernize
our OpenSSL code to make more use of 3+ features/API's and that could be a good
point to cull 1.1.1 support.

Settling for removing support for 1.0.2, which is antiques roadshow material at
this point (no TLSv1.3 support for example), removes most of the compatibility
mess we have in libpq.  1.1.1 was not a deprecation point in OpenSSL but we can
define 1.1.0 as our compatibility level to build warning-free.

The attached removes 1.0.2 support (meson build parts untested yet) with a few
small touch ups of related documentation.  I haven't yet done the research on
where that leaves LibreSSL since we don't really define anywhere what we
support (so for we've gotten by assuming it's kind of sort 1.0.2 for the parts
we care about which is skating on fairly thin ice).

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-02T21:16:39Z

    > On 2 Apr 2024, at 20:55, Daniel Gustafsson <daniel@yesql.se> wrote:

> The attached removes 1.0.2 support (meson build parts untested yet)

And a rebased version which applies over the hmac_openssl.c changes earlier
today that I hadn't pulled in.

--
Daniel Gustafsson

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-03T14:04:00Z

    On Tue, Apr 2, 2024 at 11:55 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> The attached removes 1.0.2 support (meson build parts untested yet) with a few
> small touch ups of related documentation.  I haven't yet done the research on
> where that leaves LibreSSL since we don't really define anywhere what we
> support (so for we've gotten by assuming it's kind of sort 1.0.2 for the parts
> we care about which is skating on fairly thin ice).

As far as I can tell, no versions of LibreSSL so far provide
X509_get_signature_info(), so this patch is probably a bit too
aggressive.

--Jacob

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-04-03T15:29:35Z

    Jacob Champion <jacob.champion@enterprisedb.com> writes:
> As far as I can tell, no versions of LibreSSL so far provide
> X509_get_signature_info(), so this patch is probably a bit too
> aggressive.

Another problem with cutting support is how many buildfarm members
will we lose.  I scraped recent configure logs and got the attached
results.  I count 3 machines running 1.0.1, 18 running some flavor
of 1.0.2, and 7 running various LibreSSL versions.  We could
probably retire or update the 1.0.1 installations, but the rest
would represent a heavier lift.  Notably, it seems that what macOS
is shipping is LibreSSL.

			regards, tom lane

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-03T17:25:36Z

    On Wed, Apr 3, 2024 at 8:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I count 3 machines running 1.0.1, 18 running some flavor
> of 1.0.2, and 7 running various LibreSSL versions.

I don't know all the tradeoffs with buildfarm wrangling, but IMO all
those 1.0.2 installations are the most problematic, so I dug in a bit:

  arowana CentOS 7
  batfish Ubuntu 16.04.3
  boa RHEL 7
  buri CentOS 7
  butterflyfish Photon 2.0
  clam RHEL 7.1
  cuon Ubuntu 16.04
  dhole CentOS 7.4
  hake OpenIndiana hipster
  mantid CentOS 7.9
  margay Solaris 11.4.42
  massasauga Amazon Linux 2
  myna Photon 3.0
  parula Amazon Linux 2
  rhinoceros CentOS 7.1
  shelduck SUSE 12SP5
  siskin RHEL 7.9
  snakefly Amazon Linux 2

The RHEL7-alikes are the biggest set, but that's already discussed
above. Looks like SUSE 12 goes EOL later this year (October 2024), and
it ships OpenSSL 1.1.1 as an option. Already-dead distros are Ubuntu
16.04 (April 2021), Photon 2 (January 2023), and Photon 3 (March
2024). That leaves AL2, OpenIndiana Hipster, and Solaris 11.4, all of
which appear to have newer versions of OpenSSL shipped and selectable.

--Jacob

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-04-03T17:38:50Z

    Jacob Champion <jacob.champion@enterprisedb.com> writes:
> The RHEL7-alikes are the biggest set, but that's already discussed
> above. Looks like SUSE 12 goes EOL later this year (October 2024), and
> it ships OpenSSL 1.1.1 as an option. Already-dead distros are Ubuntu
> 16.04 (April 2021), Photon 2 (January 2023), and Photon 3 (March
> 2024). That leaves AL2, OpenIndiana Hipster, and Solaris 11.4, all of
> which appear to have newer versions of OpenSSL shipped and selectable.

The discussion we had last year concluded that we were OK with
dropping 1.0.1 support when RHEL6 goes out of extended support
(June 2024 per this thread, I didn't check it).  Seems like we
should have the same policy for RHEL7.  Also, calling Photon 3
dead because it went EOL three days ago seems over-hasty.

Bottom line for me is that pulling 1.0.1 support now is OK,
but I think pulling 1.0.2 is premature.

			regards, tom lane

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-03T17:55:44Z

    On Wed, Apr 3, 2024 at 10:38 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Also, calling Photon 3
> dead because it went EOL three days ago seems over-hasty.

Well, March 1, but either way I thought "dead" for the purposes of
this thread meant "you can't build the very latest version of Postgres
on it", not "we've forgotten it exists". Back branches will continue
to need support and testing.

> Bottom line for me is that pulling 1.0.1 support now is OK,
> but I think pulling 1.0.2 is premature.

Okay, but IIUC, waiting for it to drop out of extended support means
we deal with it for four more years. That seems excessive.

--Jacob

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-04-03T18:13:57Z

    Jacob Champion <jacob.champion@enterprisedb.com> writes:
> On Wed, Apr 3, 2024 at 10:38 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Bottom line for me is that pulling 1.0.1 support now is OK,
>> but I think pulling 1.0.2 is premature.

> Okay, but IIUC, waiting for it to drop out of extended support means
> we deal with it for four more years. That seems excessive.

wikipedia says that RHEL7 ends ELS as of June 2026 [1].

			regards, tom lane

[1] https://en.wikipedia.org/wiki/Red_Hat_Enterprise_Linux#Product_life_cycle

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-03T18:17:41Z

    On Wed, Apr 3, 2024 at 11:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> wikipedia says that RHEL7 ends ELS as of June 2026 [1].

I may have misunderstood something in here then:

    https://www.redhat.com/en/blog/announcing-4-years-extended-life-cycle-support-els-red-hat-enterprise-linux-7

> ELS for RHEL 7 is now available for 4 years, starting on July 1, 2024.

Am I missing something?

--Jacob

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-03T19:08:10Z

    > On 3 Apr 2024, at 19:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Jacob Champion <jacob.champion@enterprisedb.com> writes:
>> The RHEL7-alikes are the biggest set, but that's already discussed
>> above. Looks like SUSE 12 goes EOL later this year (October 2024), and
>> it ships OpenSSL 1.1.1 as an option. Already-dead distros are Ubuntu
>> 16.04 (April 2021), Photon 2 (January 2023), and Photon 3 (March
>> 2024). That leaves AL2, OpenIndiana Hipster, and Solaris 11.4, all of
>> which appear to have newer versions of OpenSSL shipped and selectable.
> 
> The discussion we had last year concluded that we were OK with
> dropping 1.0.1 support when RHEL6 goes out of extended support
> (June 2024 per this thread, I didn't check it).  Seems like we
> should have the same policy for RHEL7.  Also, calling Photon 3
> dead because it went EOL three days ago seems over-hasty.
> 
> Bottom line for me is that pulling 1.0.1 support now is OK,
> but I think pulling 1.0.2 is premature.

Is Red Hat building and and shipping v17 packages for RHEL7 ELS customers?  If
not then it seems mostly academical to tie our dependencies to RHEL ELS unless
I'm missing something.

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-03T19:12:47Z

    > On 3 Apr 2024, at 17:29, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Jacob Champion <jacob.champion@enterprisedb.com> writes:
>> As far as I can tell, no versions of LibreSSL so far provide
>> X509_get_signature_info(), so this patch is probably a bit too
>> aggressive.
> 
> Another problem with cutting support is how many buildfarm members
> will we lose.  I scraped recent configure logs and got the attached
> results.  I count 3 machines running 1.0.1,

Support for 1.0.1 was removed with 8e278b657664 in July 2023 so those are not
building with OpenSSL enabled already.

> 18 running some flavor of 1.0.2,

massasauga and snakefly run the ssl_passphrase_callback-check test but none of
these run the ssl-check tests AFAICT, so we have very low coverage as is.  The
fact that very few animals run the ssl tests is a pet peeve of mine, it would
be nice if we could get broader coverage there.

Worth noting is that the OpenSSL check in configure.ac only reports what the
version of the OpenSSL binary in $PATH is, not which version of the library
that we build against (using --with-libs/--with-includes etc).

--
Daniel Gustafsson

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

    Andrew Dunstan <andrew@dunslane.net> — 2024-04-03T19:48:31Z

    On 2024-04-03 We 15:12, Daniel Gustafsson wrote:
> The
> fact that very few animals run the ssl tests is a pet peeve of mine, it would
> be nice if we could get broader coverage there.
>

Well, the only reason for that is that the SSL tests need to be listed 
in PG_TEST_EXTRA, and the only reason for that is that there's a 
possible hazard on multi-user servers. But I bet precious few buildfarm 
animals run in such an environment. Mine don't - I'm the only user.

Maybe we could send out an email to the buildfarm-owners list asking 
people to consider allowing the ssl tests.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-04-03T22:06:02Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> On 3 Apr 2024, at 19:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Bottom line for me is that pulling 1.0.1 support now is OK,
>> but I think pulling 1.0.2 is premature.

> Is Red Hat building and and shipping v17 packages for RHEL7 ELS customers?  If
> not then it seems mostly academical to tie our dependencies to RHEL ELS unless
> I'm missing something.

True, they won't be doing that, and neither will Devrim.  So maybe
we can leave RHEL7 out of the discussion, in which case there's
not a lot of reason to keep 1.0.2 support.  We'll need to notify
buildfarm owners to adjust their configurations.

			regards, tom lane

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-03T22:26:17Z

    > On 3 Apr 2024, at 21:48, Andrew Dunstan <andrew@dunslane.net> wrote:
> On 2024-04-03 We 15:12, Daniel Gustafsson wrote:
>>   The
>> fact that very few animals run the ssl tests is a pet peeve of mine, it would
>> be nice if we could get broader coverage there.
> 
> Well, the only reason for that is that the SSL tests need to be listed in PG_TEST_EXTRA, and the only reason for that is that       there's a possible hazard on multi-user servers. But I bet precious few buildfarm animals run in such an environment. Mine don't - I'm the only user. 
> 
> Maybe we could send out an email to the buildfarm-owners list asking people to consider allowing the ssl tests.

I think that sounds like a good idea.

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-03T22:27:27Z

    > On 4 Apr 2024, at 00:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Daniel Gustafsson <daniel@yesql.se> writes:
>> On 3 Apr 2024, at 19:38, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Bottom line for me is that pulling 1.0.1 support now is OK,
>>> but I think pulling 1.0.2 is premature.
> 
>> Is Red Hat building and and shipping v17 packages for RHEL7 ELS customers?  If
>> not then it seems mostly academical to tie our dependencies to RHEL ELS unless
>> I'm missing something.
> 
> True, they won't be doing that, and neither will Devrim.  So maybe
> we can leave RHEL7 out of the discussion, in which case there's
> not a lot of reason to keep 1.0.2 support.  We'll need to notify
> buildfarm owners to adjust their configurations.

The patch will also need to be adjusted to work with LibreSSL, but I know Jacob
was looking into that so ideally we should have something to review before
the weekend.

--
Daniel Gustafsson

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-03T22:51:22Z

    On 30.03.24 22:27, Thomas Munro wrote:
> On Sun, Mar 31, 2024 at 9:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.munro@gmail.com> writes:
>>> I was reminded of this thread by ambient security paranoia.  As it
>>> stands, we require 1.0.2 (but we very much hope that package
>>> maintainers and others in control of builds don't decide to use it).
>>> Should we skip 1.1.1 and move to requiring 3 for v17?
>>
>> I'd be kind of sad if I couldn't test SSL stuff anymore on my
>> primary workstation, which has
>>
>> $ rpm -q openssl
>> openssl-1.1.1k-12.el8_9.x86_64
>>
>> I think it's probably true that <=1.0.2 is not in any distro that
>> we still need to pay attention to, but I reject the contention
>> that RHEL8 is not in that set.
> 
> Hmm, OK so it doesn't have 3 available in parallel from base repos.
> But it's also about to reach end of "full support" in 2 months[1], so
> if we applied the policies we discussed in the LLVM-vacuuming thread
> (to wit: build farm - EOL'd OSes), then...  One question I'm unclear
> on is whether v17 will be packaged for RHEL8.

The rest of the thread talks about the end of support of RHEL 7, but you 
are here talking about RHEL 8.   It is true that "full support" for RHEL 
8 ended in May 2024, but that is the not the one we are tracking.  We 
are tracking the 10-year one, which I suppose is now called "maintenance 
support".

So if the above package list is correct, then we ought to keep 
supporting openssl 1.1.* until 2029.

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-03T23:24:25Z

    On Wed, Apr 03, 2024 at 01:38:50PM -0400, Tom Lane wrote:
> The discussion we had last year concluded that we were OK with
> dropping 1.0.1 support when RHEL6 goes out of extended support
> (June 2024 per this thread, I didn't check it).  Seems like we
> should have the same policy for RHEL7.  Also, calling Photon 3
> dead because it went EOL three days ago seems over-hasty.

Yeah.  A bunch of users of Photon are VMware (or you could say
Broadcom) product appliances, and I'd suspect that quite a lot of them
rely on Photon 3 for their base OS image.  Upgrading that stuff is not
easy work in my experience because they need to cope with a bunch of
embedded services.

> Bottom line for me is that pulling 1.0.1 support now is OK,
> but I think pulling 1.0.2 is premature.

Yeah, I guess so.  At least that seems like the safest conclusion
currently here.  The build-time check on X509_get_signature_info()
would still be required.

I'd love being able to rip out the internal locking logic currently in
libpq as LibreSSL has traces of CRYPTO_lock(), as far as I've checked,
and we rely on its existence.
--
Michael

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

    Thomas Munro <thomas.munro@gmail.com> — 2024-04-03T23:50:52Z

    On Thu, Apr 4, 2024 at 11:51 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> On 30.03.24 22:27, Thomas Munro wrote:
> > Hmm, OK so it doesn't have 3 available in parallel from base repos.
> > But it's also about to reach end of "full support" in 2 months[1], so
> > if we applied the policies we discussed in the LLVM-vacuuming thread
> > (to wit: build farm - EOL'd OSes), then...  One question I'm unclear
> > on is whether v17 will be packaged for RHEL8.
>
> The rest of the thread talks about the end of support of RHEL 7, but you
> are here talking about RHEL 8.   It is true that "full support" for RHEL
> 8 ended in May 2024, but that is the not the one we are tracking.  We
> are tracking the 10-year one, which I suppose is now called "maintenance
> support".

I might have confused myself with the two EOLs and some wishful
thinking.  I am a lot less worked up about this general topic now that
RHEL has moved to "rolling" LLVM updates in minor releases, removing a
physical-pain-inducing 10-year vacuuming horizon (that's 20 LLVM major
releases and they only fix bugs in one...).  I will leave openssl
discussions to those more knowledgeable about that.

> So if the above package list is correct, then we ought to keep
> supporting openssl 1.1.* until 2029.

That's a shame.  But it sounds like the developer burden isn't so
different from 1.1.1 to 3.x, so maybe it's not such a big deal from
our point of view.  (I have no opinion on the security ramifications
of upstream's EOL, but as a layman it sounds completely bonkers to use
it.  I wonder why the packaging community wouldn't just arrange to
have a supported-by-upstream 3.x package in their RPM repo when they
supply the newest PostgreSQL versions for the oldest RHEL, but again
not my area so I'll shut up).

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-04T06:49:46Z

    > On 4 Apr 2024, at 00:51, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 30.03.24 22:27, Thomas Munro wrote:
>> On Sun, Mar 31, 2024 at 9:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Thomas Munro <thomas.munro@gmail.com> writes:
>>>> I was reminded of this thread by ambient security paranoia.  As it
>>>> stands, we require 1.0.2 (but we very much hope that package
>>>> maintainers and others in control of builds don't decide to use it).
>>>> Should we skip 1.1.1 and move to requiring 3 for v17?
>>> 
>>> I'd be kind of sad if I couldn't test SSL stuff anymore on my
>>> primary workstation, which has
>>> 
>>> $ rpm -q openssl
>>> openssl-1.1.1k-12.el8_9.x86_64
>>> 
>>> I think it's probably true that <=1.0.2 is not in any distro that
>>> we still need to pay attention to, but I reject the contention
>>> that RHEL8 is not in that set.
>> Hmm, OK so it doesn't have 3 available in parallel from base repos.
>> But it's also about to reach end of "full support" in 2 months[1], so
>> if we applied the policies we discussed in the LLVM-vacuuming thread
>> (to wit: build farm - EOL'd OSes), then...  One question I'm unclear
>> on is whether v17 will be packaged for RHEL8.
> 
> The rest of the thread talks about the end of support of RHEL 7, but you are here talking about RHEL 8.   It is true that "full support" for RHEL 8 ended in May 2024, but that is the not the one we are tracking.  We are tracking the 10-year one, which I suppose is now called "maintenance support".
> 
> So if the above package list is correct, then we ought to keep supporting openssl 1.1.* until 2029.

Not 1.1.* but 1.1.1+.  In the old OpenSSL version numbering scheme, releases
changing the last digit would contain new features and releases that updated
the appended letter only fixed bugs.
--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-04T07:56:22Z

    > On 4 Apr 2024, at 01:24, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Wed, Apr 03, 2024 at 01:38:50PM -0400, Tom Lane wrote:
>> The discussion we had last year concluded that we were OK with
>> dropping 1.0.1 support when RHEL6 goes out of extended support
>> (June 2024 per this thread, I didn't check it).  Seems like we
>> should have the same policy for RHEL7.  Also, calling Photon 3
>> dead because it went EOL three days ago seems over-hasty.
> 
> Yeah.  A bunch of users of Photon are VMware (or you could say
> Broadcom) product appliances, and I'd suspect that quite a lot of them
> rely on Photon 3 for their base OS image.  Upgrading that stuff is not
> easy work in my experience because they need to cope with a bunch of
> embedded services.

That's true, but Photon3 won't package new major versions of PostgreSQL (the
latest RPM is 13.14).  Anyone who builds v17 on Photon 3 on their own can just
as well be expected to build an updated OpenSSL no?  This is equivalent to
RHEL7 which was discussed elsewhere in this thread.

If we are going to pin version dependencies for postgres X to available OS
release packages then it, IMHO, is reasonable to be for OS's that realistically
will package X (either by the vendor or a trusted external packager like PGDG).

It's possible, but not guaranteed, that RHEL8 ships v17 packages in ther
Application Streams Life Cycle model, they have packaged v15 so far with
retirement in 2028 so it seems likely there will be another package to retire
in 2029 when RHEL8 finally goes away (whether that will be v16 or v17 is also
speculation).  Thus, pinning on 1.1.1 is grounded in packaging reality, even
though I sincerely hope that noone who isn't paying for support is running
1.1.1 now, let alone in 4 years from now.

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-04T07:59:08Z

    > On 4 Apr 2024, at 01:50, Thomas Munro <thomas.munro@gmail.com> wrote:

> That's a shame.  But it sounds like the developer burden isn't so
> different from 1.1.1 to 3.x, so maybe it's not such a big deal from
> our point of view.

It isn't as of now since OpenSSL still supply the deprecated API's we use, but
there is no guarantee that they will do that for 5 more years.

--
Daniel Gustafsson

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-04T18:03:35Z

    On Wed, Apr 3, 2024 at 3:27 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> The patch will also need to be adjusted to work with LibreSSL, but I know Jacob
> was looking into that so ideally we should have something to review before
> the weekend.

v3 does that by putting back checks for symbols that aren't part of
LibreSSL (tested back to 2.7, which is where the 1.1.x APIs started to
arrive). It also makes adjustments for the new OPENSSL_API_COMPAT
version, getting rid of OpenSSL_add_all_algorithms() and adding a
missing header.

This patch has a deficiency where 1.1.0 itself isn't actually rejected
at configure time; Daniel's working on an explicit check for the
OPENSSL/LIBRESSL_VERSION_NUMBER that should fix that up. There's an
open question about which version we should pin for LibreSSL, which
should ultimately come down to which versions of OpenBSD we want PG17
to support.

Thanks,
--Jacob

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-05T01:37:03Z

    (Adding Mikael Kjellstrom in CC as OpenBSD owner)

On Thu, Apr 04, 2024 at 11:03:35AM -0700, Jacob Champion wrote:
> v3 does that by putting back checks for symbols that aren't part of
> LibreSSL (tested back to 2.7, which is where the 1.1.x APIs started to
> arrive).

From where did you pull the LibreSSL sources?  Directly from the
OpenBSD tree?

> It also makes adjustments for the new OPENSSL_API_COMPAT
> version, getting rid of OpenSSL_add_all_algorithms() and adding a
> missing header.

Ah, right.  OpenSSL_add_all_algorithms() is documented as having no
effect in 1.1.0.

> This patch has a deficiency where 1.1.0 itself isn't actually rejected
> at configure time; Daniel's working on an explicit check for the
> OPENSSL/LIBRESSL_VERSION_NUMBER that should fix that up. There's an
> open question about which version we should pin for LibreSSL, which
> should ultimately come down to which versions of OpenBSD we want PG17
> to support.

I would be OK to draw a line to what we test in the buildfarm if it
comes to that, down to OpenBSD 6.9.  This version is already not
supported, and we had a number of issues with older versions and
timestamps going backwards.

-/* Define to 1 if you have the `CRYPTO_lock' function. */
-#undef HAVE_CRYPTO_LOCK

I'm happy to see that gone for good.

+      # Functions introduced in OpenSSL 1.1.0/LibreSSL 2.7.0.
+      ['OPENSSL_init_ssl', {'required': true}],
+      ['BIO_meth_new', {'required': true}],
+      ['ASN1_STRING_get0_data', {'required': true}],
+      ['HMAC_CTX_new', {'required': true}],
+      ['HMAC_CTX_free', {'required': true}],

These should be removed to save cycles in ./configure and meson, no?
We don't have any more of their HAVE_* flags in the tree with this
patch applied.

-      cdata.set('OPENSSL_API_COMPAT', '0x10002000L',
+      cdata.set('OPENSSL_API_COMPAT', '0x10100000L',

Seems to me that this should also document in meson.build why 1.1.0 is
chosen, same as ./configure.

It seems to me that src/common/protocol_openssl.c could be cleaned up;
I see SSL_CTX_set_min_proto_version and SSL_CTX_set_max_proto_version
listed in LibreSSL (looking at some past version of
https://github.com/libressl/libressl.git that I still have around).

There is an extra thing in pg_strong_random.c once we cut OpenSSL <
1.1.1..  Do we still need pg_strong_random_init() and its RAND_poll()
when it comes to LibreSSL?  This is a sensitive area, so we should be
careful.
--
Michael

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

    Mikael Kjellström <mikael.kjellstrom@mksoft.nu> — 2024-04-05T06:11:43Z

    On 2024-04-05 03:37, Michael Paquier wrote:
> (Adding Mikael Kjellstrom in CC as OpenBSD owner)

My 2 OpenBSD animals (morepork 
<https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=morepork&br=HEAD> 
OpenBSD 6.9, schnauzer 
<https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=schnauzer&br=HEAD> 
OpenBSD 7.3) is using OpenSSL and not LibreSSL.

The versions are:

OpenBSD 6.9 - openssl-1.1.1k
OpenBSD 7.3 - openssl-3.1.0p1

and that is what I installed them with when I setup both animals.

Is there anything you need me change or do?

/Mikael

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-05T16:41:46Z

    On Thu, Apr 4, 2024 at 6:37 PM Michael Paquier <michael@paquier.xyz> wrote:
> From where did you pull the LibreSSL sources?  Directly from the
> OpenBSD tree?

I've been building LibreSSL Portable: https://github.com/libressl/portable

> Ah, right.  OpenSSL_add_all_algorithms() is documented as having no
> effect in 1.1.0.

I think it still has an effect, but it's unnecessary unless you need
some specific initialization other than the defaults -- for which we
now have OPENSSL_init_crypto(). Best I could tell, pgcrypto had no
such requirements (but extra eyes on that would be appreciated).

> I would be OK to draw a line to what we test in the buildfarm if it
> comes to that, down to OpenBSD 6.9.

That would correspond to LibreSSL 3.3 if I'm not mistaken. Any
particular reason for 6.9 as the dividing line, and not something
later? And by "test in the buildfarm", do you mean across all
versions, or just what we support for PG17? (For the record, I don't
think there's any reason to drop older LibreSSL testing for earlier
branches.)

> This version is already not
> supported, and we had a number of issues with older versions and
> timestamps going backwards.

Speaking of which: for completeness I should note that LibreSSL 3.2
(OpenBSD 6.8) is failing src/test/ssl because of alternate error
messages. That failure exists on HEAD and is not introduced in this
patchset. LibreSSL 3.3, which passes, has the following changelog
note:

  * If x509_verify() fails, ensure that the error is set on both
    the x509_verify_ctx() and its store context to make some failures
    visible from SSL_get_verify_result().

So that would explain that. If we drop support for 3.2 and earlier
then there's nothing to be done anyway.

> -/* Define to 1 if you have the `CRYPTO_lock' function. */
> -#undef HAVE_CRYPTO_LOCK
>
> I'm happy to see that gone for good.

+1

> +      # Functions introduced in OpenSSL 1.1.0/LibreSSL 2.7.0.
> +      ['OPENSSL_init_ssl', {'required': true}],
> +      ['BIO_meth_new', {'required': true}],
> +      ['ASN1_STRING_get0_data', {'required': true}],
> +      ['HMAC_CTX_new', {'required': true}],
> +      ['HMAC_CTX_free', {'required': true}],
>
> These should be removed to save cycles in ./configure and meson, no?
> We don't have any more of their HAVE_* flags in the tree with this
> patch applied.

True, but they are required, and I needed something in there to reject
earlier builds. Daniel's suggested approach with _VERSION_NUMBER
should be able to replace this I think.

> -      cdata.set('OPENSSL_API_COMPAT', '0x10002000L',
> +      cdata.set('OPENSSL_API_COMPAT', '0x10100000L',
>
> Seems to me that this should also document in meson.build why 1.1.0 is
> chosen, same as ./configure.

Good point.

> It seems to me that src/common/protocol_openssl.c could be cleaned up;
> I see SSL_CTX_set_min_proto_version and SSL_CTX_set_max_proto_version
> listed in LibreSSL (looking at some past version of
> https://github.com/libressl/libressl.git that I still have around).
>
> There is an extra thing in pg_strong_random.c once we cut OpenSSL <
> 1.1.1..  Do we still need pg_strong_random_init() and its RAND_poll()
> when it comes to LibreSSL?  This is a sensitive area, so we should be
> careful.

It would be cool if there are more cleanups that can happen. I agree
we need to be careful around removal, though, especially now that we
know that LibreSSL testing is spotty... I will look more into these
later today.

Thanks,
--Jacob

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-05T16:59:54Z

    > On 5 Apr 2024, at 03:37, Michael Paquier <michael@paquier.xyz> wrote:
> On Thu, Apr 04, 2024 at 11:03:35AM -0700, Jacob Champion wrote:

>> v3 does that by putting back checks for symbols that aren't part of
>> LibreSSL (tested back to 2.7, which is where the 1.1.x APIs started to
>> arrive).
> 
> From where did you pull the LibreSSL sources?  Directly from the
> OpenBSD tree?
> 
>> It also makes adjustments for the new OPENSSL_API_COMPAT
>> version, getting rid of OpenSSL_add_all_algorithms() and adding a
>> missing header.
> 
> Ah, right.  OpenSSL_add_all_algorithms() is documented as having no
> effect in 1.1.0.

This API was deprecated and made into a no-op in OpenBSD 6.4 which corresponds
to LibreSSL 2.8.3.

>> This patch has a deficiency where 1.1.0 itself isn't actually rejected
>> at configure time; Daniel's working on an explicit check for the
>> OPENSSL/LIBRESSL_VERSION_NUMBER that should fix that up. There's an
>> open question about which version we should pin for LibreSSL, which
>> should ultimately come down to which versions of OpenBSD we want PG17
>> to support.
> 
> I would be OK to draw a line to what we test in the buildfarm if it
> comes to that, down to OpenBSD 6.9.  This version is already not
> supported, and we had a number of issues with older versions and
> timestamps going backwards.

There is a member on 6.8 as well, and while 6.8 work fine the tests all fail
due to the error messages being different.  Rather than adding alternate output
for an EOL version of OpenBSD (which currently don't even run the ssl checks in
the BF) I think using 6.9 as the minimum makes sense.

> +      # Functions introduced in OpenSSL 1.1.0/LibreSSL 2.7.0.
> +      ['OPENSSL_init_ssl', {'required': true}],
> +      ['BIO_meth_new', {'required': true}],
> +      ['ASN1_STRING_get0_data', {'required': true}],
> +      ['HMAC_CTX_new', {'required': true}],
> +      ['HMAC_CTX_free', {'required': true}],
> 
> These should be removed to save cycles in ./configure and meson, no?

Correct, they are removed in favor of a compile test for OpenSSL version.

> -      cdata.set('OPENSSL_API_COMPAT', '0x10002000L',
> +      cdata.set('OPENSSL_API_COMPAT', '0x10100000L',
> 
> Seems to me that this should also document in meson.build why 1.1.0 is
> chosen, same as ./configure.

Done.

> It seems to me that src/common/protocol_openssl.c could be cleaned up;
> I see SSL_CTX_set_min_proto_version and SSL_CTX_set_max_proto_version
> listed in LibreSSL (looking at some past version of
> https://github.com/libressl/libressl.git that I still have around).

Both SSL_CTX_set_min_proto_version and SSL_CTX_set_max_proto_version are
available in at least OpenBSD 6.3 which is LibreSSL 2.7.5.  With this we can
thus remove the whole file.

> There is an extra thing in pg_strong_random.c once we cut OpenSSL <
> 1.1.1..  Do we still need pg_strong_random_init() and its RAND_poll()
> when it comes to LibreSSL?  This is a sensitive area, so we should be
> careful.

Re-reading the thread which added this comment, and the OpenSSL docs and code,
I'm leaning towards leaving this in.  The overhead is marginal and fork safety
has been broken at least once in OpenSSL since 1.1.1:

    https://github.com/openssl/openssl/issues/12377

That particular bug was thankfully caught before it shipped, but mitigating the
risk is this cheap enough that is seems reasonable to keep this in.

Attached is a WIP patch to get more eyes on it, the Meson test for 1.1.1 fails
on Windows in CI which I will investigate next.

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-05T17:03:28Z

    > On 5 Apr 2024, at 18:41, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
> On Thu, Apr 4, 2024 at 6:37 PM Michael Paquier <michael@paquier.xyz> wrote:

>> I would be OK to draw a line to what we test in the buildfarm if it
>> comes to that, down to OpenBSD 6.9.
> 
> That would correspond to LibreSSL 3.3 if I'm not mistaken. Any
> particular reason for 6.9 as the dividing line, and not something
> later? And by "test in the buildfarm", do you mean across all
> versions, or just what we support for PG17? (For the record, I don't
> think there's any reason to drop older LibreSSL testing for earlier
> branches.)

We should draw the line on something we can reliably test, so 6.9 seems fine to
me (unless there is evidence of older versions being common in the wild).
OpenBSD themselves support 2 backbranches so 6.9 is still far beyond the EOL
mark upstream.

--
Daniel Gustafsson

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-05T20:55:22Z

    On Fri, Apr 5, 2024 at 9:59 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> Attached is a WIP patch to get more eyes on it, the Meson test for 1.1.1 fails
> on Windows in CI which I will investigate next.

The changes for SSL_OP_NO_CLIENT_RENEGOTIATION and
SSL_R_VERSION_TOO_LOW look good to me.

>     -    Remove support for OpenSSL 1.0.2 and 1.1.0
>     +    Remove support for OpenSSL 1.0.2

I modified the commit message while working on v3 and forgot to put it
back before posting, sorry.

> +++ b/src/include/pg_config.h.in
> @@ -84,9 +84,6 @@
>  /* Define to 1 if you have the <crtdefs.h> header file. */
>  #undef HAVE_CRTDEFS_H
>
> -/* Define to 1 if you have the `CRYPTO_lock' function. */
> -#undef HAVE_CRYPTO_LOCK

An autoreconf run on my machine pulls in more changes (getting rid of
the symbols we no longer check for).

--Jacob

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-05T21:26:00Z

    On 05.04.24 18:59, Daniel Gustafsson wrote:
> Attached is a WIP patch to get more eyes on it, the Meson test for 1.1.1 fails
> on Windows in CI which I will investigate next.

I'm not a fan of the new PGAC_CHECK_OPENSSL.  It creates a second place 
where the OpenSSL version number has to be updated.  We had this 
carefully constructed so that there is only one place that 
OPENSSL_API_COMPAT is defined and that is the only place that needs to 
be updated.  We put the setting of OPENSSL_API_COMPAT into configure so 
that the subsequent OpenSSL tests would use it, and if the API number 
higher than what the library supports, the tests should fail.  So I 
don't understand why the configure changes have to be so expansive.

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-05T21:48:30Z

    > On 5 Apr 2024, at 23:26, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 05.04.24 18:59, Daniel Gustafsson wrote:
>> Attached is a WIP patch to get more eyes on it, the Meson test for 1.1.1 fails
>> on Windows in CI which I will investigate next.
> 
> I'm not a fan of the new PGAC_CHECK_OPENSSL.  It creates a second place where the OpenSSL version number has to be updated.  We had this carefully constructed so that there is only one place that OPENSSL_API_COMPAT is defined and that is the only place that needs to be updated.  We put the setting of OPENSSL_API_COMPAT into configure so that the subsequent OpenSSL tests would use it, and if the API number higher than what the library supports, the tests should fail.

But does that actually work?  If I change the API_COMPAT to the 1.1.1 version
number and run configure against 1.0.2 it passes just fine.  Am I missing some
clever trick here?

The reason to expand the check is to ensure that we have the version we want
for both OpenSSL and LibreSSL, and deprecating OpenSSL versions isn't all that
commonly done so having to change the version in the check didn't seem that
invasive to me.

--
Daniel Gustafsson

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-05T21:53:22Z

    On Fri, Apr 5, 2024 at 2:48 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> But does that actually work?  If I change the API_COMPAT to the 1.1.1 version
> number and run configure against 1.0.2 it passes just fine.  Am I missing some
> clever trick here?

Similarly, I changed my API_COMPAT to a nonsense 0x90100000L and
- a 1.1.1 build succeeds
- a 3.0 build fails
- LibreSSL doesn't appear to care or check that #define at all

--Jacob

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-05T22:31:57Z

    > On 5 Apr 2024, at 22:55, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
> 
> On Fri, Apr 5, 2024 at 9:59 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>> Attached is a WIP patch to get more eyes on it, the Meson test for 1.1.1 fails
>> on Windows in CI which I will investigate next.

The attached version fixes the Windows 1.1.1 check which was missing the
include directory.

> The changes for SSL_OP_NO_CLIENT_RENEGOTIATION and
> SSL_R_VERSION_TOO_LOW look good to me.

Thanks!

>> +++ b/src/include/pg_config.h.in
>> @@ -84,9 +84,6 @@
>> /* Define to 1 if you have the <crtdefs.h> header file. */
>> #undef HAVE_CRTDEFS_H
>> 
>> -/* Define to 1 if you have the `CRYPTO_lock' function. */
>> -#undef HAVE_CRYPTO_LOCK
> 
> An autoreconf run on my machine pulls in more changes (getting rid of
> the symbols we no longer check for).

Ah yes, missed updating before formatting the patch. Done in the attached.

--
Daniel Gustafsson

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-06T00:14:51Z

    On Fri, Apr 5, 2024 at 3:32 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > An autoreconf run on my machine pulls in more changes (getting rid of
> > the symbols we no longer check for).
>
> Ah yes, missed updating before formatting the patch. Done in the attached.

The commit subject may still need to be reverted to how you had it
originally, unless you are keeping my omission of "1.1.0" on purpose.

I've tested (with Meson) on LibreSSL 3.3 to 3.8, and verified that 2.7
to 3.2 now fail to configure. Similarly, OpenSSL 1.0.2 and 1.1.0 fail
to configure, and I ran tests with 1.1.1 to 3.3. I did a quick smoke
test with autoconf to make sure that old versions are rejected there
too, but I didn't run all the tests again.

Maybe there was a good reason for them to do it, but I'm kind of
amazed that LibreSSL camped on the 2.x version number, then revved to
a 3.x line anyway and camped on those numbers too, so that Meson can't
just rely on pkgconfig to figure out which version we have.

--Jacob

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-06T06:02:23Z

    On 05.04.24 23:48, Daniel Gustafsson wrote:
> The reason to expand the check is to ensure that we have the version we want
> for both OpenSSL and LibreSSL, and deprecating OpenSSL versions isn't all that
> commonly done so having to change the version in the check didn't seem that
> invasive to me.

Why do we need to check for the versions at all?  We should just check 
for the functions we need.  At least that's always been the normal 
approach in configure.

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-06T07:12:01Z

    > On 6 Apr 2024, at 08:02, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 05.04.24 23:48, Daniel Gustafsson wrote:
>> The reason to expand the check is to ensure that we have the version we want
>> for both OpenSSL and LibreSSL, and deprecating OpenSSL versions isn't all that
>> commonly done so having to change the version in the check didn't seem that
>> invasive to me.
> 
> Why do we need to check for the versions at all?  We should just check for the functions we need.  At least that's always been the normal approach in configure.

We could, but finding a stable set of functions which identifies the version of
OpenSSL *and* LibreSSL that we want, and their successors, while not matching
any older versions seemed more opaque than testing two numeric values.  The
suggested check is modelled on the LDAP check which tests for an explicit
version in a header file (albeit not for erroring out).

--
Daniel Gustafsson

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-04-06T14:04:45Z

    Daniel Gustafsson <daniel@yesql.se> writes:
>> On 6 Apr 2024, at 08:02, Peter Eisentraut <peter@eisentraut.org> wrote:
>> Why do we need to check for the versions at all?  We should just check for the functions we need.  At least that's always been the normal approach in configure.

> We could, but finding a stable set of functions which identifies the version of
> OpenSSL *and* LibreSSL that we want, and their successors, while not matching
> any older versions seemed more opaque than testing two numeric values.

I don't think you responded to Peter's point at all.  The way autoconf
is designed to work is explicitly NOT to try to identify the exact
version of $whatever.  Rather, the idea is to probe for the API
features that you want to rely on: functions, macros, struct fields,
or the like.  If you can't point to an important API difference
between 1.0.2 and 1.1.1, why drop support for 1.0.2?

			regards, tom lane

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-06T17:47:43Z

    > On 6 Apr 2024, at 16:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Daniel Gustafsson <daniel@yesql.se> writes:
>>> On 6 Apr 2024, at 08:02, Peter Eisentraut <peter@eisentraut.org> wrote:

>>> Why do we need to check for the versions at all?  We should just check for the functions we need.  At least that's always been the normal approach in configure.
> 
>> We could, but finding a stable set of functions which identifies the version of
>> OpenSSL *and* LibreSSL that we want, and their successors, while not matching
>> any older versions seemed more opaque than testing two numeric values.
> 
> I don't think you responded to Peter's point at all.  The way autoconf
> is designed to work is explicitly NOT to try to identify the exact
> version of $whatever.  Rather, the idea is to probe for the API
> features that you want to rely on: functions, macros, struct fields,
> or the like.  If you can't point to an important API difference
> between 1.0.2 and 1.1.1, why drop support for 1.0.2?

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

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.

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.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-07T22:46:18Z

    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

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-07T23:01:35Z

    > On 8 Apr 2024, at 00:46, Michael Paquier <michael@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

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-10T07:31:16Z

    On 06.04.24 19:47, Daniel Gustafsson wrote:
> 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.

I think it might be better to separate this into two steps:

1. Move to 1.1.0.  This is an API update.  Change OPENSSL_API_COMPAT, 
and remove a bunch of code that no longer needs to be conditional.  We 
could check for a representative function like OPENSSL_init_ssl() in 
configure/meson, or we could just let the compilation fail with older 
versions.

2. Move to 1.1.1.  I understand this has to do with the fork-safety of 
pg_strong_random(), and it's not an API change but a behavior change. 
Let's make this association clearer in the code.  For example, add a 
version check or assertion about this into pg_strong_random() itself.

I don't know how LibreSSL interacts with either of these two points. 
That's something that could be clearer.

Some more detailed review on the v6 patch:

* doc/src/sgml/libpq.sgml

This small documentation patch could be committed forthwith.

* src/backend/libpq/be-secure-openssl.c

+#include <openssl/bn.h>

This patch doesn't appear to add anything, so why does it need a new 
include?

Could the additions of SSL_OP_NO_CLIENT_RENEGOTIATION and 
SSL_R_VERSION_TOO_LOW be separate patches?

* src/common/hmac_openssl.c

There appears to be some unrelated refactoring happening here?

* src/include/common/openssl.h

Is the comment no longer applicable to OpenSSL, only to LibreSSL?

* src/port/pg_strong_random.c

I would prefer to remove pg_strong_random_init() if it's no longer 
useful.  I mean, if we leave it as is, and we are not removing any 
callers, then we are effectively continuing to support OpenSSL <1.1.1, 
right?

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

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-04-10T13:10:56Z

    On Wed, Apr 10, 2024 at 12:31 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> * src/backend/libpq/be-secure-openssl.c
>
> +#include <openssl/bn.h>
>
> This patch doesn't appear to add anything, so why does it need a new
> include?

This one was mine -- it was an indirect header dependency that was
effectively removed in 1.1.0 and later, due to the bump to
OPENSSL_API_COMPAT [1]. We have to depend on it directly now.

--Jacob

[1] https://github.com/openssl/openssl/blob/b372b1f764/include/openssl/dh.h#L20-L22

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-10T22:43:24Z

    On Wed, Apr 10, 2024 at 09:31:16AM +0200, Peter Eisentraut wrote:
> I think it might be better to separate this into two steps:
> 
> 1. Move to 1.1.0.  This is an API update.  Change OPENSSL_API_COMPAT, and
> remove a bunch of code that no longer needs to be conditional.  We could
> check for a representative function like OPENSSL_init_ssl() in
> configure/meson, or we could just let the compilation fail with older
> versions.
> 
> 2. Move to 1.1.1.  I understand this has to do with the fork-safety of
> pg_strong_random(), and it's not an API change but a behavior change. Let's
> make this association clearer in the code.  For example, add a version check
> or assertion about this into pg_strong_random() itself.

+1 for a split and a two-step move.  The areas cleaned up are not
really dependent.

> I don't know how LibreSSL interacts with either of these two points. That's
> something that could be clearer.

Not looked at that, unfortunately.  Cutting to one specific version of
LibreSSL would help.

> I would prefer to remove pg_strong_random_init() if it's no longer useful.
> I mean, if we leave it as is, and we are not removing any callers, then we
> are effectively continuing to support OpenSSL <1.1.1, right?

I'd rather see it gone too, at the end, but I also get that the
concerns from Daniel are worth keeping in mind.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-12T12:42:57Z

    > On 10 Apr 2024, at 09:31, Peter Eisentraut <peter@eisentraut.org> wrote:

> I think it might be better to separate this into two steps:

Fair enough.

> 1. Move to 1.1.0.  This is an API update.  Change OPENSSL_API_COMPAT, and remove a bunch of code that no longer needs to be conditional.  We could check for a representative function like OPENSSL_init_ssl() in configure/meson, or we could just let the compilation fail with older versions.

The attached 0002 bumps the minimum required version to 1.1.0 with a hard error
in autoconf/meson, and removes all the 1.0.2 support code.

I think the documentation for PQinitOpenSSL should be reworded from "you have
to do this, unless you run 1.1.0 or later" to "If you run 1.1.0, you need to do
this).  As it is now the important bit of the paragrapg is at the end rather
than in the beginning.  Trying this I didn't find a wording which seemed like
an improvement though, suggestions are welcome.

> 2. Move to 1.1.1.  I understand this has to do with the fork-safety of pg_strong_random(), and it's not an API change but a behavior change. Let's make this association clearer in the code.  For example, add a version check or assertion about this into pg_strong_random() itself.

0005 moves the fork safety init inline with calling pg_strong_random, and
removes it for everyone else.  This allows 1.1.0 to be supported as we
effectively are at the 1.1.0 API level, at the cost of calls for strong random
being slower on 1.1.0.  An unscientific guess based on packaged OpenSSL
versions and the EOL and ELS/LTS status of 1.1.0, is that the number of
production installs of postgres 17 using OpenSSL 1.1.0 is close to zero.

> I don't know how LibreSSL interacts with either of these two points. That's something that could be clearer.

The oldest LibreSSL we have in the buildfarm is 3.2 (from OpenBSD 6.8), which
the attached version supports and passes tests with.  LibreSSL has been
providing fork safety since 2.0.2 which is well into the past.

> * doc/src/sgml/libpq.sgml
> 
> This small documentation patch could be committed forthwith.

Agreed, separated into 0001 in the attached and can be committed regardless of
the remaining ones.

> * src/backend/libpq/be-secure-openssl.c
> 
> +#include <openssl/bn.h>
> 
> This patch doesn't appear to add anything, so why does it need a new include?

As mentioned downthread, an indirect inclusion was removed so we need to
explicitly include it.

> Could the additions of SSL_OP_NO_CLIENT_RENEGOTIATION and SSL_R_VERSION_TOO_LOW be separate patches?

They can, done in the attached.

SSL_R_VERSION_TOO_LOW was introduced quite recently in LibreSSL 3.6.3 (OpenBSD
7.2), but splitting the check for TOO_LOW and TOO_HIGH into two seems pretty
uncontroversial and a good way to get ever so slightly better error handling on
recent LibreSSL.

SSL renegotiation has been supported for much longer in LibreSSL so adding that
to make OpenSSL and LibreSSL support slightly more on par seems seems like a
good idea regardless of version bump.

> * src/common/hmac_openssl.c
> 
> There appears to be some unrelated refactoring happening here?

I assume you mean changing back to FRONTEND from USE_RESOWNER_FOR_HMAC, the
latter was added recently in 38698dd38 and have never shipped, so it seemed
more backpatch-friendly to move back.  Given the amount of changes it probably
won't move the needle though so reverted.

> * src/include/common/openssl.h
> 
> Is the comment no longer applicable to OpenSSL, only to LibreSSL?

OpenSSL has since 0.9.8 defined TLS_MAX_VERSION which points highest version
TLS protocol supported, but AFAIK there is no such construction in LibreSSL.
Assuming I didn't totally misunderstand the comment of course.

> * src/port/pg_strong_random.c
> 
> I would prefer to remove pg_strong_random_init() if it's no longer useful.  I mean, if we leave it as is, and we are not removing any callers, then we are effectively continuing to support OpenSSL <1.1.1, right?

The attached removes pg_strong_random_init and instead calls it explicitly for
1.1.0 users by checking the OpenSSL version.

Is the attached split in line with how you were thinking about it?

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-15T05:04:12Z

    On Fri, Apr 12, 2024 at 02:42:57PM +0200, Daniel Gustafsson wrote:
>> On 10 Apr 2024, at 09:31, Peter Eisentraut <peter@eisentraut.org> wrote:
>> 2. Move to 1.1.1.  I understand this has to do with the fork-safety of pg_strong_random(), and it's not an API change but a behavior change. Let's make this association clearer in the code.  For example, add a version check or assertion about this into pg_strong_random() itself.

> 0005 moves the fork safety init inline with calling pg_strong_random, and
> removes it for everyone else.  This allows 1.1.0 to be supported as we
> effectively are at the 1.1.0 API level, at the cost of calls for strong random
> being slower on 1.1.0.  An unscientific guess based on packaged OpenSSL
> versions and the EOL and ELS/LTS status of 1.1.0, is that the number of
> production installs of postgres 17 using OpenSSL 1.1.0 is close to zero.

It is only necessary to call RAND_poll once after forking.  Wouldn't
it be OK to use a static flag and use the initialization once?

>> * src/port/pg_strong_random.c
>> 
>> I would prefer to remove pg_strong_random_init() if it's no longer
>> useful.  I mean, if we leave it as is, and we are not removing any
>> callers, then we are effectively continuing to support OpenSSL
>> <1.1.1, right?
> 
> The attached removes pg_strong_random_init and instead calls it explicitly for
> 1.1.0 users by checking the OpenSSL version.
> 
> Is the attached split in line with how you were thinking about it?

If I may, 0001 looks sensible here.  The bits from 0003 and 0004 could
be applied before 0002, as well.

--- a/src/backend/postmaster/fork_process.c
+++ b/src/backend/postmaster/fork_process.c
@@ -110,9 +110,6 @@ fork_process(void)
 				close(fd);
 			}
 		}
-
-		/* do post-fork initialization for random number generation */
-		pg_strong_random_init();

Perhaps you intented this diff to be in 0005 rather than in 0002?
With 0002 applied, only support for 1.0.2 is removed, not 1.1.0 yet.

 pg_strong_random(void *buf, size_t len)
 {
 	int			i;
 
+#if (OPENSSL_VERSION_NUMBER <= 0x10100000L)
+	/*
+	 * Make sure processes do not share OpenSSL randomness state.  This is not
+	 * requred on LibreSSL and no longer required in OpenSSL 1.1.1 and later
+	 * versions.
+	 */
+	RAND_poll();
+#endif

s/requred/required/.  Rather than calling always RAND_poll(), this
could use a static flag to call it only once when pg_strong_random is
called for the first time.  I would not mind seeing this part entirely
gone with the removal of support for 1.1.0.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-15T09:07:05Z

    > On 15 Apr 2024, at 07:04, Michael Paquier <michael@paquier.xyz> wrote:
> On Fri, Apr 12, 2024 at 02:42:57PM +0200, Daniel Gustafsson wrote:

>> Is the attached split in line with how you were thinking about it?
> 
> If I may, 0001 looks sensible here.  The bits from 0003 and 0004 could
> be applied before 0002, as well.

Agreed, once we are in post-freeze I think those three are mostly ready to go.

> - /* do post-fork initialization for random number generation */
> - pg_strong_random_init();
> 
> Perhaps you intented this diff to be in 0005 rather than in 0002?
> With 0002 applied, only support for 1.0.2 is removed, not 1.1.0 yet.

Yes, nice catch, that was a mistake in splitting up the patch into multiple
pieces, it should be in the 0005 patch for strong random. Fixed.

> s/requred/required/.

Fixed.

> Rather than calling always RAND_poll(), this
> could use a static flag to call it only once when pg_strong_random is
> called for the first time.

Agreed, we can good that. Fixed.

> I would not mind seeing this part entirely
> gone with the removal of support for 1.1.0.

If we want to keep autoconf from checking versions and just check compatibility
(with our code) then we will remain at 1.1.0 compatibility.  The only 1.1.1 API
we use is not present in LibreSSL so we can't really place a hard restriction
on that.  It might be that keeping it for now, and removing it later during the
v18 cycle as we modernize our OpenSSL code (which I hope to find time to work
on) and make use of newer 1.1.1 API:s.  That way we can keep our autoconf/meson
checks consistent across library checks.  If we end up with no new API:s to
check for by the time the last commitfest of v18 rolls around, we can revisit
the decision then.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-15T23:03:41Z

    On Mon, Apr 15, 2024 at 11:07:05AM +0200, Daniel Gustafsson wrote:
> On 15 Apr 2024, at 07:04, Michael Paquier <michael@paquier.xyz> wrote:
>> On Fri, Apr 12, 2024 at 02:42:57PM +0200, Daniel Gustafsson wrote:
>>> Is the attached split in line with how you were thinking about it?
>> 
>> If I may, 0001 looks sensible here.  The bits from 0003 and 0004 could
>> be applied before 0002, as well.
> 
> Agreed, once we are in post-freeze I think those three are mostly
> ready to go.

Is there a point to wait for 0001, 0003 and 0004, though, and
shouldn't these three be backpatched?  0001 is certainly OK as a
doc-only change to be consistent across the board without waiting 5
more years.  0003 and 0004 are conditional and depend on if
SSL_R_VERSION_TOO_LOW and SSL_OP_NO_CLIENT_RENEGOTIATION are defined
at compile-time.  0003 is much more important, and note that
01e6f1a842f4 has been backpatched all the way down.  0004 is nice,
still not strongly mandatory.

>> Rather than calling always RAND_poll(), this
>> could use a static flag to call it only once when pg_strong_random is
>> called for the first time.
> 
> Agreed, we can good that. Fixed.

+#if (OPENSSL_VERSION_NUMBER <= 0x10100000L)
+	static bool rand_initialized = false;

This does not look right.  At the top of upstream's branch
OpenSSL_1_1_0-stable, OPENSSL_VERSION_NUMBER is 0x101000d0L, so the
initialization would be missed for any version in the 1.1.0 series
except the GA one without this code being compiled, no?

>> I would not mind seeing this part entirely
>> gone with the removal of support for 1.1.0.
> 
> If we want to keep autoconf from checking versions and just check compatibility
> (with our code) then we will remain at 1.1.0 compatibility.  The only 1.1.1 API
> we use is not present in LibreSSL so we can't really place a hard restriction
> on that.  It might be that keeping it for now, and removing it later during the
> v18 cycle as we modernize our OpenSSL code (which I hope to find time to work
> on) and make use of newer 1.1.1 API:s.  That way we can keep our autoconf/meson
> checks consistent across library checks.  If we end up with no new API:s to
> check for by the time the last commitfest of v18 rolls around, we can revisit
> the decision then.

Okay, fine by me.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-16T08:17:26Z

    
> On 16 Apr 2024, at 01:03, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Mon, Apr 15, 2024 at 11:07:05AM +0200, Daniel Gustafsson wrote:
>> On 15 Apr 2024, at 07:04, Michael Paquier <michael@paquier.xyz> wrote:
>>> On Fri, Apr 12, 2024 at 02:42:57PM +0200, Daniel Gustafsson wrote:
>>>> Is the attached split in line with how you were thinking about it?
>>> 
>>> If I may, 0001 looks sensible here.  The bits from 0003 and 0004 could
>>> be applied before 0002, as well.
>> 
>> Agreed, once we are in post-freeze I think those three are mostly
>> ready to go.
> 
> Is there a point to wait for 0001, 0003 and 0004, though, and
> shouldn't these three be backpatched?  0001 is certainly OK as a
> doc-only change to be consistent across the board without waiting 5
> more years.  0003 and 0004 are conditional and depend on if
> SSL_R_VERSION_TOO_LOW and SSL_OP_NO_CLIENT_RENEGOTIATION are defined
> at compile-time.  0003 is much more important, and note that
> 01e6f1a842f4 has been backpatched all the way down.  0004 is nice,
> still not strongly mandatory.

I forgot (and didn't check) that we backpatched 01e6f1a842f4, with that in mind
I agree that we should backpatch 0003 as well to put LibreSSL on par as much as
we can.  0004 is a fix for the LibreSSL support, not adding anything new, so
pushing that to master now makes sense.  Unless objections are raised I'll push
0001, 0003 and 0004 shortly.  0002 and 0005 can hopefully be addressed in the
July commitfest.

>>> Rather than calling always RAND_poll(), this
>>> could use a static flag to call it only once when pg_strong_random is
>>> called for the first time.
>> 
>> Agreed, we can good that. Fixed.
> 
> +#if (OPENSSL_VERSION_NUMBER <= 0x10100000L)
> + static bool rand_initialized = false;
> 
> This does not look right.  At the top of upstream's branch
> OpenSSL_1_1_0-stable, OPENSSL_VERSION_NUMBER is 0x101000d0L, so the
> initialization would be missed for any version in the 1.1.0 series
> except the GA one without this code being compiled, no?

Meh, I was clearly not caffeinated as that's a thinko with a typo attached to
it.  The check should be "< 0x10101000" to catch any version prior to 1.1.1.
Fixed.

--
Daniel Gustafsson

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-18T10:53:43Z

    On 16.04.24 10:17, Daniel Gustafsson wrote:
> I forgot (and didn't check) that we backpatched 01e6f1a842f4, with that in mind
> I agree that we should backpatch 0003 as well to put LibreSSL on par as much as
> we can.  0004 is a fix for the LibreSSL support, not adding anything new, so
> pushing that to master now makes sense.  Unless objections are raised I'll push
> 0001, 0003 and 0004 shortly.  0002 and 0005 can hopefully be addressed in the
> July commitfest.

Review of the latest batch:

* v9-0001-Doc-Use-past-tense-for-things-which-happened-in-t.patch

Ok

8 v9-0002-Remove-support-for-OpenSSL-1.0.2.patch

Ok, but maybe make the punctuation consistent here:

+      # Function introduced in OpenSSL 1.0.2, not in LibreSSL.
+      ['SSL_CTX_set_cert_cb'],
+
+      # Function introduced in OpenSSL 1.1.1, not in LibreSSL
        ['X509_get_signature_info'],

* v9-0003-Support-disallowing-SSL-renegotiation-in-LibreSSL.patch

ok

* v9-0004-Support-SSL_R_VERSION_TOO_LOW-on-LibreSSL.patch

Seems ok, but the reason isn't clear to me.  Are there LibreSSL versions 
that have SSL_R_VERSION_TOO_LOW but not SSL_R_VERSION_TOO_HIGH?  Maybe 
this could be explained better.

Also, "OpenSSL 7.2" in the commit message probably meant "OpenBSD"?

* v9-0005-Remove-pg_strong_random-initialization.patch

I don't understand the reason for this phrase in the commit message: 
"1.1.1 is being increasingly phased out from production use".  Did you 
mean 1.1.0 there?

Conditionally sticking the RAND_poll() into pg_strong_random(), does 
that have the effect we want?  It wouldn't reinitialize after a fork, 
AFAICT.


If everything is addressed, I agree that 0001, 0003, and 0004 can go 
into PG17, the rest later.

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-18T20:26:33Z

    > On 18 Apr 2024, at 12:53, Peter Eisentraut <peter@eisentraut.org> wrote:

> Review of the latest batch:

Thanks for reviewing!

> 8 v9-0002-Remove-support-for-OpenSSL-1.0.2.patch
> 
> Ok, but maybe make the punctuation consistent here:

Fixed.

> * v9-0004-Support-SSL_R_VERSION_TOO_LOW-on-LibreSSL.patch
> 
> Seems ok, but the reason isn't clear to me.  Are there LibreSSL versions that have SSL_R_VERSION_TOO_LOW but not SSL_R_VERSION_TOO_HIGH?  Maybe this could be explained better.

LibreSSL doesn't support SSL_R_VERSION_TOO_HIGH at all, they only support
_TOO_LOW starting with the OpenBSD 7.2 release.  I've expanded the commit
message to document this.

> Also, "OpenSSL 7.2" in the commit message probably meant "OpenBSD"?

Ah yes, fixed.

> * v9-0005-Remove-pg_strong_random-initialization.patch
> 
> I don't understand the reason for this phrase in the commit message: "1.1.1 is being increasingly phased out from production use".  Did you mean 1.1.0 there?

Correct, I got lost among the version numbers it seems. Fixed.

> Conditionally sticking the RAND_poll() into pg_strong_random(), does that have the effect we want?  It wouldn't reinitialize after a fork, AFAICT.

No I think you're right, my previous version would have worked (but was ugly)
but this doesn't guarantee that.  Thinking more about it maybe it's best to
just keep the init function and have a version check for 1.1.0 there, making it
an empty no-op for all other cases.  When we move past 1.1.0 due to a new API
requirement we can blow it all away.

> If everything is addressed, I agree that 0001, 0003, and 0004 can go into PG17, the rest later.

Agreed, 0002 and 0005 are clearly for the v18 cycle.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-19T05:37:35Z

    On Thu, Apr 18, 2024 at 12:53:43PM +0200, Peter Eisentraut wrote:
> If everything is addressed, I agree that 0001, 0003, and 0004 can go into
> PG17, the rest later.

About the PG17 bits, would you agree about a backpatch?  Or perhaps
you disagree?
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-19T06:55:32Z

    > On 19 Apr 2024, at 07:37, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Thu, Apr 18, 2024 at 12:53:43PM +0200, Peter Eisentraut wrote:
>> If everything is addressed, I agree that 0001, 0003, and 0004 can go into
>> PG17, the rest later.
> 
> About the PG17 bits, would you agree about a backpatch?  Or perhaps
> you disagree?

If we want to 0001 can be baclpatched to v15, 0004 to v13 and 0003 all the way.
I don't have strong opinions either way.

--
Daniel Gustafsson

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-04-19T08:06:26Z

    On 19.04.24 07:37, Michael Paquier wrote:
> On Thu, Apr 18, 2024 at 12:53:43PM +0200, Peter Eisentraut wrote:
>> If everything is addressed, I agree that 0001, 0003, and 0004 can go into
>> PG17, the rest later.
> 
> About the PG17 bits, would you agree about a backpatch?  Or perhaps
> you disagree?

I don't think any of these need to be backpatched, at least right now.

0001 is just a cosmetic documentation tweak, has no reason to be 
backpatched.

0003 adds new functionality for LibreSSL.  While the code looks 
straightforward, we have little knowledge about how it works in 
practice.  How is the buildfarm coverage of LibreSSL (with SSL tests 
enabled!)?  If people are keen on this, it might be better to get it 
into PG17 and at least let to go through a few months of beta testing.

0004 effectively just enhances an error message for LibreSSL; there is 
little reason to backpatch this.

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-23T20:08:13Z

    > On 19 Apr 2024, at 10:06, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 19.04.24 07:37, Michael Paquier wrote:
>> On Thu, Apr 18, 2024 at 12:53:43PM +0200, Peter Eisentraut wrote:
>>> If everything is addressed, I agree that 0001, 0003, and 0004 can go into
>>> PG17, the rest later.
>> About the PG17 bits, would you agree about a backpatch?  Or perhaps
>> you disagree?
> 
> I don't think any of these need to be backpatched, at least right now.
> 
> 0001 is just a cosmetic documentation tweak, has no reason to be backpatched.
> 
> 0003 adds new functionality for LibreSSL.  While the code looks straightforward, we have little knowledge about how it works in practice.  How is the buildfarm coverage of LibreSSL (with SSL tests enabled!)?  If people are keen on this, it might be better to get it into PG17 and at least let to go through a few months of beta testing.
> 
> 0004 effectively just enhances an error message for LibreSSL; there is little reason to backpatch this.

Hearing no objections to this plan (and the posted v10), I'll go ahead with
0001, 0003 and 0004 into v17 tomorrow.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-23T22:20:27Z

    On Tue, Apr 23, 2024 at 10:08:13PM +0200, Daniel Gustafsson wrote:
> Hearing no objections to this plan (and the posted v10), I'll go ahead with
> 0001, 0003 and 0004 into v17 tomorrow.

WFM, thanks.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-24T11:31:12Z

    > On 24 Apr 2024, at 00:20, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Tue, Apr 23, 2024 at 10:08:13PM +0200, Daniel Gustafsson wrote:
>> Hearing no objections to this plan (and the posted v10), I'll go ahead with
>> 0001, 0003 and 0004 into v17 tomorrow.
> 
> WFM, thanks.

Done.  Attached are the two remaining patches, rebased over HEAD, for removing
support for OpenSSL 1.0.2 in v18. Parking them in the commitfest for now.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-04-25T03:49:16Z

    On Wed, Apr 24, 2024 at 01:31:12PM +0200, Daniel Gustafsson wrote:
> Done.  Attached are the two remaining patches, rebased over HEAD, for removing
> support for OpenSSL 1.0.2 in v18. Parking them in the commitfest for now.

You have mentioned once upthread the documentation of PQinitOpenSSL():
   However, this is unnecessary when using <productname>OpenSSL</productname>
   version 1.1.0 or later, as duplicate initializations are no longer problematic.

With 1.0.2's removal in place, this could be simplified more and the
patch does not touch it.  This relates also to pq_init_crypto_lib,
which is gone with 0001.  Of course, it is not possible to just remove
PQinitOpenSSL() or old application may fail linking.  Removing
pq_init_crypto_lib reduces any confusion around this part of the
initialization.

0002 looks OK here.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-27T18:32:19Z

    > On 25 Apr 2024, at 05:49, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Wed, Apr 24, 2024 at 01:31:12PM +0200, Daniel Gustafsson wrote:
>> Done.  Attached are the two remaining patches, rebased over HEAD, for removing
>> support for OpenSSL 1.0.2 in v18. Parking them in the commitfest for now.
> 
> You have mentioned once upthread the documentation of PQinitOpenSSL():
>   However, this is unnecessary when using <productname>OpenSSL</productname>
>   version 1.1.0 or later, as duplicate initializations are no longer problematic.
> 
> With 1.0.2's removal in place, this could be simplified more and the
> patch does not touch it.  This relates also to pq_init_crypto_lib,
> which is gone with 0001.  Of course, it is not possible to just remove
> PQinitOpenSSL() or old application may fail linking.  Removing
> pq_init_crypto_lib reduces any confusion around this part of the
> initialization.

That's a good point, there is potential for more code removal here.  The
attached 0001 takes a stab at it while it's fresh in mind, I'll revisit before
the July CF to see if there is more that can be done.

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-04-27T18:33:55Z

    > On 27 Apr 2024, at 20:32, Daniel Gustafsson <daniel@yesql.se> wrote:

> That's a good point, there is potential for more code removal here.  The
> attached 0001 takes a stab at it while it's fresh in mind, I'll revisit before
> the July CF to see if there is more that can be done.

..and again with the attachment. Not enough coffee.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-05-01T04:21:32Z

    On Sat, Apr 27, 2024 at 08:33:55PM +0200, Daniel Gustafsson wrote:
> > On 27 Apr 2024, at 20:32, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
> > That's a good point, there is potential for more code removal here.  The
> > attached 0001 takes a stab at it while it's fresh in mind, I'll revisit before
> > the July CF to see if there is more that can be done.
> 
> ..and again with the attachment. Not enough coffee.

My remark was originally about pq_init_crypto_lib that does the
locking initialization, and your new patch a bit more, as of:

-    /* This stuff need be done only once. */
-    if (!SSL_initialized)
-    {
-#ifdef HAVE_OPENSSL_INIT_SSL
-        OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL);
-#else
-        OPENSSL_config(NULL);
-        SSL_library_init();
-        SSL_load_error_strings();
-#endif
-        SSL_initialized = true;
-    }

OPENSSL_init_ssl() has replaced SSL_library_init(), marked as
deprecated, and even this step is mentioned as not required anymore
with 1.1.0~:
https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_init_ssl.html

Same with OPENSSL_init_crypto(), replacing OPENSSL_config(), again not
required in 1.1.0~:
https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_init_crypto.html

SSL_load_error_strings() is recommended as not to use in 1.1.0,
replaced by the others:
https://www.openssl.org/docs/man3.2/man3/SSL_load_error_strings.html

While OpenSSL will be able to cope with that, how much of that applies
to LibreSSL?  SSL_load_error_strings(), OPENSSL_init_ssl(),
OPENSSL_CONFIG() are OK based on the docs:
https://man.archlinux.org/man/extra/libressl/libressl-OPENSSL_config.3.en
https://man.archlinux.org/man/extra/libressl/libressl-OPENSSL_init_ssl.3.en
https://man.archlinux.org/man/extra/libressl/libressl-ERR_load_crypto_strings.3.en

So +1 to remove all this code after a closer lookup.  I would
recommend to update the documentation of PQinitSSL and PQinitOpenSSL
to tell that these become useless and are deprecated.

    ERR_clear_error();
-
#ifdef USE_RESOWNER_FOR_HMAC

Some noise diff.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-05-03T08:39:15Z

    > On 1 May 2024, at 06:21, Michael Paquier <michael@paquier.xyz> wrote:

> My remark was originally about pq_init_crypto_lib that does the
> locking initialization, and your new patch a bit more, as of:
> 
> ...
> 
> So +1 to remove all this code after a closer lookup.

Thanks for review.

>  I would
> recommend to update the documentation of PQinitSSL and PQinitOpenSSL
> to tell that these become useless and are deprecated.

They are no-ops when linking against v18, but writing an extension which
targets all supported versions of postgres along with their respective
supported OpenSSL versions make them still required, or am I missing something?

>    ERR_clear_error();
> -
> #ifdef USE_RESOWNER_FOR_HMAC
> 
> Some noise diff.

Will fix with a new rebase when once the tree has settled down from the
post-freeze fixups in this area.

--
Daniel Gustafsson

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-05-03T19:02:12Z

    On 03.05.24 10:39, Daniel Gustafsson wrote:
>>   I would
>> recommend to update the documentation of PQinitSSL and PQinitOpenSSL
>> to tell that these become useless and are deprecated.
> They are no-ops when linking against v18, but writing an extension which
> targets all supported versions of postgres along with their respective
> supported OpenSSL versions make them still required, or am I missing something?

I don't think extensions come into play here, since this is libpq, so 
only the shared library interface compatibility matters.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-05-03T19:21:33Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> On 03.05.24 10:39, Daniel Gustafsson wrote:
>> They are no-ops when linking against v18, but writing an extension which
>> targets all supported versions of postgres along with their respective
>> supported OpenSSL versions make them still required, or am I missing something?

> I don't think extensions come into play here, since this is libpq, so 
> only the shared library interface compatibility matters.

Yeah, server-side extensions don't really seem to be at hazard,
but doesn't the argument apply to client-side applications and
libraries that want to work across different PG/OpenSSL versions?

			regards, tom lane

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-05-04T08:08:48Z

    > On 3 May 2024, at 21:21, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Peter Eisentraut <peter@eisentraut.org> writes:
>> On 03.05.24 10:39, Daniel Gustafsson wrote:
>>> They are no-ops when linking against v18, but writing an extension which
>>> targets all supported versions of postgres along with their respective
>>> supported OpenSSL versions make them still required, or am I missing something?
> 
>> I don't think extensions come into play here, since this is libpq, so 
>> only the shared library interface compatibility matters.
> 
> Yeah, server-side extensions don't really seem to be at hazard,
> but doesn't the argument apply to client-side applications and
> libraries that want to work across different PG/OpenSSL versions?

Right, I was using "extension" a bit carelessly, what I meant was client-side
applications using libpq. 

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-05-06T23:31:29Z

    On Fri, May 03, 2024 at 10:39:15AM +0200, Daniel Gustafsson wrote:
> They are no-ops when linking against v18, but writing an extension which
> targets all supported versions of postgres along with their respective
> supported OpenSSL versions make them still required, or am I missing something?

Yeah, that depends on how much version you expect your application to
work on.  Still it seems to me that there's value in mentioning that
if your application does not care about anything older than OpenSSL 
1.1.0, like PG 18 assuming that this patch is merged, then these calls
are pointless for HEAD.  The routine definitions would be around only
for the .so compatibility.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-05-07T10:36:24Z

    > On 7 May 2024, at 01:31, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Fri, May 03, 2024 at 10:39:15AM +0200, Daniel Gustafsson wrote:
>> They are no-ops when linking against v18, but writing an extension which
>> targets all supported versions of postgres along with their respective
>> supported OpenSSL versions make them still required, or am I missing something?
> 
> Yeah, that depends on how much version you expect your application to
> work on.  Still it seems to me that there's value in mentioning that
> if your application does not care about anything older than OpenSSL 
> 1.1.0, like PG 18 assuming that this patch is merged, then these calls
> are pointless for HEAD.  The routine definitions would be around only
> for the .so compatibility.

Fair enough.  I've taken a stab at documenting that the functions are
deprecated, while at the same time documenting when and how they can be used
(and be useful).  The attached also removes one additional comment in the
testcode which is now obsolete (since removing 1.0.1 support really), and fixes
the spurious whitespace you detected upthread.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-05-08T00:45:24Z

    On Tue, May 07, 2024 at 12:36:24PM +0200, Daniel Gustafsson wrote:
> Fair enough.  I've taken a stab at documenting that the functions are
> deprecated, while at the same time documenting when and how they can be used
> (and be useful).  The attached also removes one additional comment in the
> testcode which is now obsolete (since removing 1.0.1 support really), and fixes
> the spurious whitespace you detected upthread.

+       This function is deprecated and only present for backwards compatibility,
+       it does nothing.
[...]
+       <xref linkend="libpq-PQinitSSL"/> and <xref linkend="libpq-PQinitOpenSSL"/>
+       are maintained for backwards compatibility, but are no longer required
+       since <productname>PostgreSQL</productname> 18.

LGTM, thanks for doing this!
--
Michael

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-07-11T21:22:50Z

    On 07.05.24 11:36, Daniel Gustafsson wrote:
>> Yeah, that depends on how much version you expect your application to
>> work on.  Still it seems to me that there's value in mentioning that
>> if your application does not care about anything older than OpenSSL
>> 1.1.0, like PG 18 assuming that this patch is merged, then these calls
>> are pointless for HEAD.  The routine definitions would be around only
>> for the .so compatibility.
> 
> Fair enough.  I've taken a stab at documenting that the functions are
> deprecated, while at the same time documenting when and how they can be used
> (and be useful).  The attached also removes one additional comment in the
> testcode which is now obsolete (since removing 1.0.1 support really), and fixes
> the spurious whitespace you detected upthread.

The 0001 patch removes the functions pgtls_init_library() and 
pgtls_init() but keeps the declarations in libpq-int.h.  This should be 
fixed.

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-07-12T20:42:25Z

    > On 11 Jul 2024, at 23:22, Peter Eisentraut <peter@eisentraut.org> wrote:

> The 0001 patch removes the functions pgtls_init_library() and pgtls_init() but keeps the declarations in libpq-int.h.  This should be fixed.

Ah, nice catch. Done in the attached rebase.

--
Daniel Gustafsson

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-07-14T12:03:35Z

    On 12.07.24 21:42, Daniel Gustafsson wrote:
>> On 11 Jul 2024, at 23:22, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
>> The 0001 patch removes the functions pgtls_init_library() and pgtls_init() but keeps the declarations in libpq-int.h.  This should be fixed.
> 
> Ah, nice catch. Done in the attached rebase.

This patch version looks good to me.

Small comments on the commit message of 0002: Typo "checkig".  Also, 
maybe the commit message title can be qualified a little, since we're 
not really doing "Remove pg_strong_random initialization." but something 
like "Remove unnecessary ..."?

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-07-15T21:42:05Z

    > On 14 Jul 2024, at 14:03, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 12.07.24 21:42, Daniel Gustafsson wrote:
>>> On 11 Jul 2024, at 23:22, Peter Eisentraut <peter@eisentraut.org> wrote:
>>> The 0001 patch removes the functions pgtls_init_library() and pgtls_init() but keeps the declarations in libpq-int.h.  This should be fixed.
>> Ah, nice catch. Done in the attached rebase.
> 
> This patch version looks good to me.

Thanks for review, I will go ahead with this once back from vacation at the
tail end of July when I can properly handle the BF.

> Small comments on the commit message of 0002: Typo "checkig".  Also, maybe the commit message title can be qualified a little, since we're not really doing "Remove pg_strong_random initialization." but something like "Remove unnecessary ..."?

Good points, will address before pushing.

--
Daniel Gustafsson

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

    Joe Conway <mail@joeconway.com> — 2024-08-05T12:38:00Z

    On 7/15/24 17:42, Daniel Gustafsson wrote:
>> On 14 Jul 2024, at 14:03, Peter Eisentraut <peter@eisentraut.org>
>> wrote:
>> 
>> On 12.07.24 21:42, Daniel Gustafsson wrote:
>>>> On 11 Jul 2024, at 23:22, Peter Eisentraut
>>>> <peter@eisentraut.org> wrote: The 0001 patch removes the
>>>> functions pgtls_init_library() and pgtls_init() but keeps the
>>>> declarations in libpq-int.h.  This should be fixed.
>>> Ah, nice catch. Done in the attached rebase.
>> 
>> This patch version looks good to me.
> 
> Thanks for review, I will go ahead with this once back from vacation
> at the tail end of July when I can properly handle the BF.
> 
>> Small comments on the commit message of 0002: Typo "checkig".
>> Also, maybe the commit message title can be qualified a little,
>> since we're not really doing "Remove pg_strong_random
>> initialization." but something like "Remove unnecessary ..."?
> 
> Good points, will address before pushing.

I know I am way late to this thread, and I have only tried a cursory 
skim of it given the length, but have we made any kind of announcement 
(packagers at least?) that we intend to not support Postgres 18 with ssl 
on RHEL 7.9 and derivatives? Yes, RHEL 7 just passed EOL, but there is 
commercial extended support available until July 2028[1] which means 
many people will continue to use it.

Joe

[1] 
https://www.redhat.com/en/blog/announcing-4-years-extended-life-cycle-support-els-red-hat-enterprise-linux-7
-- 
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

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

    Devrim Gündüz <devrim@gunduz.org> — 2024-08-05T13:13:38Z

    Hi Joe,

On Mon, 2024-08-05 at 08:38 -0400, Joe Conway wrote:
> I know I am way late to this thread, and I have only tried a cursory 
> skim of it given the length, but have we made any kind of announcement
> (packagers at least?) that we intend to not support Postgres 18 with
> ssl  on RHEL 7.9 and derivatives?

I dropped RHEL 7 support as of PostgreSQL 16:
https://yum.postgresql.org/news/rhel7-postgresql-rpms-end-of-life/

and also recently stopped providing updates except PostgreSQL packages:

https://yum.postgresql.org/news/rhel7-end-of-life/

Min OS version is RHEL 8 / SLES 15.

-HTH

Regards,
-- 
Devrim Gündüz
Open Source Solution Architect, PostgreSQL Major Contributor
Twitter: @DevrimGunduz , @DevrimGunduzTR

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-08-05T13:14:49Z

    Joe Conway <mail@joeconway.com> writes:
> I know I am way late to this thread, and I have only tried a cursory 
> skim of it given the length, but have we made any kind of announcement 
> (packagers at least?) that we intend to not support Postgres 18 with ssl 
> on RHEL 7.9 and derivatives? Yes, RHEL 7 just passed EOL, but there is 
> commercial extended support available until July 2028[1] which means 
> many people will continue to use it.

PG v16 will be in-support until November 2028, so it's not like
we are leaving RHEL 7 completely in the lurch.  I doubt that the
sort of people who are still running an EOL OS are looking to put
a bleeding-edge database on it, so this seems sufficient to me.

As for notifying packagers --- Red Hat themselves will certainly
not be trying to put new major versions of anything on RHEL 7,
and Devrim has stopped packaging newer PG for RHEL 7 altogether,
so who among them is going to care?

			regards, tom lane

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

    Joe Conway <mail@joeconway.com> — 2024-08-05T13:36:21Z

    On 8/5/24 09:14, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> I know I am way late to this thread, and I have only tried a cursory 
>> skim of it given the length, but have we made any kind of announcement 
>> (packagers at least?) that we intend to not support Postgres 18 with ssl 
>> on RHEL 7.9 and derivatives? Yes, RHEL 7 just passed EOL, but there is 
>> commercial extended support available until July 2028[1] which means 
>> many people will continue to use it.
> 
> PG v16 will be in-support until November 2028, so it's not like
> we are leaving RHEL 7 completely in the lurch.  I doubt that the
> sort of people who are still running an EOL OS are looking to put
> a bleeding-edge database on it, so this seems sufficient to me.

ok

> As for notifying packagers --- Red Hat themselves will certainly
> not be trying to put new major versions of anything on RHEL 7,
> and Devrim has stopped packaging newer PG for RHEL 7 altogether,
> so who among them is going to care?


Perhaps no one on packagers. It would not shock me to see complaints 
from others after we rip out support for 1.0.2, but maybe not ¯\_(ツ)_/¯

-- 
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-08-07T13:49:37Z

    > On 5 Aug 2024, at 15:36, Joe Conway <mail@joeconway.com> wrote:

> It would not shock me to see complaints from others after we rip out support
> for 1.0.2, but maybe not ¯\_(ツ)_/¯


I think it's highly likely that we will see complaints for any support we
deprecate.  OpenSSL 1.0.2 will however still be supported for another 5 years
with v17 (which is ~9years past its EOL date) so I don't feel too bad about it.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-08-07T23:59:17Z

    > On Aug 7, 2024, at 22:49, Daniel Gustafsson <daniel@yesql.se> wrote:
> I think it's highly likely that we will see complaints for any support we
> deprecate.  OpenSSL 1.0.2 will however still be supported for another 5 years
> with v17 (which is ~9years past its EOL date) so I don't feel too bad about it.

I’ve cut 1.0.1 at least by myself last year, and definitely not regret it while not hearing complains on the matter. 1.0.2 being a LTS matters more in the likeliness to see complaints, but I think it’s time to just let it go. So let’s do it and move on.

I’d seriously consider 1.1.0 as well for this release cycle as something to drop but I’m ok to keep it as the code gains are not really here. The 1.0.2 cut removes a lot of code and concepts particularly with the libcrypto threading and its locks!
--
Michael

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-08-21T13:01:08Z

    On 07.08.24 15:49, Daniel Gustafsson wrote:
>> On 5 Aug 2024, at 15:36, Joe Conway <mail@joeconway.com> wrote:
> 
>> It would not shock me to see complaints from others after we rip out support
>> for 1.0.2, but maybe not ¯\_(ツ)_/¯
> 
> 
> I think it's highly likely that we will see complaints for any support we
> deprecate.  OpenSSL 1.0.2 will however still be supported for another 5 years
> with v17 (which is ~9years past its EOL date) so I don't feel too bad about it.

Is anything -- other than this inquiry -- preventing this patch set from 
getting committed?

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

    Joe Conway <mail@joeconway.com> — 2024-08-21T14:48:38Z

    On 8/21/24 09:01, Peter Eisentraut wrote:
> On 07.08.24 15:49, Daniel Gustafsson wrote:
>>> On 5 Aug 2024, at 15:36, Joe Conway <mail@joeconway.com> wrote:
>> 
>>> It would not shock me to see complaints from others after we rip out support
>>> for 1.0.2, but maybe not ¯\_(ツ)_/¯
>> 
>> 
>> I think it's highly likely that we will see complaints for any support we
>> deprecate.  OpenSSL 1.0.2 will however still be supported for another 5 years
>> with v17 (which is ~9years past its EOL date) so I don't feel too bad about it.
> 
> Is anything -- other than this inquiry -- preventing this patch set from
> getting committed?

The overwhelming consensus seemed to be "just do it", so FWIW consider 
my reservations withdrawn ;-)

-- 
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

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

    Michael Paquier <michael@paquier.xyz> — 2024-08-22T00:31:21Z

    On Wed, Aug 21, 2024 at 10:48:38AM -0400, Joe Conway wrote:
> On 8/21/24 09:01, Peter Eisentraut wrote:
>> Is anything -- other than this inquiry -- preventing this patch set from
>> getting committed?
> 
> The overwhelming consensus seemed to be "just do it", so FWIW consider my
> reservations withdrawn ;-)

Just do it :)

I am pretty sure that Daniel is just on vacations and that this will
happen sooner than later during the next commit fest, so I'd rather
wait for him for an update on this thread first.
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-08-22T21:13:15Z

    > On 22 Aug 2024, at 02:31, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Wed, Aug 21, 2024 at 10:48:38AM -0400, Joe Conway wrote:
>> On 8/21/24 09:01, Peter Eisentraut wrote:
>>> Is anything -- other than this inquiry -- preventing this patch set from
>>> getting committed?

That, and available time.

>> The overwhelming consensus seemed to be "just do it", so FWIW consider my
>> reservations withdrawn ;-)
> 
> Just do it :)

That's my plan, I wanted to wait a bit to see if anyone else chimed in with
concerns.

--
Daniel Gustafsson

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

    Michael Paquier <michael@paquier.xyz> — 2024-08-22T23:56:01Z

    On Thu, Aug 22, 2024 at 11:13:15PM +0200, Daniel Gustafsson wrote:
> On 22 Aug 2024, at 02:31, Michael Paquier <michael@paquier.xyz> wrote:
>> Just do it :)
> 
> That's my plan, I wanted to wait a bit to see if anyone else chimed in with
> concerns.

Cool, thanks!
--
Michael

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-09-02T08:03:14Z

    > On 23 Aug 2024, at 01:56, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Thu, Aug 22, 2024 at 11:13:15PM +0200, Daniel Gustafsson wrote:
>> On 22 Aug 2024, at 02:31, Michael Paquier <michael@paquier.xyz> wrote:
>>> Just do it :)
>> 
>> That's my plan, I wanted to wait a bit to see if anyone else chimed in with
>> concerns.
> 
> Cool, thanks!

Attached is a rebased v15 (only changes are commit-message changes noted by
Peter upthread) for the sake of archives, and for a green-check run in the
CFBot.  Assuming this builds green I intend to push this.

--
Daniel Gustafsson

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-09-02T12:26:54Z

    > On 2 Sep 2024, at 10:03, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
>> On 23 Aug 2024, at 01:56, Michael Paquier <michael@paquier.xyz> wrote:
>> 
>> On Thu, Aug 22, 2024 at 11:13:15PM +0200, Daniel Gustafsson wrote:
>>> On 22 Aug 2024, at 02:31, Michael Paquier <michael@paquier.xyz> wrote:
>>>> Just do it :)
>>> 
>>> That's my plan, I wanted to wait a bit to see if anyone else chimed in with
>>> concerns.
>> 
>> Cool, thanks!
> 
> Attached is a rebased v15 (only changes are commit-message changes noted by
> Peter upthread) for the sake of archives, and for a green-check run in the
> CFBot.  Assuming this builds green I intend to push this.

And pushed.  All BF owners with animals using 1.0.2 have been notified but not
all have been updated (or modified to skip SSL) so there will be some failing.

--
Daniel Gustafsson

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

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-10T08:01:22Z

    On 02.09.24 14:26, Daniel Gustafsson wrote:
>> On 2 Sep 2024, at 10:03, Daniel Gustafsson <daniel@yesql.se> wrote:
>>
>>> On 23 Aug 2024, at 01:56, Michael Paquier <michael@paquier.xyz> wrote:
>>>
>>> On Thu, Aug 22, 2024 at 11:13:15PM +0200, Daniel Gustafsson wrote:
>>>> On 22 Aug 2024, at 02:31, Michael Paquier <michael@paquier.xyz> wrote:
>>>>> Just do it :)
>>>>
>>>> That's my plan, I wanted to wait a bit to see if anyone else chimed in with
>>>> concerns.
>>>
>>> Cool, thanks!
>>
>> Attached is a rebased v15 (only changes are commit-message changes noted by
>> Peter upthread) for the sake of archives, and for a green-check run in the
>> CFBot.  Assuming this builds green I intend to push this.
> 
> And pushed.  All BF owners with animals using 1.0.2 have been notified but not
> all have been updated (or modified to skip SSL) so there will be some failing.

A small follow-up for this:  With the current minimum OpenSSL version 
being 1.1.0, we can remove an unconstify() call; see attached patch.

See this OpenSSL commit: 
<https://github.com/openssl/openssl/commit/8ab31975ba>.  The analogous 
LibreSSL change is here: 
<https://cvsweb.openbsd.org/src/lib/libcrypto/bio/bss_mem.c?rev=1.17&content-type=text/x-cvsweb-markup>. 
  I don't know if we have a concrete minimum LibreSSL version, but the 
change is about as old as the OpenSSL change.

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

    Daniel Gustafsson <daniel@yesql.se> — 2024-09-10T08:11:25Z

    > On 10 Sep 2024, at 10:01, Peter Eisentraut <peter@eisentraut.org> wrote:

>> And pushed.  All BF owners with animals using 1.0.2 have been notified but not
>> all have been updated (or modified to skip SSL) so there will be some failing.
> 
> A small follow-up for this:  With the current minimum OpenSSL version being 1.1.0, we can remove an unconstify() call; see attached patch.

Nice catch.

> See this OpenSSL commit: <https://github.com/openssl/openssl/commit/8ab31975ba>.  The analogous LibreSSL change is here: <https://cvsweb.openbsd.org/src/lib/libcrypto/bio/bss_mem.c?rev=1.17&content-type=text/x-cvsweb-markup>.  

> I don't know if we have a concrete minimum LibreSSL version, but the change is about as old as the OpenSSL change.

We've never documented the minimum LibreSSL version we support, but given that
we regularly test LibreSSL and fix breakage in our support I think we should.

--
Daniel Gustafsson

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

    Robins Tharakan <tharakan@gmail.com> — 2024-12-17T02:54:49Z

    Hi,

On Thu, 4 Apr 2024 at 05:42, Daniel Gustafsson <daniel@yesql.se> wrote:

>
> massasauga and snakefly run the ssl_passphrase_callback-check test but
> none of
> these run the ssl-check tests AFAICT, so we have very low coverage as is.
> The
> fact that very few animals run the ssl tests is a pet peeve of mine, it
> would
> be nice if we could get broader coverage there.
>


I am quite late to this discussion, but I am unable to get snakefly up and
running,
i.e. I see that all versions failed today (most on
ssl_passphrase_callback-check
test [2], but v15/v16 on sslCheck [1] too)

This machine has OpenSSL v1.0.2k so I am not expecting it to work on v17+,
but I was assuming at least the older versions should still be working.
Also despite
adding --without-openssl the older versions still fail [2].

Is upgrading OpenSSL the only way to get snakefly back to green?

Reference:
1.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snakefly&dt=2024-12-16%2014%3A04%3A25
2.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snakefly&dt=2024-12-16%2014%3A01%3A21

-
robins

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-12-17T04:17:17Z

    Robins Tharakan <tharakan@gmail.com> writes:
> I am quite late to this discussion, but I am unable to get snakefly up and
> running,
> i.e. I see that all versions failed today (most on
> ssl_passphrase_callback-check
> test [2], but v15/v16 on sslCheck [1] too)

The failures all seem to be in "checkprep", ie the test setup
stage not the test proper.  The log output from that isn't being
captured, making it hard to diagnose.  Could we prevail on you
to install a more up-to-date buildfarm client script on that
machine?  It looks to be running v11, which is rather old.

			regards, tom lane

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

    Michael Paquier <michael@paquier.xyz> — 2024-12-17T06:28:51Z

    On Mon, Dec 16, 2024 at 11:17:17PM -0500, Tom Lane wrote:
> The failures all seem to be in "checkprep", ie the test setup
> stage not the test proper.  The log output from that isn't being
> captured, making it hard to diagnose.  Could we prevail on you
> to install a more up-to-date buildfarm client script on that
> machine?  It looks to be running v11, which is rather old.

+1.  Robins, feel free to ping me directly if you need help with this
host.
--
Michael

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

    Robins Tharakan <tharakan@gmail.com> — 2024-12-17T11:50:43Z

    On Tue, 17 Dec 2024 at 16:59, Michael Paquier <michael@paquier.xyz> wrote:

> +1.  Robins, feel free to ping me directly if you need help with this
> host.
>

Much appreciated Michael. Thankfully the new client worked without much
hiccup
(once it got the correct config file).

This is an old instance and IIRC gcc 7.3.1 gave me trouble sometime back. So
like alligator, I upgraded gcc recently. That (along with what feels like
cron config)
needs some attention and I should be able to stabilize the results soon.

-
robins