* Robert Haas (robertmh...@gmail.com) wrote:
> On Mon, Feb 29, 2016 at 10:02 PM, Stephen Frost <sfr...@snowman.net> wrote:
> > Attached is a stripped-down version of the default roles patch which
> > includes only the 'pg_signal_backend' default role.  This provides the
> > framework and structure for other default roles to be added and formally
> > reserves the 'pg_' role namespace.  This is split into two patches, the
> > first to formally reserve 'pg_', the second to add the new default role.
> >
> > Will add to the March commitfest for review.
> 
> Here is a review of the first patch:
> 
> +       if (!IsA(node, RoleSpec))
> +               elog(ERROR, "invalid node type %d", node->type);
> 
> That looks strange.  Why not just Assert(IsA(node, RoleSpec))?

Sure, that works, done.

> +
> +       return;
> 
> Useless return.

Removed.

> @@ -673,6 +673,7 @@ dumpRoles(PGconn *conn)
>                          "pg_catalog.shobj_description(oid,
> 'pg_authid') as rolcomment, "
>                                                   "rolname =
> current_user AS is_current_user "
>                                                   "FROM pg_authid "
> +                                                 "WHERE rolname !~ '^pg_' "
>                                                   "ORDER BY 2");
>         else if (server_version >= 90100)
>                 printfPQExpBuffer(buf,
> @@ -895,6 +896,7 @@ dumpRoleMembership(PGconn *conn)
>                                            "LEFT JOIN pg_authid ur on
> ur.oid = a.roleid "
>                                            "LEFT JOIN pg_authid um on
> um.oid = a.member "
>                                            "LEFT JOIN pg_authid ug on
> ug.oid = a.grantor "
> +                                          "WHERE NOT (ur.rolname ~
> '^pg_' AND um.rolname ~ '^pg_')"
>                                            "ORDER BY 1,2,3");
> 
> If I'm reading this correctly, when dumping a 9.5 server, we'll
> silently drop any roles whose names start with pg_ from the dump, and
> hope that doesn't break anything.  When dumping a 9.4 or older server,
> we'll include those roles and hope that they miraculously restore on
> the new server.  I'm thinking neither of those approaches is going to
> work out, and the difference between them seems totally unprincipled.

That needed to be updated to be 9.6 and 9.5, of course.

Further, you make a good point that 9.6's pg_dumpall should really
always exclude any roles which start with 'pg_', throwing a warning if
it finds them.

Note that pg_upgrade won't proceed with an upgrade of a system that has
any 'pg_' roles.

