From c82207a548db47623a2bfa2447babdaa630302b9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 28 Nov 2023 12:34:03 -0500 Subject: [PATCH] Use BIO_{get,set}_app_data instead of BIO_{get,set}_data. We should have done it this way all along, but we accidentally got away with using the wrong BIO field up until OpenSSL 3.2. There, the library's BIO routines that we rely on use the "data" field for their own purposes, and our conflicting use causes assorted weird behaviors up to and including core dumps when SSL connections are attempted. Switch to using the approved field for the purpose, i.e. app_data. While at it, remove our configure probes for BIO_get_data as well as the fallback implementation. BIO_{get,set}_app_data have been there since long before any OpenSSL version that we still support, even in the back branches. Also, update src/test/ssl/t/001_ssltests.pl to allow for a minor change in an error message spelling that evidently came in with 3.2. Tristan Partin and Bo Andreson. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAN55FZ1eDDYsYaL7mv+oSLUij2h_u6hvD4Qmv-7PK7jkji0uyQ@mail.gmail.com --- configure | 2 +- configure.ac | 2 +- meson.build | 1 - src/backend/libpq/be-secure-openssl.c | 11 +++-------- src/include/pg_config.h.in | 3 --- src/interfaces/libpq/fe-secure-openssl.c | 11 +++-------- src/test/ssl/t/001_ssltests.pl | 6 +++--- src/tools/msvc/Solution.pm | 2 -- 8 files changed, 11 insertions(+), 27 deletions(-) diff --git a/configure b/configure index c0641150383..bf3ea690db3 100755 --- a/configure +++ b/configure @@ -12836,7 +12836,7 @@ done # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it # doesn't have these OpenSSL 1.1.0 functions. So check for individual # functions. - for ac_func in OPENSSL_init_ssl BIO_get_data BIO_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free + for ac_func in OPENSSL_init_ssl BIO_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free 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" diff --git a/configure.ac b/configure.ac index f220b379b30..fed7167e65d 100644 --- a/configure.ac +++ b/configure.ac @@ -1367,7 +1367,7 @@ if test "$with_ssl" = openssl ; then # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it # doesn't have these OpenSSL 1.1.0 functions. So check for individual # functions. - AC_CHECK_FUNCS([OPENSSL_init_ssl BIO_get_data BIO_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free]) + AC_CHECK_FUNCS([OPENSSL_init_ssl BIO_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free]) # OpenSSL versions before 1.1.0 required setting callback functions, for # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock() # function was removed. diff --git a/meson.build b/meson.build index ee58ee7a065..0095fb183af 100644 --- a/meson.build +++ b/meson.build @@ -1285,7 +1285,6 @@ if sslopt in ['auto', 'openssl'] # doesn't have these OpenSSL 1.1.0 functions. So check for individual # functions. ['OPENSSL_init_ssl'], - ['BIO_get_data'], ['BIO_meth_new'], ['ASN1_STRING_get0_data'], ['HMAC_CTX_new'], diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 31b6a6eacdf..1b8b32c5b39 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -842,11 +842,6 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor) * see sock_read() and sock_write() in OpenSSL's crypto/bio/bss_sock.c. */ -#ifndef HAVE_BIO_GET_DATA -#define BIO_get_data(bio) (bio->ptr) -#define BIO_set_data(bio, data) (bio->ptr = data) -#endif - static BIO_METHOD *my_bio_methods = NULL; static int @@ -856,7 +851,7 @@ my_sock_read(BIO *h, char *buf, int size) if (buf != NULL) { - res = secure_raw_read(((Port *) BIO_get_data(h)), buf, size); + res = secure_raw_read(((Port *) BIO_get_app_data(h)), buf, size); BIO_clear_retry_flags(h); if (res <= 0) { @@ -876,7 +871,7 @@ my_sock_write(BIO *h, const char *buf, int size) { int res = 0; - res = secure_raw_write(((Port *) BIO_get_data(h)), buf, size); + res = secure_raw_write(((Port *) BIO_get_app_data(h)), buf, size); BIO_clear_retry_flags(h); if (res <= 0) { @@ -952,7 +947,7 @@ my_SSL_set_fd(Port *port, int fd) SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB); goto err; } - BIO_set_data(bio, port); + BIO_set_app_data(bio, port); BIO_set_fd(bio, fd, BIO_NOCLOSE); SSL_set_bio(port->ssl, bio, bio); diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index d8a2985567f..5f16918243c 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -66,9 +66,6 @@ /* Define to 1 if you have the `backtrace_symbols' function. */ #undef HAVE_BACKTRACE_SYMBOLS -/* Define to 1 if you have the `BIO_get_data' function. */ -#undef HAVE_BIO_GET_DATA - /* Define to 1 if you have the `BIO_meth_new' function. */ #undef HAVE_BIO_METH_NEW diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index 4aeaf08312c..e669bdbf1d2 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -1815,11 +1815,6 @@ PQsslAttribute(PGconn *conn, const char *attribute_name) * see sock_read() and sock_write() in OpenSSL's crypto/bio/bss_sock.c. */ -#ifndef HAVE_BIO_GET_DATA -#define BIO_get_data(bio) (bio->ptr) -#define BIO_set_data(bio, data) (bio->ptr = data) -#endif - /* protected by ssl_config_mutex */ static BIO_METHOD *my_bio_methods; @@ -1828,7 +1823,7 @@ my_sock_read(BIO *h, char *buf, int size) { int res; - res = pqsecure_raw_read((PGconn *) BIO_get_data(h), buf, size); + res = pqsecure_raw_read((PGconn *) BIO_get_app_data(h), buf, size); BIO_clear_retry_flags(h); if (res < 0) { @@ -1858,7 +1853,7 @@ my_sock_write(BIO *h, const char *buf, int size) { int res; - res = pqsecure_raw_write((PGconn *) BIO_get_data(h), buf, size); + res = pqsecure_raw_write((PGconn *) BIO_get_app_data(h), buf, size); BIO_clear_retry_flags(h); if (res < 0) { @@ -1968,7 +1963,7 @@ my_SSL_set_fd(PGconn *conn, int fd) SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB); goto err; } - BIO_set_data(bio, conn); + BIO_set_app_data(bio, conn); SSL_set_bio(conn->ssl, bio, bio); BIO_set_fd(bio, fd, BIO_NOCLOSE); diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index a049fd2ff03..d921f1dde9f 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -776,7 +776,7 @@ $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt " . sslkey('client-revoked.key'), "certificate authorization fails with revoked client cert", - expected_stderr => qr/SSL error: sslv3 alert certificate revoked/, + expected_stderr => qr|SSL error: ssl[a-z0-9/]* alert certificate revoked|, # temporarily(?) skip this check due to timing issue # log_like => [ # qr{Client certificate verification failed at depth 0: certificate revoked}, @@ -881,7 +881,7 @@ $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt " . sslkey('client-revoked.key'), "certificate authorization fails with revoked client cert with server-side CRL directory", - expected_stderr => qr/SSL error: sslv3 alert certificate revoked/, + expected_stderr => qr|SSL error: ssl[a-z0-9/]* alert certificate revoked|, # temporarily(?) skip this check due to timing issue # log_like => [ # qr{Client certificate verification failed at depth 0: certificate revoked}, @@ -894,7 +894,7 @@ $node->connect_fails( "$common_connstr user=ssltestuser sslcert=ssl/client-revoked-utf8.crt " . sslkey('client-revoked-utf8.key'), "certificate authorization fails with revoked UTF-8 client cert with server-side CRL directory", - expected_stderr => qr/SSL error: sslv3 alert certificate revoked/, + expected_stderr => qr|SSL error: ssl[a-z0-9/]* alert certificate revoked|, # temporarily(?) skip this check due to timing issue # log_like => [ # qr{Client certificate verification failed at depth 0: certificate revoked}, diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index 98a5b5d872b..a980b51f5d9 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -224,7 +224,6 @@ sub GenerateFiles HAVE_ATOMICS => 1, HAVE_ATOMIC_H => undef, HAVE_BACKTRACE_SYMBOLS => undef, - HAVE_BIO_GET_DATA => undef, HAVE_BIO_METH_NEW => undef, HAVE_COMPUTED_GOTO => undef, HAVE_COPYFILE => undef, @@ -502,7 +501,6 @@ sub GenerateFiles || ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0')) { $define{HAVE_ASN1_STRING_GET0_DATA} = 1; - $define{HAVE_BIO_GET_DATA} = 1; $define{HAVE_BIO_METH_NEW} = 1; $define{HAVE_HMAC_CTX_FREE} = 1; $define{HAVE_HMAC_CTX_NEW} = 1;