Alena0704 commented on PR #1804:
URL: https://github.com/apache/cloudberry/pull/1804#issuecomment-4635983525
> > > This PR's permission check looks up the role by name, not by a fixed
OID:
> > > ```
> > > role = get_role_oid("mdb_admin", true); /* look up the role by the
name "mdb_admin" */
> > > if (!is_member_of_role(GetUserId(), role))
> > > ereport(ERROR, ...);
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > This means: members of any role that happens to be named mdb_admin can
manage resource groups. The code doesn't care who created that role or when.
> >
> >
> > Yep, exactly :) We only care about rolsuper or not, if mdb-admin patch
does not allow to gain superuser priviledge - we consider it safe :)
>
> Thanks for the iterations on this. I agree with the constraint driving the
current design — a new bootstrap entry in pg_authid.dat bumps
CATALOG_VERSION_NO and breaks minor-version upgrades, so a runtime lookup is
the right shape for the stable branch.
>
> How about Proposal: use a reserved-prefix role name
>
> Rename the role to e.g. pg_manage_resgroup and keep the same runtime
get_role_oid() lookup. The point is that the pg_ namespace is already reserved
for roles:
>
> * CreateRole() rejects any pg_-prefixed name via IsReservedName()
(src/backend/commands/user.c),
> so a CREATEROLE user cannot forge it;
> * RenameRole() rejects renaming any role to a pg_ name, so it cannot be
smuggled in via ALTER
> ROLE ... RENAME either;
>
> One caveat: DROP is not name-protected
We can't use a pg_ name because creating it requires changes in
pg_authid.dat (a bootstrap role), which bumps CATALOG_VERSION_NO and breaks
in-place minor upgrades.
postgres=# CREATE ROLE pg_manage_resgroup;
ERROR: role name "pg_manage_resgroup" is reserved
DETAIL: Role names starting with "pg_" are reserved.
I added in the last commit a check in user.c that restricts
CREATE/ALTER/RENAME/DROP/GRANT/REVOKE of the mdb_admin and mdb_superuser roles
to superusers only, so an ordinary CREATEROLE user cannot hijack the by-name
privilege gate (create, rename, reset the password of, or self-grant the role)
and thereby escalate privileges.
DROP ROLE
NOTICE: role "rg_attacker" does not exist, skipping
DROP ROLE
NOTICE: resource queue required -- using default resource queue "pg_default"
CREATE ROLE
NOTICE: resource queue required -- using default resource queue "pg_default"
CREATE ROLE
SET
ERROR: must be superuser to grant membership in role "mdb_admin"
ERROR: must be superuser to drop role "mdb_admin"
ERROR: must be superuser to rename role "mdb_admin"
ERROR: must be superuser to alter role "mdb_admin"
ERROR: must be superuser to create role "mdb_superuser"
RESET
DROP ROLE
DROP ROLE
--
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]
