Direct SSL connection and ALPN loose ends

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 60cf68aac4..be41d75de0 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -67,6 +67,7 @@ static int	ssl_external_passwd_cb(char *buf, int size, int rwflag, void *userdat
 static int	dummy_ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
 static int	verify_cb(int ok, X509_STORE_CTX *ctx);
 static void info_cb(const SSL *ssl, int type, int args);
+static int client_hello_cb(SSL *ssl, int *al,	void *arg);
 static int	alpn_cb(SSL *ssl,
 					const unsigned char **out,
 					unsigned char *outlen,
@@ -443,6 +444,9 @@ be_tls_open_server(Port *port)
 	/* set up debugging/info callback */
 	SSL_CTX_set_info_callback(SSL_context, info_cb);
 
+	/* enable ClientHello */
+	SSL_CTX_set_client_hello_cb(SSL_context, client_hello_cb, NULL);
+
 	/* enable ALPN */
 	SSL_CTX_set_alpn_select_cb(SSL_context, alpn_cb, port);
 
@@ -1304,6 +1308,30 @@ info_cb(const SSL *ssl, int type, int args)
 /* See pqcomm.h comments on OpenSSL implementation of ALPN (RFC 7301) */
 static const unsigned char alpn_protos[] = PG_ALPN_PROTOCOL_VECTOR;
 
+/*
+ * Server callback for ClientHello validation.
+ */
+static int
+client_hello_cb(SSL *ssl,
+		int *al,
+		void *arg)
+{
+	const unsigned char * data;
+    size_t len;
+
+    if (SSL_client_hello_get0_ext(ssl, TLSEXT_TYPE_application_layer_protocol_negotiation, &data, &len) == 1)
+        return SSL_CLIENT_HELLO_SUCCESS;
+    else
+    {
+ 		/*
+		 * The client doesn't support ALPN negotiation protocol.  Reject the connection
+		 * with TLS "no_application_protocol" alert, per RFC 7301.
+		 */
+       *al = TLS1_AD_NO_APPLICATION_PROTOCOL;
+        return SSL_CLIENT_HELLO_ERROR;
+    }
+}
+
 /*
  * Server callback for ALPN negotiation. We use the standard "helper" function
  * even though currently we only accept one value.

From 4b988b1c74066fe8b5f98acb4ecd20150a47bc33 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Mon, 24 Jun 2024 20:41:41 +0300
Subject: [PATCH 1/4] Fix outdated comment after removal of direct SSL fallback

The option to fall back from direct SSL to negotiated SSL or a
plaintext connection was removed in commit fb5718f35f.
---
 src/interfaces/libpq/fe-connect.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 071b1b34aa1..e003279fb6c 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3564,8 +3564,8 @@ keep_going:						/* We will come back to here until there is
 				if (pollres == PGRES_POLLING_FAILED)
 				{
 					/*
-					 * Failed direct ssl connection, possibly try a new
-					 * connection with postgres negotiation
+					 * SSL handshake failed.  We will retry with a plaintext
+					 * connection, if permitted by sslmode.
 					 */
 					CONNECTION_FAILED();
 				}
-- 
2.39.2

From 7df581e902b76665de4c751ddbc1309143e6c0b8 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Mon, 24 Jun 2024 18:14:52 +0300
Subject: [PATCH 2/4] Fix fallback behavior when server sends an ERROR early at
 startup

With sslmode=prefer, the desired behavior is to completely fail the
connection attempt, *not* fall back to a plaintext connection, if the
server responds to the SSLRequest with an error ('E') response instead
of rejecting SSL with an 'N' response. This was broken in commit
05fd30c0e7.

Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
---
 src/interfaces/libpq/fe-connect.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e003279fb6c..360d9a45476 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3521,6 +3521,12 @@ keep_going:						/* We will come back to here until there is
 						 * byte here.
 						 */
 						conn->status = CONNECTION_AWAITING_RESPONSE;
+
+						/*
+						 * Don't fall back to a plaintext connection after
+						 * reading the error.
+						 */
+						conn->failed_enc_methods |= conn->allowed_enc_methods & (~conn->current_enc_method);
 						goto keep_going;
 					}
 					else