> @@ -631,7 +637,8 @@ check_is_install_user(ClusterInfo *cluster)
>         res = executeQueryOrDie(conn,
>                                                         "SELECT rolsuper, oid 
> "
>                                                         "FROM
> pg_catalog.pg_roles "
> -                                                       "WHERE rolname
> = current_user");
> +                                                       "WHERE rolname
> = current_user "
> +                                                       "AND rolname
> !~ '^pg_'");
> 
>         /*
>          * We only allow the install user in the new cluster (see comment 
> below)
> @@ -647,7 +654,8 @@ check_is_install_user(ClusterInfo *cluster)
> 
>         res = executeQueryOrDie(conn,
>                                                         "SELECT COUNT(*) "
> -                                                       "FROM
> pg_catalog.pg_roles ");
> +                                                       "FROM
> pg_catalog.pg_roles "
> +                                                       "WHERE rolname
> !~ '^pg_'");
> 
>         if (PQntuples(res) != 1)
>                 pg_fatal("could not determine the number of users\n");
> 
> What bad thing would happen without these changes that would be
> avoided with these changes?  I can't think of anything.

This function ("check_is_install_user") is simply checking that the user
we are logged in as is the 'install' user and that there aren't any
other users in the destination cluster.  The initial check is perhaps a
bit paranoid- it shouldn't be possible for a 'pg_' role to be the one
which pg_upgrade has logged into the cluster with as none of the 'pg_'
roles have the 'login' privilege.

For the second check, pg_upgrade expects a pristine cluster to perform
the binary upgrade into, which includes checking that there aren't any
roles besides the 'install' role in the destination cluster.  Since
default roles are created at initdb time, the destination cluster *will*
have other roles in it besides the install role, but only roles whose
names start with 'pg_', and those are ok because they're from initdb.

Updated (and rebased) patch attached.

Thanks for the review!

Stephen
From 090994ca5a9ae0c64f8752b43801141582f31af7 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Mon, 29 Feb 2016 21:27:46 -0500
Subject: [PATCH 1/2] Reserve the "pg_" namespace for roles

This will prevent users from creating roles which begin with "pg_" and
will check for those roles before allowing an upgrade using pg_upgrade.

This will allow for default roles to be provided at initdb time.
---
 doc/src/sgml/ref/psql-ref.sgml          |  8 +++++--
 src/backend/catalog/catalog.c           |  5 +++--
 src/backend/commands/user.c             | 40 +++++++++++++++++++++++++++++++++
 src/backend/utils/adt/acl.c             | 39 ++++++++++++++++++++++++++++++++
 src/bin/pg_dump/pg_dumpall.c            | 11 ++++++++-
 src/bin/pg_upgrade/check.c              | 40 +++++++++++++++++++++++++++++++--
 src/bin/psql/command.c                  |  4 ++--
 src/bin/psql/describe.c                 |  5 ++++-
 src/bin/psql/describe.h                 |  2 +-
 src/bin/psql/help.c                     |  4 ++--
 src/include/utils/acl.h                 |  1 +
 src/test/regress/expected/rolenames.out | 18 +++++++++++++++
 src/test/regress/sql/rolenames.sql      |  8 +++++++
 13 files changed, 172 insertions(+), 13 deletions(-)

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 8a85804..20f2e9f 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1365,13 +1365,15 @@ testdb=&gt;
 
 
       <varlistentry>
-        <term><literal>\dg[+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+        <term><literal>\dg[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
         <listitem>
         <para>
         Lists database roles.
         (Since the concepts of <quote>users</> and <quote>groups</> have been
         unified into <quote>roles</>, this command is now equivalent to
         <literal>\du</literal>.)
+        By default, only user-created roles are shown; supply the
+        <literal>S</literal> modifier to include system roles.
         If <replaceable class="parameter">pattern</replaceable> is specified,
         only those roles whose names match the pattern are listed.
         If the form <literal>\dg+</literal> is used, additional information
@@ -1525,13 +1527,15 @@ testdb=&gt;
       </varlistentry>
 
       <varlistentry>
-        <term><literal>\du[+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+        <term><literal>\du[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
         <listitem>
         <para>
         Lists database roles.
         (Since the concepts of <quote>users</> and <quote>groups</> have been
         unified into <quote>roles</>, this command is now equivalent to
         <literal>\dg</literal>.)
+        By default, only user-created roles are shown; supply the
+        <literal>S</literal> modifier to include system roles.
         If <replaceable class="parameter">pattern</replaceable> is specified,
         only those roles whose names match the pattern are listed.
         If the form <literal>\du+</literal> is used, additional information
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index bead2c1..d1cf45b 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -184,8 +184,9 @@ IsToastNamespace(Oid namespaceId)
  *		True iff name starts with the pg_ prefix.
  *
  *		For some classes of objects, the prefix pg_ is reserved for
- *		system objects only.  As of 8.0, this is only true for
- *		schema and tablespace names.
+ *		system objects only.  As of 8.0, this was only true for
+ *		schema and tablespace names.  With 9.6, this is also true
+ *		for roles.
  */
 bool
 IsReservedName(const char *name)
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 4baeaa2..ee37397 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -17,6 +17,7 @@
 #include "access/htup_details.h"
 #include "access/xact.h"
 #include "catalog/binary_upgrade.h"
+#include "catalog/catalog.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
 #include "catalog/objectaccess.h"
@@ -312,6 +313,17 @@ CreateRole(CreateRoleStmt *stmt)
 	}
 
 	/*
+	 * Check that the user is not trying to create a role in the reserved
+	 * "pg_" namespace.
+	 */
+	if (IsReservedName(stmt->role))
+		ereport(ERROR,
+				(errcode(ERRCODE_RESERVED_NAME),
+				 errmsg("role name \"%s\" is reserved",
+					 stmt->role),
+				 errdetail("Role names starting with \"pg_\" are reserved.")));
+
+	/*
 	 * Check the pg_authid relation to be certain the role doesn't already
 	 * exist.
 	 */
