[PATCH] Avoid mixing custom and OpenSSL BIO functions

From e29b816dfb7ee5ed9f3389d7f06408f83783258a Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 11 Feb 2024 10:42:25 -0500
Subject: [PATCH] Avoid mixing custom and OpenSSL BIO functions

This addresses the root cause of the BIO_set_data conflict that
c82207a548db47623a2bfa2447babdaa630302b9 attempted to address. That fix was
really just a workaround. The real root cause was that postgres was mixing up
two BIO implementations, each of which expected to be driving the BIO.

Mixing them up did not actually do any good. The Port and PGconn structures
already maintained the file descriptors. The socket BIO's copy, configured via
BIO_set_fd, wasn't even being used because my_BIO_s_socket replaced read and
write anyway. We've been essentially keeping extra state around, and relying
on it being unused:

- gets - Not implemented by sockets and not used by libssl.
- puts - Not used by libssl. If it were, it would break the special SIGPIPE and
  interrupt handling postgres aiming for.
- ctrl - (More on this later)
- create - This is just setting up state that we don't use.
- destroy - This is a no-op because we use BIO_NOCLOSE. In fact it's important
  that it's a no-op because otherwise OpenSSL would close the socket under
  postgres' feet!
- callback_ctrl - Not implemented by sockets.

That leaves ctrl. ctrl is a bunch of operations (it's ioctl). The only
operation that matters is BIO_CTRL_FLUSH, which is implemented as a no-op. All
other operations are unused. It's once again good that they're unused because
otherwise OpenSSL might mess with postgres's socket, break the assumptions
around interrupt handling, etc.

Instead, simply implement a very basic ctrl ourselves and drop the other
functions. This avoids the risk that future OpenSSL upgrades add some feature
to BIO_s_socket's ctrl which conflicts with postgres.

Once we've done that, we're free to use BIO_set_data. While BIO_set_app_data
works fine, I've reverted back to BIO_set_data because it's more commonly used.
app_data depends on OpenSSL's "ex_data" mechanism, which is a tad heavier under
the hood.

As this is no longer related to BIO_s_socket or calling SSL_set_fd, I've
renamed the methods to reference the PGconn and Port types instead.
---
 configure                                |   2 +-
 configure.ac                             |   2 +-
 meson.build                              |   1 +
 src/backend/libpq/be-secure-openssl.c    | 106 +++++++++++++++--------
 src/include/libpq/libpq-be.h             |   1 +
 src/include/pg_config.h.in               |   3 +
 src/interfaces/libpq/fe-secure-openssl.c |  96 +++++++++++++-------
 src/interfaces/libpq/libpq-int.h         |   1 +
 8 files changed, 141 insertions(+), 71 deletions(-)

diff --git a/configure b/configure
index 6b87e5c9a8..cacda708b0 100755
--- a/configure
+++ b/configure
@@ -12870,7 +12870,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_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free
+  for ac_func in OPENSSL_init_ssl BIO_get_data 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 6e64ece11d..083bbabfbf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1374,7 +1374,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_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free])
+  AC_CHECK_FUNCS([OPENSSL_init_ssl BIO_get_data 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/meson.build b/meson.build
index 8ed51b6aae..ba133b6e11 100644
--- a/meson.build
+++ b/meson.build
@@ -1290,6 +1290,7 @@ 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 e12b1cc9e3..05cfb8c5d3 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -56,10 +56,10 @@
 static void default_openssl_tls_init(SSL_CTX *context, bool isServerStart);
 openssl_tls_init_hook_typ openssl_tls_init_hook = default_openssl_tls_init;
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(Port *port, int fd);
+static int	port_bio_read(BIO *h, char *buf, int size);
+static int	port_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *port_bio_method(void);
+static int	ssl_set_port_bio(Port *port);
 
 static DH  *load_dh_file(char *filename, bool isServerStart);
 static DH  *load_dh_buffer(const char *buffer, size_t len);
@@ -440,7 +440,7 @@ be_tls_open_server(Port *port)
 						SSLerrmessage(ERR_get_error()))));
 		return -1;
 	}