@@ -3612,6 +3618,13 @@ keep_going:						/* We will come back to here until there is
 						 * into AWAITING_RESPONSE state and let the code there
 						 * deal with it.  Note we have *not* consumed the "E"
 						 * byte here.
+						 *
+						 * Note that unlike on an error response to
+						 * SSLRequest, we allow falling back to SSL or
+						 * plaintext connection here.  GSS support was
+						 * introduced in PostgreSQL version 12, so an error
+						 * response might mean that we are connecting to a
+						 * pre-v12 server.
 						 */
 						conn->status = CONNECTION_AWAITING_RESPONSE;
 						goto keep_going;
@@ -3659,6 +3672,10 @@ keep_going:						/* We will come back to here until there is
 				}
 				else if (pollres == PGRES_POLLING_FAILED)
 				{
+					/*
+					 * GSS handshake failed.  We will retry with an SSL or
+					 * plaintext connection, if permitted by the options.
+					 */
 					CONNECTION_FAILED();
 				}
 				/* Else, return POLLING_READING or POLLING_WRITING status */
-- 
2.39.2

From e893d2e53a9bde6ddf4462d9f8b206c51ebd40cb Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Mon, 24 Jun 2024 18:15:53 +0300
Subject: [PATCH 3/4] WIP: Add injection points and test for early backend
 startup errors

---
 src/backend/tcop/backend_startup.c            |  9 ++++
 src/backend/utils/misc/injection_point.c      | 33 +++++++++++-
 src/include/utils/injection_point.h           |  1 +
 src/interfaces/libpq/meson.build              |  1 +
 .../libpq/t/005_negotiate_encryption.pl       | 50 +++++++++++++++++++
 5 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index cfa27551964..7b4a3e23458 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -33,6 +33,7 @@
 #include "tcop/backend_startup.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/timeout.h"
@@ -160,6 +161,14 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
 
 	whereToSendOutput = DestRemote; /* now safe to ereport to client */
 
+	/* For testing client error handling */
+	INJECTION_POINT("backend-initialize");
+	if (IsInjectionPointAttached("backend-initialize-v2-error"))
+	{
+		FrontendProtocol = PG_PROTOCOL(2,0);
+		elog(FATAL, "protocol version 2 error triggered");
+	}
+
 	/* set these to empty in case they are needed before we set them up */
 	port->remote_host = "";
 	port->remote_port = "";
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 5c2a0d2297e..d3995dfd6fc 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -24,6 +24,7 @@
 #include "port/pg_bitutils.h"
 #include "storage/fd.h"
 #include "storage/lwlock.h"
+#include "storage/proc.h"
 #include "storage/shmem.h"
 #include "utils/hsearch.h"
 #include "utils/injection_point.h"
@@ -281,6 +282,31 @@ InjectionPointDetach(const char *name)
 #endif
 }
 
+/*
+ * Test if an injection point is defined.
+ */
+bool
+IsInjectionPointAttached(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	bool		found;
+
+	/* FIXME: just skip locking if we don't have a PGPROC entry yet. Obviously unsafe.. */
+	if (MyProc != NULL && IsUnderPostmaster)
+		LWLockAcquire(InjectionPointLock, LW_SHARED);
+
+	(void) hash_search(InjectionPointHash, name, HASH_FIND, &found);
+
+	if (MyProc != NULL && IsUnderPostmaster)
+		LWLockRelease(InjectionPointLock);
+
+	return found;
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+	return false;				/* silence compiler */
+#endif
+}
+
 /*
  * Execute an injection point, if defined.
  *
@@ -296,11 +322,14 @@ InjectionPointRun(const char *name)
 	InjectionPointCallback injection_callback;
 	const void *private_data;
 
-	LWLockAcquire(InjectionPointLock, LW_SHARED);
+	/* FIXME: just skip locking if we don't have a PGPROC entry yet. Obviously unsafe.. */
+	if (MyProc != NULL && IsUnderPostmaster)
+		LWLockAcquire(InjectionPointLock, LW_SHARED);
 	entry_by_name = (InjectionPointEntry *)
 		hash_search(InjectionPointHash, name,
 					HASH_FIND, &found);