@@ -1117,6 +1129,7 @@ RenameRole(const char *oldname, const char *newname)
 	int			i;
 	Oid			roleid;
 	ObjectAddress address;
+	Form_pg_authid authform;
 
 	rel = heap_open(AuthIdRelationId, RowExclusiveLock);
 	dsc = RelationGetDescr(rel);
@@ -1136,6 +1149,7 @@ RenameRole(const char *oldname, const char *newname)
 	 */
 
 	roleid = HeapTupleGetOid(oldtuple);
+	authform = (Form_pg_authid) GETSTRUCT(oldtuple);
 
 	if (roleid == GetSessionUserId())
 		ereport(ERROR,
@@ -1146,6 +1160,24 @@ RenameRole(const char *oldname, const char *newname)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("current user cannot be renamed")));
 
+	/*
+	 * Check that the user is not trying to rename a system role and
+	 * not trying to rename a role into the reserved "pg_" namespace.
+	 */
+	if (IsReservedName(NameStr(authform->rolname)))
+		ereport(ERROR,
+				(errcode(ERRCODE_RESERVED_NAME),
+				 errmsg("role name \"%s\" is reserved",
+					 NameStr(authform->rolname)),
+				 errdetail("Role names starting with \"pg_\" are reserved.")));
+
+	if (IsReservedName(newname))
+		ereport(ERROR,
+				(errcode(ERRCODE_RESERVED_NAME),
+				 errmsg("role name \"%s\" is reserved",
+					 newname),
+				 errdetail("Role names starting with \"pg_\" are reserved.")));
+
 	/* make sure the new name doesn't exist */
 	if (SearchSysCacheExists1(AUTHNAME, CStringGetDatum(newname)))
 		ereport(ERROR,
@@ -1224,10 +1256,18 @@ GrantRole(GrantRoleStmt *stmt)
 	ListCell   *item;
 
 	if (stmt->grantor)
+	{
+		check_rolespec_name(stmt->grantor,
+							"Cannot specify reserved role as grantor.");
 		grantor = get_rolespec_oid(stmt->grantor, false);
+	}
 	else
 		grantor = GetUserId();
 
+	foreach(item, stmt->grantee_roles)
+		check_rolespec_name(lfirst(item),
+							"Cannot GRANT roles to a reserved role.");
+
 	grantee_ids = roleSpecsToIds(stmt->grantee_roles);
 
 	/* AccessShareLock is enough since we aren't modifying pg_authid */
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 63fbce0..d2b23d0 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -17,6 +17,7 @@
 #include <ctype.h>
 
 #include "access/htup_details.h"
+#include "catalog/catalog.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_authid.h"
 #include "catalog/pg_auth_members.h"
@@ -5247,3 +5248,41 @@ get_rolespec_name(const Node *node)
 
 	return rolename;
 }
