Use BIO_{get,set}_app_data instead of BIO_{get,set}_data.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 28 Nov 2023 17:34:03 +0000 (12:34 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
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
meson.build
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 82e45657b219923cf3b2cb4ea39802d22a7ff7e9..907c777b9cc342c8a35683d5da95bdacbff8eeb0 100755 (executable)
--- a/configure
+++ b/configure
@@ -12982,7 +12982,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 fcea0bcab42595866dfcb018f16560a96c67d577..ab32bfdd0828037c5e54d300adf26a61ce174caf 100644 (file)
@@ -1385,7 +1385,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 51b5285924ab7390f569462dbe69bf7bfe5465a4..96fc2e139afc9c629cabf0f127d6b7184febce30 100644 (file)
@@ -1278,7 +1278,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'],
index e9c86d08df25b25d9df55333ef13d3fb069ab30d..49dca0cda9fa808bfbe79afcd15d83e66ab4d547 100644 (file)
@@ -844,11 +844,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
@@ -858,7 +853,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)
        {
@@ -878,7 +873,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)
    {
@@ -954,7 +949,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 6d572c38204201221b00dc39381a3dfec99991ee..174544630e39ae1258fafb6fa071aa5fcfcba857 100644 (file)
@@ -70,9 +70,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 5a44ac5c3053902b2356d6ae463772c38749af47..93cf70b0ed63eb6f7ba44b3b9c466b637f8613a7 100644 (file)
@@ -1830,11 +1830,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;
 
@@ -1843,7 +1838,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)
    {
@@ -1873,7 +1868,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)
    {
@@ -1992,7 +1987,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 76442de063f17670217178f15356da6a1223b85f..9bb28fbc832de5d76221185e889345abe0805103 100644 (file)
@@ -781,7 +781,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},
@@ -886,7 +886,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},
@@ -899,7 +899,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},
index b6d31c358356f767c438fc42821ec3049d9d2f67..711fae853f83a802c0be8786c407a9753a36aef4 100644 (file)
@@ -225,7 +225,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,
@@ -503,7 +502,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;