-	LWLockRelease(InjectionPointLock);
+	if (MyProc != NULL && IsUnderPostmaster)
+		LWLockRelease(InjectionPointLock);
 
 	/*
 	 * If not found, do nothing and remove it from the local cache if it
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index a61d5d44391..5715eefb31e 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -35,6 +35,7 @@ extern void InjectionPointAttach(const char *name,
 								 const void *private_data,
 								 int private_data_size);
 extern void InjectionPointRun(const char *name);
+extern bool IsInjectionPointAttached(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
 #endif							/* INJECTION_POINT_H */
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index ed2a4048d18..7623aeadab7 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -121,6 +121,7 @@ tests += {
       't/005_negotiate_encryption.pl',
     ],
     'env': {
+      'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
       'with_ssl': ssl_library,
       'with_gssapi': gssapi.found() ? 'yes' : 'no',
       'with_krb_srvnam': 'postgres',
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index c3f70d31bc8..e6bea0f04f8 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -90,6 +90,8 @@ my $kerberos_enabled =
   $ENV{PG_TEST_EXTRA} && $ENV{PG_TEST_EXTRA} =~ /\bkerberos\b/;
 my $ssl_supported = $ENV{with_ssl} eq 'openssl';
 
+my $injection_points_supported = $ENV{enable_injection_points} eq 'yes';
+
 ###
 ### Prepare test server for GSSAPI and SSL authentication, with a few
 ### different test users and helper functions. We don't actually
@@ -155,6 +157,10 @@ $node->safe_psql('postgres', 'CREATE USER ssluser;');
 $node->safe_psql('postgres', 'CREATE USER nossluser;');
 $node->safe_psql('postgres', 'CREATE USER gssuser;');
 $node->safe_psql('postgres', 'CREATE USER nogssuser;');
+if ($injection_points_supported != 0)
+{
+	$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;')
+}
 
 my $unixdir = $node->safe_psql('postgres', 'SHOW unix_socket_directories;');
 chomp($unixdir);
@@ -320,6 +326,27 @@ nossluser   .            disable      postgres       connect, authok
 		\@all_sslmodes, \@all_sslnegotiations,
 		parse_table($test_table));
 
+	if ($injection_points_supported != 0)
+	{
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'backenderror -> fail');
+		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize-v2-error', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'v2error -> fail');
+		$node->restart;
+	}
+
 	# Disable SSL again
 	$node->adjust_conf('postgresql.conf', 'ssl', 'off');
 	$node->reload;
@@ -403,6 +430,27 @@ nogssuser   disable      disable      postgres       connect, authok
 	test_matrix($node, $server_config, [ 'testuser', 'gssuser', 'nogssuser' ],
 		\@all_gssencmodes, $sslmodes, $sslnegotiations,
 		parse_table($test_table));
+
+	if ($injection_points_supported != 0)
+	{
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'backenderror, backenderror -> fail');
+		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize-v2-error', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'v2error -> fail');
+		$node->restart;
+	}
 }
 
 ###
@@ -750,6 +798,8 @@ sub parse_log_events
 		push @events, "gssreject" if $line =~ /GSSENCRequest rejected/;
 		push @events, "authfail" if $line =~ /no pg_hba.conf entry/;
 		push @events, "authok" if $line =~ /connection authenticated/;
+		push @events, "backenderror" if $line =~ /error triggered for injection point backend-/;
+		push @events, "v2error" if $line =~ /protocol version 2 error triggered/;
 	}
 
 	# No events at all is represented by "-"
-- 
2.39.2

From 572b3e8ba2ab33fa695cb557ab6d0b6ae0e0ae63 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Mon, 24 Jun 2024 23:13:28 +0300
Subject: [PATCH 4/4] WIP: Add tests for errors during ssl or gssapi handshake

---
 src/backend/libpq/be-secure-gssapi.c           |  3 +++
 src/backend/libpq/be-secure.c                  |  3 +++
 .../libpq/t/005_negotiate_encryption.pl        | 18 ++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index bc04e78abba..483636503c1 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -21,6 +21,7 @@
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
 #include "pgstat.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -499,6 +500,8 @@ secure_open_gssapi(Port *port)
 				minor;
 	gss_cred_id_t delegated_creds;
 
+	INJECTION_POINT("backend-gssapi-startup");
+
 	/*
 	 * Allocate subsidiary Port data for GSSAPI operations.
 	 */
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 1663f36b6b8..ef20ea755b7 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -30,6 +30,7 @@
 #include "libpq/libpq.h"
 #include "miscadmin.h"
 #include "tcop/tcopprot.h"
+#include "utils/injection_point.h"
 #include "utils/wait_event.h"
 
 char	   *ssl_library;
@@ -129,6 +130,8 @@ secure_open_server(Port *port)
 	}
 	Assert(pq_buffer_remaining_data() == 0);
 
