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

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
Date: 2023-05-25 08:16:27
Message-ID: B7197D6F-C8A4-467D-9792-EBE9125ED433@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 25 May 2023, at 00:18, Michael Paquier <michael(at)paquier(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2023-05-25 08:27:02 Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
Previous Message Masahiko Sawada 2023-05-25 07:02:50 Re: running logical replication as the subscription owner