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
From b59a9a192d88bfd6261a3559ace8caced2e25d9a Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Fri, 26 May 2023 10:45:35 +0900
Subject: [PATCH v2] Remove support for OpenSSL 1.0.1
A few notes about that:
- This simplifies a lot of code areas related to channel binding, as
X509_get_signature_nid() always exists.
- This removes the comments related to 1.0.1e introduced by 74242c2.
- Do we need to care about the case of building the Postgres code with
LibreSSL on Windows for the MSVC scripts, leading to the removal of the
check with HAVE_SSL_CTX_SET_CERT_CB?
---
src/include/libpq/libpq-be.h | 6 ----
src/include/pg_config.h.in | 3 --
src/backend/libpq/auth-scram.c | 20 ++++++------
src/backend/libpq/be-secure-openssl.c | 4 ---
src/interfaces/libpq/fe-auth-scram.c | 8 ++---
src/interfaces/libpq/fe-auth.c | 2 +-
src/interfaces/libpq/fe-secure-openssl.c | 4 ---
src/interfaces/libpq/libpq-int.h | 6 ----
src/test/ssl/t/002_scram.pl | 41 ++++--------------------
doc/src/sgml/installation.sgml | 2 +-
configure | 16 ++++-----
configure.ac | 8 ++---
meson.build | 7 ++--
src/tools/msvc/Solution.pm | 10 +-----
14 files changed, 37 insertions(+), 100 deletions(-)
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 3b2ce9908f..a0b74c8095 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -305,14 +305,8 @@ extern void be_tls_get_peer_serial(Port *port, char *ptr, size_t len);
*
* The result is a palloc'd hash of the server certificate with its
* size, and NULL if there is no certificate available.
- *
- * This is not supported with old versions of OpenSSL that don't have
- * the X509_get_signature_nid() function.
*/
-#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
-#define HAVE_BE_TLS_GET_CERTIFICATE_HASH
extern char *be_tls_get_certificate_hash(Port *port, size_t *len);
-#endif
/* init hook for SSL, the default sets the password callback if appropriate */
#ifdef USE_OPENSSL
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 6d572c3820..ca3a49c552 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -529,9 +529,6 @@
/* Define to 1 if you have the `X509_get_signature_info' function. */
#undef HAVE_X509_GET_SIGNATURE_INFO
-/* Define to 1 if you have the `X509_get_signature_nid' function. */
-#undef HAVE_X509_GET_SIGNATURE_NID
-
/* Define to 1 if the assembler supports X86_64's POPCNTQ instruction. */
#undef HAVE_X86_64_POPCNTQ
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 9b286aa4d7..118d15b1a1 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -209,10 +209,9 @@ scram_get_mechanisms(Port *port, StringInfo buf)
/*
* Advertise the mechanisms in decreasing order of importance. So the
* channel-binding variants go first, if they are supported. Channel
- * binding is only supported with SSL, and only if the SSL implementation
- * has a function to get the certificate's hash.
+ * binding is only supported with SSL.
*/
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
+#ifdef USE_SSL
if (port->ssl_in_use)
{
appendStringInfoString(buf, SCRAM_SHA_256_PLUS_NAME);
@@ -251,13 +250,12 @@ scram_init(Port *port, const char *selected_mech, const char *shadow_pass)
/*
* Parse the selected mechanism.
*
- * Note that if we don't support channel binding, either because the SSL
- * implementation doesn't support it or we're not using SSL at all, we
- * would not have advertised the PLUS variant in the first place. If the
- * client nevertheless tries to select it, it's a protocol violation like
- * selecting any other SASL mechanism we don't support.
+ * Note that if we don't support channel binding, or if we're not using
+ * SSL at all, we would not have advertised the PLUS variant in the first
+ * place. If the client nevertheless tries to select it, it's a protocol
+ * violation like selecting any other SASL mechanism we don't support.
*/
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
+#ifdef USE_SSL
if (strcmp(selected_mech, SCRAM_SHA_256_PLUS_NAME) == 0 && port->ssl_in_use)
state->channel_binding_in_use = true;
else
@@ -1010,7 +1008,7 @@ read_client_first_message(scram_state *state, const char *input)
errmsg("malformed SCRAM message"),
errdetail("The client selected SCRAM-SHA-256-PLUS, but the SCRAM message does not include channel binding data.")));
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
+#ifdef USE_SSL
if (state->port->ssl_in_use)
ereport(ERROR,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
@@ -1306,7 +1304,7 @@ read_client_final_message(scram_state *state, const char *input)
channel_binding = read_attr_value(&p, 'c');
if (state->channel_binding_in_use)
{
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
+#ifdef USE_SSL
const char *cbind_data = NULL;
size_t cbind_data_len = 0;
size_t cbind_header_len;
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 05276ab95c..658b09988d 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -831,8 +831,6 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
*
* These functions are closely modelled on the standard socket BIO in OpenSSL;
* see sock_read() and sock_write() in OpenSSL's crypto/bio/bss_sock.c.
- * XXX OpenSSL 1.0.1e considers many more errcodes than just EINTR as reasons
- * to retry; do we need to adopt their logic for that?
*/
#ifndef HAVE_BIO_GET_DATA
@@ -1429,7 +1427,6 @@ be_tls_get_peer_serial(Port *port, char *ptr, size_t len)
ptr[0] = '\0';
}
-#if defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO)
char *
be_tls_get_certificate_hash(Port *port, size_t *len)
{
@@ -1488,7 +1485,6 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
return cert_hash;
}
-#endif
/*
* Convert an X509 subject name to a cstring.
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 6b779ec7ff..61e6cd84d2 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -401,7 +401,7 @@ build_client_first_message(fe_scram_state *state)
Assert(conn->ssl_in_use);
appendPQExpBufferStr(&buf, "p=tls-server-end-point");
}
-#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
+#ifdef USE_SSL
else if (conn->channel_binding[0] != 'd' && /* disable */
conn->ssl_in_use)
{
@@ -474,7 +474,7 @@ build_client_final_message(fe_scram_state *state)
*/
if (strcmp(state->sasl_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0)
{
-#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
+#ifdef USE_SSL
char *cbind_data = NULL;
size_t cbind_data_len = 0;
size_t cbind_header_len;
@@ -540,9 +540,9 @@ build_client_final_message(fe_scram_state *state)
appendPQExpBufferStr(&conn->errorMessage,
"channel binding not supported by this build\n");
return NULL;
-#endif /* HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH */
+#endif /* USE_SSL */
}
-#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
+#ifdef USE_SSL
else if (conn->channel_binding[0] != 'd' && /* disable */
conn->ssl_in_use)
appendPQExpBufferStr(&buf, "c=eSws"); /* base64 of "y,," */
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 88fd0f3d80..f8e09d3b41 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -478,7 +478,7 @@ pg_SASL_init(PGconn *conn, int payloadlen)
{
/* The server has offered SCRAM-SHA-256-PLUS. */
-#ifdef HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
+#ifdef USE_SSL
/*
* The client supports channel binding, which is chosen if
* channel_binding is not disabled.
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 390c888c96..bea71660ab 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -364,7 +364,6 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
return n;
}
-#if defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO)
char *
pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
{
@@ -439,7 +438,6 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
return cert_hash;
}
-#endif /* HAVE_X509_GET_SIGNATURE_NID */
/* ------------------------------------------------------------ */
/* OpenSSL specific code */
@@ -1826,8 +1824,6 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
*
* These functions are closely modelled on the standard socket BIO in OpenSSL;
* see sock_read() and sock_write() in OpenSSL's crypto/bio/bss_sock.c.
- * XXX OpenSSL 1.0.1e considers many more errcodes than just EINTR as reasons
- * to retry; do we need to adopt their logic for that?
*/
#ifndef HAVE_BIO_GET_DATA
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 0045f83cbf..b70d4aee6a 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -833,14 +833,8 @@ extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
*
* NULL is sent back to the caller in the event of an error, with an
* error message for the caller to consume.
- *
- * This is not supported with old versions of OpenSSL that don't have
- * the X509_get_signature_nid() function.
*/
-#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
-#define HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
extern char *pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len);
-#endif
/*
* Verify that the server certificate matches the host name we connected to.
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 28c54bdb09..3a798e1a56 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -44,9 +44,6 @@ my $SERVERHOSTADDR = '127.0.0.1';
# This is the pattern to use in pg_hba.conf to match incoming connections.
my $SERVERHOSTCIDR = '127.0.0.1/32';
-# Determine whether build supports tls-server-end-point.
-my $supports_tls_server_end_point =
- check_pg_config("#define HAVE_X509_GET_SIGNATURE_NID 1");
# Determine whether build supports detection of hash algorithms for
# RSA-PSS certificates.
my $supports_rsapss_certs =
@@ -90,21 +87,9 @@ $node->connect_fails(
expected_stderr => qr/invalid channel_binding value: "invalid_value"/);
$node->connect_ok("$common_connstr user=ssltestuser channel_binding=disable",
"SCRAM with SSL and channel_binding=disable");
-if ($supports_tls_server_end_point)
-{
- $node->connect_ok(
- "$common_connstr user=ssltestuser channel_binding=require",
- "SCRAM with SSL and channel_binding=require");
-}
-else
-{
- $node->connect_fails(
- "$common_connstr user=ssltestuser channel_binding=require",
- "SCRAM with SSL and channel_binding=require",
- expected_stderr =>
- qr/channel binding is required, but server did not offer an authentication method that supports channel binding/
- );
-}
+$node->connect_ok(
+ "$common_connstr user=ssltestuser channel_binding=require",
+ "SCRAM with SSL and channel_binding=require");
# Now test when the user has an MD5-encrypted password; should fail
$node->connect_fails(
@@ -152,22 +137,10 @@ $node->connect_fails(
expected_stderr =>
qr/channel binding required but not supported by server's authentication request/
);
-if ($supports_tls_server_end_point)
-{
- $node->connect_ok(
- "$common_connstr user=ssltestuser channel_binding=require require_auth=scram-sha-256",
- "SCRAM with SSL, channel_binding=require, and require_auth=scram-sha-256"
- );
-}
-else
-{
- $node->connect_fails(
- "$common_connstr user=ssltestuser channel_binding=require require_auth=scram-sha-256",
- "SCRAM with SSL, channel_binding=require, and require_auth=scram-sha-256",
- expected_stderr =>
- qr/channel binding is required, but server did not offer an authentication method that supports channel binding/
- );
-}
+$node->connect_ok(
+ "$common_connstr user=ssltestuser channel_binding=require require_auth=scram-sha-256",
+ "SCRAM with SSL, channel_binding=require, and require_auth=scram-sha-256"
+);
# Now test with a server certificate that uses the RSA-PSS algorithm.
# This checks that the certificate can be loaded and that channel binding
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 75dc81a0a9..e9d797417a 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -275,7 +275,7 @@ documentation. See standalone-profile.xsl for details.
encrypted client connections. <productname>OpenSSL</productname> is
also required for random number generation on platforms that do not
have <filename>/dev/urandom</filename> (except Windows). The minimum
- required version is 1.0.1.
+ required version is 1.0.2.
</para>
</listitem>
diff --git a/configure b/configure
index 1b415142d1..c25089f2c1 100755
--- a/configure
+++ b/configure
@@ -12744,9 +12744,9 @@ if test "$with_openssl" = yes ; then
fi
if test "$with_ssl" = openssl ; then
- # Minimum required OpenSSL version is 1.0.1
+ # Minimum required OpenSSL version is 1.0.2
-$as_echo "#define OPENSSL_API_COMPAT 0x10001000L" >>confdefs.h
+$as_echo "#define OPENSSL_API_COMPAT 0x10002000L" >>confdefs.h
if test "$PORTNAME" != "win32"; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for CRYPTO_new_ex_data in -lcrypto" >&5
@@ -12961,15 +12961,13 @@ else
fi
fi
- # Functions introduced in OpenSSL 1.0.2. LibreSSL does not have
- # SSL_CTX_set_cert_cb().
- for ac_func in X509_get_signature_nid SSL_CTX_set_cert_cb
+ # LibreSSL does not have SSL_CTX_set_cert_cb().
+ for ac_func in SSL_CTX_set_cert_cb
do :
- as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
-ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
-if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
+ ac_fn_c_check_func "$LINENO" "SSL_CTX_set_cert_cb" "ac_cv_func_SSL_CTX_set_cert_cb"
+if test "x$ac_cv_func_SSL_CTX_set_cert_cb" = xyes; then :
cat >>confdefs.h <<_ACEOF
-#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
+#define HAVE_SSL_CTX_SET_CERT_CB 1
_ACEOF
fi
diff --git a/configure.ac b/configure.ac
index 09558ada0f..093bfad082 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1367,8 +1367,8 @@ fi
if test "$with_ssl" = openssl ; then
dnl Order matters!
- # Minimum required OpenSSL version is 1.0.1
- AC_DEFINE(OPENSSL_API_COMPAT, [0x10001000L],
+ # Minimum required OpenSSL version is 1.0.2
+ AC_DEFINE(OPENSSL_API_COMPAT, [0x10002000L],
[Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.])
if test "$PORTNAME" != "win32"; then
AC_CHECK_LIB(crypto, CRYPTO_new_ex_data, [], [AC_MSG_ERROR([library 'crypto' is required for OpenSSL])])
@@ -1377,9 +1377,9 @@ if test "$with_ssl" = openssl ; then
AC_SEARCH_LIBS(CRYPTO_new_ex_data, [eay32 crypto], [], [AC_MSG_ERROR([library 'eay32' or 'crypto' is required for OpenSSL])])
AC_SEARCH_LIBS(SSL_new, [ssleay32 ssl], [], [AC_MSG_ERROR([library 'ssleay32' or 'ssl' is required for OpenSSL])])
fi
- # 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])
# Functions introduced in OpenSSL 1.1.0. We used to check for
# OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
# defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
diff --git a/meson.build b/meson.build
index 16b2e86646..7d0122ab32 100644
--- a/meson.build
+++ b/meson.build
@@ -1268,9 +1268,8 @@ if sslopt in ['auto', 'openssl']
['CRYPTO_new_ex_data', {'required': true}],
['SSL_new', {'required': true}],
- # Functions introduced in OpenSSL 1.0.2.
- ['X509_get_signature_nid'],
- ['SSL_CTX_set_cert_cb'], # not in LibreSSL
+ # Functions introduced in OpenSSL 1.0.2, not in LibreSSL.
+ ['SSL_CTX_set_cert_cb'],
# Functions introduced in OpenSSL 1.1.0. We used to check for
# OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
@@ -1312,7 +1311,7 @@ if sslopt in ['auto', 'openssl']
if are_openssl_funcs_complete
cdata.set('USE_OPENSSL', 1,
description: 'Define to 1 to build with OpenSSL support. (-Dssl=openssl)')
- cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
+ cdata.set('OPENSSL_API_COMPAT', '0x10002000L',
description: '''Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.''')
ssl_library = 'openssl'
else
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index b6d31c3583..e6d8f9fedc 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -371,7 +371,6 @@ sub GenerateFiles
HAVE_UUID_UUID_H => undef,
HAVE_WCSTOMBS_L => 1,
HAVE_VISIBILITY_ATTRIBUTE => undef,
- HAVE_X509_GET_SIGNATURE_NID => 1,
HAVE_X509_GET_SIGNATURE_INFO => undef,
HAVE_X86_64_POPCNTQ => undef,
HAVE__BOOL => undef,
@@ -488,6 +487,7 @@ sub GenerateFiles
if ($self->{options}->{openssl})
{
$define{USE_OPENSSL} = 1;
+ $define{HAVE_SSL_CTX_SET_CERT_CB} = 1;
my ($digit1, $digit2, $digit3) = $self->GetOpenSSLVersion();
@@ -509,14 +509,6 @@ sub GenerateFiles
$define{HAVE_HMAC_CTX_NEW} = 1;
$define{HAVE_OPENSSL_INIT_SSL} = 1;
}
-
- # Symbols needed with OpenSSL 1.0.2 and above.
- if ( ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
- || ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0')
- || ($digit1 >= '1' && $digit2 >= '0' && $digit3 >= '2'))
- {
- $define{HAVE_SSL_CTX_SET_CERT_CB} = 1;
- }
}
$self->GenerateConfigHeader('src/include/pg_config.h', \%define, 1);
--
2.40.1
signature.asc
Description: PGP signature