+	INJECTION_POINT("backend-ssl-startup");
+
 	r = be_tls_open_server(port);
 
 	if (port->raw_buf_remaining > 0)
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index e6bea0f04f8..991badf28ec 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -345,6 +345,15 @@ nossluser   .            disable      postgres       connect, authok
 			"user=testuser sslmode=prefer",
 			'v2error -> fail');
 		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-ssl-startup', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'connect, sslaccept, backenderror, reconnect, authok -> plain');
+		$node->restart;
 	}
 
 	# Disable SSL again
@@ -450,6 +459,15 @@ nogssuser   disable      disable      postgres       connect, authok
 			"user=testuser gssencmode=prefer sslmode=disable",
 			'v2error -> fail');
 		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-gssapi-startup', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'connect, gssaccept, backenderror, reconnect, authok -> plain');
+		$node->restart;
 	}
 }
 
-- 
2.39.2

From 651db616fcb19b89c94cacb30c27267ac7c17070 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 23 Jul 2024 20:03:27 +0300
Subject: [PATCH v2 1/3] Fix fallback behavior when server sends an ERROR early
 at startup

With sslmode=prefer, the desired behavior is to completely fail the
connection attempt, *not* fall back to a plaintext connection, if the
server responds to the SSLRequest with an error ('E') response instead
of rejecting SSL with an 'N' response. This was broken in commit
05fd30c0e7.

Reported-by: Jacob Champion
Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
---
 src/interfaces/libpq/fe-connect.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e003279fb6..360d9a4547 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3521,6 +3521,12 @@ keep_going:						/* We will come back to here until there is
 						 * byte here.
 						 */
 						conn->status = CONNECTION_AWAITING_RESPONSE;
+
+						/*
+						 * Don't fall back to a plaintext connection after
+						 * reading the error.
+						 */
+						conn->failed_enc_methods |= conn->allowed_enc_methods & (~conn->current_enc_method);
 						goto keep_going;
 					}
 					else
@@ -3612,6 +3618,13 @@ keep_going:						/* We will come back to here until there is
 						 * into AWAITING_RESPONSE state and let the code there
 						 * deal with it.  Note we have *not* consumed the "E"
 						 * byte here.
+						 *
+						 * Note that unlike on an error response to
+						 * SSLRequest, we allow falling back to SSL or
+						 * plaintext connection here.  GSS support was
+						 * introduced in PostgreSQL version 12, so an error
+						 * response might mean that we are connecting to a
+						 * pre-v12 server.
 						 */
 						conn->status = CONNECTION_AWAITING_RESPONSE;
 						goto keep_going;
@@ -3659,6 +3672,10 @@ keep_going:						/* We will come back to here until there is
 				}
 				else if (pollres == PGRES_POLLING_FAILED)
 				{
+					/*
+					 * GSS handshake failed.  We will retry with an SSL or
+					 * plaintext connection, if permitted by the options.
+					 */
 					CONNECTION_FAILED();
 				}
 				/* Else, return POLLING_READING or POLLING_WRITING status */
-- 
2.39.2

From 649e2c89f62484940c55b02635e393bdd85654af Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 23 Jul 2024 20:20:37 +0300
Subject: [PATCH v2 2/3] Add test for early backend startup errors

The new test tests the libpq fallback behavior on an early error.

This adds an IS_INJECTION_POINT_ATTACHED() macro, to allow writing the
test code alongside the normal source code. In principle, the new test
could've been written by an extra test module with a callback that
sets the FrontendProtocol global variable, but I think it's more
straightforward to have the test code

Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
---
 doc/src/sgml/xfunc.sgml                       | 25 ++++++++++
 src/backend/tcop/backend_startup.c            | 16 ++++++
 src/backend/utils/misc/injection_point.c      | 15 ++++++
 src/include/utils/injection_point.h           |  3 ++
 src/interfaces/libpq/Makefile                 |  4 +-
 src/interfaces/libpq/meson.build              |  1 +
 .../libpq/t/005_negotiate_encryption.pl       | 50 +++++++++++++++++++
 7 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 7e92e89846..5b584a4f14 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3672,6 +3672,31 @@ custom_injection_callback(const char *name, const void *private_data)
      logic.
     </para>
 
