Skip to content

Commit

Permalink
Use BIO_{get,set}_app_data instead of BIO_{get,set}_data.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tglsfdc committed Nov 28, 2023
1 parent 10a5992 commit c82207a
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 27 deletions.
2 changes: 1 addition & 1 deletion configure
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
11 changes: 3 additions & 8 deletions src/backend/libpq/be-secure-openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
{
Expand All @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/include/pg_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 3 additions & 8 deletions src/interfaces/libpq/fe-secure-openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/test/ssl/t/001_ssltests.pl
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ sub switch_server_cert
"$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},
Expand Down Expand Up @@ -881,7 +881,7 @@ sub switch_server_cert
"$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},
Expand All @@ -894,7 +894,7 @@ sub switch_server_cert
"$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},
Expand Down
2 changes: 0 additions & 2 deletions src/tools/msvc/Solution.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit c82207a

Please sign in to comment.