+
+/*
+ * Given a RoleSpec, throw an error if the name is reserved, using detail_msg,
+ * if provided.
+ *
+ * If node is NULL, no error is thrown.  If detail_msg is NULL then no detail
+ * message is provided.
+ */
+void
+check_rolespec_name(const Node *node, const char *detail_msg)
+{
+	RoleSpec   *role;
+
+	if (!node)
+		return;
+
+	role = (RoleSpec *) node;
+
+	Assert(IsA(node, RoleSpec));
+
+	if (role->roletype != ROLESPEC_CSTRING)
+		return;
+
+	if (IsReservedName(role->rolename))
+	{
+		if (detail_msg)
+			ereport(ERROR,
+					(errcode(ERRCODE_RESERVED_NAME),
+					 errmsg("role \"%s\" is reserved",
+						 role->rolename),
+					 errdetail("%s", detail_msg)));
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_RESERVED_NAME),
+					 errmsg("role \"%s\" is reserved",
+						 role->rolename)));
+	}
+}
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index be6b4a8..a4bb30f 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -664,7 +664,7 @@ dumpRoles(PGconn *conn)
 	int			i;
 
 	/* note: rolconfig is dumped later */
-	if (server_version >= 90500)
+	if (server_version >= 90600)
 		printfPQExpBuffer(buf,
 						  "SELECT oid, rolname, rolsuper, rolinherit, "
 						  "rolcreaterole, rolcreatedb, "
@@ -673,6 +673,7 @@ dumpRoles(PGconn *conn)
 			 "pg_catalog.shobj_description(oid, 'pg_authid') as rolcomment, "
 						  "rolname = current_user AS is_current_user "
 						  "FROM pg_authid "
+						  "WHERE rolname !~ '^pg_' "
 						  "ORDER BY 2");
 	else if (server_version >= 90100)
 		printfPQExpBuffer(buf,
@@ -770,6 +771,13 @@ dumpRoles(PGconn *conn)
 		auth_oid = atooid(PQgetvalue(res, i, i_oid));
 		rolename = PQgetvalue(res, i, i_rolname);
 
+		if (strncmp(rolename,"pg_",3) != 0)
+		{
+			fprintf(stderr, _("%s: role name starting with 'pg_' skipped (%s)\n"),
+					progname, rolename);
+			continue;
+		}
+
 		resetPQExpBuffer(buf);
 
 		if (binary_upgrade)
@@ -895,6 +903,7 @@ dumpRoleMembership(PGconn *conn)
 					   "LEFT JOIN pg_authid ur on ur.oid = a.roleid "
 					   "LEFT JOIN pg_authid um on um.oid = a.member "
 					   "LEFT JOIN pg_authid ug on ug.oid = a.grantor "
+					   "WHERE NOT (ur.rolname ~ '^pg_' AND um.rolname ~ '^pg_')"
 					   "ORDER BY 1,2,3");
 
 	if (PQntuples(res) > 0)
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f932094..6b6f5ba 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -24,6 +24,7 @@ static void check_for_prepared_transactions(ClusterInfo *cluster);
 static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
 static void check_for_reg_data_type_usage(ClusterInfo *cluster);
 static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
+static void check_for_pg_role_prefix(ClusterInfo *cluster);
 static void get_bin_version(ClusterInfo *cluster);
 static char *get_canonical_locale_name(int category, const char *locale);
 
@@ -96,6 +97,11 @@ check_and_dump_old_cluster(bool live_check)
 	check_for_prepared_transactions(&old_cluster);
 	check_for_reg_data_type_usage(&old_cluster);
 	check_for_isn_and_int8_passing_mismatch(&old_cluster);
+
+	/* 9.5 and below should not have roles starting with pg_ */
+	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 905)
+		check_for_pg_role_prefix(&old_cluster);
+
 	if (GET_MAJOR_VERSION(old_cluster.major_version) == 904 &&
 		old_cluster.controldata.cat_ver < JSONB_FORMAT_CHANGE_CAT_VER)
 		check_for_jsonb_9_4_usage(&old_cluster);
