v10-0011-Add-_pq_.protocol_managed_params-protocol-extens.patch

From d8bbd84c7e6b8aa6db343567bc1db36269c38ce2 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Fri, 5 Jan 2024 14:50:53 +0100
Subject: [PATCH v10 11/13] Add _pq_.protocol_managed_params protocol extension
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds a new protocol extension that allows changing certain GUCs to be
protocol extensions, i.e. changing their context to PGC_PROTOCOL or
PGC_SU_PROTOCOL.

One benefit this provides is the ability to sandbox SQL scripts by
downgrading the connection to a low privileged user, without having the
ability to change back by using SET ROLE:

```
psql "user=postgres _pq_.protocol_managed_params=role options='-c role=pg_read_all_data'"

> SELECT CURRENT_USER;
   current_user
──────────────────
 pg_read_all_data
(1 row)

> SET ROLE postgres;
ERROR:  parameter can only be set at the protocol level "role"
```
---
 doc/src/sgml/config.sgml            | 49 ++++++++++++++
 doc/src/sgml/libpq.sgml             | 10 +++
 src/backend/utils/misc/guc.c        | 99 +++++++++++++++++++++++++++++
 src/backend/utils/misc/guc_tables.c | 12 ++++
 src/include/utils/guc.h             |  2 +
 src/include/utils/guc_hooks.h       |  2 +
 src/interfaces/libpq/fe-connect.c   |  4 ++
 src/interfaces/libpq/fe-protocol3.c |  3 +
 src/interfaces/libpq/libpq-int.h    |  1 +
 9 files changed, 182 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 65a6e6c4086..f7a44ec7c61 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11215,6 +11215,55 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
     </variablelist>
    </sect1>
 
+   <sect1 id="runtime-config-protocol">
+    <title>Protocol extension parameters</title>
+    <para>
+     Some parameters control the behaviour of the protocol, these are called
+     protocol extension parameters. These types of parameters are different
+     from all other runtime parameters in a few important ways. First, they all
+     start with a <literal>_pq_.</literal> prefix. Secondly, they can only be
+     set at the protocol level using the StartupMessage and ParameterSet
+     messages.
+     It's not possible to set them in any other way (not through command line
+     arguments, configuration files, SET, etc). Finally, if you configure one
+     of them in the StartupMessage, but the server doesn't support the
+     parameter the connection attempt does not fail like with other options.
+     Instead the connection attempt continues as normal as these protocol
+     parameters are considered optional to implement by the server. To check if
+     the parameter was set you need to check the return value of
+     <xref linkend="libpq-PQunsupportedProtocolExtensionParameters"/>. If one of the
+     requested parameters was not supported by the server it will be listed
+     there.
+    </para>
+
+     <variablelist>
+
+     <varlistentry id="guc-pq-protocol-managed-params" xreflabel="_pq_.protocol_managed_params">
+      <term><varname>_pq_.protocol_managed_params</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>_pq_.protocol_managed_params</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        This parameter can be used to "upgrade" regular runtime parameters to a
+        protocol extension parameter. Thus disallowing it to be set in any
+        other way than through the StartupMessage and ParameterSet protocol
+        messages.
+        This can be useful to limit the power of an attacker with arbitrary SQL
+        execution. For example, if you set
+        <literal>_pq_.protocol_managed_params</literal> to
+        <literal>role</literal> then you can connect as a
+        highly privileged user to <productname>PostgreSQL</productname> but
+        change role to a user with fewer privileges. And then the attacker with
+        only SQL access is unable to change back the session authorization.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     </variablelist>
+   </sect1>
+
    <sect1 id="runtime-config-custom">
     <title>Customized Options</title>
 
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index ec00fe7b7f9..dd58a8cabab 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2212,6 +2212,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
        </para>
       </listitem>
      </varlistentry>
+
+     <varlistentry id="libpq-pq-protocol-managed-params" xreflabel="load_balance_hosts">
+      <term><literal>_pq_.protocol_managed_params</literal></term>
+      <listitem>
+       <para>
+        Specifies a value for the <xref linkend="guc-pq-protocol-managed-params"/>
+        configuration parameter.
+       </para>
+      </listitem>
+     </varlistentry>
     </variablelist>
    </para>
   </sect2>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f8de9ca7b18..72a4273222b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -47,9 +47,11 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/conffiles.h"
+#include "utils/guc_hooks.h"
 #include "utils/guc_tables.h"
 #include "utils/memutils.h"
 #include "utils/timestamp.h"
+#include "utils/varlena.h"
 
 
 #define CONFIG_FILENAME "postgresql.conf"
@@ -6971,3 +6973,100 @@ call_enum_check_hook(struct config_enum *conf, int *newval, void **extra,
 
 	return true;
 }
