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

diff --git a/configure b/configure
index d83a402ea11108817ede566aa84b559c744cf3ca..d55440cd6a40c7d9a4130f3a8e835d91564ba9a9 100755 (executable)
--- a/configure
+++ b/configure
@@ -13239,7 +13239,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 570daced81b133bef78d94b633d1e18d30e369de..2bc752ca1ab96545c0fec8ca868076ab64e59452 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -1347,7 +1347,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/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index f5c5ed210e22c214d0e64d21599e8ba50fec7c44..aed8a75345aaf4059cd5a1f516d36016b3e05f12 100644 (file)
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -839,11 +839,6 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
  * to retry; do we need to adopt their logic for that?
  */
 
-#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
@@ -853,7 +848,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)
        {
@@ -873,7 +868,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)
    {
@@ -949,7 +944,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 d09e9f9a1c3fa55cad86374c1f435d3e77eaa958..768e3d719c152a83d8e91f7798d3253f039f77cd 100644 (file)
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -77,9 +77,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 62f813df68db7d6464b4a967b6d3e3f627033030..d863d279a07bdc1698e6fb30becec7446fb4499a 100644 (file)
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1800,11 +1800,6 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
  * to retry; do we need to adopt their logic for that?
  */
 
-#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;
 
@@ -1813,7 +1808,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)
    {
@@ -1843,7 +1838,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)
    {
@@ -1962,7 +1957,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 707f4005af50076f9c348475c4a5c03a0da92b55..c570b48a1bddd19628e739c068bb363e2240ab65 100644 (file)
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -682,7 +682,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|,
    # revoked certificates should not authenticate the user
    log_unlike => [qr/connection authenticated:/],);
 
@@ -743,6 +743,6 @@ $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|);
 
 done_testing();

diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 790f03b05e654e97f5ea01baa4419eccc459dad8..a53239fa2870fb06ec823ba5e99a49a4df10ebf3 100644 (file)
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -226,7 +226,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_CLOCK_GETTIME         => undef,
        HAVE_COMPUTED_GOTO         => undef,
@@ -566,7 +565,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;