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

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