@@ -629,7 +635,8 @@ check_is_install_user(ClusterInfo *cluster)
 	res = executeQueryOrDie(conn,
 							"SELECT rolsuper, oid "
 							"FROM pg_catalog.pg_roles "
-							"WHERE rolname = current_user");
+							"WHERE rolname = current_user "
+							"AND rolname !~ '^pg_'");
 
 	/*
 	 * We only allow the install user in the new cluster (see comment below)
@@ -645,7 +652,8 @@ check_is_install_user(ClusterInfo *cluster)
 
 	res = executeQueryOrDie(conn,
 							"SELECT COUNT(*) "
-							"FROM pg_catalog.pg_roles ");
+							"FROM pg_catalog.pg_roles "
+							"WHERE rolname !~ '^pg_'");
 
 	if (PQntuples(res) != 1)
 		pg_fatal("could not determine the number of users\n");
@@ -1033,6 +1041,34 @@ check_for_jsonb_9_4_usage(ClusterInfo *cluster)
 		check_ok();
 }
 
+/*
+ * check_for_pg_role_prefix()
+ *
+ *	Versions older than 9.6 should not have any pg_* roles
+ */
+static void
+check_for_pg_role_prefix(ClusterInfo *cluster)
+{
+	PGresult   *res;
+	PGconn	   *conn = connectToServer(cluster, "template1");
+
+	prep_status("Checking for roles starting with 'pg_'");
+
+	res = executeQueryOrDie(conn,
+							"SELECT * "
+							"FROM pg_catalog.pg_roles "
+							"WHERE rolname ~ '^pg_'");
+
+	if (PQntuples(res) != 0)
+		pg_fatal("The %s cluster contains roles starting with 'pg_'\n",
+				 CLUSTER_NAME(cluster));
+
+	PQclear(res);
+
+	PQfinish(conn);
+
+	check_ok();
+}
 
 static void
 get_bin_version(ClusterInfo *cluster)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9750a5b..bd521ef 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -429,7 +429,7 @@ exec_command(const char *cmd,
 				break;
 			case 'g':
 				/* no longer distinct from \du */
-				success = describeRoles(pattern, show_verbose);
+				success = describeRoles(pattern, show_verbose, show_system);
 				break;
 			case 'l':
 				success = do_lo_list();
@@ -474,7 +474,7 @@ exec_command(const char *cmd,
 					success = PSQL_CMD_UNKNOWN;
 				break;
 			case 'u':
-				success = describeRoles(pattern, show_verbose);
+				success = describeRoles(pattern, show_verbose, show_system);
 				break;
 			case 'F':			/* text search subsystem */
 				switch (cmd[2])
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index fd8dc91..5722848 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2646,7 +2646,7 @@ add_tablespace_footer(printTableContent *const cont, char relkind,
  * Describes roles.  Any schema portion of the pattern is ignored.
  */
 bool
-describeRoles(const char *pattern, bool verbose)
+describeRoles(const char *pattern, bool verbose, bool showSystem)
 {
 	PQExpBufferData buf;
 	PGresult   *res;
@@ -2691,6 +2691,9 @@ describeRoles(const char *pattern, bool verbose)
 
 		appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_roles r\n");
 
+		if (!showSystem && !pattern)
+			appendPQExpBufferStr(&buf, "WHERE r.rolname !~ '^pg_'\n");
+
 		processSQLNamePattern(pset.db, &buf, pattern, false, false,
 							  NULL, "r.rolname", NULL, NULL);
 	}
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index e4fc79e..9672275 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -25,7 +25,7 @@ extern bool describeTypes(const char *pattern, bool verbose, bool showSystem);
 extern bool describeOperators(const char *pattern, bool verbose, bool showSystem);
 
 /* \du, \dg */
-extern bool describeRoles(const char *pattern, bool verbose);
+extern bool describeRoles(const char *pattern, bool verbose, bool showSystem);
 
 /* \drds */
 extern bool listDbRoleSettings(const char *pattern1, const char *pattern2);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 59f6f25..30232b5 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -227,7 +227,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\dFd[+] [PATTERN]      list text search dictionaries\n"));
 	fprintf(output, _("  \\dFp[+] [PATTERN]      list text search parsers\n"));
 	fprintf(output, _("  \\dFt[+] [PATTERN]      list text search templates\n"));
-	fprintf(output, _("  \\dg[+]  [PATTERN]      list roles\n"));
+	fprintf(output, _("  \\dg[S+] [PATTERN]      list roles\n"));
 	fprintf(output, _("  \\di[S+] [PATTERN]      list indexes\n"));
 	fprintf(output, _("  \\dl                    list large objects, same as \\lo_list\n"));
 	fprintf(output, _("  \\dL[S+] [PATTERN]      list procedural languages\n"));
@@ -240,7 +240,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\ds[S+] [PATTERN]      list sequences\n"));
 	fprintf(output, _("  \\dt[S+] [PATTERN]      list tables\n"));
 	fprintf(output, _("  \\dT[S+] [PATTERN]      list data types\n"));
-	fprintf(output, _("  \\du[+]  [PATTERN]      list roles\n"));
+	fprintf(output, _("  \\du[S+] [PATTERN]      list roles\n"));
 	fprintf(output, _("  \\dv[S+] [PATTERN]      list views\n"));
 	fprintf(output, _("  \\dE[S+] [PATTERN]      list foreign tables\n"));
 	fprintf(output, _("  \\dx[+]  [PATTERN]      list extensions\n"));
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 4e15a14..d91437b 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -231,6 +231,7 @@ extern void check_is_member_of_role(Oid member, Oid role);
 extern Oid	get_role_oid(const char *rolename, bool missing_ok);
 extern Oid	get_role_oid_or_public(const char *rolename);
 extern Oid	get_rolespec_oid(const Node *node, bool missing_ok);
+extern void	check_rolespec_name(const Node *node, const char *detail_msg);
 extern HeapTuple get_rolespec_tuple(const Node *node);
 extern char *get_rolespec_name(const Node *node);
 
diff --git a/src/test/regress/expected/rolenames.out b/src/test/regress/expected/rolenames.out
index 8f88c02..c9be282 100644
--- a/src/test/regress/expected/rolenames.out
+++ b/src/test/regress/expected/rolenames.out
@@ -78,6 +78,18 @@ CREATE ROLE "none"; -- error
 ERROR:  role name "none" is reserved
 LINE 1: CREATE ROLE "none";
                     ^
+CREATE ROLE pg_abc; -- error
+ERROR:  role name "pg_abc" is reserved
+DETAIL:  Role names starting with "pg_" are reserved.
+CREATE ROLE "pg_abc"; -- error
+ERROR:  role name "pg_abc" is reserved
+DETAIL:  Role names starting with "pg_" are reserved.
+CREATE ROLE pg_backup; -- error
+ERROR:  role name "pg_backup" is reserved
+DETAIL:  Role names starting with "pg_" are reserved.
+CREATE ROLE "pg_backup"; -- error
+ERROR:  role name "pg_backup" is reserved
+DETAIL:  Role names starting with "pg_" are reserved.
 CREATE ROLE testrol0 SUPERUSER LOGIN;
 CREATE ROLE testrolx SUPERUSER LOGIN;
 CREATE ROLE testrol2 SUPERUSER;
@@ -804,6 +816,12 @@ LINE 1: DROP USER MAPPING IF EXISTS FOR CURRENT_ROLE SERVER sv9;
 DROP USER MAPPING IF EXISTS FOR nonexistent SERVER sv9;  -- error
 NOTICE:  role "nonexistent" does not exist, skipping
 -- GRANT/REVOKE
+GRANT testrol0 TO pg_backup; -- error
+ERROR:  role "pg_backup" is reserved
+DETAIL:  Cannot GRANT roles to a reserved role.
+GRANT pg_backup TO pg_monitor; -- error
+ERROR:  role "pg_monitor" is reserved
+DETAIL:  Cannot GRANT roles to a reserved role.
 UPDATE pg_proc SET proacl = null WHERE proname LIKE 'testagg_';
 SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_';
  proname  | proacl 
diff --git a/src/test/regress/sql/rolenames.sql b/src/test/regress/sql/rolenames.sql
index e8c6b33..65c97ec 100644
--- a/src/test/regress/sql/rolenames.sql
+++ b/src/test/regress/sql/rolenames.sql
@@ -57,6 +57,11 @@ CREATE ROLE "public"; -- error
 CREATE ROLE none; -- error
 CREATE ROLE "none"; -- error
 
+CREATE ROLE pg_abc; -- error
+CREATE ROLE "pg_abc"; -- error
+CREATE ROLE pg_backup; -- error
+CREATE ROLE "pg_backup"; -- error
+
 CREATE ROLE testrol0 SUPERUSER LOGIN;
 CREATE ROLE testrolx SUPERUSER LOGIN;
 CREATE ROLE testrol2 SUPERUSER;
@@ -376,6 +381,9 @@ DROP USER MAPPING IF EXISTS FOR CURRENT_ROLE SERVER sv9; --error
 DROP USER MAPPING IF EXISTS FOR nonexistent SERVER sv9;  -- error
 
 -- GRANT/REVOKE
+GRANT testrol0 TO pg_backup; -- error
+GRANT pg_backup TO pg_monitor; -- error
+
 UPDATE pg_proc SET proacl = null WHERE proname LIKE 'testagg_';
 SELECT proname, proacl FROM pg_proc WHERE proname LIKE 'testagg_';
 
-- 
2.5.0


From f6c227203ea8f68a92154224db0b3d3d9fb99f88 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Wed, 30 Sep 2015 07:08:03 -0400
Subject: [PATCH 2/2] Create default roles

This creates an initial set of default roles which administrators may
use to grant access to, historically, superuser-only functions.  Using
these roles instead of granting superuser access reduces the number of
superuser roles required for a system.  Documention for each of the
default roles has been added to user-manag.sgml.
---
 doc/src/sgml/func.sgml          |  8 ++++---
 doc/src/sgml/user-manag.sgml    | 51 +++++++++++++++++++++++++++++++++++++++++
 src/backend/utils/adt/misc.c    |  8 ++++---
 src/include/catalog/pg_authid.h |  8 ++++++-
 4 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 000489d..24248fd 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17296,7 +17296,8 @@ SELECT set_config('log_statement_stats', 'off', false);
         </entry>
        <entry><type>boolean</type></entry>
        <entry>Cancel a backend's current query.  This is also allowed if the
-        calling role is a member of the role whose backend is being canceled,
+        calling role is a member of the role whose backend is being canceled or
+        the calling role has been granted <literal>pg_signal_backend</literal>,
         however only superusers can cancel superuser backends.
         </entry>
       </row>
@@ -17320,8 +17321,9 @@ SELECT set_config('log_statement_stats', 'off', false);
         </entry>
        <entry><type>boolean</type></entry>
        <entry>Terminate a backend.  This is also allowed if the calling role
-        is a member of the role whose backend is being terminated, however only
-        superusers can terminate superuser backends.
+        is a member of the role whose backend is being terminated or the
+        calling role has been granted <literal>pg_signal_backend</literal>,
+        however only superusers can terminate superuser backends.
        </entry>
       </row>
      </tbody>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index d1b6e59..7eaefe5 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -483,6 +483,57 @@ DROP ROLE doomed_role;
   </para>
  </sect1>
 
+ <sect1 id="default-roles">
+  <title>Default Roles</title>
+
+  <indexterm zone="default-roles">
+   <primary>role</>
+  </indexterm>
+
+  <para>
+   <productname>PostgreSQL</productname> provides a set of default roles
+   which provide access to certain, commonly needed, privileged capabilities
+   and information.  Administrators can GRANT these roles to users and/or
+   other roles in their environment, providing those users with access to
+   the specified capabilities and information.
+  </para>
+
+  <para>
+   The default roles are described in <xref linkend="default-roles-table">.
+   Note that the specific permissions for each of the default roles may
+   change in the future as additional capabilities are added.  Administrators
+   should monitor the release notes for changes.
+  </para>
+
+   <table tocentry="1" id="default-roles-table">
+    <title>Default Roles</title>
+    <tgroup cols="2">
+     <thead>
+      <row>
+       <entry>Role</entry>
+       <entry>Allowed Access</entry>
+      </row>
+     </thead>
+     <tbody>
+      <row>
+       <entry>pg_signal_backend</entry>
+       <entry>Send signals to other backends (eg: cancel query, terminate).</entry>
+      </row>
+     </tbody>
+    </tgroup>
+   </table>
+
+  <para>
+   Administrators can grant access to these roles to users using the GRANT
+   command:
+
+<programlisting>
+GRANT pg_signal_backend TO admin_user;
+</programlisting>
+  </para>
+
+ </sect1>
+
  <sect1 id="perm-functions">
   <title>Function and Trigger Security</title>
 
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 43f36db..28feb55 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -21,6 +21,7 @@
 #include <unistd.h>
 
 #include "access/sysattr.h"
+#include "catalog/pg_authid.h"
 #include "catalog/catalog.h"
 #include "catalog/pg_tablespace.h"
 #include "catalog/pg_type.h"
@@ -243,7 +244,8 @@ pg_signal_backend(int pid, int sig)
 		return SIGNAL_BACKEND_NOSUPERUSER;
 
 	/* Users can signal backends they have role membership in. */
-	if (!has_privs_of_role(GetUserId(), proc->roleId))
+	if (!has_privs_of_role(GetUserId(), proc->roleId) &&
+		!has_privs_of_role(GetUserId(), DEFAULT_ROLE_SIGNAL_BACKENDID))
 		return SIGNAL_BACKEND_NOPERMISSION;
 
 	/*
@@ -289,7 +291,7 @@ pg_cancel_backend(PG_FUNCTION_ARGS)
 	if (r == SIGNAL_BACKEND_NOPERMISSION)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 (errmsg("must be a member of the role whose query is being canceled"))));
+				 (errmsg("must be a member of the role whose query is being canceled or member of pg_signal_backend"))));
 
 	PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
 }
@@ -313,7 +315,7 @@ pg_terminate_backend(PG_FUNCTION_ARGS)
 	if (r == SIGNAL_BACKEND_NOPERMISSION)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 (errmsg("must be a member of the role whose process is being terminated"))));
+				 (errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend"))));
 
 	PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
 }
diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
index c163083..533081d 100644
--- a/src/include/catalog/pg_authid.h
+++ b/src/include/catalog/pg_authid.h
@@ -93,10 +93,16 @@ typedef FormData_pg_authid *Form_pg_authid;
  *
  * The uppercase quantities will be replaced at initdb time with
  * user choices.
+ *
+ * If adding new default roles or changing the OIDs below, be sure to add or
+ * update the #defines which follow as appropriate.
  * ----------------
  */
 DATA(insert OID = 10 ( "POSTGRES" t t t t t t t -1 _null_ _null_));
+DATA(insert OID = 4200 ( "pg_signal_backend" f t f f f f f -1 _null_ _null_));
+
+#define BOOTSTRAP_SUPERUSERID			10
 
-#define BOOTSTRAP_SUPERUSERID 10
+#define DEFAULT_ROLE_SIGNAL_BACKENDID	4200
 
 #endif   /* PG_AUTHID_H */
-- 
2.5.0

Attachment: signature.asc
Description: Digital signature

Reply via email to