Let mdb_admin manage resource groups

[Alena0704]

Let mdb_admin manage resource groups

In Greenplum/Cloudberry only a superuser may CREATE/ALTER/DROP resource
groups. In managed-service deployments the client never gets superuser,
so a cloud admin can't tune their own CPU/memory limits.

Instead of pinning a predefined role OID (which bumps CATALOG_VERSION_NO
and needs a fresh initdb), gate the four entry points on membership of
the mdb_admin role, resolved by name at runtime. mdb_admin is an ordinary
role created by the control plane with CREATE ROLE. If it doesn't exist,
the check falls back to superuser-only, so a stock cluster is unchanged.

admin_group stays superuser-only for ALTER/DROP.

Since mdb_admin and mdb_superuser gate privileges by name, also restrict
their CREATE/ALTER/RENAME/DROP to superusers, so a CREATEROLE
user can't hijack them to escalate.

Adapted from open-gpdb/gpdb 3ac99962ad2. Tests from apache/cloudberry #1763.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

• Bug fix (non-breaking change)
• New feature (non-breaking change)
• Breaking change (fix or feature with breaking changes)
• Documentation update

Breaking Changes

Test Plan

• Unit tests added/updated
• Integration tests added/updated
• Passed make installcheck
• Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

• Followed contribution guide
• Added/updated documentation
• Reviewed code for security implications
• This PR contains AI-assisted code generation
• Requested review from cloudberry committers

Additional Context

CI Skip Instructions

---

[Alena0704]

I rewrote the patch completely because a new bootstrap catalog entry bumps CATALOG_VERSION_NO and incompatible with minor gpupgrade - I adapted the commit from open-gpdb 3ac99962ad2 and added my tests.

[my-ship-it]

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.

Where the hole is: This assumption overlooks CREATEROLE users. Suppose a DBA has granted some user CREATEROLE (quite common on self-hosted clusters,
e.g. to let someone manage ordinary user accounts):

  -- Executed as an ordinary CREATEROLE user:
  CREATE ROLE mdb_admin;          -- CREATEROLE allows creating any non-superuser role ✓
  GRANT mdb_admin TO myself;      -- Under PG14 semantics, a CREATEROLE user can grant
                                  -- any non-superuser role to anyone, including themselves ✓
  ALTER RESOURCE GROUP default_group SET concurrency 1;  -- now passes the permission check!

Three SQL statements, and a user who was only supposed to "manage accounts" has self-granted resource group administration rights.

[Alena0704]

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.

Where the hole is: This assumption overlooks CREATEROLE users. Suppose a DBA has granted some user CREATEROLE (quite common on self-hosted clusters, e.g. to let someone manage ordinary user accounts):

  -- Executed as an ordinary CREATEROLE user:
  CREATE ROLE mdb_admin;          -- CREATEROLE allows creating any non-superuser role ✓
  GRANT mdb_admin TO myself;      -- Under PG14 semantics, a CREATEROLE user can grant
                                  -- any non-superuser role to anyone, including themselves ✓
  ALTER RESOURCE GROUP default_group SET concurrency 1;  -- now passes the permission check!

Three SQL statements, and a user who was only supposed to "manage accounts" has self-granted resource group administration rights.

Agree, I have fixed it by checking that it is included in admin and system group.

[reshke]

Where the hole is: This assumption overlooks CREATEROLE users. Suppose a DBA has granted some user CREATEROLE (quite common on self-hosted clusters, e.g. to let someone manage ordinary user accounts):

  -- Executed as an ordinary CREATEROLE user:
  CREATE ROLE mdb_admin;          -- CREATEROLE allows creating any non-superuser role ✓
  GRANT mdb_admin TO myself;      -- Under PG14 semantics, a CREATEROLE user can grant
                                  -- any non-superuser role to anyone, including themselves ✓
  ALTER RESOURCE GROUP default_group SET concurrency 1;  -- now passes the permission check!

Three SQL statements, and a user who was only supposed to "manage accounts" has self-granted resource group administration rights.

for our use in Cloud we just do not consider this as privilege escalation, because we grant this role to cloud users(non-superuser) by request (you just click in UI like that this role to this role)

Anyway issue looks valid in general use

[reshke]

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 :)

[my-ship-it]

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

[Alena0704]

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.

CREATE ROLE rg_attacker CREATEROLE;
SET ROLE rg_attacker;
GRANT mdb_admin TO rg_attacker; -- self-grant membership
DROP ROLE mdb_admin; -- drop the gate
ALTER ROLE mdb_admin RENAME TO rg_attacker_admin; -- move the real role aside
ALTER ROLE mdb_admin PASSWORD 'hijack'; -- take it over via password reset
CREATE ROLE mdb_superuser; -- create a protected role
RESET ROLE;
DROP ROLE rg_attacker;

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