v6-0005-Remove-grantor-field-from-pg_auth_members.patch

application/octet-stream

Filename: v6-0005-Remove-grantor-field-from-pg_auth_members.patch
Type: application/octet-stream
Part: 4
Message: Re: CREATEROLE and role ownership hierarchies

Patch

Format: format-patch
Series: patch v6-0005
Subject: Remove grantor field from pg_auth_members
File+
src/backend/commands/user.c 0 17
src/bin/pg_dump/pg_dumpall.c 2 15
src/include/catalog/pg_auth_members.h 0 1
src/test/regress/expected/oidjoins.out 0 1
From 13000e50d611071eafc0b5bebfa33c2cedec1f90 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 11 Jan 2022 10:26:05 -0800
Subject: [PATCH v6 5/5] Remove grantor field from pg_auth_members

The "grantor" field of pg_auth_members is unused and, more to the
point, unreliable.  The system does not avoid dangling references
from the grantor field to dropped roles in pg_authid.  This creates
the risk that, if an oid is reused for a new role, the grantor field
may point to a role other than the one that performed the grant.

We could fix the bug, but there is no clear solution to the problem
that existing installations may have broken data.  Since the field
is not used for any purpose, removing it seems the best option.
---
 src/backend/commands/user.c            | 17 -----------------
 src/bin/pg_dump/pg_dumpall.c           | 17 ++---------------
 src/include/catalog/pg_auth_members.h  |  1 -
 src/test/regress/expected/oidjoins.out |  1 -
 4 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index cc33277c20..b36b00d46c 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -1455,21 +1455,6 @@ AddRoleMems(const char *rolename, Oid roleid,
 		ereport(ERROR,
 				errmsg("role \"%s\" cannot have explicit members", rolename));
 
-	/*
-	 * The role membership grantor of record has little significance at
-	 * present.  Nonetheless, inasmuch as users might look to it for a crude
-	 * audit trail, let only superusers impute the grant to a third party.
-	 *
-	 * Before lifting this restriction, give the member == role case of
-	 * is_admin_of_role() a fresh look.  Ensure that the current role cannot
-	 * use an explicit grantor specification to take advantage of the session
-	 * user's self-admin right.
-	 */
-	if (grantorId != GetUserId() && !superuser())
-		ereport(ERROR,
-				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("must be superuser to set grantor")));
-
 	pg_authmem_rel = table_open(AuthMemRelationId, RowExclusiveLock);
 	pg_authmem_dsc = RelationGetDescr(pg_authmem_rel);
 
@@ -1545,12 +1530,10 @@ AddRoleMems(const char *rolename, Oid roleid,
 
 		new_record[Anum_pg_auth_members_roleid - 1] = ObjectIdGetDatum(roleid);
 		new_record[Anum_pg_auth_members_member - 1] = ObjectIdGetDatum(memberid);
-		new_record[Anum_pg_auth_members_grantor - 1] = ObjectIdGetDatum(grantorId);
 		new_record[Anum_pg_auth_members_admin_option - 1] = BoolGetDatum(admin_opt);
 
 		if (HeapTupleIsValid(authmem_tuple))
 		{
-			new_record_repl[Anum_pg_auth_members_grantor - 1] = true;
 			new_record_repl[Anum_pg_auth_members_admin_option - 1] = true;
 			tuple = heap_modify_tuple(authmem_tuple, pg_authmem_dsc,
 									  new_record,
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 10383c713f..cfc997bb73 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -945,14 +945,12 @@ dumpRoleMembership(PGconn *conn)
 
 	printfPQExpBuffer(buf, "SELECT ur.rolname AS roleid, "
 					  "um.rolname AS member, "
-					  "a.admin_option, "
-					  "ug.rolname AS grantor "
+					  "a.admin_option "
 					  "FROM pg_auth_members a "
 					  "LEFT JOIN %s ur on ur.oid = a.roleid "
 					  "LEFT JOIN %s um on um.oid = a.member "
-					  "LEFT JOIN %s ug on ug.oid = a.grantor "
 					  "WHERE NOT (ur.rolname ~ '^pg_' AND um.rolname ~ '^pg_')"
-					  "ORDER BY 1,2,3", role_catalog, role_catalog, role_catalog);
+					  "ORDER BY 1,2,3", role_catalog, role_catalog);
 	res = executeQuery(conn, buf->data);
 
 	if (PQntuples(res) > 0)
@@ -968,17 +966,6 @@ dumpRoleMembership(PGconn *conn)
 		fprintf(OPF, " TO %s", fmtId(member));
 		if (*option == 't')
 			fprintf(OPF, " WITH ADMIN OPTION");
-
-		/*
-		 * We don't track the grantor very carefully in the backend, so cope
-		 * with the possibility that it has been dropped.
-		 */
-		if (!PQgetisnull(res, i, 3))
-		{
-			char	   *grantor = PQgetvalue(res, i, 3);
-
-			fprintf(OPF, " GRANTED BY %s", fmtId(grantor));
-		}
 		fprintf(OPF, ";\n");
 	}
 
diff --git a/src/include/catalog/pg_auth_members.h b/src/include/catalog/pg_auth_members.h
index 1bc027f133..c873201688 100644
--- a/src/include/catalog/pg_auth_members.h
+++ b/src/include/catalog/pg_auth_members.h
@@ -31,7 +31,6 @@ CATALOG(pg_auth_members,1261,AuthMemRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_
 {
 	Oid			roleid BKI_LOOKUP(pg_authid);	/* ID of a role */
 	Oid			member BKI_LOOKUP(pg_authid);	/* ID of a member of that role */
-	Oid			grantor BKI_LOOKUP(pg_authid);	/* who granted the membership */
 	bool		admin_option;	/* granted with admin option? */
 } FormData_pg_auth_members;
 
diff --git a/src/test/regress/expected/oidjoins.out b/src/test/regress/expected/oidjoins.out
index 266a30a85b..a6a73df7ff 100644
--- a/src/test/regress/expected/oidjoins.out
+++ b/src/test/regress/expected/oidjoins.out
@@ -197,7 +197,6 @@ NOTICE:  checking pg_tablespace {spcowner} => pg_authid {oid}
 NOTICE:  checking pg_authid {rolowner} => pg_authid {oid}
 NOTICE:  checking pg_auth_members {roleid} => pg_authid {oid}
 NOTICE:  checking pg_auth_members {member} => pg_authid {oid}
-NOTICE:  checking pg_auth_members {grantor} => pg_authid {oid}
 NOTICE:  checking pg_shdepend {dbid} => pg_database {oid}
 NOTICE:  checking pg_shdepend {classid} => pg_class {oid}
 NOTICE:  checking pg_shdepend {refclassid} => pg_class {oid}
-- 
2.21.1 (Apple Git-122.3)