v1-0001-Add-new-predefined-role-pg_create_subscriptions.patch

From cad995bc66aef479d7a075fe9ff1af195de04a3c Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 20 Jan 2023 09:52:58 -0500
Subject: [PATCH v1] Add new predefined role pg_create_subscriptions.

This role can be granted to non-superusers to allow them to issue
CREATE SUBSCRIPTION. If a connection string passed to CREATE
SUBSCRIPTION or ALTER SUBSCRIPTION would trigger access to local
files (e.g. because it specifies host=/someplace or
sslcert=/some/file), then pg_read_server_files is also required.

This commit also allows non-superusers to use ALTER SUBSCRIPTION
.. SKIP. It is good enough to be the owner of the subscription.

XXX. Documentation changes not included.
---
 src/backend/commands/subscriptioncmds.c       | 33 ++++++++-----
 .../libpqwalreceiver/libpqwalreceiver.c       | 46 +++++++++++++++++--
 src/include/catalog/pg_authid.dat             |  5 ++
 src/include/replication/walreceiver.h         |  2 +-
 src/test/regress/expected/subscription.out    |  4 +-
 5 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index baff00dd74..258414dc5c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -23,6 +23,7 @@
 #include "catalog/namespace.h"
 #include "catalog/objectaccess.h"
 #include "catalog/objectaddress.h"
+#include "catalog/pg_authid_d.h"
 #include "catalog/pg_subscription.h"
 #include "catalog/pg_subscription_rel.h"
 #include "catalog/pg_type.h"
@@ -572,10 +573,15 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
 	if (opts.create_slot)
 		PreventInTransactionBlock(isTopLevel, "CREATE SUBSCRIPTION ... WITH (create_slot = true)");
 
-	if (!superuser())
+	/*
+	 * We don't want to allow unprivileged users to be able to trigger attempts
+	 * to access arbitrary network destinations, so require the user to have
+	 * been specifically authorized to perform this operation.
+	 */
+	if (!has_privs_of_role(owner, ROLE_PG_CREATE_SUBSCRIPTION))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser to create subscriptions")));
+				 errmsg("must have privileges of pg_create_subscription to create subscriptions")));
 
 	/*
 	 * If built with appropriate switch, whine when regression-testing
@@ -614,7 +620,11 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
 	load_file("libpqwalreceiver", false);
 
 	/* Check the connection info string. */
-	walrcv_check_conninfo(conninfo);
+	if (walrcv_check_conninfo(conninfo) &&
+		!has_privs_of_role(owner, ROLE_PG_READ_SERVER_FILES))
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("must have privileges of pg_read_server_files to specify this connection string")));
 
 	/* Everything ok, form a new tuple. */
 	memset(values, 0, sizeof(values));
@@ -1148,7 +1158,11 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
 			/* Load the library providing us libpq calls. */
 			load_file("libpqwalreceiver", false);
 			/* Check the connection info string. */
-			walrcv_check_conninfo(stmt->conninfo);
+			if (walrcv_check_conninfo(stmt->conninfo) &&
+				!has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
+				ereport(ERROR,
+						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+						 errmsg("must have privileges of pg_read_server_files to specify this connection string")));
 
 			values[Anum_pg_subscription_subconninfo - 1] =
 				CStringGetTextDatum(stmt->conninfo);
@@ -1305,11 +1319,6 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
 				/* ALTER SUBSCRIPTION ... SKIP supports only LSN option */
 				Assert(IsSet(opts.specified_opts, SUBOPT_LSN));
 
-				if (!superuser())
-					ereport(ERROR,
-							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-							 errmsg("must be superuser to skip transaction")));
-
 				/*
 				 * If the user sets subskiplsn, we do a sanity check to make
 				 * sure that the specified LSN is a probable value.
@@ -1717,13 +1726,13 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION,
 					   NameStr(form->subname));
 
-	/* New owner must be a superuser */
-	if (!superuser_arg(newOwnerId))
+	/* New owner must have pg_create_subscription */
+	if (!has_privs_of_role(newOwnerId, ROLE_PG_CREATE_SUBSCRIPTION))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied to change owner of subscription \"%s\"",
 						NameStr(form->subname)),
-				 errhint("The owner of a subscription must be a superuser.")));
+				 errhint("The owner of a subscription must have the privileges of pg_create_subscription.")));
 
 	form->subowner = newOwnerId;
 	CatalogTupleUpdate(rel, &tup->t_self, tup);
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index c40c6220db..d70bca87fb 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -50,7 +50,7 @@ struct WalReceiverConn
 static WalReceiverConn *libpqrcv_connect(const char *conninfo,
 										 bool logical, const char *appname,
 										 char **err);
