v7-0010-Move-code-to-check-for-alpn.patch

From 3b1955bae494593dd9e7479ee64d854c4b5dfda7 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 10 Jan 2024 09:05:29 +0200
Subject: [PATCH v7 10/12] Move code to check for alpn

- if non-direct SSL is used, and client sends an unexpected ALPN
  protocol, that's now an error ?
---
 src/backend/libpq/be-secure-openssl.c | 49 +++++++++++++++++----------
 src/backend/postmaster/postmaster.c   | 18 ++--------
 src/include/libpq/libpq-be.h          |  2 +-
 3 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index d3c8b4117fb..efe59f672d2 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -584,6 +584,32 @@ aloop:
 		return -1;
 	}
 
+	/* Get protocol selected by ALPN */
+	port->alpn_used = false;
+	{
+		const unsigned char	*selected;
+		unsigned int len;
+
+		SSL_get0_alpn_selected(port->ssl, &selected, &len);
+
+		/* If ALPN is used, check that we negotiated the expected protocol */
+		if (selected != NULL)
+		{
+			if (len == strlen(PG_ALPN_PROTOCOL) &&
+				memcmp(selected, PG_ALPN_PROTOCOL, strlen(PG_ALPN_PROTOCOL)) == 0)
+			{
+				port->alpn_used = true;
+			}
+			else
+			{
+				/* shouldn't happen */
+				ereport(COMMERROR,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 errmsg("Received SSL connection request with unexpected ALPN protocol")));
+			}
+		}
+	}
+
 	/* Get client certificate, if available. */
 	port->peer = SSL_get_peer_certificate(port->ssl);
 
@@ -715,12 +741,6 @@ be_tls_close(Port *port)
 		pfree(port->peer_dn);
 		port->peer_dn = NULL;
 	}
-
-	if (port->ssl_alpn_protocol)
-	{
-		pfree(port->ssl_alpn_protocol);
-		port->ssl_alpn_protocol = NULL;
-	}
 }
 
 ssize_t
@@ -1279,7 +1299,7 @@ info_cb(const SSL *ssl, int type, int args)
 }
 
 /* See pqcomm.h comments on OpenSSL implementation of ALPN (RFC 7301) */
-static unsigned char alpn_protos[] = PG_ALPN_PROTOCOL_VECTOR;
+static const unsigned char alpn_protos[] = PG_ALPN_PROTOCOL_VECTOR;
 
 /*
  * Server callback for ALPN negotiation. We use use the standard "helper"
@@ -1298,30 +1318,25 @@ static int  alpn_cb(SSL *ssl,
 	 * vector when the callback is declared to take a const vector? What are
 	 * we to do with that?
 	 */
-	int retval;
+	int			retval;
+
 	Assert(userdata != NULL);
 	Assert(out != NULL);
 	Assert(outlen != NULL);
 	Assert(in != NULL);
 
-	retval = SSL_select_next_proto((unsigned char **)out, outlen,
+	retval = SSL_select_next_proto((unsigned char **) out, outlen,
 								   alpn_protos, sizeof(alpn_protos),
 								   in, inlen);
 	if (*out == NULL || *outlen > sizeof(alpn_protos) || outlen <= 0)
 		return SSL_TLSEXT_ERR_NOACK; /* can't happen */
 
 	if (retval == OPENSSL_NPN_NEGOTIATED)
-	{
-		struct Port *port = (struct Port *)userdata;
-		char *alpn_protocol = MemoryContextAllocZero(TopMemoryContext, *outlen+1);
-		memcpy(alpn_protocol, *out, *outlen);
-		port->ssl_alpn_protocol = alpn_protocol;
 		return SSL_TLSEXT_ERR_OK;
-	} else if (retval == OPENSSL_NPN_NO_OVERLAP) {
+	else if (retval == OPENSSL_NPN_NO_OVERLAP)
 		return SSL_TLSEXT_ERR_NOACK;
-	} else {
+	else
 		return SSL_TLSEXT_ERR_NOACK;
-	}
 }
 
 
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 872bbd79f09..3ed6134941e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1933,11 +1933,6 @@ ServerLoop(void)
  * This happens before startup packets so we are careful not to actual read
  * any bytes from the stream if it's not a direct SSL connection.
  */
-
-#ifdef USE_SSL
-static const char *expected_alpn_protocol = PG_ALPN_PROTOCOL;
-#endif
-
 static int
 ProcessSSLStartup(Port *port)
 {
@@ -1989,22 +1984,13 @@ ProcessSSLStartup(Port *port)
 			return STATUS_ERROR;
 		}
 
-		if (port->ssl_alpn_protocol == NULL)
+		if (!port->alpn_used)
 		{
 			ereport(COMMERROR,
 					(errcode(ERRCODE_PROTOCOL_VIOLATION),
 					 errmsg("Received direct SSL connection request without required ALPN protocol negotiation extension")));
 			return STATUS_ERROR;
 		}
-		if (strcmp(port->ssl_alpn_protocol, expected_alpn_protocol) != 0)
-		{
-			ereport(COMMERROR,
-					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("Received direct SSL connection request with unexpected ALPN protocol \"%s\" expected \"%s\"",
-							port->ssl_alpn_protocol,
-							expected_alpn_protocol)));
-			return STATUS_ERROR;
-		}
 #else
 		// XXX: Send TLS alert ?
 		return STATUS_ERROR;
@@ -2151,7 +2137,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
 
 			if (secure_open_server(port) == -1)
 				return STATUS_ERROR;
-			
+
 			/*
 			 * At this point we should have no data already buffered.  If we do,
 			 * it was received before we performed the SSL handshake, so it wasn't
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index e947e6f7cf0..44ad2a0848f 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -218,7 +218,7 @@ typedef struct Port
 	char	   *peer_cn;
 	char	   *peer_dn;
 	bool		peer_cert_valid;
-	char       *ssl_alpn_protocol;
+	bool		alpn_used;
 
 	/*
 	 * OpenSSL structures. (Keep these last so that the locations of other
-- 
2.39.2