Alena0704 commented on PR #1804: URL: https://github.com/apache/cloudberry/pull/1804#issuecomment-4680696155
> > Reworked the patch following the pg_aux commit - moved the mdb_admin role into a contrib extension, pg_aux_catalog: it's created via pg_create_mdb_admin_role() (fixed OID 8067, superuser-only). Resource-group operations are gated on mdb_admin membership, identified by that fixed OID. > > Because the role is created at a single fixed OID and only by a superuser, it cannot be re-created or taken over by another user - a CREATEROLE user can neither establish mdb_admin nor recreate it at that OID: > > ``` > > postgres=# CREATE ROLE rg_attacker CREATEROLE; > > CREATE ROLE > > postgres=# SET ROLE rg_attacker; > > SET > > postgres=> ALTER ROLE mdb_admin RENAME TO rg_attacker_admin; > > ERROR: role "mdb_admin" does not exist > > postgres=> ALTER ROLE mdb_admin PASSWORD 'hijack'; > > ERROR: role "mdb_admin" does not exist > > ``` > > > > > > > > > > > > > > > > > > > > > > > > Tests were moved into the extension as well. > > Thanks for the rework — moving to a fixed-OID role provisioned through contrib/pg_aux_catalog is the right call, and I think it closes the escalation path we were discussing. A few notes: > > Security model — LGTM. The original concern (a CREATEROLE user forging a role named mdb_admin and self-granting it) is now moot, because the gate checks membership in OID 8067 rather than the name: > > * A plain CREATE ROLE mdb_admin gets an ordinary OID (≥ FirstNormalObjectId), never 8067, so a forged role is inert. > * next_aux_pg_authid_oid is only set inside the superuser-only function, and 8067 sits in the reserved range that normal OID allocation never hands out. > * I confirmed is_member_of_role() short-circuits for superusers (src/backend/utils/adt/acl.c), so when the extension isn't installed (OID 8067 absent) the behavior cleanly degrades to the previous superuser-only semantics, and superusers never lose access. Nice fallback. Thanks! Yes, that's the intent - the check is on membership in OID 8067, and only a superuser can create the role, so a plain CREATE ROLE mdb_admin is harmless. It's all in the current commit (ae95005). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