+    <para>
+     An alternative way to define the action to take when an injection point
+     is reached is to add the testing code alongside the normal source
+     code. This can be useful if the action e.g. depends on local variables
+     that are not accessible to loaded modules. The
+     <function>IS_INJECTION_POINT_ATTACHED</function> macro can then be used
+     to check if an injection point is attached, for example:
+<programlisting>
+#ifdef USE_INJECTION_POINTS
+if (IS_INJECTION_POINT_ATTACHED("before-foobar"))
+{
+    /* change a local variable if injection point is attached */
+    local_var = 123;
+
+    /* also execute the callback */
+    INJECTION_POINT_CACHED("before-foobar");
+}
+#endif
+</programlisting>
+     Note that the callback attached to the injection point will not be
+     executed by the <function>IS_INJECTION_POINT_ATTACHED</function>
+     macro. If you want to execute the callback, you must also call
+     <function>INJECTION_POINT_CACHED</function> like in the above example.
+    </para>
+
     <para>
      Optionally, it is possible to detach an injection point by calling:
 <programlisting>
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index a2f94b1050..b840d95e4d 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -33,6 +33,7 @@
 #include "tcop/backend_startup.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/timeout.h"
@@ -213,6 +214,21 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
 							remote_host)));
 	}
 
+	/* For testing client error handling */
+#ifdef USE_INJECTION_POINTS
+	INJECTION_POINT("backend-initialize");
+	if (IS_INJECTION_POINT_ATTACHED("backend-initialize-v2-error"))
+	{
+		/*
+		 * This simulates an early error from a pre-v14 server, which used the
+		 * version 2 protocol for any errors that occurred before processing
+		 * the startup packet.
+		 */
+		FrontendProtocol = PG_PROTOCOL(2, 0);
+		elog(FATAL, "protocol version 2 error triggered");
+	}
+#endif
+
 	/*
 	 * If we did a reverse lookup to name, we might as well save the results
 	 * rather than possibly repeating the lookup during authentication.
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 8ad0c27bc8..ec23f1f4d5 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -23,6 +23,7 @@
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include "storage/lwlock.h"
+#include "storage/proc.h"
 #include "storage/shmem.h"
 #include "utils/hsearch.h"
 #include "utils/injection_point.h"
@@ -570,3 +571,17 @@ InjectionPointCached(const char *name)
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
 }
+
+/*
+ * Test if an injection point is defined.
+ */
+bool
+IsInjectionPointAttached(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	return InjectionPointCacheRefresh(name) != NULL;
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+	return false;				/* silence compiler */
+#endif
+}
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index a385e3df64..6be20b4857 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -18,10 +18,12 @@
 #define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
 #define INJECTION_POINT(name) InjectionPointRun(name)
 #define INJECTION_POINT_CACHED(name) InjectionPointCached(name)
+#define IS_INJECTION_POINT_ATTACHED(name) IsInjectionPointAttached(name)
 #else
 #define INJECTION_POINT_LOAD(name) ((void) name)
 #define INJECTION_POINT(name) ((void) name)
 #define INJECTION_POINT_CACHED(name) ((void) name)
+#define IS_INJECTION_POINT_ATTACHED(name) (false)
 #endif
 
 /*
@@ -41,6 +43,7 @@ extern void InjectionPointAttach(const char *name,
 extern void InjectionPointLoad(const char *name);
 extern void InjectionPointRun(const char *name);
 extern void InjectionPointCached(const char *name);
+extern bool IsInjectionPointAttached(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
 #endif							/* INJECTION_POINT_H */
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index b36a765764..27f8499d8a 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -9,11 +9,13 @@
 #
 #-------------------------------------------------------------------------
 
+EXTRA_INSTALL=src/test/modules/injection_points
+
 subdir = src/interfaces/libpq
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-export with_ssl with_gssapi with_krb_srvnam
+export with_ssl with_gssapi with_krb_srvnam enable_injection_points
 
 PGFILEDESC = "PostgreSQL Access Library"
 
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index ed2a4048d1..7623aeadab 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -121,6 +121,7 @@ tests += {
       't/005_negotiate_encryption.pl',
     ],
     'env': {
+      'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
       'with_ssl': ssl_library,
       'with_gssapi': gssapi.found() ? 'yes' : 'no',
       'with_krb_srvnam': 'postgres',
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index 251c405926..e21c883ab4 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -90,6 +90,8 @@ my $kerberos_enabled =
   $ENV{PG_TEST_EXTRA} && $ENV{PG_TEST_EXTRA} =~ /\bkerberos\b/;
 my $ssl_supported = $ENV{with_ssl} eq 'openssl';
 
+my $injection_points_supported = $ENV{enable_injection_points} eq 'yes';
+
 ###
 ### Prepare test server for GSSAPI and SSL authentication, with a few
 ### different test users and helper functions. We don't actually
@@ -155,6 +157,10 @@ $node->safe_psql('postgres', 'CREATE USER ssluser;');
 $node->safe_psql('postgres', 'CREATE USER nossluser;');
 $node->safe_psql('postgres', 'CREATE USER gssuser;');
 $node->safe_psql('postgres', 'CREATE USER nogssuser;');
+if ($injection_points_supported != 0)
+{
+	$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;')
+}
 
 my $unixdir = $node->safe_psql('postgres', 'SHOW unix_socket_directories;');
 chomp($unixdir);
@@ -312,6 +318,27 @@ nossluser   .            disable      postgres       connect, authok
 		['disable'], \@all_sslmodes, \@all_sslnegotiations,
 		parse_table($test_table));
 
