diff options
| author | Tom Lane | 2024-04-29 23:26:19 +0000 |
|---|---|---|
| committer | Tom Lane | 2024-04-29 23:26:19 +0000 |
| commit | 534287403914cc9760db98f7320ac4e92f5d416e (patch) | |
| tree | 69106855f7d48855411ba4703e120fe3aa155441 /src/include | |
| parent | dd0183469bb779247c96e86c2272dca7ff4ec9e7 (diff) | |
Fix failure to track role dependencies of pg_init_privs entries.
If an ACL recorded in pg_init_privs mentions a non-pinned role,
that reference must also be noted in pg_shdepend so that we know
that the role can't go away without removing the ACL reference.
Otherwise, DROP ROLE could succeed and leave dangling entries
behind, which is what's causing the recent upgrade-check failures
on buildfarm member copperhead.
This has been wrong since pg_init_privs was introduced, but it's
escaped notice because typical pg_init_privs entries would only
mention the bootstrap superuser (pinned) or at worst the owner
of the extension (who can't go away before the extension does).
We lack even a representation of such a role reference for
pg_shdepend. My first thought for a solution was entries listing
pg_init_privs in classid, but that doesn't work because then there's
noplace to put the granted-on object's classid. Rather than adding
a new column to pg_shdepend, let's add a new deptype code
SHARED_DEPENDENCY_INITACL. Much of the associated boilerplate
code can be cribbed from code for SHARED_DEPENDENCY_ACL.
A lot of the bulk of this patch just stems from the new need to pass
the object's owner ID to recordExtensionInitPriv, so that we can
consult it while updating pg_shdepend. While many callers have that
at hand already, a few places now need to fetch the owner ID of an
arbitrary privilege-bearing object. For that, we assume that there
is a catcache on the relevant catalog's OID column, which is an
assumption already made in ExecGrant_common so it seems okay here.
We do need an entirely new routine RemoveRoleFromInitPriv to perform
cleanup of pg_init_privs ACLs during DROP OWNED BY. It's analogous
to RemoveRoleFromObjectACL, but we can't share logic because that
function operates by building a command parsetree and invoking
existing GRANT/REVOKE infrastructure. There is of course no SQL
command that would update pg_init_privs entries when we're not in
process of creating their extension, so we need a routine that can
do the updates directly.
catversion bump because this changes the expected contents of
pg_shdepend. For the same reason, there's no hope of back-patching
this, even though it fixes a longstanding bug. Fortunately, the
case where it's a problem seems to be near nonexistent in the field.
If it weren't for the buildfarm breakage, I'd have been content to
leave this for v18.
Patch by me; thanks to Daniel Gustafsson for review and discussion.
Discussion: https://postgr.es/m/1745535.1712358659@sss.pgh.pa.us
Diffstat (limited to 'src/include')
| -rw-r--r-- | src/include/catalog/catversion.h | 2 | ||||
| -rw-r--r-- | src/include/catalog/dependency.h | 16 | ||||
| -rw-r--r-- | src/include/utils/acl.h | 2 |
3 files changed, 17 insertions, 3 deletions
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 950f00bed48..8618ea59076 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202404021 +#define CATALOG_VERSION_NO 202404291 #endif diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h index ec654010d43..e0dcc0b069e 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -56,11 +56,17 @@ typedef enum DependencyType * created for the owner of an object; hence two objects may be linked by * one or the other, but not both, of these dependency types.) * - * (c) a SHARED_DEPENDENCY_POLICY entry means that the referenced object is + * (c) a SHARED_DEPENDENCY_INITACL entry means that the referenced object is + * a role mentioned in a pg_init_privs entry for the dependent object. The + * referenced object must be a pg_authid entry. (SHARED_DEPENDENCY_INITACL + * entries are not created for the owner of an object; hence two objects may + * be linked by one or the other, but not both, of these dependency types.) + * + * (d) a SHARED_DEPENDENCY_POLICY entry means that the referenced object is * a role mentioned in a policy object. The referenced object must be a * pg_authid entry. * - * (d) a SHARED_DEPENDENCY_TABLESPACE entry means that the referenced + * (e) a SHARED_DEPENDENCY_TABLESPACE entry means that the referenced * object is a tablespace mentioned in a relation without storage. The * referenced object must be a pg_tablespace entry. (Relations that have * storage don't need this: they are protected by the existence of a physical @@ -73,6 +79,7 @@ typedef enum SharedDependencyType { SHARED_DEPENDENCY_OWNER = 'o', SHARED_DEPENDENCY_ACL = 'a', + SHARED_DEPENDENCY_INITACL = 'i', SHARED_DEPENDENCY_POLICY = 'r', SHARED_DEPENDENCY_TABLESPACE = 't', SHARED_DEPENDENCY_INVALID = 0, @@ -201,6 +208,11 @@ extern void updateAclDependencies(Oid classId, Oid objectId, int32 objsubId, int noldmembers, Oid *oldmembers, int nnewmembers, Oid *newmembers); +extern void updateInitAclDependencies(Oid classId, Oid objectId, int32 objsubId, + Oid ownerId, + int noldmembers, Oid *oldmembers, + int nnewmembers, Oid *newmembers); + extern bool checkSharedDependencies(Oid classId, Oid objectId, char **detail_msg, char **detail_log_msg); diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 3a0baf30395..1a554c66997 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -276,6 +276,8 @@ extern void aclcheck_error_type(AclResult aclerr, Oid typeOid); extern void recordExtObjInitPriv(Oid objoid, Oid classoid); extern void removeExtObjInitPriv(Oid objoid, Oid classoid); +extern void RemoveRoleFromInitPriv(Oid roleid, + Oid classid, Oid objid, int32 objsubid); /* ownercheck routines just return true (owner) or false (not) */ |