-static void libpqrcv_check_conninfo(const char *conninfo);
+static bool libpqrcv_check_conninfo(const char *conninfo);
 static char *libpqrcv_get_conninfo(WalReceiverConn *conn);
 static void libpqrcv_get_senderinfo(WalReceiverConn *conn,
 									char **sender_host, int *sender_port);
@@ -247,13 +247,20 @@ libpqrcv_connect(const char *conninfo, bool logical, const char *appname,
 }
 
 /*
- * Validate connection info string (just try to parse it)
+ * Validate connection info string, and determine whether it might cause
+ * local filesystem access to be attempted.
+ *
+ * If the connection string can't be parsed, this function will raise
+ * an error and will not return. If it can, it will return true if local
+ * filesystem access may be attempted and false otherwise.
  */
-static void
+static bool
 libpqrcv_check_conninfo(const char *conninfo)
 {
 	PQconninfoOption *opts = NULL;
+	PQconninfoOption *opt;
 	char	   *err = NULL;
+	bool		result = false;
 
 	opts = PQconninfoParse(conninfo, &err);
 	if (opts == NULL)
@@ -267,7 +274,40 @@ libpqrcv_check_conninfo(const char *conninfo)
 				 errmsg("invalid connection string syntax: %s", errcopy)));
 	}
 
+	for (opt = opts; opt->keyword != NULL; ++opt)
+	{
+		/* Ignore connection options that are not present. */
+		if (opt->val == NULL)
+			continue;
+
+		/* For all these parameters, the value is a local filename. */
+		if (strcmp(opt->keyword, "passfile") == 0 ||
+			strcmp(opt->keyword, "sslcert") == 0 ||
+			strcmp(opt->keyword, "sslkey") == 0 ||
+			strcmp(opt->keyword, "sslrootcert") == 0 ||
+			strcmp(opt->keyword, "sslcrl") == 0 ||
+			strcmp(opt->keyword, "sslcrldir") == 0 ||
+			strcmp(opt->keyword, "service") == 0)
+		{
+			result = true;
+			break;
+		}
+
+		/*
+		 * For the host parameter, the value might be a local filename.
+		 * It might also be a reference to the local host's abstract UNIX
+		 * socket namespace, which we consider equivalent to a local pathname
+		 * for security purporses.
+		 */
+		if (strcmp(opt->keyword, "host") == 0 && is_unixsock_path(opt->val))
+		{
+			result = true;
+			break;
+		}
+	}
+
 	PQconninfoFree(opts);
+	return result;
 }
 
 /*
diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat
index 2a2fee7d28..7daad12e96 100644
--- a/src/include/catalog/pg_authid.dat
+++ b/src/include/catalog/pg_authid.dat
@@ -89,5 +89,10 @@
   rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
   rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
   rolpassword => '_null_', rolvaliduntil => '_null_' },
+{ oid => '9535', oid_symbol => 'ROLE_PG_CREATE_SUBSCRIPTION',
+  rolname => 'pg_create_subscription', rolsuper => 'f', rolinherit => 't',
+  rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f',
+  rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1',
+  rolpassword => '_null_', rolvaliduntil => '_null_' },
 
 ]
diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index decffe352d..1227650a85 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -247,7 +247,7 @@ typedef WalReceiverConn *(*walrcv_connect_fn) (const char *conninfo,
  *
  * Parse and validate the connection string given as of 'conninfo'.
  */
-typedef void (*walrcv_check_conninfo_fn) (const char *conninfo);
+typedef bool (*walrcv_check_conninfo_fn) (const char *conninfo);
 
 /*
  * walrcv_get_conninfo_fn
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 4e5cb0d3a9..7843867695 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -78,7 +78,7 @@ ERROR:  subscription "regress_testsub" already exists
 -- fail - must be superuser
 SET SESSION AUTHORIZATION 'regress_subscription_user2';
 CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION foo WITH (connect = false);
-ERROR:  must be superuser to create subscriptions
+ERROR:  must have privileges of pg_create_subscription to create subscriptions
 SET SESSION AUTHORIZATION 'regress_subscription_user';
 -- fail - invalid option combinations
 CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, copy_data = true);
@@ -213,7 +213,7 @@ ALTER SUBSCRIPTION regress_testsub_foo RENAME TO regress_testsub;
 -- fail - new owner must be superuser
 ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
 ERROR:  permission denied to change owner of subscription "regress_testsub"
-HINT:  The owner of a subscription must be a superuser.
+HINT:  The owner of a subscription must have the privileges of pg_create_subscription.
 ALTER ROLE regress_subscription_user2 SUPERUSER;
 -- now it works
 ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user2;
-- 
2.37.1 (Apple Git-137.1)