-	if (!my_SSL_set_fd(port, port->sock))
+	if (!ssl_set_port_bio(port))
 	{
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
@@ -849,17 +849,24 @@ 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.
  */
 
-static BIO_METHOD *my_bio_methods = NULL;
+#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 *port_bio_method_ptr = NULL;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+port_bio_read(BIO *h, char *buf, int size)
 {
 	int			res = 0;
+	Port	   *port = (Port *) BIO_get_data(h);
 
 	if (buf != NULL)
 	{
-		res = secure_raw_read(((Port *) BIO_get_app_data(h)), buf, size);
+		res = secure_raw_read(port, buf, size);
 		BIO_clear_retry_flags(h);
+		port->last_read_was_eof = res == 0;
 		if (res <= 0)
 		{
 			/* If we were interrupted, tell caller to retry */
@@ -874,11 +881,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+port_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res = 0;
 
-	res = secure_raw_write(((Port *) BIO_get_app_data(h)), buf, size);
+	res = secure_raw_write(((Port *) BIO_get_data(h)), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res <= 0)
 	{
@@ -892,56 +899,81 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+port_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	Port	   *port = (Port *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. port_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = port->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+port_bio_method(void)
 {
-	if (!my_bio_methods)
+	if (!port_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 #ifdef HAVE_BIO_METH_NEW
 		int			my_bio_index;
 
 		my_bio_index = BIO_get_new_index();
 		if (my_bio_index == -1)
 			return NULL;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
-		my_bio_methods = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
-		if (!my_bio_methods)
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
+		port_bio_method_ptr = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
+		if (!port_bio_method_ptr)
 			return NULL;
-		if (!BIO_meth_set_write(my_bio_methods, my_sock_write) ||
-			!BIO_meth_set_read(my_bio_methods, my_sock_read) ||
-			!BIO_meth_set_gets(my_bio_methods, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(my_bio_methods, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(port_bio_method_ptr, port_bio_write) ||
+			!BIO_meth_set_read(port_bio_method_ptr, port_bio_read) ||
+			!BIO_meth_set_ctrl(port_bio_method_ptr, port_bio_ctrl))
 		{
-			BIO_meth_free(my_bio_methods);
-			my_bio_methods = NULL;
+			BIO_meth_free(port_bio_method_ptr);
+			port_bio_method_ptr = NULL;
 			return NULL;
 		}
 #else
-		my_bio_methods = malloc(sizeof(BIO_METHOD));
-		if (!my_bio_methods)
+		port_bio_method_ptr = malloc(sizeof(BIO_METHOD));
+		if (!port_bio_method_ptr)
 			return NULL;
-		memcpy(my_bio_methods, biom, sizeof(BIO_METHOD));
-		my_bio_methods->bread = my_sock_read;
-		my_bio_methods->bwrite = my_sock_write;
+		memset(port_bio_method_ptr, 0, sizeof(BIO_METHOD));
+		port_bio_method_ptr->type = BIO_TYPE_SOURCE_SINK;
+		port_bio_method_ptr->name = "libpq socket";
+		port_bio_method_ptr->bread = port_bio_read;
+		port_bio_method_ptr->bwrite = port_bio_write;
+		port_bio_method_ptr->ctrl = port_bio_ctrl;
 #endif
 	}
-	return my_bio_methods;
+	return port_bio_method_ptr;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(Port *port, int fd)
+ssl_set_port_bio(Port *port)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = port_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -954,9 +986,9 @@ my_SSL_set_fd(Port *port, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, port);
+	BIO_set_data(bio, port);
+	BIO_set_init(bio, 1);
 
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	SSL_set_bio(port->ssl, bio, bio);
 	ret = 1;
 err:
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 47d66d5524..014c067e81 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -218,6 +218,7 @@ typedef struct Port
 	char	   *peer_cn;
 	char	   *peer_dn;
 	bool		peer_cert_valid;
+	bool		last_read_was_eof;
 
 	/*
 	 * OpenSSL structures. (Keep these last so that the locations of other
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 07e73567dc..c7b88945fa 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -66,6 +66,9 @@
 /* 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 8110882262..cadb5b32f4 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -78,10 +78,10 @@ static char *SSLerrmessage(unsigned long ecode);
 static void SSLerrfree(char *buf);
 static int	PQssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(PGconn *conn, int fd);
+static int	pgconn_bio_read(BIO *h, char *buf, int size);
+static int	pgconn_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *pgconn_bio_method(void);
+static int	ssl_set_pgconn_bio(PGconn *conn);
 
 
 static bool pq_init_ssl_lib = true;
@@ -1191,7 +1191,7 @@ initialize_SSL(PGconn *conn)
 	 */
 	if (!(conn->ssl = SSL_new(SSL_context)) ||
 		!SSL_set_app_data(conn->ssl, conn) ||
-		!my_SSL_set_fd(conn, conn->sock))
+		!ssl_set_pgconn_bio(conn))
 	{
 		char	   *err = SSLerrmessage(ERR_get_error());
 
@@ -1802,16 +1802,23 @@ 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;
+static BIO_METHOD *pgconn_bio_method_ptr;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+pgconn_bio_read(BIO *h, char *buf, int size)
 {
 	int			res;
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
 
-	res = pqsecure_raw_read((PGconn *) BIO_get_app_data(h), buf, size);
+	res = pqsecure_raw_read(conn, buf, size);
 	BIO_clear_retry_flags(h);
+	conn->last_read_was_eof = res == 0;
 	if (res < 0)
 	{
 		/* If we were interrupted, tell caller to retry */
@@ -1836,11 +1843,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+pgconn_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res;
 
-	res = pqsecure_raw_write((PGconn *) BIO_get_app_data(h), buf, size);
+	res = pqsecure_raw_write((PGconn *) BIO_get_data(h), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res < 0)
 	{
@@ -1865,26 +1872,54 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+pgconn_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. pgconn_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = conn->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+pgconn_bio_method(void)
 {
 	BIO_METHOD *res;
 
 	if (pthread_mutex_lock(&ssl_config_mutex))
 		return NULL;
 
-	res = my_bio_methods;
+	res = pgconn_bio_method_ptr;
 
-	if (!my_bio_methods)
+	if (!pgconn_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 #ifdef HAVE_BIO_METH_NEW
 		int			my_bio_index;
 
 		my_bio_index = BIO_get_new_index();
 		if (my_bio_index == -1)
 			goto err;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
 		res = BIO_meth_new(my_bio_index, "libpq socket");
 		if (!res)
 			goto err;
@@ -1893,14 +1928,9 @@ my_BIO_s_socket(void)
 		 * As of this writing, these functions never fail. But check anyway,
 		 * like OpenSSL's own examples do.
 		 */
-		if (!BIO_meth_set_write(res, my_sock_write) ||
-			!BIO_meth_set_read(res, my_sock_read) ||
-			!BIO_meth_set_gets(res, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(res, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(res, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(res, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(res, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(res, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(res, pgconn_bio_write) ||
+			!BIO_meth_set_read(res, pgconn_bio_read) ||
+			!BIO_meth_set_ctrl(res, pgconn_bio_ctrl))
 		{
 			goto err;
 		}
@@ -1908,13 +1938,16 @@ my_BIO_s_socket(void)
 		res = malloc(sizeof(BIO_METHOD));
 		if (!res)
 			goto err;
-		memcpy(res, biom, sizeof(BIO_METHOD));
-		res->bread = my_sock_read;
-		res->bwrite = my_sock_write;
+		memset(res, 0, sizeof(BIO_METHOD));
+		res->type = BIO_TYPE_SOURCE_SINK;
+		res->name = "libpq socket";
+		res->bread = pgconn_bio_read;
+		res->bwrite = pgconn_bio_write;
+		res->ctrl = pgconn_bio_ctrl;
 #endif
 	}
 
-	my_bio_methods = res;
+	pgconn_bio_method_ptr = res;
 	pthread_mutex_unlock(&ssl_config_mutex);
 	return res;
 
@@ -1930,15 +1963,14 @@ err:
 	return NULL;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(PGconn *conn, int fd)
+ssl_set_pgconn_bio(PGconn *conn)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = pgconn_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -1950,10 +1982,10 @@ my_SSL_set_fd(PGconn *conn, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, conn);
+	BIO_set_data(bio, conn);
+	BIO_set_init(bio, 1);
 
 	SSL_set_bio(conn->ssl, bio, bio);
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	ret = 1;
 err:
 	return ret;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 82c18f870d..a6c36073be 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -547,6 +547,7 @@ struct pg_conn
 	bool		ssl_in_use;
 	bool		ssl_cert_requested; /* Did the server ask us for a cert? */
 	bool		ssl_cert_sent;	/* Did we send one in reply? */
+	bool		last_read_was_eof;
 
 #ifdef USE_SSL
 	bool		allow_ssl_try;	/* Allowed to try SSL negotiation */
-- 
2.44.0.rc0.258.g7320e95886-goog

From d949ff826aed2a7a9107be4b166fd48bcae38227 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 11 Feb 2024 10:42:25 -0500
Subject: [PATCH] Avoid mixing custom and OpenSSL BIO functions

This addresses the root cause of the BIO_set_data conflict that
c82207a548db47623a2bfa2447babdaa630302b9 attempted to address. That fix was
really just a workaround. The real root cause was that postgres was mixing up
two BIO implementations, each of which expected to be driving the BIO.

Mixing them up did not actually do any good. The Port and PGconn structures
already maintained the file descriptors. The socket BIO's copy, configured via
BIO_set_fd, wasn't even being used because my_BIO_s_socket replaced read and
write anyway. We've been essentially keeping extra state around, and relying
on it being unused:

- gets - Not implemented by sockets and not used by libssl.
- puts - Not used by libssl. If it were, it would break the special SIGPIPE and
  interrupt handling postgres aiming for.
- ctrl - (More on this later)
- create - This is just setting up state that we don't use.
- destroy - This is a no-op because we use BIO_NOCLOSE. In fact it's important
  that it's a no-op because otherwise OpenSSL would close the socket under
  postgres' feet!
- callback_ctrl - Not implemented by sockets.

That leaves ctrl. ctrl is a bunch of operations (it's ioctl). The only
operation that matters is BIO_CTRL_FLUSH, which is implemented as a no-op. All
other operations are unused. It's once again good that they're unused because
otherwise OpenSSL might mess with postgres's socket, break the assumptions
around interrupt handling, etc.

Instead, simply implement a very basic ctrl ourselves and drop the other
functions. This avoids the risk that future OpenSSL upgrades add some feature
to BIO_s_socket's ctrl which conflicts with postgres.

Once we've done that, we're free to use BIO_set_data. While BIO_set_app_data
works fine, I've reverted back to BIO_set_data because it's more commonly used.
app_data depends on OpenSSL's "ex_data" mechanism, which is a tad heavier under
the hood.

As this is no longer related to BIO_s_socket or calling SSL_set_fd, I've
renamed the methods to reference the PGconn and Port types instead.
---
 configure                                |   2 +-
 configure.ac                             |   2 +-
 meson.build                              |   1 +
 src/backend/libpq/be-secure-openssl.c    | 106 +++++++++++++++--------
 src/include/libpq/libpq-be.h             |   1 +
 src/include/pg_config.h.in               |   3 +
 src/interfaces/libpq/fe-secure-openssl.c |  94 +++++++++++++-------
 src/interfaces/libpq/libpq-int.h         |   1 +
 8 files changed, 140 insertions(+), 70 deletions(-)

diff --git a/configure b/configure
index 8e7704d54b..538975530f 100755
--- a/configure
+++ b/configure
@@ -12564,7 +12564,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_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free
+  for ac_func in OPENSSL_init_ssl BIO_get_data 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 c7322e292c..4e34539ea2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1352,7 +1352,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_meth_new ASN1_STRING_get0_data HMAC_CTX_new HMAC_CTX_free])
+  AC_CHECK_FUNCS([OPENSSL_init_ssl BIO_get_data 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/meson.build b/meson.build
index 1c0579d5a6..853e1aa9f8 100644
--- a/meson.build
+++ b/meson.build
@@ -1281,6 +1281,7 @@ 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 39b1a66236..05b995ff6c 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -56,10 +56,10 @@
 static void default_openssl_tls_init(SSL_CTX *context, bool isServerStart);
 openssl_tls_init_hook_typ openssl_tls_init_hook = default_openssl_tls_init;
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(Port *port, int fd);
+static int	port_bio_read(BIO *h, char *buf, int size);
+static int	port_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *port_bio_method(void);
+static int	ssl_set_port_bio(Port *port);
 
 static DH  *load_dh_file(char *filename, bool isServerStart);
 static DH  *load_dh_buffer(const char *buffer, size_t len);
@@ -454,7 +454,7 @@ be_tls_open_server(Port *port)
 						SSLerrmessage(ERR_get_error()))));
 		return -1;
 	}
-	if (!my_SSL_set_fd(port, port->sock))
+	if (!ssl_set_port_bio(port))
 	{
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
@@ -891,17 +891,24 @@ 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.
  */
 
-static BIO_METHOD *my_bio_methods = NULL;
+#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 *port_bio_method_ptr = NULL;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+port_bio_read(BIO *h, char *buf, int size)
 {
 	int			res = 0;
+	Port	   *port = (Port *) BIO_get_data(h);
 
 	if (buf != NULL)
 	{
-		res = secure_raw_read(((Port *) BIO_get_app_data(h)), buf, size);
+		res = secure_raw_read(port, buf, size);
 		BIO_clear_retry_flags(h);
+		port->last_read_was_eof = res == 0;
 		if (res <= 0)
 		{
 			/* If we were interrupted, tell caller to retry */
@@ -916,11 +923,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+port_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res = 0;
 
-	res = secure_raw_write(((Port *) BIO_get_app_data(h)), buf, size);
+	res = secure_raw_write(((Port *) BIO_get_data(h)), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res <= 0)
 	{
@@ -934,56 +941,81 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+port_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	Port	   *port = (Port *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. port_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = port->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+port_bio_method(void)
 {
-	if (!my_bio_methods)
+	if (!port_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 #ifdef HAVE_BIO_METH_NEW
 		int			my_bio_index;
 
 		my_bio_index = BIO_get_new_index();
 		if (my_bio_index == -1)
 			return NULL;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
-		my_bio_methods = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
-		if (!my_bio_methods)
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
+		port_bio_method_ptr = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
+		if (!port_bio_method_ptr)
 			return NULL;
-		if (!BIO_meth_set_write(my_bio_methods, my_sock_write) ||
-			!BIO_meth_set_read(my_bio_methods, my_sock_read) ||
-			!BIO_meth_set_gets(my_bio_methods, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(my_bio_methods, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(port_bio_method_ptr, port_bio_write) ||
+			!BIO_meth_set_read(port_bio_method_ptr, port_bio_read) ||
+			!BIO_meth_set_ctrl(port_bio_method_ptr, port_bio_ctrl))
 		{
-			BIO_meth_free(my_bio_methods);
-			my_bio_methods = NULL;
+			BIO_meth_free(port_bio_method_ptr);
+			port_bio_method_ptr = NULL;
 			return NULL;
 		}
 #else
-		my_bio_methods = malloc(sizeof(BIO_METHOD));
-		if (!my_bio_methods)
+		port_bio_method_ptr = malloc(sizeof(BIO_METHOD));
+		if (!port_bio_method_ptr)
 			return NULL;
-		memcpy(my_bio_methods, biom, sizeof(BIO_METHOD));
-		my_bio_methods->bread = my_sock_read;
-		my_bio_methods->bwrite = my_sock_write;
+		memset(port_bio_method_ptr, 0, sizeof(BIO_METHOD));
+		port_bio_method_ptr->type = BIO_TYPE_SOURCE_SINK;
+		port_bio_method_ptr->name = "libpq socket";
+		port_bio_method_ptr->bread = port_bio_read;
+		port_bio_method_ptr->bwrite = port_bio_write;
+		port_bio_method_ptr->ctrl = port_bio_ctrl;
 #endif
 	}
-	return my_bio_methods;
+	return port_bio_method_ptr;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(Port *port, int fd)
+ssl_set_port_bio(Port *port)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = port_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -996,9 +1028,9 @@ my_SSL_set_fd(Port *port, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, port);
+	BIO_set_data(bio, port);
+	BIO_set_init(bio, 1);
 
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	SSL_set_bio(port->ssl, bio, bio);
 	ret = 1;
 err:
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 05cb1874c5..d97d1e5f6b 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -204,6 +204,7 @@ typedef struct Port
 	char	   *peer_dn;
 	bool		peer_cert_valid;
 	bool		alpn_used;
+	bool		last_read_was_eof;
 
 	/*
 	 * OpenSSL structures. (Keep these last so that the locations of other
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index f8d3e3b6b8..01a76aef08 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -66,6 +66,9 @@
 /* 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 bf9dfbf918..883e09afe9 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -78,10 +78,10 @@ static char *SSLerrmessage(unsigned long ecode);
 static void SSLerrfree(char *buf);
 static int	PQssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(PGconn *conn, int fd);
+static int	pgconn_bio_read(BIO *h, char *buf, int size);
+static int	pgconn_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *pgconn_bio_method(void);
+static int	ssl_set_pgconn_bio(PGconn *conn);
 
 
 static bool pq_init_ssl_lib = true;
@@ -1193,7 +1193,7 @@ initialize_SSL(PGconn *conn)
 	 */
 	if (!(conn->ssl = SSL_new(SSL_context)) ||
 		!SSL_set_app_data(conn->ssl, conn) ||
-		!my_SSL_set_fd(conn, conn->sock))
+		!ssl_set_pgconn_bio(conn))
 	{
 		char	   *err = SSLerrmessage(ERR_get_error());
 
@@ -1900,17 +1900,24 @@ 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;
+static BIO_METHOD *pgconn_bio_method_ptr;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+pgconn_bio_read(BIO *h, char *buf, int size)
 {
 	PGconn	   *conn = (PGconn *) BIO_get_app_data(h);
 	int			res;
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
 
 	res = pqsecure_raw_read(conn, buf, size);
 	BIO_clear_retry_flags(h);
+	conn->last_read_was_eof = res == 0;
 	if (res < 0)
 	{
 		/* If we were interrupted, tell caller to retry */
@@ -1938,11 +1945,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+pgconn_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res;
 
-	res = pqsecure_raw_write((PGconn *) BIO_get_app_data(h), buf, size);
+	res = pqsecure_raw_write((PGconn *) BIO_get_data(h), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res < 0)
 	{
@@ -1967,26 +1974,54 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+pgconn_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. pgconn_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = conn->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+pgconn_bio_method(void)
 {
 	BIO_METHOD *res;
 
 	if (pthread_mutex_lock(&ssl_config_mutex))
 		return NULL;
 
-	res = my_bio_methods;
+	res = pgconn_bio_method_ptr;
 
-	if (!my_bio_methods)
+	if (!pgconn_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 #ifdef HAVE_BIO_METH_NEW
 		int			my_bio_index;
 
 		my_bio_index = BIO_get_new_index();
 		if (my_bio_index == -1)
 			goto err;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
 		res = BIO_meth_new(my_bio_index, "libpq socket");
 		if (!res)
 			goto err;
@@ -1995,14 +2030,9 @@ my_BIO_s_socket(void)
 		 * As of this writing, these functions never fail. But check anyway,
 		 * like OpenSSL's own examples do.
 		 */
-		if (!BIO_meth_set_write(res, my_sock_write) ||
-			!BIO_meth_set_read(res, my_sock_read) ||
-			!BIO_meth_set_gets(res, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(res, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(res, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(res, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(res, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(res, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(res, pgconn_bio_write) ||
+			!BIO_meth_set_read(res, pgconn_bio_read) ||
+			!BIO_meth_set_ctrl(res, pgconn_bio_ctrl))
 		{
 			goto err;
 		}
@@ -2010,13 +2040,16 @@ my_BIO_s_socket(void)
 		res = malloc(sizeof(BIO_METHOD));
 		if (!res)
 			goto err;
-		memcpy(res, biom, sizeof(BIO_METHOD));
-		res->bread = my_sock_read;
-		res->bwrite = my_sock_write;
+		memset(res, 0, sizeof(BIO_METHOD));
+		res->type = BIO_TYPE_SOURCE_SINK;
+		res->name = "libpq socket";
+		res->bread = pgconn_bio_read;
+		res->bwrite = pgconn_bio_write;
+		res->ctrl = pgconn_bio_ctrl;
 #endif
 	}
 
-	my_bio_methods = res;
+	pgconn_bio_method_ptr = res;
 	pthread_mutex_unlock(&ssl_config_mutex);
 	return res;
 
@@ -2032,15 +2065,14 @@ err:
 	return NULL;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(PGconn *conn, int fd)
+ssl_set_pgconn_bio(PGconn *conn)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = pgconn_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -2052,10 +2084,10 @@ my_SSL_set_fd(PGconn *conn, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, conn);
+	BIO_set_data(bio, conn);
+	BIO_set_init(bio, 1);
 
 	SSL_set_bio(conn->ssl, bio, bio);
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	ret = 1;
 err:
 	return ret;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 3886204c70..c6c19e6a19 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -567,6 +567,7 @@ struct pg_conn
 	bool		ssl_handshake_started;
 	bool		ssl_cert_requested; /* Did the server ask us for a cert? */
 	bool		ssl_cert_sent;	/* Did we send one in reply? */
+	bool		last_read_was_eof;
 
 #ifdef USE_SSL
 #ifdef USE_OPENSSL
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog

From c33884464f5d8b5f7e336be5357fc5c8114a4b34 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 3 Sep 2024 14:13:05 +0200
Subject: [PATCH v4] Avoid mixing custom and OpenSSL BIO functions

This addresses the root cause of the BIO_set_data conflict that
c82207a548db47623a2bfa2447babdaa630302b9 attempted to address. That fix was
really just a workaround. The real root cause was that postgres was mixing up
two BIO implementations, each of which expected to be driving the BIO.

Mixing them up did not actually do any good. The Port and PGconn structures
already maintained the file descriptors. The socket BIO's copy, configured via
BIO_set_fd, wasn't even being used because my_BIO_s_socket replaced read and
write anyway. We've been essentially keeping extra state around, and relying
on it being unused:

- gets - Not implemented by sockets and not used by libssl.
- puts - Not used by libssl. If it were, it would break the special SIGPIPE and
  interrupt handling postgres aiming for.
- ctrl - (More on this later)
- create - This is just setting up state that we don't use.
- destroy - This is a no-op because we use BIO_NOCLOSE. In fact it's important
  that it's a no-op because otherwise OpenSSL would close the socket under
  postgres' feet!
- callback_ctrl - Not implemented by sockets.

That leaves ctrl. ctrl is a bunch of operations (it's ioctl). The only
operation that matters is BIO_CTRL_FLUSH, which is implemented as a no-op. All
other operations are unused. It's once again good that they're unused because
otherwise OpenSSL might mess with postgres's socket, break the assumptions
around interrupt handling, etc.

Instead, simply implement a very basic ctrl ourselves and drop the other
functions. This avoids the risk that future OpenSSL upgrades add some feature
to BIO_s_socket's ctrl which conflicts with postgres.

Once we've done that, we're free to use BIO_set_data. While BIO_set_app_data
works fine, I've reverted back to BIO_set_data because it's more commonly used.
app_data depends on OpenSSL's "ex_data" mechanism, which is a tad heavier under
the hood.

As this is no longer related to BIO_s_socket or calling SSL_set_fd, I've
renamed the methods to reference the PGconn and Port types instead.
---
 src/backend/libpq/be-secure-openssl.c    | 94 +++++++++++++++---------
 src/include/libpq/libpq-be.h             |  1 +
 src/interfaces/libpq/fe-secure-openssl.c | 81 ++++++++++++--------
 src/interfaces/libpq/libpq-int.h         |  1 +
 4 files changed, 113 insertions(+), 64 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 1ebd3f2e6d..1f86b0cd98 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -57,10 +57,10 @@
 static void default_openssl_tls_init(SSL_CTX *context, bool isServerStart);
 openssl_tls_init_hook_typ openssl_tls_init_hook = default_openssl_tls_init;
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(Port *port, int fd);
+static int	port_bio_read(BIO *h, char *buf, int size);
+static int	port_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *port_bio_method(void);
+static int	ssl_set_port_bio(Port *port);
 
 static DH  *load_dh_file(char *filename, bool isServerStart);
 static DH  *load_dh_buffer(const char *buffer, size_t len);
@@ -453,7 +453,7 @@ be_tls_open_server(Port *port)
 						SSLerrmessage(ERR_get_error()))));
 		return -1;
 	}
-	if (!my_SSL_set_fd(port, port->sock))
+	if (!ssl_set_port_bio(port))
 	{
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
@@ -890,17 +890,19 @@ 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.
  */
 
-static BIO_METHOD *my_bio_methods = NULL;
+static BIO_METHOD *port_bio_method_ptr = NULL;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+port_bio_read(BIO *h, char *buf, int size)
 {
 	int			res = 0;
+	Port	   *port = (Port *) BIO_get_data(h);
 
 	if (buf != NULL)
 	{
-		res = secure_raw_read(((Port *) BIO_get_app_data(h)), buf, size);
+		res = secure_raw_read(port, buf, size);
 		BIO_clear_retry_flags(h);
+		port->last_read_was_eof = res == 0;
 		if (res <= 0)
 		{
 			/* If we were interrupted, tell caller to retry */
@@ -915,11 +917,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+port_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res = 0;
 
-	res = secure_raw_write(((Port *) BIO_get_app_data(h)), buf, size);
+	res = secure_raw_write(((Port *) BIO_get_data(h)), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res <= 0)
 	{
@@ -933,47 +935,69 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+port_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	Port	   *port = (Port *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. port_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = port->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+port_bio_method(void)
 {
-	if (!my_bio_methods)
+	if (!port_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 		int			my_bio_index;
 
-		my_bio_index = BIO_get_new_index();
-		if (my_bio_index == -1)
-			return NULL;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
-		my_bio_methods = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
-		if (!my_bio_methods)
+ 		my_bio_index = BIO_get_new_index();
+ 		if (my_bio_index == -1)
+ 			return NULL;
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
+		port_bio_method_ptr = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
+		if (!port_bio_method_ptr)
 			return NULL;
-		if (!BIO_meth_set_write(my_bio_methods, my_sock_write) ||
-			!BIO_meth_set_read(my_bio_methods, my_sock_read) ||
-			!BIO_meth_set_gets(my_bio_methods, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(my_bio_methods, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(port_bio_method_ptr, port_bio_write) ||
+			!BIO_meth_set_read(port_bio_method_ptr, port_bio_read) ||
+			!BIO_meth_set_ctrl(port_bio_method_ptr, port_bio_ctrl))
 		{
-			BIO_meth_free(my_bio_methods);
-			my_bio_methods = NULL;
+			BIO_meth_free(port_bio_method_ptr);
+			port_bio_method_ptr = NULL;
 			return NULL;
 		}
 	}
-	return my_bio_methods;
+	return port_bio_method_ptr;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(Port *port, int fd)
+ssl_set_port_bio(Port *port)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = port_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -986,9 +1010,9 @@ my_SSL_set_fd(Port *port, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, port);
+	BIO_set_data(bio, port);
+	BIO_set_init(bio, 1);
 
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	SSL_set_bio(port->ssl, bio, bio);
 	ret = 1;
 err:
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 05cb1874c5..d97d1e5f6b 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -204,6 +204,7 @@ typedef struct Port
 	char	   *peer_dn;
 	bool		peer_cert_valid;
 	bool		alpn_used;
+	bool		last_read_was_eof;
 
 	/*
 	 * OpenSSL structures. (Keep these last so that the locations of other
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index b5749d0292..420443a031 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -77,10 +77,10 @@ static char *SSLerrmessage(unsigned long ecode);
 static void SSLerrfree(char *buf);
 static int	PQssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(PGconn *conn, int fd);
+static int	pgconn_bio_read(BIO *h, char *buf, int size);
+static int	pgconn_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *pgconn_bio_method(void);
+static int	ssl_set_pgconn_bio(PGconn *conn);
 
 static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
 
@@ -989,7 +989,7 @@ initialize_SSL(PGconn *conn)
 	 */
 	if (!(conn->ssl = SSL_new(SSL_context)) ||
 		!SSL_set_app_data(conn->ssl, conn) ||
-		!my_SSL_set_fd(conn, conn->sock))
+		!ssl_set_pgconn_bio(conn))
 	{
 		char	   *err = SSLerrmessage(ERR_get_error());
 
@@ -1670,16 +1670,17 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
  */
 
 /* protected by ssl_config_mutex */
-static BIO_METHOD *my_bio_methods;
+static BIO_METHOD *pgconn_bio_method_ptr;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+pgconn_bio_read(BIO *h, char *buf, int size)
 {
-	PGconn	   *conn = (PGconn *) BIO_get_app_data(h);
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
 	int			res;
 
 	res = pqsecure_raw_read(conn, buf, size);
 	BIO_clear_retry_flags(h);
+	conn->last_read_was_eof = res == 0;
 	if (res < 0)
 	{
 		/* If we were interrupted, tell caller to retry */
@@ -1707,11 +1708,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+pgconn_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res;
 
-	res = pqsecure_raw_write((PGconn *) BIO_get_app_data(h), buf, size);
+	res = pqsecure_raw_write((PGconn *) BIO_get_data(h), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res < 0)
 	{
@@ -1736,25 +1737,53 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+pgconn_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. pgconn_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = conn->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+pgconn_bio_method(void)
 {
 	BIO_METHOD *res;
 
 	if (pthread_mutex_lock(&ssl_config_mutex))
 		return NULL;
 
-	res = my_bio_methods;
+	res = pgconn_bio_method_ptr;
 
-	if (!my_bio_methods)
+	if (!pgconn_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 		int			my_bio_index;
 
 		my_bio_index = BIO_get_new_index();
 		if (my_bio_index == -1)
 			goto err;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
 		res = BIO_meth_new(my_bio_index, "libpq socket");
 		if (!res)
 			goto err;
@@ -1763,20 +1792,15 @@ my_BIO_s_socket(void)
 		 * As of this writing, these functions never fail. But check anyway,
 		 * like OpenSSL's own examples do.
 		 */
-		if (!BIO_meth_set_write(res, my_sock_write) ||
-			!BIO_meth_set_read(res, my_sock_read) ||
-			!BIO_meth_set_gets(res, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(res, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(res, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(res, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(res, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(res, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(res, pgconn_bio_write) ||
+			!BIO_meth_set_read(res, pgconn_bio_read) ||
+			!BIO_meth_set_ctrl(res, pgconn_bio_ctrl))
 		{
 			goto err;
 		}
 	}
 
-	my_bio_methods = res;
+	pgconn_bio_method_ptr = res;
 	pthread_mutex_unlock(&ssl_config_mutex);
 	return res;
 
@@ -1787,15 +1811,14 @@ err:
 	return NULL;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(PGconn *conn, int fd)
+ssl_set_pgconn_bio(PGconn *conn)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = pgconn_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -1807,10 +1830,10 @@ my_SSL_set_fd(PGconn *conn, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, conn);
+	BIO_set_data(bio, conn);
+	BIO_set_init(bio, 1);
 
 	SSL_set_bio(conn->ssl, bio, bio);
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	ret = 1;
 err:
 	return ret;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 9579f80353..08cc391cbd 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -581,6 +581,7 @@ struct pg_conn
 	bool		ssl_handshake_started;
 	bool		ssl_cert_requested; /* Did the server ask us for a cert? */
 	bool		ssl_cert_sent;	/* Did we send one in reply? */
+	bool		last_read_was_eof;
 
 #ifdef USE_SSL
 #ifdef USE_OPENSSL
-- 
2.39.3 (Apple Git-146)

From e0f64c6f81843afcf174774a7c64874bdeb06ba5 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 3 Sep 2024 14:13:05 +0200
Subject: [PATCH v5 1/2] Avoid mixing custom and OpenSSL BIO functions

This addresses the root cause of the BIO_set_data conflict that
c82207a548db47623a2bfa2447babdaa630302b9 attempted to address. That fix was
really just a workaround. The real root cause was that postgres was mixing up
two BIO implementations, each of which expected to be driving the BIO.

Mixing them up did not actually do any good. The Port and PGconn structures
already maintained the file descriptors. The socket BIO's copy, configured via
BIO_set_fd, wasn't even being used because my_BIO_s_socket replaced read and
write anyway. We've been essentially keeping extra state around, and relying
on it being unused:

- gets - Not implemented by sockets and not used by libssl.
- puts - Not used by libssl. If it were, it would break the special SIGPIPE and
  interrupt handling postgres aiming for.
- ctrl - (More on this later)
- create - This is just setting up state that we don't use.
- destroy - This is a no-op because we use BIO_NOCLOSE. In fact it's important
  that it's a no-op because otherwise OpenSSL would close the socket under
  postgres' feet!
- callback_ctrl - Not implemented by sockets.

That leaves ctrl. ctrl is a bunch of operations (it's ioctl). The only
operation that matters is BIO_CTRL_FLUSH, which is implemented as a no-op. All
other operations are unused. It's once again good that they're unused because
otherwise OpenSSL might mess with postgres's socket, break the assumptions
around interrupt handling, etc.

Instead, simply implement a very basic ctrl ourselves and drop the other
functions. This avoids the risk that future OpenSSL upgrades add some feature
to BIO_s_socket's ctrl which conflicts with postgres.

Once we've done that, we're free to use BIO_set_data. While BIO_set_app_data
works fine, I've reverted back to BIO_set_data because it's more commonly used.
app_data depends on OpenSSL's "ex_data" mechanism, which is a tad heavier under
the hood.

As this is no longer related to BIO_s_socket or calling SSL_set_fd, I've
renamed the methods to reference the PGconn and Port types instead.
---
 src/backend/libpq/be-secure-openssl.c    | 94 +++++++++++++++---------
 src/include/libpq/libpq-be.h             |  1 +
 src/interfaces/libpq/fe-secure-openssl.c | 81 ++++++++++++--------
 src/interfaces/libpq/libpq-int.h         |  1 +
 4 files changed, 113 insertions(+), 64 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 1ebd3f2e6d..1f86b0cd98 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -57,10 +57,10 @@
 static void default_openssl_tls_init(SSL_CTX *context, bool isServerStart);
 openssl_tls_init_hook_typ openssl_tls_init_hook = default_openssl_tls_init;
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(Port *port, int fd);
+static int	port_bio_read(BIO *h, char *buf, int size);
+static int	port_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *port_bio_method(void);
+static int	ssl_set_port_bio(Port *port);
 
 static DH  *load_dh_file(char *filename, bool isServerStart);
 static DH  *load_dh_buffer(const char *buffer, size_t len);
@@ -453,7 +453,7 @@ be_tls_open_server(Port *port)
 						SSLerrmessage(ERR_get_error()))));
 		return -1;
 	}
-	if (!my_SSL_set_fd(port, port->sock))
+	if (!ssl_set_port_bio(port))
 	{
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
@@ -890,17 +890,19 @@ 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.
  */
 
-static BIO_METHOD *my_bio_methods = NULL;
+static BIO_METHOD *port_bio_method_ptr = NULL;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+port_bio_read(BIO *h, char *buf, int size)
 {
 	int			res = 0;
+	Port	   *port = (Port *) BIO_get_data(h);
 
 	if (buf != NULL)
 	{
-		res = secure_raw_read(((Port *) BIO_get_app_data(h)), buf, size);
+		res = secure_raw_read(port, buf, size);
 		BIO_clear_retry_flags(h);
+		port->last_read_was_eof = res == 0;
 		if (res <= 0)
 		{
 			/* If we were interrupted, tell caller to retry */
@@ -915,11 +917,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+port_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res = 0;
 
-	res = secure_raw_write(((Port *) BIO_get_app_data(h)), buf, size);
+	res = secure_raw_write(((Port *) BIO_get_data(h)), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res <= 0)
 	{
@@ -933,47 +935,69 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+port_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	Port	   *port = (Port *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. port_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = port->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+port_bio_method(void)
 {
-	if (!my_bio_methods)
+	if (!port_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 		int			my_bio_index;
 
-		my_bio_index = BIO_get_new_index();
-		if (my_bio_index == -1)
-			return NULL;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
-		my_bio_methods = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
-		if (!my_bio_methods)
+ 		my_bio_index = BIO_get_new_index();
+ 		if (my_bio_index == -1)
+ 			return NULL;
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
+		port_bio_method_ptr = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
+		if (!port_bio_method_ptr)
 			return NULL;
-		if (!BIO_meth_set_write(my_bio_methods, my_sock_write) ||
-			!BIO_meth_set_read(my_bio_methods, my_sock_read) ||
-			!BIO_meth_set_gets(my_bio_methods, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(my_bio_methods, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(my_bio_methods, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(my_bio_methods, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(my_bio_methods, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(my_bio_methods, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(port_bio_method_ptr, port_bio_write) ||
+			!BIO_meth_set_read(port_bio_method_ptr, port_bio_read) ||
+			!BIO_meth_set_ctrl(port_bio_method_ptr, port_bio_ctrl))
 		{
-			BIO_meth_free(my_bio_methods);
-			my_bio_methods = NULL;
+			BIO_meth_free(port_bio_method_ptr);
+			port_bio_method_ptr = NULL;
 			return NULL;
 		}
 	}
-	return my_bio_methods;
+	return port_bio_method_ptr;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(Port *port, int fd)
+ssl_set_port_bio(Port *port)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = port_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -986,9 +1010,9 @@ my_SSL_set_fd(Port *port, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, port);
+	BIO_set_data(bio, port);
+	BIO_set_init(bio, 1);
 
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	SSL_set_bio(port->ssl, bio, bio);
 	ret = 1;
 err:
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 05cb1874c5..d97d1e5f6b 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -204,6 +204,7 @@ typedef struct Port
 	char	   *peer_dn;
 	bool		peer_cert_valid;
 	bool		alpn_used;
+	bool		last_read_was_eof;
 
 	/*
 	 * OpenSSL structures. (Keep these last so that the locations of other
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index b5749d0292..420443a031 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -77,10 +77,10 @@ static char *SSLerrmessage(unsigned long ecode);
 static void SSLerrfree(char *buf);
 static int	PQssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
 
-static int	my_sock_read(BIO *h, char *buf, int size);
-static int	my_sock_write(BIO *h, const char *buf, int size);
-static BIO_METHOD *my_BIO_s_socket(void);
-static int	my_SSL_set_fd(PGconn *conn, int fd);
+static int	pgconn_bio_read(BIO *h, char *buf, int size);
+static int	pgconn_bio_write(BIO *h, const char *buf, int size);
+static BIO_METHOD *pgconn_bio_method(void);
+static int	ssl_set_pgconn_bio(PGconn *conn);
 
 static pthread_mutex_t ssl_config_mutex = PTHREAD_MUTEX_INITIALIZER;
 
@@ -989,7 +989,7 @@ initialize_SSL(PGconn *conn)
 	 */
 	if (!(conn->ssl = SSL_new(SSL_context)) ||
 		!SSL_set_app_data(conn->ssl, conn) ||
-		!my_SSL_set_fd(conn, conn->sock))
+		!ssl_set_pgconn_bio(conn))
 	{
 		char	   *err = SSLerrmessage(ERR_get_error());
 
@@ -1670,16 +1670,17 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
  */
 
 /* protected by ssl_config_mutex */
-static BIO_METHOD *my_bio_methods;
+static BIO_METHOD *pgconn_bio_method_ptr;
 
 static int
-my_sock_read(BIO *h, char *buf, int size)
+pgconn_bio_read(BIO *h, char *buf, int size)
 {
-	PGconn	   *conn = (PGconn *) BIO_get_app_data(h);
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
 	int			res;
 
 	res = pqsecure_raw_read(conn, buf, size);
 	BIO_clear_retry_flags(h);
+	conn->last_read_was_eof = res == 0;
 	if (res < 0)
 	{
 		/* If we were interrupted, tell caller to retry */
@@ -1707,11 +1708,11 @@ my_sock_read(BIO *h, char *buf, int size)
 }
 
 static int
-my_sock_write(BIO *h, const char *buf, int size)
+pgconn_bio_write(BIO *h, const char *buf, int size)
 {
 	int			res;
 
-	res = pqsecure_raw_write((PGconn *) BIO_get_app_data(h), buf, size);
+	res = pqsecure_raw_write((PGconn *) BIO_get_data(h), buf, size);
 	BIO_clear_retry_flags(h);
 	if (res < 0)
 	{
@@ -1736,25 +1737,53 @@ my_sock_write(BIO *h, const char *buf, int size)
 	return res;
 }
 
+static long
+pgconn_bio_ctrl(BIO *h, int cmd, long num, void *ptr)
+{
+	long		res;
+	PGconn	   *conn = (PGconn *) BIO_get_data(h);
+
+	switch (cmd)
+	{
+		case BIO_CTRL_EOF:
+			/*
+			 * This should not be needed. pgconn_bio_read already has a way to
+			 * signal EOF to OpenSSL. However, OpenSSL made an undocumented,
+			 * backwards-incompatible change and now expects EOF via BIO_ctrl.
+			 * See https://github.com/openssl/openssl/issues/8208
+			 */
+			res = conn->last_read_was_eof;
+			break;
+		case BIO_CTRL_FLUSH:
+			/* libssl expects all BIOs to support BIO_flush. */
+			res = 1;
+			break;
+		default:
+			res = 0;
+			break;
+	}
+
+	return res;
+}
+
 static BIO_METHOD *
-my_BIO_s_socket(void)
+pgconn_bio_method(void)
 {
 	BIO_METHOD *res;
 
 	if (pthread_mutex_lock(&ssl_config_mutex))
 		return NULL;
 
-	res = my_bio_methods;
+	res = pgconn_bio_method_ptr;
 
-	if (!my_bio_methods)
+	if (!pgconn_bio_method_ptr)
 	{
-		BIO_METHOD *biom = (BIO_METHOD *) BIO_s_socket();
 		int			my_bio_index;
 
 		my_bio_index = BIO_get_new_index();
 		if (my_bio_index == -1)
 			goto err;
-		my_bio_index |= (BIO_TYPE_DESCRIPTOR | BIO_TYPE_SOURCE_SINK);
+		my_bio_index |= BIO_TYPE_SOURCE_SINK;
 		res = BIO_meth_new(my_bio_index, "libpq socket");
 		if (!res)
 			goto err;
@@ -1763,20 +1792,15 @@ my_BIO_s_socket(void)
 		 * As of this writing, these functions never fail. But check anyway,
 		 * like OpenSSL's own examples do.
 		 */
-		if (!BIO_meth_set_write(res, my_sock_write) ||
-			!BIO_meth_set_read(res, my_sock_read) ||
-			!BIO_meth_set_gets(res, BIO_meth_get_gets(biom)) ||
-			!BIO_meth_set_puts(res, BIO_meth_get_puts(biom)) ||
-			!BIO_meth_set_ctrl(res, BIO_meth_get_ctrl(biom)) ||
-			!BIO_meth_set_create(res, BIO_meth_get_create(biom)) ||
-			!BIO_meth_set_destroy(res, BIO_meth_get_destroy(biom)) ||
-			!BIO_meth_set_callback_ctrl(res, BIO_meth_get_callback_ctrl(biom)))
+		if (!BIO_meth_set_write(res, pgconn_bio_write) ||
+			!BIO_meth_set_read(res, pgconn_bio_read) ||
+			!BIO_meth_set_ctrl(res, pgconn_bio_ctrl))
 		{
 			goto err;
 		}
 	}
 
-	my_bio_methods = res;
+	pgconn_bio_method_ptr = res;
 	pthread_mutex_unlock(&ssl_config_mutex);
 	return res;
 
@@ -1787,15 +1811,14 @@ err:
 	return NULL;
 }
 
-/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
 static int
-my_SSL_set_fd(PGconn *conn, int fd)
+ssl_set_pgconn_bio(PGconn *conn)
 {
 	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
-	bio_method = my_BIO_s_socket();
+	bio_method = pgconn_bio_method();
 	if (bio_method == NULL)
 	{
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
@@ -1807,10 +1830,10 @@ my_SSL_set_fd(PGconn *conn, int fd)
 		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
 		goto err;
 	}
-	BIO_set_app_data(bio, conn);
+	BIO_set_data(bio, conn);
+	BIO_set_init(bio, 1);
 
 	SSL_set_bio(conn->ssl, bio, bio);
-	BIO_set_fd(bio, fd, BIO_NOCLOSE);
 	ret = 1;
 err:
 	return ret;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 9579f80353..08cc391cbd 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -581,6 +581,7 @@ struct pg_conn
 	bool		ssl_handshake_started;
 	bool		ssl_cert_requested; /* Did the server ask us for a cert? */
 	bool		ssl_cert_sent;	/* Did we send one in reply? */
+	bool		last_read_was_eof;
 
 #ifdef USE_SSL
 #ifdef USE_OPENSSL
-- 
2.39.3 (Apple Git-146)

From d87de7ec139023b6e7f9c808e70913e919047b18 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Wed, 4 Sep 2024 15:20:00 +0200
Subject: [PATCH v5 2/2] Review comments

---
 src/backend/libpq/be-secure-openssl.c    | 18 +++++-------------
 src/interfaces/libpq/fe-secure-openssl.c | 17 +++++------------
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 1f86b0cd98..b248ab384c 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -993,30 +993,22 @@ port_bio_method(void)
 static int
 ssl_set_port_bio(Port *port)
 {
-	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
 	bio_method = port_bio_method();
 	if (bio_method == NULL)
-	{
-		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
-		goto err;
-	}
-	bio = BIO_new(bio_method);
+		return 0;
 
+	bio = BIO_new(bio_method);
 	if (bio == NULL)
-	{
-		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
-		goto err;
-	}
+		return 0;
+
 	BIO_set_data(bio, port);
 	BIO_set_init(bio, 1);
 
 	SSL_set_bio(port->ssl, bio, bio);
-	ret = 1;
-err:
-	return ret;
+	return 1;
 }
 
 /*
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 420443a031..6739e009bd 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1814,29 +1814,22 @@ err:
 static int
 ssl_set_pgconn_bio(PGconn *conn)
 {
-	int			ret = 0;
 	BIO		   *bio;
 	BIO_METHOD *bio_method;
 
 	bio_method = pgconn_bio_method();
 	if (bio_method == NULL)
-	{
-		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
-		goto err;
-	}
+		return 0;
+
 	bio = BIO_new(bio_method);
 	if (bio == NULL)
-	{
-		SSLerr(SSL_F_SSL_SET_FD, ERR_R_BUF_LIB);
-		goto err;
-	}
+		return 0;
+
 	BIO_set_data(bio, conn);
 	BIO_set_init(bio, 1);
 
 	SSL_set_bio(conn->ssl, bio, bio);
-	ret = 1;
-err:
-	return ret;
+	return 1;
 }
 
 /*
-- 
2.39.3 (Apple Git-146)