+	if ($injection_points_supported != 0)
+	{
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'connect, backenderror -> fail');
+		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize-v2-error', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'connect, v2error -> fail');
+		$node->restart;
+	}
+
 	# Disable SSL again
 	$node->adjust_conf('postgresql.conf', 'ssl', 'off');
 	$node->reload;
@@ -393,6 +420,27 @@ nogssuser   disable      disable      postgres       connect, authok
 	test_matrix($node, [ 'testuser', 'gssuser', 'nogssuser' ],
 		\@all_gssencmodes, $sslmodes, $sslnegotiations,
 		parse_table($test_table));
+
+	if ($injection_points_supported != 0)
+	{
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'backenderror, backenderror -> fail');
+		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize-v2-error', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'v2error -> fail');
+		$node->restart;
+	}
 }
 
 ###
@@ -738,6 +786,8 @@ sub parse_log_events
 		push @events, "gssreject" if $line =~ /GSSENCRequest rejected/;
 		push @events, "authfail" if $line =~ /no pg_hba.conf entry/;
 		push @events, "authok" if $line =~ /connection authenticated/;
+		push @events, "backenderror" if $line =~ /error triggered for injection point backend-/;
+		push @events, "v2error" if $line =~ /protocol version 2 error triggered/;
 	}
 
 	# No events at all is represented by "-"
-- 
2.39.2

From 87d8fbd5e65a7298eeb3335280e15c6da2d2a4bc Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 23 Jul 2024 20:21:54 +0300
Subject: [PATCH v2 3/3] Add tests for errors during SSL or GSSAPI handshake

These test that libpq correctly falls back to a plaintext connection
on handshake error, in the "prefer" modes.
---
 src/backend/libpq/be-secure-gssapi.c           |  3 +++
 src/backend/libpq/be-secure.c                  |  3 +++
 .../libpq/t/005_negotiate_encryption.pl        | 18 ++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index bc04e78abb..483636503c 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -21,6 +21,7 @@
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
 #include "pgstat.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 
@@ -499,6 +500,8 @@ secure_open_gssapi(Port *port)
 				minor;
 	gss_cred_id_t delegated_creds;
 
+	INJECTION_POINT("backend-gssapi-startup");
+
 	/*
 	 * Allocate subsidiary Port data for GSSAPI operations.
 	 */
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 1663f36b6b..ef20ea755b 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -30,6 +30,7 @@
 #include "libpq/libpq.h"
 #include "miscadmin.h"
 #include "tcop/tcopprot.h"
+#include "utils/injection_point.h"
 #include "utils/wait_event.h"
 
 char	   *ssl_library;
@@ -129,6 +130,8 @@ secure_open_server(Port *port)
 	}
 	Assert(pq_buffer_remaining_data() == 0);
 
+	INJECTION_POINT("backend-ssl-startup");
+
 	r = be_tls_open_server(port);
 
 	if (port->raw_buf_remaining > 0)
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index e21c883ab4..eadec9145f 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -337,6 +337,15 @@ nossluser   .            disable      postgres       connect, authok
 			"user=testuser sslmode=prefer",
 			'connect, v2error -> fail');
 		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-ssl-startup', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'connect, sslaccept, backenderror, reconnect, authok -> plain');
+		$node->restart;
 	}
 
 	# Disable SSL again
@@ -440,6 +449,15 @@ nogssuser   disable      disable      postgres       connect, authok
 			"user=testuser gssencmode=prefer sslmode=disable",
 			'v2error -> fail');
 		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-gssapi-startup', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'connect, gssaccept, backenderror, reconnect, authok -> plain');
+		$node->restart;
 	}
 }
 
-- 
2.39.2