v8-0004-Direct-SSL-connections-ALPN-support.patch

From 3a6f22bdd0b231cc645b67cccd7511d6a38095c4 Mon Sep 17 00:00:00 2001
From: Greg Stark <stark@mit.edu>
Date: Mon, 20 Mar 2023 14:09:30 -0400
Subject: [PATCH v8 4/6] Direct SSL connections ALPN support

Move code to check for alpn

- if non-direct SSL is used, and client sends an unexpected ALPN
  protocol, that's now an error ?

Author: Greg Stark, Heikki Linnakangas
---
 src/backend/libpq/be-secure-openssl.c         | 84 +++++++++++++++++++
 src/backend/libpq/be-secure.c                 |  3 +
 src/backend/postmaster/postmaster.c           | 13 +++
 src/backend/utils/misc/guc_tables.c           |  9 ++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/bin/psql/command.c                        |  7 +-
 src/include/libpq/libpq-be.h                  |  1 +
 src/include/libpq/libpq.h                     |  1 +
 src/include/libpq/pqcomm.h                    | 19 +++++
 src/interfaces/libpq/fe-connect.c             |  5 ++
 src/interfaces/libpq/fe-secure-openssl.c      | 35 ++++++++
 src/interfaces/libpq/libpq-int.h              |  1 +
 12 files changed, 177 insertions(+), 2 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index e12b1cc9e3b..afde13c2fad 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -67,6 +67,12 @@ 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	alpn_cb(SSL *ssl,
+					const unsigned char **out,
+					unsigned char *outlen,
+					const unsigned char *in,
+					unsigned int inlen,
+					void *userdata);
 static bool initialize_dh(SSL_CTX *context, bool isServerStart);
 static bool initialize_ecdh(SSL_CTX *context, bool isServerStart);
 static const char *SSLerrmessage(unsigned long ecode);
@@ -432,6 +438,16 @@ be_tls_open_server(Port *port)
 	/* set up debugging/info callback */
 	SSL_CTX_set_info_callback(SSL_context, info_cb);
 
+	if (ssl_enable_alpn)
+	{
+		elog(DEBUG2, "Enabling OpenSSL ALPN callback");
+		SSL_CTX_set_alpn_select_cb(SSL_context, alpn_cb, port);
+	}
+	else
+	{
+		elog(DEBUG2, "OpenSSL ALPN is disabled, not setting callback");
+	}
+
 	if (!(port->ssl = SSL_new(SSL_context)))
 	{
 		ereport(COMMERROR,
@@ -571,6 +587,32 @@ aloop:
 		return -1;
 	}
 
+	/* Get the 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);
 
@@ -1259,6 +1301,48 @@ 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 ALPN negotiation. We use the standard "helper" function
+ * even though currently we only accept one value.
+ */
+static int
+alpn_cb(SSL *ssl,
+		const unsigned char **out,
+		unsigned char *outlen,
+		const unsigned char *in,
+		unsigned int inlen,
+		void *userdata)
+{
+	/*
+	 * Why does OpenSSL provide a helper function that requires a nonconst
+	 * vector when the callback is declared to take a const vector? What are
+	 * we to do with that?
+	 */
+	int			retval;
+
+	Assert(userdata != NULL);
+	Assert(out != NULL);
+	Assert(outlen != NULL);
+	Assert(in != NULL);
+
+	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)
+		return SSL_TLSEXT_ERR_OK;
+	else if (retval == OPENSSL_NPN_NO_OVERLAP)
+		return SSL_TLSEXT_ERR_NOACK;
+	else
+		return SSL_TLSEXT_ERR_NOACK;
+}
+
+
 /*
  * Set DH parameters for generating ephemeral DH keys.  The
  * DH parameters can take a long time to compute, so they must be
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 1d1329d1d95..20a1a4ad551 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -58,6 +58,9 @@ bool		SSLPreferServerCiphers;
 int			ssl_min_protocol_version = PG_TLS1_2_VERSION;
 int			ssl_max_protocol_version = PG_TLS_ANY;
 
+/* GUC variable: if false ignore ALPN negotiation */
+bool		ssl_enable_alpn = true;
+
 /* ------------------------------------------------------------ */
 /*			 Procedures common to all secure sessions			*/
 /* ------------------------------------------------------------ */
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index ab82faf6aa0..cc6d7b477e2 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1958,6 +1958,11 @@ ProcessSSLStartup(Port *port)
 	{
 		elog(LOG, "Detected direct SSL handshake");
 
+		if (!ssl_enable_alpn)
+		{
+			elog(WARNING, "Received direct SSL connection without ssl_enable_alpn enabled");
+		}
+
 #ifdef USE_SSL
 		if (!LoadedSSL || port->laddr.addr.ss_family == AF_UNIX)
 		{
@@ -1972,6 +1977,14 @@ ProcessSSLStartup(Port *port)
 			 */
 			return STATUS_ERROR;
 		}
+
+		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;
+		}
 #else
 		/* SSL not supported by this build */
 		return STATUS_ERROR;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 45013582a74..b7f7b2837aa 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1085,6 +1085,15 @@ struct config_bool ConfigureNamesBool[] =
 		true,
 		NULL, NULL, NULL
 	},
