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

application/octet-stream

Filename: v4-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 v4-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 3ce6981776a708f4c865e04cfb05b0357f39c1d6 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 4 Jan 2022 19:51:44 -0800
Subject: [PATCH v4 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 44f3f54b15..c70ee87f9d 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -1481,21 +1481,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);
 
@@ -1571,12 +1556,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 9ff0c091a9..a20778299d 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -940,14 +940,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)
@@ -963,17 +961,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 b9d3ffd1c5..21b805f6de 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)