From 534287403914cc9760db98f7320ac4e92f5d416e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 29 Apr 2024 19:26:19 -0400 Subject: [PATCH] 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 --- doc/src/sgml/catalogs.sgml | 14 + src/backend/catalog/aclchk.c | 262 ++++++++++++++++-- src/backend/catalog/pg_shdepend.c | 56 +++- src/include/catalog/catversion.h | 2 +- src/include/catalog/dependency.h | 16 +- src/include/utils/acl.h | 2 + .../test_pg_dump/expected/test_pg_dump.out | 183 ++++++++++++ .../modules/test_pg_dump/sql/test_pg_dump.sql | 37 +++ 8 files changed, 549 insertions(+), 23 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 2907079e2a..c8cb46c5b9 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -7184,6 +7184,20 @@ SCRAM-SHA-256$<iteration count>:&l + + SHARED_DEPENDENCY_INITACL (i) + + + The referenced object (which must be a role) is mentioned in a + pg_init_privs + entry for the dependent object. + (A SHARED_DEPENDENCY_INITACL entry is not made for + the owner of the object, since the owner will have + a SHARED_DEPENDENCY_OWNER entry anyway.) + + + + SHARED_DEPENDENCY_POLICY (r) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 7abf3c2a74..e6cc720579 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -165,9 +165,9 @@ static AclMode pg_type_aclmask_ext(Oid type_oid, Oid roleid, AclMode mask, AclMaskHow how, bool *is_missing); static void recordExtensionInitPriv(Oid objoid, Oid classoid, int objsubid, - Acl *new_acl); + Oid ownerId, Acl *new_acl); static void recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, - Acl *new_acl); + Oid ownerId, Acl *new_acl); /* @@ -1447,7 +1447,19 @@ SetDefaultACL(InternalDefaultACL *iacls) /* * RemoveRoleFromObjectACL * - * Used by shdepDropOwned to remove mentions of a role in ACLs + * Used by shdepDropOwned to remove mentions of a role in ACLs. + * + * Notice that this doesn't accept an objsubid parameter, which is a bit bogus + * since the pg_shdepend record that caused us to call it certainly had one. + * If, for example, pg_shdepend records the existence of a permission on + * mytable.mycol, this function will effectively issue a REVOKE ALL ON TABLE + * mytable. That gets the job done because (per SQL spec) such a REVOKE also + * revokes per-column permissions. We could not recreate a situation where + * the role has table-level but not column-level permissions; but it's okay + * (for now anyway) because this is only used when we're dropping the role + * and so all its permissions everywhere must go away. At worst it's a bit + * inefficient if the role has column permissions on several columns of the + * same table. */ void RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid) @@ -1790,7 +1802,7 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid, const char *relname, CatalogTupleUpdate(attRelation, &newtuple->t_self, newtuple); /* Update initial privileges for extensions */ - recordExtensionInitPriv(relOid, RelationRelationId, attnum, + recordExtensionInitPriv(relOid, RelationRelationId, attnum, ownerId, ACL_NUM(new_acl) > 0 ? new_acl : NULL); /* Update the shared dependency ACL info */ @@ -2050,7 +2062,8 @@ ExecGrant_Relation(InternalGrant *istmt) CatalogTupleUpdate(relation, &newtuple->t_self, newtuple); /* Update initial privileges for extensions */ - recordExtensionInitPriv(relOid, RelationRelationId, 0, new_acl); + recordExtensionInitPriv(relOid, RelationRelationId, 0, + ownerId, new_acl); /* Update the shared dependency ACL info */ updateAclDependencies(RelationRelationId, relOid, 0, @@ -2251,7 +2264,7 @@ ExecGrant_common(InternalGrant *istmt, Oid classid, AclMode default_privs, CatalogTupleUpdate(relation, &newtuple->t_self, newtuple); /* Update initial privileges for extensions */ - recordExtensionInitPriv(objectid, classid, 0, new_acl); + recordExtensionInitPriv(objectid, classid, 0, ownerId, new_acl); /* Update the shared dependency ACL info */ updateAclDependencies(classid, @@ -2403,7 +2416,8 @@ ExecGrant_Largeobject(InternalGrant *istmt) CatalogTupleUpdate(relation, &newtuple->t_self, newtuple); /* Update initial privileges for extensions */ - recordExtensionInitPriv(loid, LargeObjectRelationId, 0, new_acl); + recordExtensionInitPriv(loid, LargeObjectRelationId, 0, + ownerId, new_acl); /* Update the shared dependency ACL info */ updateAclDependencies(LargeObjectRelationId, @@ -2575,7 +2589,7 @@ ExecGrant_Parameter(InternalGrant *istmt) /* Update initial privileges for extensions */ recordExtensionInitPriv(parameterId, ParameterAclRelationId, 0, - new_acl); + ownerId, new_acl); /* Update the shared dependency ACL info */ updateAclDependencies(ParameterAclRelationId, parameterId, 0, @@ -4463,6 +4477,7 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) } recordExtensionInitPrivWorker(objoid, classoid, curr_att, + pg_class_tuple->relowner, DatumGetAclP(attaclDatum)); ReleaseSysCache(attTuple); @@ -4475,6 +4490,7 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) /* Add the record, if any, for the top-level object */ if (!isNull) recordExtensionInitPrivWorker(objoid, classoid, 0, + pg_class_tuple->relowner, DatumGetAclP(aclDatum)); ReleaseSysCache(tuple); @@ -4485,6 +4501,7 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) Datum aclDatum; bool isNull; HeapTuple tuple; + Form_pg_largeobject_metadata form_lo_meta; ScanKeyData entry[1]; SysScanDesc scan; Relation relation; @@ -4509,6 +4526,7 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) tuple = systable_getnext(scan); if (!HeapTupleIsValid(tuple)) elog(ERROR, "could not find tuple for large object %u", objoid); + form_lo_meta = (Form_pg_largeobject_metadata) GETSTRUCT(tuple); aclDatum = heap_getattr(tuple, Anum_pg_largeobject_metadata_lomacl, @@ -4517,6 +4535,7 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) /* Add the record, if any, for the top-level object */ if (!isNull) recordExtensionInitPrivWorker(objoid, classoid, 0, + form_lo_meta->lomowner, DatumGetAclP(aclDatum)); systable_endscan(scan); @@ -4524,24 +4543,29 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) /* This will error on unsupported classoid. */ else if (get_object_attnum_acl(classoid) != InvalidAttrNumber) { + int cacheid; + Oid ownerId; Datum aclDatum; bool isNull; HeapTuple tuple; - tuple = SearchSysCache1(get_object_catcache_oid(classoid), - ObjectIdGetDatum(objoid)); + cacheid = get_object_catcache_oid(classoid); + tuple = SearchSysCache1(cacheid, ObjectIdGetDatum(objoid)); if (!HeapTupleIsValid(tuple)) elog(ERROR, "cache lookup failed for %s %u", get_object_class_descr(classoid), objoid); - aclDatum = SysCacheGetAttr(get_object_catcache_oid(classoid), tuple, + ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid, + tuple, + get_object_attnum_owner(classoid))); + aclDatum = SysCacheGetAttr(cacheid, tuple, get_object_attnum_acl(classoid), &isNull); /* Add the record, if any, for the top-level object */ if (!isNull) recordExtensionInitPrivWorker(objoid, classoid, 0, - DatumGetAclP(aclDatum)); + ownerId, DatumGetAclP(aclDatum)); ReleaseSysCache(tuple); } @@ -4554,6 +4578,8 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) void removeExtObjInitPriv(Oid objoid, Oid classoid) { + Oid ownerId; + /* * If this is a relation then we need to see if there are any sub-objects * (eg: columns) for it and, if so, be sure to call @@ -4568,6 +4594,7 @@ removeExtObjInitPriv(Oid objoid, Oid classoid) if (!HeapTupleIsValid(tuple)) elog(ERROR, "cache lookup failed for relation %u", objoid); pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple); + ownerId = pg_class_tuple->relowner; /* * Indexes don't have permissions, neither do the pg_class rows for @@ -4604,7 +4631,8 @@ removeExtObjInitPriv(Oid objoid, Oid classoid) /* when removing, remove all entries, even dropped columns */ - recordExtensionInitPrivWorker(objoid, classoid, curr_att, NULL); + recordExtensionInitPrivWorker(objoid, classoid, curr_att, + ownerId, NULL); ReleaseSysCache(attTuple); } @@ -4612,9 +4640,35 @@ removeExtObjInitPriv(Oid objoid, Oid classoid) ReleaseSysCache(tuple); } + else + { + /* Must find out the owner's OID the hard way */ + AttrNumber ownerattnum; + int cacheid; + HeapTuple tuple; + + /* + * If the object is of a kind that has no owner, it should not have + * any pg_init_privs entry either. + */ + ownerattnum = get_object_attnum_owner(classoid); + if (ownerattnum == InvalidAttrNumber) + return; + cacheid = get_object_catcache_oid(classoid); + tuple = SearchSysCache1(cacheid, ObjectIdGetDatum(objoid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for %s %u", + get_object_class_descr(classoid), objoid); + + ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid, + tuple, + ownerattnum)); + + ReleaseSysCache(tuple); + } /* Remove the record, if any, for the top-level object */ - recordExtensionInitPrivWorker(objoid, classoid, 0, NULL); + recordExtensionInitPrivWorker(objoid, classoid, 0, ownerId, NULL); } /* @@ -4626,7 +4680,8 @@ removeExtObjInitPriv(Oid objoid, Oid classoid) * Pass in the object OID, the OID of the class (the OID of the table which * the object is defined in) and the 'sub' id of the object (objsubid), if * any. If there is no 'sub' id (they are currently only used for columns of - * tables) then pass in '0'. Finally, pass in the complete ACL to store. + * tables) then pass in '0'. Also pass the OID of the object's owner. + * Finally, pass in the complete ACL to store. * * If an ACL already exists for this object/sub-object then we will replace * it with what is passed in. @@ -4635,7 +4690,8 @@ removeExtObjInitPriv(Oid objoid, Oid classoid) * removed, if one is found. */ static void -recordExtensionInitPriv(Oid objoid, Oid classoid, int objsubid, Acl *new_acl) +recordExtensionInitPriv(Oid objoid, Oid classoid, int objsubid, + Oid ownerId, Acl *new_acl) { /* * Generally, we only record the initial privileges when an extension is @@ -4648,7 +4704,7 @@ recordExtensionInitPriv(Oid objoid, Oid classoid, int objsubid, Acl *new_acl) if (!creating_extension && !binary_upgrade_record_init_privs) return; - recordExtensionInitPrivWorker(objoid, classoid, objsubid, new_acl); + recordExtensionInitPrivWorker(objoid, classoid, objsubid, ownerId, new_acl); } /* @@ -4664,14 +4720,23 @@ recordExtensionInitPriv(Oid objoid, Oid classoid, int objsubid, Acl *new_acl) * EXTENSION ... ADD/DROP. */ static void -recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, Acl *new_acl) +recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, + Oid ownerId, Acl *new_acl) { Relation relation; ScanKeyData key[3]; SysScanDesc scan; HeapTuple tuple; HeapTuple oldtuple; + int noldmembers; + int nnewmembers; + Oid *oldmembers; + Oid *newmembers; + /* We'll need the role membership of the new ACL. */ + nnewmembers = aclmembers(new_acl, &newmembers); + + /* Search pg_init_privs for an existing entry. */ relation = table_open(InitPrivsRelationId, RowExclusiveLock); ScanKeyInit(&key[0], @@ -4699,6 +4764,23 @@ recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, Acl *new_a Datum values[Natts_pg_init_privs] = {0}; bool nulls[Natts_pg_init_privs] = {0}; bool replace[Natts_pg_init_privs] = {0}; + Datum oldAclDatum; + bool isNull; + Acl *old_acl; + + /* Update pg_shdepend for roles mentioned in the old/new ACLs. */ + oldAclDatum = heap_getattr(oldtuple, Anum_pg_init_privs_initprivs, + RelationGetDescr(relation), &isNull); + if (!isNull) + old_acl = DatumGetAclP(oldAclDatum); + else + old_acl = NULL; /* this case shouldn't happen, probably */ + noldmembers = aclmembers(old_acl, &oldmembers); + + updateInitAclDependencies(classoid, objoid, objsubid, + ownerId, + noldmembers, oldmembers, + nnewmembers, newmembers); /* If we have a new ACL to set, then update the row with it. */ if (new_acl) @@ -4744,6 +4826,15 @@ recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, Acl *new_a tuple = heap_form_tuple(RelationGetDescr(relation), values, nulls); CatalogTupleInsert(relation, tuple); + + /* Update pg_shdepend, too. */ + noldmembers = 0; + oldmembers = NULL; + + updateInitAclDependencies(classoid, objoid, objsubid, + ownerId, + noldmembers, oldmembers, + nnewmembers, newmembers); } } @@ -4754,3 +4845,138 @@ recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, Acl *new_a table_close(relation, RowExclusiveLock); } + +/* + * RemoveRoleFromInitPriv + * + * Used by shdepDropOwned to remove mentions of a role in pg_init_privs. + */ +void +RemoveRoleFromInitPriv(Oid roleid, Oid classid, Oid objid, int32 objsubid) +{ + Relation rel; + ScanKeyData key[3]; + SysScanDesc scan; + HeapTuple oldtuple; + int cacheid; + HeapTuple objtuple; + Oid ownerId; + Datum oldAclDatum; + bool isNull; + Acl *old_acl; + Acl *new_acl; + HeapTuple newtuple; + int noldmembers; + int nnewmembers; + Oid *oldmembers; + Oid *newmembers; + + /* Search for existing pg_init_privs entry for the target object. */ + rel = table_open(InitPrivsRelationId, RowExclusiveLock); + + ScanKeyInit(&key[0], + Anum_pg_init_privs_objoid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(objid)); + ScanKeyInit(&key[1], + Anum_pg_init_privs_classoid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(classid)); + ScanKeyInit(&key[2], + Anum_pg_init_privs_objsubid, + BTEqualStrategyNumber, F_INT4EQ, + Int32GetDatum(objsubid)); + + scan = systable_beginscan(rel, InitPrivsObjIndexId, true, + NULL, 3, key); + + /* There should exist only one entry or none. */ + oldtuple = systable_getnext(scan); + + if (!HeapTupleIsValid(oldtuple)) + { + /* + * Hmm, why are we here if there's no entry? But pack up and go away + * quietly. + */ + systable_endscan(scan); + table_close(rel, RowExclusiveLock); + return; + } + + /* Get a writable copy of the existing ACL. */ + oldAclDatum = heap_getattr(oldtuple, Anum_pg_init_privs_initprivs, + RelationGetDescr(rel), &isNull); + if (!isNull) + old_acl = DatumGetAclPCopy(oldAclDatum); + else + old_acl = NULL; /* this case shouldn't happen, probably */ + + /* + * We need the members of both old and new ACLs so we can correct the + * shared dependency information. Collect data before + * merge_acl_with_grant throws away old_acl. + */ + noldmembers = aclmembers(old_acl, &oldmembers); + + /* Must find out the owner's OID the hard way. */ + cacheid = get_object_catcache_oid(classid); + objtuple = SearchSysCache1(cacheid, ObjectIdGetDatum(objid)); + if (!HeapTupleIsValid(objtuple)) + elog(ERROR, "cache lookup failed for %s %u", + get_object_class_descr(classid), objid); + + ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid, + objtuple, + get_object_attnum_owner(classid))); + ReleaseSysCache(objtuple); + + /* + * Generate new ACL. Grantor of rights is always the same as the owner. + */ + new_acl = merge_acl_with_grant(old_acl, + false, /* is_grant */ + false, /* grant_option */ + DROP_RESTRICT, + list_make1_oid(roleid), + ACLITEM_ALL_PRIV_BITS, + ownerId, + ownerId); + + /* If we end with an empty ACL, delete the pg_init_privs entry. */ + if (new_acl == NULL || ACL_NUM(new_acl) == 0) + { + CatalogTupleDelete(rel, &oldtuple->t_self); + } + else + { + Datum values[Natts_pg_init_privs] = {0}; + bool nulls[Natts_pg_init_privs] = {0}; + bool replaces[Natts_pg_init_privs] = {0}; + + /* Update existing entry. */ + values[Anum_pg_init_privs_initprivs - 1] = PointerGetDatum(new_acl); + replaces[Anum_pg_init_privs_initprivs - 1] = true; + + newtuple = heap_modify_tuple(oldtuple, RelationGetDescr(rel), + values, nulls, replaces); + CatalogTupleUpdate(rel, &newtuple->t_self, newtuple); + } + + /* + * Update the shared dependency ACL info. + */ + nnewmembers = aclmembers(new_acl, &newmembers); + + updateInitAclDependencies(classid, objid, objsubid, + ownerId, + noldmembers, oldmembers, + nnewmembers, newmembers); + + systable_endscan(scan); + + /* prevent error when processing objects multiple times */ + CommandCounterIncrement(); + + table_close(rel, RowExclusiveLock); +} diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c index cb31590339..20bcfd779b 100644 --- a/src/backend/catalog/pg_shdepend.c +++ b/src/backend/catalog/pg_shdepend.c @@ -84,6 +84,11 @@ static void shdepChangeDep(Relation sdepRel, Oid classid, Oid objid, int32 objsubid, Oid refclassid, Oid refobjid, SharedDependencyType deptype); +static void updateAclDependenciesWorker(Oid classId, Oid objectId, + int32 objsubId, Oid ownerId, + SharedDependencyType deptype, + int noldmembers, Oid *oldmembers, + int nnewmembers, Oid *newmembers); static void shdepAddDependency(Relation sdepRel, Oid classId, Oid objectId, int32 objsubId, Oid refclassId, Oid refobjId, @@ -340,6 +345,11 @@ changeDependencyOnOwner(Oid classId, Oid objectId, Oid newOwnerId) AuthIdRelationId, newOwnerId, SHARED_DEPENDENCY_ACL); + /* The same applies to SHARED_DEPENDENCY_INITACL */ + shdepDropDependency(sdepRel, classId, objectId, 0, true, + AuthIdRelationId, newOwnerId, + SHARED_DEPENDENCY_INITACL); + table_close(sdepRel, RowExclusiveLock); } @@ -478,6 +488,38 @@ updateAclDependencies(Oid classId, Oid objectId, int32 objsubId, Oid ownerId, int noldmembers, Oid *oldmembers, int nnewmembers, Oid *newmembers) +{ + updateAclDependenciesWorker(classId, objectId, objsubId, + ownerId, SHARED_DEPENDENCY_ACL, + noldmembers, oldmembers, + nnewmembers, newmembers); +} + +/* + * updateInitAclDependencies + * Update the pg_shdepend info for a pg_init_privs entry. + * + * Exactly like updateAclDependencies, except we are considering a + * pg_init_privs ACL for the specified object. + */ +void +updateInitAclDependencies(Oid classId, Oid objectId, int32 objsubId, + Oid ownerId, + int noldmembers, Oid *oldmembers, + int nnewmembers, Oid *newmembers) +{ + updateAclDependenciesWorker(classId, objectId, objsubId, + ownerId, SHARED_DEPENDENCY_INITACL, + noldmembers, oldmembers, + nnewmembers, newmembers); +} + +/* Common code for the above two functions */ +static void +updateAclDependenciesWorker(Oid classId, Oid objectId, int32 objsubId, + Oid ownerId, SharedDependencyType deptype, + int noldmembers, Oid *oldmembers, + int nnewmembers, Oid *newmembers) { Relation sdepRel; int i; @@ -513,7 +555,7 @@ updateAclDependencies(Oid classId, Oid objectId, int32 objsubId, shdepAddDependency(sdepRel, classId, objectId, objsubId, AuthIdRelationId, roleid, - SHARED_DEPENDENCY_ACL); + deptype); } /* Drop no-longer-used old dependencies */ @@ -532,7 +574,7 @@ updateAclDependencies(Oid classId, Oid objectId, int32 objsubId, shdepDropDependency(sdepRel, classId, objectId, objsubId, false, /* exact match on objsubId */ AuthIdRelationId, roleid, - SHARED_DEPENDENCY_ACL); + deptype); } table_close(sdepRel, RowExclusiveLock); @@ -1249,6 +1291,8 @@ storeObjectDescription(StringInfo descs, appendStringInfo(descs, _("owner of %s"), objdesc); else if (deptype == SHARED_DEPENDENCY_ACL) appendStringInfo(descs, _("privileges for %s"), objdesc); + else if (deptype == SHARED_DEPENDENCY_INITACL) + appendStringInfo(descs, _("initial privileges for %s"), objdesc); else if (deptype == SHARED_DEPENDENCY_POLICY) appendStringInfo(descs, _("target of %s"), objdesc); else if (deptype == SHARED_DEPENDENCY_TABLESPACE) @@ -1431,6 +1475,14 @@ shdepDropOwned(List *roleids, DropBehavior behavior) add_exact_object_address(&obj, deleteobjs); } break; + case SHARED_DEPENDENCY_INITACL: + /* Shouldn't see a role grant here */ + Assert(sdepForm->classid != AuthMemRelationId); + RemoveRoleFromInitPriv(roleid, + sdepForm->classid, + sdepForm->objid, + sdepForm->objsubid); + break; } } diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 950f00bed4..8618ea5907 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 ec654010d4..e0dcc0b069 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 3a0baf3039..1a554c6699 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) */ diff --git a/src/test/modules/test_pg_dump/expected/test_pg_dump.out b/src/test/modules/test_pg_dump/expected/test_pg_dump.out index f57c96aeb9..fcfa78aafc 100644 --- a/src/test/modules/test_pg_dump/expected/test_pg_dump.out +++ b/src/test/modules/test_pg_dump/expected/test_pg_dump.out @@ -62,6 +62,113 @@ GRANT SELECT ON ft1 TO regress_dump_test_role; GRANT UPDATE ON test_pg_dump_mv1 TO regress_dump_test_role; GRANT USAGE ON SCHEMA test_pg_dump_s1 TO regress_dump_test_role; GRANT USAGE ON TYPE test_pg_dump_e1 TO regress_dump_test_role; +-- Substitute for current user's name to keep test output consistent +SELECT s.obj, + CASE WHEN a.grantor::regrole::name = current_user THEN 'postgres' + ELSE a.grantor::regrole END, + CASE WHEN a.grantee::regrole::name = current_user THEN 'postgres' + ELSE a.grantee::regrole END, + a.privilege_type, a.is_grantable +FROM + (SELECT pg_describe_object(classoid,objoid,objsubid) AS obj, initprivs + FROM pg_init_privs WHERE privtype = 'e' ORDER BY 1) s, + aclexplode(s.initprivs) a; + obj | grantor | grantee | privilege_type | is_grantable +----------------------------------------------------+----------+------------------------+----------------+-------------- + column col1 of table regress_pg_dump_table | postgres | - | SELECT | f + function regress_pg_dump_schema.test_agg(smallint) | postgres | - | EXECUTE | f + function regress_pg_dump_schema.test_agg(smallint) | postgres | postgres | EXECUTE | f + function regress_pg_dump_schema.test_agg(smallint) | postgres | regress_dump_test_role | EXECUTE | f + function regress_pg_dump_schema.test_func() | postgres | - | EXECUTE | f + function regress_pg_dump_schema.test_func() | postgres | postgres | EXECUTE | f + function regress_pg_dump_schema.test_func() | postgres | regress_dump_test_role | EXECUTE | f + function wgo_then_no_access() | postgres | - | EXECUTE | f + function wgo_then_no_access() | postgres | postgres | EXECUTE | f + function wgo_then_no_access() | postgres | pg_signal_backend | EXECUTE | t + sequence regress_pg_dump_schema.test_seq | postgres | postgres | SELECT | f + sequence regress_pg_dump_schema.test_seq | postgres | postgres | UPDATE | f + sequence regress_pg_dump_schema.test_seq | postgres | postgres | USAGE | f + sequence regress_pg_dump_schema.test_seq | postgres | regress_dump_test_role | USAGE | f + sequence regress_pg_dump_seq | postgres | postgres | SELECT | f + sequence regress_pg_dump_seq | postgres | postgres | UPDATE | f + sequence regress_pg_dump_seq | postgres | postgres | USAGE | f + sequence regress_pg_dump_seq | postgres | regress_dump_test_role | USAGE | f + sequence regress_seq_dumpable | postgres | postgres | SELECT | f + sequence regress_seq_dumpable | postgres | postgres | UPDATE | f + sequence regress_seq_dumpable | postgres | postgres | USAGE | f + sequence regress_seq_dumpable | postgres | - | SELECT | f + sequence wgo_then_regular | postgres | postgres | SELECT | f + sequence wgo_then_regular | postgres | postgres | UPDATE | f + sequence wgo_then_regular | postgres | postgres | USAGE | f + sequence wgo_then_regular | postgres | pg_signal_backend | SELECT | f + sequence wgo_then_regular | postgres | pg_signal_backend | UPDATE | t + sequence wgo_then_regular | postgres | pg_signal_backend | USAGE | t + table regress_pg_dump_schema.test_table | postgres | postgres | INSERT | f + table regress_pg_dump_schema.test_table | postgres | postgres | SELECT | f + table regress_pg_dump_schema.test_table | postgres | postgres | UPDATE | f + table regress_pg_dump_schema.test_table | postgres | postgres | DELETE | f + table regress_pg_dump_schema.test_table | postgres | postgres | TRUNCATE | f + table regress_pg_dump_schema.test_table | postgres | postgres | REFERENCES | f + table regress_pg_dump_schema.test_table | postgres | postgres | TRIGGER | f + table regress_pg_dump_schema.test_table | postgres | postgres | MAINTAIN | f + table regress_pg_dump_schema.test_table | postgres | regress_dump_test_role | SELECT | f + table regress_pg_dump_table | postgres | postgres | INSERT | f + table regress_pg_dump_table | postgres | postgres | SELECT | f + table regress_pg_dump_table | postgres | postgres | UPDATE | f + table regress_pg_dump_table | postgres | postgres | DELETE | f + table regress_pg_dump_table | postgres | postgres | TRUNCATE | f + table regress_pg_dump_table | postgres | postgres | REFERENCES | f + table regress_pg_dump_table | postgres | postgres | TRIGGER | f + table regress_pg_dump_table | postgres | postgres | MAINTAIN | f + table regress_pg_dump_table | postgres | regress_dump_test_role | SELECT | f + table regress_table_dumpable | postgres | postgres | INSERT | f + table regress_table_dumpable | postgres | postgres | SELECT | f + table regress_table_dumpable | postgres | postgres | UPDATE | f + table regress_table_dumpable | postgres | postgres | DELETE | f + table regress_table_dumpable | postgres | postgres | TRUNCATE | f + table regress_table_dumpable | postgres | postgres | REFERENCES | f + table regress_table_dumpable | postgres | postgres | TRIGGER | f + table regress_table_dumpable | postgres | postgres | MAINTAIN | f + table regress_table_dumpable | postgres | - | SELECT | f + type regress_pg_dump_schema.test_type | postgres | - | USAGE | f + type regress_pg_dump_schema.test_type | postgres | postgres | USAGE | f + type regress_pg_dump_schema.test_type | postgres | regress_dump_test_role | USAGE | f +(58 rows) + +SELECT pg_describe_object(classid,objid,objsubid) AS obj, + pg_describe_object(refclassid,refobjid,0) AS refobj, + deptype + FROM pg_shdepend JOIN pg_database d ON dbid = d.oid + WHERE d.datname = current_database() + ORDER BY 1, 3; + obj | refobj | deptype +----------------------------------------------------+-----------------------------+--------- + column c1 of foreign table ft1 | role regress_dump_test_role | a + column c1 of table test_pg_dump_t1 | role regress_dump_test_role | a + foreign table ft1 | role regress_dump_test_role | a + foreign-data wrapper dummy | role regress_dump_test_role | a + function regress_pg_dump_schema.test_agg(smallint) | role regress_dump_test_role | a + function regress_pg_dump_schema.test_agg(smallint) | role regress_dump_test_role | i + function regress_pg_dump_schema.test_func() | role regress_dump_test_role | a + function regress_pg_dump_schema.test_func() | role regress_dump_test_role | i + function test_pg_dump(integer) | role regress_dump_test_role | a + materialized view test_pg_dump_mv1 | role regress_dump_test_role | a + schema test_pg_dump_s1 | role regress_dump_test_role | a + sequence regress_pg_dump_schema.test_seq | role regress_dump_test_role | a + sequence regress_pg_dump_schema.test_seq | role regress_dump_test_role | i + sequence regress_pg_dump_seq | role regress_dump_test_role | a + sequence regress_pg_dump_seq | role regress_dump_test_role | i + server s0 | role regress_dump_test_role | a + table regress_pg_dump_schema.test_table | role regress_dump_test_role | a + table regress_pg_dump_schema.test_table | role regress_dump_test_role | i + table regress_pg_dump_table | role regress_dump_test_role | a + table regress_pg_dump_table | role regress_dump_test_role | i + type regress_pg_dump_schema.test_type | role regress_dump_test_role | a + type regress_pg_dump_schema.test_type | role regress_dump_test_role | i + type test_pg_dump_e1 | role regress_dump_test_role | a + view test_pg_dump_v1 | role regress_dump_test_role | a +(24 rows) + ALTER EXTENSION test_pg_dump ADD ACCESS METHOD gist2; ALTER EXTENSION test_pg_dump ADD AGGREGATE newavg(int4); ALTER EXTENSION test_pg_dump ADD CAST (text AS casttesttype); @@ -92,4 +199,80 @@ ALTER EXTENSION test_pg_dump DROP TABLE test_pg_dump_t1; ALTER EXTENSION test_pg_dump DROP TYPE test_pg_dump_e1; ALTER EXTENSION test_pg_dump DROP VIEW test_pg_dump_v1; DROP OWNED BY regress_dump_test_role RESTRICT; +-- Substitute for current user's name to keep test output consistent +SELECT s.obj, + CASE WHEN a.grantor::regrole::name = current_user THEN 'postgres' + ELSE a.grantor::regrole END, + CASE WHEN a.grantee::regrole::name = current_user THEN 'postgres' + ELSE a.grantee::regrole END, + a.privilege_type, a.is_grantable +FROM + (SELECT pg_describe_object(classoid,objoid,objsubid) AS obj, initprivs + FROM pg_init_privs WHERE privtype = 'e' ORDER BY 1) s, + aclexplode(s.initprivs) a; + obj | grantor | grantee | privilege_type | is_grantable +----------------------------------------------------+----------+-------------------+----------------+-------------- + column col1 of table regress_pg_dump_table | postgres | - | SELECT | f + function regress_pg_dump_schema.test_agg(smallint) | postgres | - | EXECUTE | f + function regress_pg_dump_schema.test_agg(smallint) | postgres | postgres | EXECUTE | f + function regress_pg_dump_schema.test_func() | postgres | - | EXECUTE | f + function regress_pg_dump_schema.test_func() | postgres | postgres | EXECUTE | f + function wgo_then_no_access() | postgres | - | EXECUTE | f + function wgo_then_no_access() | postgres | postgres | EXECUTE | f + function wgo_then_no_access() | postgres | pg_signal_backend | EXECUTE | t + sequence regress_pg_dump_schema.test_seq | postgres | postgres | SELECT | f + sequence regress_pg_dump_schema.test_seq | postgres | postgres | UPDATE | f + sequence regress_pg_dump_schema.test_seq | postgres | postgres | USAGE | f + sequence regress_pg_dump_seq | postgres | postgres | SELECT | f + sequence regress_pg_dump_seq | postgres | postgres | UPDATE | f + sequence regress_pg_dump_seq | postgres | postgres | USAGE | f + sequence regress_seq_dumpable | postgres | postgres | SELECT | f + sequence regress_seq_dumpable | postgres | postgres | UPDATE | f + sequence regress_seq_dumpable | postgres | postgres | USAGE | f + sequence regress_seq_dumpable | postgres | - | SELECT | f + sequence wgo_then_regular | postgres | postgres | SELECT | f + sequence wgo_then_regular | postgres | postgres | UPDATE | f + sequence wgo_then_regular | postgres | postgres | USAGE | f + sequence wgo_then_regular | postgres | pg_signal_backend | SELECT | f + sequence wgo_then_regular | postgres | pg_signal_backend | UPDATE | t + sequence wgo_then_regular | postgres | pg_signal_backend | USAGE | t + table regress_pg_dump_schema.test_table | postgres | postgres | INSERT | f + table regress_pg_dump_schema.test_table | postgres | postgres | SELECT | f + table regress_pg_dump_schema.test_table | postgres | postgres | UPDATE | f + table regress_pg_dump_schema.test_table | postgres | postgres | DELETE | f + table regress_pg_dump_schema.test_table | postgres | postgres | TRUNCATE | f + table regress_pg_dump_schema.test_table | postgres | postgres | REFERENCES | f + table regress_pg_dump_schema.test_table | postgres | postgres | TRIGGER | f + table regress_pg_dump_schema.test_table | postgres | postgres | MAINTAIN | f + table regress_pg_dump_table | postgres | postgres | INSERT | f + table regress_pg_dump_table | postgres | postgres | SELECT | f + table regress_pg_dump_table | postgres | postgres | UPDATE | f + table regress_pg_dump_table | postgres | postgres | DELETE | f + table regress_pg_dump_table | postgres | postgres | TRUNCATE | f + table regress_pg_dump_table | postgres | postgres | REFERENCES | f + table regress_pg_dump_table | postgres | postgres | TRIGGER | f + table regress_pg_dump_table | postgres | postgres | MAINTAIN | f + table regress_table_dumpable | postgres | postgres | INSERT | f + table regress_table_dumpable | postgres | postgres | SELECT | f + table regress_table_dumpable | postgres | postgres | UPDATE | f + table regress_table_dumpable | postgres | postgres | DELETE | f + table regress_table_dumpable | postgres | postgres | TRUNCATE | f + table regress_table_dumpable | postgres | postgres | REFERENCES | f + table regress_table_dumpable | postgres | postgres | TRIGGER | f + table regress_table_dumpable | postgres | postgres | MAINTAIN | f + table regress_table_dumpable | postgres | - | SELECT | f + type regress_pg_dump_schema.test_type | postgres | - | USAGE | f + type regress_pg_dump_schema.test_type | postgres | postgres | USAGE | f +(51 rows) + +SELECT pg_describe_object(classid,objid,objsubid) AS obj, + pg_describe_object(refclassid,refobjid,0) AS refobj, + deptype + FROM pg_shdepend JOIN pg_database d ON dbid = d.oid + WHERE d.datname = current_database() + ORDER BY 1, 3; + obj | refobj | deptype +-----+--------+--------- +(0 rows) + DROP ROLE regress_dump_test_role; diff --git a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql index 4f1eb9d429..41f1d8dfc5 100644 --- a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql +++ b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql @@ -75,6 +75,24 @@ GRANT UPDATE ON test_pg_dump_mv1 TO regress_dump_test_role; GRANT USAGE ON SCHEMA test_pg_dump_s1 TO regress_dump_test_role; GRANT USAGE ON TYPE test_pg_dump_e1 TO regress_dump_test_role; +-- Substitute for current user's name to keep test output consistent +SELECT s.obj, + CASE WHEN a.grantor::regrole::name = current_user THEN 'postgres' + ELSE a.grantor::regrole END, + CASE WHEN a.grantee::regrole::name = current_user THEN 'postgres' + ELSE a.grantee::regrole END, + a.privilege_type, a.is_grantable +FROM + (SELECT pg_describe_object(classoid,objoid,objsubid) AS obj, initprivs + FROM pg_init_privs WHERE privtype = 'e' ORDER BY 1) s, + aclexplode(s.initprivs) a; +SELECT pg_describe_object(classid,objid,objsubid) AS obj, + pg_describe_object(refclassid,refobjid,0) AS refobj, + deptype + FROM pg_shdepend JOIN pg_database d ON dbid = d.oid + WHERE d.datname = current_database() + ORDER BY 1, 3; + ALTER EXTENSION test_pg_dump ADD ACCESS METHOD gist2; ALTER EXTENSION test_pg_dump ADD AGGREGATE newavg(int4); ALTER EXTENSION test_pg_dump ADD CAST (text AS casttesttype); @@ -108,4 +126,23 @@ ALTER EXTENSION test_pg_dump DROP TYPE test_pg_dump_e1; ALTER EXTENSION test_pg_dump DROP VIEW test_pg_dump_v1; DROP OWNED BY regress_dump_test_role RESTRICT; + +-- Substitute for current user's name to keep test output consistent +SELECT s.obj, + CASE WHEN a.grantor::regrole::name = current_user THEN 'postgres' + ELSE a.grantor::regrole END, + CASE WHEN a.grantee::regrole::name = current_user THEN 'postgres' + ELSE a.grantee::regrole END, + a.privilege_type, a.is_grantable +FROM + (SELECT pg_describe_object(classoid,objoid,objsubid) AS obj, initprivs + FROM pg_init_privs WHERE privtype = 'e' ORDER BY 1) s, + aclexplode(s.initprivs) a; +SELECT pg_describe_object(classid,objid,objsubid) AS obj, + pg_describe_object(refclassid,refobjid,0) AS refobj, + deptype + FROM pg_shdepend JOIN pg_database d ON dbid = d.oid + WHERE d.datname = current_database() + ORDER BY 1, 3; + DROP ROLE regress_dump_test_role;