Use BIO_{get,set}_app_data instead of BIO_{get,set}_data.
authorTom Lane
Tue, 28 Nov 2023 17:34:03 +0000 (12:34 -0500)
committerTom Lane
Tue, 28 Nov 2023 17:34:03 +0000 (12:34 -0500)
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
configure.ac
src/backend/libpq/be-secure-openssl.c
src/include/pg_config.h.in
src/interfaces/libpq/fe-secure-openssl.c
src/test/ssl/t/001_ssltests.pl
src/tools/msvc/Solution.pm

index 62a921b5e7f5ddb64e99ab2d4d389117d38b98a9..f74b9862a0e75d5218b09a86e441f1dcf68bf1ed 100755 (executable)
--- a/configure
+++ b/configure
@@ -13071,7 +13071,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"
index a3243cc7e8c04e7c57202744958dcbc87da8ed9d..46624d2a115bf81e45e27cb9e19f19381ec4c37d 100644 (file)
@@ -1311,7 +1311,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.
index 13ac9614422fd73747e8eebe9a13113907ed796f..e39952494e65349b70bbc9a01065da3d2bc3aae7 100644 (file)
@@ -823,11 +823,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
@@ -837,7 +832,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)
        {
@@ -857,7 +852,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)
    {
@@ -933,7 +928,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);
index 40d513c1288baecc69af6be5a8c5683a33c40638..51fa911fb6aeef7fa7dfe45503c2ad6fe22d29e0 100644 (file)
@@ -86,9 +86,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
 
index c451360586336b1fdf6251a48d6b6bb2a4959297..b42b4ca5002c73ee4076e5a95d3555aff8dabf12 100644 (file)
@@ -1661,11 +1661,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;
 
@@ -1674,7 +1669,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)
    {
@@ -1704,7 +1699,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)
    {
@@ -1823,7 +1818,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);
index 8cdd0d2e6864f98aa8c2a59994c657f17a26d60e..cc7bd98c83c99e59cdf157b88627fe011549992a 100644 (file)
@@ -538,7 +538,7 @@ $node->connect_fails(
 $node->connect_fails(
    "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.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:/],);
 
@@ -591,7 +591,7 @@ switch_server_cert($node, 'server-cn-only', undef, undef,
 $node->connect_fails(
    "$common_connstr user=ssltestuser sslcert=ssl/client-revoked.crt sslkey=ssl/client-revoked_tmp.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|);
 
 # clean up
 foreach my $key (@keys)
index 577b5afea72053dacd27dbfadce0aa3ed593cd02..53d60dbd259e4364e8e375ca280dd8669ccb8dba 100644 (file)
@@ -229,7 +229,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,
@@ -562,7 +561,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;