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

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

---
 src/backend/libpq/be-secure-openssl.c         | 66 +++++++++++++++++++
 src/backend/libpq/be-secure.c                 |  3 +
 src/backend/postmaster/postmaster.c           | 25 +++++++
 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      | 31 +++++++++
 src/interfaces/libpq/libpq-int.h              |  1 +
 12 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 685aa2ed69..620ffafb0b 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,13 @@ 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,
@@ -692,6 +705,12 @@ 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
@@ -1250,6 +1269,53 @@ 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;
+
+/*
+ * Server callback for ALPN negotiation. We use use the standard "helper"
+ * function even though currently we only accept one value. We store the
+ * negotiated protocol in Port->ssl_alpn_protocol and rely on higher level
+ * logic (in postmaster.c) to decide what to do with that info.
+ */
+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)
+	{
+		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) {
+		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 1020b3adb0..79a61900ba 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -61,6 +61,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 ec1d895a23..9b4b37b997 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1934,6 +1934,10 @@ ServerLoop(void)
  * 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)
 {
@@ -1970,6 +1974,10 @@ ProcessSSLStartup(Port *port)
 		char *buf = NULL;
 		elog(LOG, "Detected direct SSL handshake");
 
+		if (!ssl_enable_alpn) {
+			elog(WARNING, "Received direct SSL connection without ssl_enable_alpn enabled");
+		}
+
 		/* push unencrypted buffered data back through SSL setup */
 		len = pq_buffer_has_data();
 		if (len > 0)
@@ -2000,6 +2008,23 @@ ProcessSSLStartup(Port *port)
 			return STATUS_ERROR;
 		}
 		pfree(port->raw_buf);
+
+		if (port->ssl_alpn_protocol == NULL)
+		{
+			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
 		ereport(COMMERROR,
 				(errcode(ERRCODE_PROTOCOL_VIOLATION),
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8062589efd..79a1d0a100 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1088,6 +1088,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 ee49ca3937..f93bcc23d0 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -118,6 +118,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 d7731234b6..816b336973 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3715,6 +3715,7 @@ printSSLInfo(void)
 	const char *protocol;
 	const char *cipher;
 	const char *compression;
+	const char *alpn;
 
 	if (!PQsslInUse(pset.db))
 		return;					/* no SSL */
@@ -3722,11 +3723,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 824b28e824..2258241770 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -217,6 +217,7 @@ typedef struct Port
 	char	   *peer_cn;
 	char	   *peer_dn;
 	bool		peer_cert_valid;
+	char       *ssl_alpn_protocol;
 
 	/*
 	 * 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 2b02f67257..e7adf4a989 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 c85090259d..d9fc02adfc 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -152,6 +152,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 7cd0eb261f..3118c19edd 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -314,6 +314,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)},
@@ -4487,6 +4491,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 4d1e4009ef..02af7bf571 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -912,6 +912,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.
@@ -1240,6 +1243,20 @@ 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
@@ -1723,6 +1740,7 @@ PQsslAttributeNames(PGconn *conn)
 		"cipher",
 		"compression",
 		"protocol",
+		"alpn",
 		NULL
 	};
 	static const char *const empty_attrs[] = {NULL};
@@ -1777,6 +1795,19 @@ 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 db63afd786..17e5b2528f 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -400,6 +400,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.40.0