Here is a v2 which adds back a comment that was not meant to be removed.
-- Tristan Partin Neon (https://neon.tech)
From 4bcb73eab9ceba950581a890c52820a81134f7e4 Mon Sep 17 00:00:00 2001 From: Tristan Partin <tris...@neon.tech> Date: Mon, 27 Nov 2023 11:49:52 -0600 Subject: [PATCH v2] Use BIO_{get,set}_app_data() instead of BIO_{get,set}_data() Compiling Postgres against OpenSSL 3.2 exposed a regression: $ psql "postgresql://$DB?sslmode=require" psql: error: connection to server at "redacted" (redacted), port 5432 failed: ERROR: Parameter 'user' is missing in startup packet. double free or corruption (out) Aborted (core dumped) Analyzing the backtrace, openssl was overwriting heap-allocated data in our PGconn struct because it thought BIO::ptr was a struct bss_sock_st *. OpenSSL then called a memset() on a member of that struct, and we zeroed out data in our PGconn struct. BIO_get_data(3) says the following: > These functions are mainly useful when implementing a custom BIO. > > The BIO_set_data() function associates the custom data pointed to by ptr > with the BIO a. This data can subsequently be retrieved via a call to > BIO_get_data(). This can be used by custom BIOs for storing > implementation specific information. If you take a look at my_BIO_s_socket(), we create a partially custom BIO, but for the most part are defaulting to the methods defined by BIO_s_socket(). We need to set application-specific data and not BIO private data, so that the BIO implementation we rely on, can properly assert that its private data is what it expects. Co-authored-by: Bo Andreson <m...@boanderson.me> --- 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/tools/msvc/Solution.pm | 2 -- 5 files changed, 6 insertions(+), 22 deletions(-) diff --git a/meson.build b/meson.build index ee58ee7a06..0095fb183a 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 31b6a6eacd..1b8b32c5b3 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 d8a2985567..5f16918243 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 4aeaf08312..e669bdbf1d 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/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index 98a5b5d872..a980b51f5d 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; -- Tristan Partin Neon (https://neon.tech)