+	{
+		{"ssl_enable_alpn", PGC_SIGHUP, CONN_AUTH_SSL,
+			gettext_noop("Respond to TLS ALPN Extension Requests."),
+			NULL,
+		},
+		&ssl_enable_alpn,
+		true,
+		NULL, NULL, NULL
+	},
 	{
 		{"fsync", PGC_SIGHUP, WAL_SETTINGS,
 			gettext_noop("Forces synchronization of updates to disk."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index edcc0282b2d..880cd9438cb 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -127,6 +127,7 @@
 #ssl_dh_params_file = ''
 #ssl_passphrase_command = ''
 #ssl_passphrase_command_supports_reload = off
+#ssl_enable_alpn = on
 
 
 #------------------------------------------------------------------------------
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 5c906e48068..07f13f9c04e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3814,6 +3814,7 @@ printSSLInfo(void)
 	const char *protocol;
 	const char *cipher;
 	const char *compression;
+	const char *alpn;
 
 	if (!PQsslInUse(pset.db))
 		return;					/* no SSL */
@@ -3821,11 +3822,13 @@ printSSLInfo(void)
 	protocol = PQsslAttribute(pset.db, "protocol");
 	cipher = PQsslAttribute(pset.db, "cipher");
 	compression = PQsslAttribute(pset.db, "compression");
+	alpn = PQsslAttribute(pset.db, "alpn");
 
-	printf(_("SSL connection (protocol: %s, cipher: %s, compression: %s)\n"),
+	printf(_("SSL connection (protocol: %s, cipher: %s, compression: %s, ALPN: %s)\n"),
 		   protocol ? protocol : _("unknown"),
 		   cipher ? cipher : _("unknown"),
-		   (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"));
+		   (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"),
+		   alpn ? alpn : _("none"));
 }
 
 /*
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 1f7ce041359..53c08370a4c 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		alpn_used;
 
 	/*
 	 * OpenSSL structures. (Keep these last so that the locations of other
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 79c5f6b754b..b65b32f2093 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -123,6 +123,7 @@ extern PGDLLIMPORT char *SSLECDHCurve;
 extern PGDLLIMPORT bool SSLPreferServerCiphers;
 extern PGDLLIMPORT int ssl_min_protocol_version;
 extern PGDLLIMPORT int ssl_max_protocol_version;
+extern PGDLLIMPORT bool ssl_enable_alpn;
 
 enum ssl_protocol_versions
 {
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 9ae469c86c4..fb93c820530 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -139,6 +139,25 @@ typedef struct CancelRequestPacket
 	uint32		cancelAuthCode; /* secret key to authorize cancel */
 } CancelRequestPacket;
 
+/* Application-Layer Protocol Negotiation is required for direct connections
+ * to avoid protocol confusion attacks (e.g https://alpaca-attack.com/).
+ *
+ * ALPN is specified in RFC 7301
+ *
+ * This string should be registered at:
+ * https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
+ *
+ * OpenSSL uses this wire-format for the list of alpn protocols even in the
+ * API. Both server and client take the same format parameter but the client
+ * actually sends it to the server as-is and the server it specifies the
+ * preference order to use to choose the one selected to send back.
+ *
+ * c.f. https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_alpn_select_cb.html
+ *
+ * The #define can be used to initialize a char[] vector to use directly in the API
+ */
+#define PG_ALPN_PROTOCOL "TBD-pgsql"
+#define PG_ALPN_PROTOCOL_VECTOR { 9, 'T','B','D','-','p','g','s','q','l' }
 
 /*
  * A client can also start by sending a SSL or GSSAPI negotiation request to
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 7bd371fafb5..4bbed0451ed 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -313,6 +313,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-SNI", "", 1,
 	offsetof(struct pg_conn, sslsni)},
 
+	{"sslalpn", "PGSSLALPN", "1", NULL,
+		"SSL-ALPN", "", 1,
+	offsetof(struct pg_conn, sslalpn)},
+
 	{"requirepeer", "PGREQUIREPEER", NULL, NULL,
 		"Require-Peer", "", 10,
 	offsetof(struct pg_conn, requirepeer)},
@@ -4473,6 +4477,7 @@ freePGconn(PGconn *conn)
 	free(conn->sslcrldir);
 	free(conn->sslcompression);
 	free(conn->sslsni);
+	free(conn->sslalpn);
 	free(conn->requirepeer);
 	free(conn->require_auth);
 	free(conn->ssl_min_protocol_version);
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 81108822620..a8fd4c19efb 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -885,6 +885,9 @@ destroy_ssl_system(void)
 #endif
 }
 
+/* See pqcomm.h comments on OpenSSL implementation of ALPN (RFC 7301) */
+static unsigned char alpn_protos[] = PG_ALPN_PROTOCOL_VECTOR;
+
 /*
  *	Create per-connection SSL object, and load the client certificate,
  *	private key, and trusted CA certs.
@@ -1234,6 +1237,22 @@ initialize_SSL(PGconn *conn)
 		}
 	}
 
+	if (conn->sslalpn && conn->sslalpn[0] == '1')
+	{
+		int			retval;
+
+		retval = SSL_set_alpn_protos(conn->ssl, alpn_protos, sizeof(alpn_protos));
+
+		if (retval != 0)
+		{
+			char	   *err = SSLerrmessage(ERR_get_error());
+
+			libpq_append_conn_error(conn, "could not set ssl alpn extension: %s", err);
+			SSLerrfree(err);
+			return -1;
+		}
+	}
+
 	/*
 	 * Read the SSL key. If a key is specified, treat it as an engine:key
 	 * combination if there is colon present - we don't support files with
@@ -1736,6 +1755,7 @@ PQsslAttributeNames(PGconn *conn)
 		"cipher",
 		"compression",
 		"protocol",
+		"alpn",
 		NULL
 	};
 	static const char *const empty_attrs[] = {NULL};
@@ -1790,6 +1810,21 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
 	if (strcmp(attribute_name, "protocol") == 0)
 		return SSL_get_version(conn->ssl);
 
+	if (strcmp(attribute_name, "alpn") == 0)
+	{
+		const unsigned char *data;
+		unsigned int len;
+		static char alpn_str[256];	/* alpn doesn't support longer than 255
+									 * bytes */
+
+		SSL_get0_alpn_selected(conn->ssl, &data, &len);
+		if (data == NULL || len == 0 || len > sizeof(alpn_str) - 1)
+			return NULL;
+		memcpy(alpn_str, data, len);
+		alpn_str[len] = 0;
+		return alpn_str;
+	}
+
 	return NULL;				/* unknown attribute */
 }
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 640a3ae0f5a..97d78f02b92 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -399,6 +399,7 @@ struct pg_conn
 	char	   *sslcrl;			/* certificate revocation list filename */
 	char	   *sslcrldir;		/* certificate revocation list directory name */
 	char	   *sslsni;			/* use SSL SNI extension (0 or 1) */
+	char	   *sslalpn;		/* use SSL ALPN extension (0 or 1) */
 	char	   *requirepeer;	/* required peer credentials for local sockets */
 	char	   *gssencmode;		/* GSS mode (require,prefer,disable) */
 	char	   *krbsrvname;		/* Kerberos service name */
-- 
2.39.2