+
+/*
+ * GUC check_hook for protocol_managed_params
+ */
+bool
+check_protocol_managed_params(char **newval, void **extra, GucSource source)
+{
+	List	   *namelist;
+	char	   *protocol_params_str = pstrdup(*newval);
+
+	if (!SplitIdentifierString(protocol_params_str, ',', &namelist))
+	{
+		/* syntax error in name list */
+		GUC_check_errdetail("List syntax is invalid.");
+		pfree(protocol_params_str);
+		list_free(namelist);
+		return false;
+	}
+
+	foreach_ptr(char, pname, namelist)
+	{
+		/*
+		 * We explicitly allow unknown parameters here (but we still warn for
+		 * them). So that it is possible to add version specific parameters to
+		 * the protocol_managed_parameters list in the StartupMessage without
+		 * knowing the current server version yet.
+		 */
+		struct config_generic *config = find_option(pname, false, false, WARNING);
+
+		if (strncmp(pname, "_pq_.", 5) == 0)
+		{
+			GUC_check_errdetail("Parameter \"%s\" is a protocol extension.", pname);
+			pfree(protocol_params_str);
+			list_free(namelist);
+			return false;
+		}
+
+		if (!config)
+			continue;
+
+		if (config->context != PGC_PROTOCOL && config->context != PGC_USERSET && config->context != PGC_SUSET)
+		{
+			GUC_check_errdetail("Parameter \"%s\" is not a user-settable parameter.", pname);
+			pfree(protocol_params_str);
+			list_free(namelist);
+			return false;
+		}
+	}
+
+
+	pfree(protocol_params_str);
+	list_free(namelist);
+	return true;
+}
+
+/*
+ * GUC check_hook for protocol_managed_params
+ */
+void
+assign_protocol_managed_params(const char *newval, void *extra)
+{
+	List	   *namelist;
+	char	   *old_protocol_params_str = pstrdup(protocol_managed_params);
+	char	   *protocol_params_str = pstrdup(newval);
+
+	if (!SplitIdentifierString(old_protocol_params_str, ',', &namelist))
+	{
+		elog(ERROR, "List syntax is invalid and check hook should have checked.");
+	}
+
+	foreach_ptr(char, pname, namelist)
+	{
+		struct config_generic *config = find_option(pname, false, false, ERROR);
+
+		if (config)
+			config->context = config->context == PGC_PROTOCOL ? PGC_USERSET : PGC_SUSET;
+	}
+
+	list_free(namelist);
+
+	if (!SplitIdentifierString(protocol_params_str, ',', &namelist))
+	{
+		elog(ERROR, "List syntax is invalid and check hook should have checked.");
+	}
+
+	foreach_ptr(char, pname, namelist)
+	{
+		struct config_generic *config = find_option(pname, false, true, ERROR);
+
+		if (config)
+			config->context = config->context == PGC_USERSET ? PGC_PROTOCOL : PGC_SU_PROTOCOL;
+	}
+
+	pfree(old_protocol_params_str);
+	pfree(protocol_params_str);
+	list_free(namelist);
+}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 09084db0c05..5ce3ae7153d 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -494,6 +494,8 @@ extern const struct config_enum_entry dynamic_shared_memory_options[];
 /*
  * GUC option variables that are exported from this module
  */
+char	   *protocol_managed_params = "";
+
 bool		log_duration = false;
 bool		Debug_print_plan = false;
 bool		Debug_print_parse = false;
@@ -3918,6 +3920,16 @@ struct config_real ConfigureNamesReal[] =
 
 struct config_string ConfigureNamesString[] =
 {
+	{
+		{"_pq_.protocol_managed_params", PGC_PROTOCOL, PROTOCOL_EXTENSION,
+			gettext_noop("List of additional parameters to be only managed at the protocol level."),
+			NULL,
+			GUC_LIST_INPUT | GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE
+		},
+		&protocol_managed_params,
+		"",
+		check_protocol_managed_params, assign_protocol_managed_params, NULL
+	},
 	{
 		{"archive_command", PGC_SIGHUP, WAL_ARCHIVING,
 			gettext_noop("Sets the shell command that will be called to archive a WAL file."),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c7a9efe0adf..85f880dc6cd 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -242,6 +242,8 @@ typedef enum
 
 
 /* GUC vars that are actually defined in guc_tables.c, rather than elsewhere */
+extern PGDLLIMPORT char *protocol_managed_params;
+
 extern PGDLLIMPORT bool Debug_print_plan;
 extern PGDLLIMPORT bool Debug_print_parse;
 extern PGDLLIMPORT bool Debug_print_rewritten;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index d64dc5fcdb0..ebe048b5561 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -25,6 +25,8 @@
  * Please keep the declarations in order by GUC variable name.
  */
 
+extern bool check_protocol_managed_params(char **newval, void **extra, GucSource source);
+extern void assign_protocol_managed_params(const char *newval, void *extra);
 extern bool check_application_name(char **newval, void **extra,
 								   GucSource source);
 extern void assign_application_name(const char *newval, void *extra);
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index aca78a683fe..606f5c80489 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -359,6 +359,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"Load-Balance-Hosts", "", 8,	/* sizeof("disable") = 8 */
 	offsetof(struct pg_conn, load_balance_hosts)},
 
+	{"_pq_.protocol_managed_params", NULL, NULL, NULL,
+		"Pq-Protocol-Managed-Params", "", 40,
+	offsetof(struct pg_conn, pq_protocol_managed_params)},
+
 	/* Terminating entry --- MUST BE LAST */
 	{NULL, NULL, NULL, NULL,
 	NULL, NULL, 0}
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 129de550953..ee9205494d7 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -2319,6 +2319,9 @@ build_startup_packet(const PGconn *conn, char *packet,
 		}
 	}
 
+	if (conn->pq_protocol_managed_params && conn->pq_protocol_managed_params[0])
+		ADD_STARTUP_OPTION("_pq_.protocol_managed_params", conn->pq_protocol_managed_params);
+
 	/* Add trailing terminator */
 	if (packet)
 		packet[packet_len] = '\0';
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index e312819ca3a..f065d77cecc 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -409,6 +409,7 @@ struct pg_conn
 	char	   *target_session_attrs;	/* desired session properties */
 	char	   *require_auth;	/* name of the expected auth method */
 	char	   *load_balance_hosts; /* load balance over hosts */
+	char	   *pq_protocol_managed_params; /* _pq_.protocol_managed_params */
 
 	/* Optional file to write trace info to */
 	FILE	   *Pfdebug;
-- 
2.34.1