Обсуждение: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
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
Вложения
> 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
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
Вложения
> 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
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
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
Вложения
> 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
> 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
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
Вложения
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
Вложения
> 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
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
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
Вложения
> 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
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
Вложения
> 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
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
Вложения
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).
> 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
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
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
Вложения
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
Вложения
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
Вложения
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.
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
Вложения
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?
> 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
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
Вложения
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.
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.
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
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
> 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
Вложения
> 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
Вложения
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
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
sysname | l
---------------+----------------------------------------------------------------------------------------------
alabio | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
alimoche | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022
arowana | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
avocet | configure: using openssl: OpenSSL 1.1.1l-fips 24 Aug 2021 SUSE release 150400.7.60.2
ayu | configure: using openssl: OpenSSL 1.1.0l 10 Sep 2019
babbler | configure: using openssl: OpenSSL 1.1.1k FIPS 25 Mar 2021
basilisk | configure: using openssl: OpenSSL 3.1.4 24 Oct 2023 (Library: OpenSSL 3.1.4 24 Oct 2023)
batfish | configure: using openssl: OpenSSL 1.0.2g 1 Mar 2016
batta | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
blackneck | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022
boa | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
boomslang | configure: using openssl: OpenSSL 1.1.1k 25 Mar 2021
broadbill | configure: using openssl: OpenSSL 1.1.1k FIPS 25 Mar 2021
bulbul | configure: using openssl: OpenSSL 1.1.1k FIPS 25 Mar 2021
buri | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
bushmaster | configure: using openssl: OpenSSL 3.1.5 30 Jan 2024 (Library: OpenSSL 3.1.5 30 Jan 2024)
butterflyfish | configure: using openssl: OpenSSL 1.0.2p-fips 14 Aug 2018
caiman | configure: using openssl: OpenSSL 3.2.1 30 Jan 2024 (Library: OpenSSL 3.2.1 30 Jan 2024)
canebrake | configure: using openssl: OpenSSL 3.1.5 30 Jan 2024 (Library: OpenSSL 3.1.5 30 Jan 2024)
cascabel | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022 (Library: OpenSSL 1.1.1w 11 Sep 2023)
cavefish | configure: using openssl: OpenSSL 1.1.1 11 Sep 2018
chevrotain | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022 (Library: OpenSSL 1.1.1w 11 Sep 2023)
chimaera | configure: using openssl: OpenSSL 1.1.0l 10 Sep 2019
chipmunk | configure: using openssl: OpenSSL 1.0.1t 3 May 2016
cisticola | configure: using openssl: OpenSSL 1.1.1g FIPS 21 Apr 2020
clam | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
conchuela | configure: using openssl: LibreSSL 3.2.5
copperhead | configure: using openssl: OpenSSL 1.1.1k 25 Mar 2021
culpeo | configure: using openssl: OpenSSL 3.0.11 19 Sep 2023 (Library: OpenSSL 3.0.11 19 Sep 2023)
cuon | configure: using openssl: OpenSSL 1.0.2g 1 Mar 2016
demoiselle | configure: using openssl: OpenSSL 1.1.0h-fips 27 Mar 2018
desman | configure: using openssl: OpenSSL 3.0.9 30 May 2023 (Library: OpenSSL 3.0.9 30 May 2023)
dhole | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
dikkop | configure: using openssl: OpenSSL 3.0.10 1 Aug 2023 (Library: OpenSSL 3.0.10 1 Aug 2023)
elasmobranch | configure: using openssl: OpenSSL 1.1.0h-fips 27 Mar 2018
gokiburi | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
grison | configure: using openssl: OpenSSL 1.1.0l 10 Sep 2019
grison | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022
guaibasaurus | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
gull | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
habu | configure: using openssl: OpenSSL 3.0.9 30 May 2023 (Library: OpenSSL 3.0.9 30 May 2023)
hachi | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
hake | configure: using openssl: OpenSSL 1.0.2u 20 Dec 2019
hippopotamus | configure: using openssl: OpenSSL 1.1.1l-fips 24 Aug 2021 SUSE release 150400.7.60.2
indri | configure: using openssl: OpenSSL 3.2.0 23 Nov 2023 (Library: OpenSSL 3.2.0 23 Nov 2023)
jackdaw | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022 (Library: OpenSSL 1.1.1w 11 Sep 2023)
jay | configure: using openssl: OpenSSL 1.1.1l-fips 24 Aug 2021 SUSE release 150400.7.60.2
kingsnake | configure: using openssl: OpenSSL 3.0.9 30 May 2023 (Library: OpenSSL 3.0.9 30 May 2023)
krait | configure: using openssl: OpenSSL 1.1.1k FIPS 25 Mar 2021
lancehead | configure: using openssl: OpenSSL 1.1.1k FIPS 25 Mar 2021
lapwing | configure: using openssl: OpenSSL 1.0.1t 3 May 2016
loach | configure: using openssl: OpenSSL 1.1.1q-freebsd 5 Jul 2022
longfin | configure: using openssl: LibreSSL 3.3.6
lora | configure: using openssl: OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022)
lorikeet | configure: using openssl: OpenSSL 3.0.12 24 Oct 2023 (Library: OpenSSL 3.0.12 24 Oct 2023)
mamba | configure: using openssl: OpenSSL 3.0.12 24 Oct 2023 (Library: OpenSSL 3.0.12 24 Oct 2023)
mamushi | configure: using openssl: OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022)
mantid | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
margay | configure: using openssl: OpenSSL 1.0.2za 24 Aug 2021
massasauga | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
mereswine | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
morepork | configure: using openssl: LibreSSL 3.3.2
motmot | configure: using openssl: OpenSSL 3.0.9 30 May 2023 (Library: OpenSSL 3.0.9 30 May 2023)
mule | configure: using openssl: OpenSSL 3.0.11 19 Sep 2023 (Library: OpenSSL 3.0.11 19 Sep 2023)
myna | configure: using openssl: OpenSSL 1.0.2r-fips 26 Feb 2019
nicator | configure: using openssl: OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022)
nuthatch | configure: using openssl: OpenSSL 3.1.4 24 Oct 2023 (Library: OpenSSL 3.1.4 24 Oct 2023)
opaleye | configure: using openssl: OpenSSL 1.1.1o-freebsd 3 May 2022
oystercatcher | configure: using openssl: OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022)
parula | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
perentie | configure: using openssl: OpenSSL 3.1.1 30 May 2023 (Library: OpenSSL 3.1.1 30 May 2023)
pike | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022 (Library: OpenSSL 1.1.1w 11 Sep 2023)
plover | configure: using openssl: LibreSSL 3.3.2
pollock | configure: using openssl: OpenSSL 3.1.4 24 Oct 2023 (Library: OpenSSL 3.1.4 24 Oct 2023)
potoo | configure: using openssl: OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022)
prion | configure: using openssl: OpenSSL 3.0.8 7 Feb 2023 (Library: OpenSSL 3.0.8 7 Feb 2023)
pytilia | configure: using openssl: OpenSSL 1.1.1k FIPS 25 Mar 2021
rhinoceros | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
rinkhals | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022 (Library: OpenSSL 1.1.1w 11 Sep 2023)
rudd | configure: using openssl: OpenSSL 1.1.1f 31 Mar 2020
ruddy | configure: using openssl: OpenSSL 1.1.1d 10 Sep 2019
sarus | configure: using openssl: OpenSSL 3.0.2 15 Mar 2022 (Library: OpenSSL 3.0.2 15 Mar 2022)
sawshark | configure: using openssl: LibreSSL 3.8.2
schnauzer | configure: using openssl: LibreSSL 3.7.2
shelduck | configure: using openssl: OpenSSL 1.0.2p-fips 14 Aug 2018
shiner | configure: using openssl: OpenSSL 3.0.2 15 Mar 2022 (Library: OpenSSL 3.0.2 15 Mar 2022)
sidewinder | configure: using openssl: OpenSSL 1.1.1k 25 Mar 2021
sifaka | configure: using openssl: LibreSSL 3.3.6
siskin | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
snakefly | configure: using openssl: OpenSSL 1.0.2k-fips 26 Jan 2017
splitfin | configure: using openssl: OpenSSL 1.1.1f 31 Mar 2020
taipan | configure: using openssl: OpenSSL 3.1.5 30 Jan 2024 (Library: OpenSSL 3.1.5 30 Jan 2024)
tayra | configure: using openssl: OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022)
topminnow | configure: using openssl: OpenSSL 1.0.1t 3 May 2016
trilobite | configure: using openssl: OpenSSL 1.1.1l-fips 24 Aug 2021 SUSE release 150400.7.60.2
turaco | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
turbot | configure: using openssl: OpenSSL 3.0.2 15 Mar 2022 (Library: OpenSSL 3.0.2 15 Mar 2022)
twinspot | configure: using openssl: OpenSSL 3.0.7 1 Nov 2022 (Library: OpenSSL 3.0.7 1 Nov 2022)
urocryon | configure: using openssl: OpenSSL 1.1.0l 10 Sep 2019
urutu | configure: using openssl: OpenSSL 3.1.5 30 Jan 2024 (Library: OpenSSL 3.1.5 30 Jan 2024)
vimba | configure: using openssl: OpenSSL 1.1.1 11 Sep 2018
whinchat | configure: using openssl: OpenSSL 1.1.1n 15 Mar 2022 (Library: OpenSSL 1.1.1w 11 Sep 2023)
widowbird | configure: using openssl: OpenSSL 1.1.1w 11 Sep 2023
ziege | configure: using openssl: OpenSSL 1.1.1k FIPS 25 Mar 2021
(104 rows)
sysname | l
---------------+------------------------------------------------
adder | Run-time dependency openssl found: YES 3.1.5
akepa | Run-time dependency openssl found: YES 3.1.1
calliphoridae | Run-time dependency openssl found: YES 3.1.5
crake | Run-time dependency openssl found: YES 3.1.1
culicidae | Run-time dependency openssl found: YES 3.1.5
dogfish | Run-time dependency openssl found: YES 3.1.4
fairywren | Run-time dependency openssl found: YES 3.1.0\r
flaviventris | Run-time dependency openssl found: YES 3.1.5
francolin | Run-time dependency openssl found: YES 3.1.5
grassquit | Run-time dependency openssl found: YES 3.1.5
kestrel | Run-time dependency openssl found: YES 3.1.5
koel | Run-time dependency openssl found: YES 3.1.1
mylodon | Run-time dependency openssl found: YES 3.1.5
olingo | Run-time dependency openssl found: YES 3.1.5
piculet | Run-time dependency openssl found: YES 3.1.5
rorqual | Run-time dependency openssl found: YES 3.1.5
serinus | Run-time dependency openssl found: YES 3.1.5
sevengill | Run-time dependency openssl found: YES 3.2.1
skink | Run-time dependency openssl found: YES 3.1.5
tamandua | Run-time dependency openssl found: YES 3.1.5
(20 rows)
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
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
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
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
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
> 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
> 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
On 2024-04-03 We 15:12, Daniel Gustafsson wrote:
Thefact 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
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
> 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 isthat 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
> 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
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.
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
Вложения
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).
> 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-yearone, 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
> 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
> 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
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
Вложения
(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
Вложения
On 2024-04-05 03:37, Michael Paquier wrote:
(Adding Mikael Kjellstrom in CC as OpenBSD owner)
My 2 OpenBSD animals (morepork OpenBSD 6.9, schnauzer 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
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
> 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
Вложения
> 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
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
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.
> 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 isthe only place that needs to be updated. We put the setting of OPENSSL_API_COMPAT into configure so that the subsequentOpenSSL 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
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
> 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
Вложения
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
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.
> 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 alwaysbeen 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
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
alwaysbeen 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
> 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 alwaysbeen 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
Вложения
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
Вложения
> 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
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?
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
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
Вложения
> 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 tobe conditional. We could check for a representative function like OPENSSL_init_ssl() in configure/meson, or we could justlet 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 buta behavior change. Let's make this association clearer in the code. For example, add a version check or assertion aboutthis 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 notremoving 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
Вложения
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
changebut a behavior change. Let's make this association clearer in the code. For example, add a version check or
assertionabout 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
Вложения
> 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
Вложения
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
Вложения
> 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
Вложения
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.
> 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 productionuse". 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 reinitializeafter 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
Вложения
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
Вложения
> 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
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.
> 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 itworks in practice. How is the buildfarm coverage of LibreSSL (with SSL tests enabled!)? If people are keen on this, itmight 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
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
Вложения
> 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
Вложения
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
Вложения
> 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
> 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
Вложения
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
Вложения
> 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
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.
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
> 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
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
Вложения
> 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
Вложения
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