diff --git a/.github/workflows/build-cloudberry-rocky8.yml b/.github/workflows/build-cloudberry-rocky8.yml index 2abf88060e3..c8068f098cb 100644 --- a/.github/workflows/build-cloudberry-rocky8.yml +++ b/.github/workflows/build-cloudberry-rocky8.yml @@ -311,6 +311,7 @@ jobs: "contrib/pgstattuple:installcheck", "contrib/tablefunc:installcheck", "contrib/passwordcheck:installcheck", + "contrib/pg_aux_catalog:installcheck", "contrib/pg_buffercache:installcheck", "contrib/sslinfo:installcheck"] }, diff --git a/.github/workflows/build-cloudberry.yml b/.github/workflows/build-cloudberry.yml index ca75f7b42e7..99bc67c99b7 100644 --- a/.github/workflows/build-cloudberry.yml +++ b/.github/workflows/build-cloudberry.yml @@ -304,6 +304,7 @@ jobs: "contrib/pgstattuple:installcheck", "contrib/tablefunc:installcheck", "contrib/passwordcheck:installcheck", + "contrib/pg_aux_catalog:installcheck", "contrib/pg_buffercache:installcheck", "contrib/sslinfo:installcheck"] }, diff --git a/.github/workflows/build-deb-cloudberry.yml b/.github/workflows/build-deb-cloudberry.yml index 85d917b8ff0..fee69b073f7 100644 --- a/.github/workflows/build-deb-cloudberry.yml +++ b/.github/workflows/build-deb-cloudberry.yml @@ -243,6 +243,7 @@ jobs: "contrib/pgstattuple:installcheck", "contrib/tablefunc:installcheck", "contrib/passwordcheck:installcheck", + "contrib/pg_aux_catalog:installcheck", "contrib/pg_buffercache:installcheck", "contrib/sslinfo:installcheck"] }, diff --git a/contrib/Makefile b/contrib/Makefile index b14600e3557..01315b1f6f8 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -34,6 +34,7 @@ SUBDIRS = \ old_snapshot \ pageinspect \ passwordcheck \ + pg_aux_catalog \ postgres_fdw \ pg_buffercache \ pg_freespacemap \ diff --git a/contrib/pg_aux_catalog/.gitignore b/contrib/pg_aux_catalog/.gitignore new file mode 100644 index 00000000000..c4ec060cefb --- /dev/null +++ b/contrib/pg_aux_catalog/.gitignore @@ -0,0 +1,6 @@ +# Generated test output +/log/ +/results/ +/tmp_check/ +/isolation2/results/ +/isolation2/output_iso/ diff --git a/contrib/pg_aux_catalog/Makefile b/contrib/pg_aux_catalog/Makefile new file mode 100644 index 00000000000..439ecf31a76 --- /dev/null +++ b/contrib/pg_aux_catalog/Makefile @@ -0,0 +1,37 @@ +# contrib/pg_aux_catalog/Makefile + +MODULE_big = pg_aux_catalog +OBJS = \ + $(WIN32RES) \ + pg_aux_catalog.o + +EXTENSION = pg_aux_catalog +DATA = pg_aux_catalog--1.0.sql + +PGFILEDESC = "pg_aux_catalog - auxiliary catalog management" + +REGRESS = pg_aux_catalog +REGRESS_OPTS = --init-file=$(top_srcdir)/src/test/regress/init_file + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/pg_aux_catalog +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif + +# Multi-session resource-group permission tests, run with the GPDB isolation2 +# harness. Requires a running cluster with resource groups enabled +# (gp_resource_manager=group); see installcheck-resgroup in +# src/test/isolation2. Not part of the default "installcheck". +installcheck-isolation2: install + $(pg_isolation2_regress_installcheck) \ + --init-file=$(top_builddir)/src/test/regress/init_file \ + --inputdir=$(srcdir)/isolation2 \ + --outputdir=isolation2 \ + --schedule=$(srcdir)/isolation2/isolation2_schedule + diff --git a/contrib/pg_aux_catalog/README.md b/contrib/pg_aux_catalog/README.md new file mode 100644 index 00000000000..a89535838ee --- /dev/null +++ b/contrib/pg_aux_catalog/README.md @@ -0,0 +1,71 @@ +# pg_aux_catalog + +Auxiliary catalog management for Apache Cloudberry. + +This extension provisions the **`mdb_admin`** privilege role, which lets a +non-superuser manage resource groups in managed-service deployments where the +client is never given superuser. + +## Background + +In Greenplum/Cloudberry only a superuser may `CREATE`/`ALTER`/`DROP` resource +groups or move a running query between groups with `pg_resgroup_move_query()`. +The server gates those four entry points on membership of `mdb_admin`, +identified by a **fixed OID (8067)** rather than by name, so the privilege is +recognised reliably across the coordinator and all segments. + +A fixed OID cannot be obtained from a plain `CREATE ROLE` (that assigns an +ordinary OID). This extension provides the one supported way to create the +role at OID 8067. + +## Functions + +### `pg_create_mdb_admin_role() returns oid` + +Creates the `mdb_admin` role with its fixed OID (8067). +Returns the OID of the created role (8067). Errors if a role with that OID or +the name `mdb_admin` already exists. The OID assignment is dispatched to the +segments, so the role has the same OID cluster-wide. + +## Usage + +```sql +CREATE EXTENSION pg_aux_catalog; + +-- Provision the role (the control plane does this once per cluster). +SELECT pg_create_mdb_admin_role(); + +-- Grant the capability to a tenant admin. +GRANT mdb_admin TO cloud_admin; + +-- cloud_admin can now manage resource groups without superuser: +SET ROLE cloud_admin; +CREATE RESOURCE GROUP rg_tenant WITH (concurrency = 4, cpu_max_percent = 20); +ALTER RESOURCE GROUP rg_tenant SET cpu_max_percent 30; +DROP RESOURCE GROUP rg_tenant; +``` + +`admin_group` and `system_group` remain superuser-only for `ALTER`/`DROP`: +they are infrastructure, not user-tunable groups. + +## Building and testing + +```sh +make -C contrib/pg_aux_catalog install +make -C contrib/pg_aux_catalog installcheck +``` + +`installcheck` runs a single-session regression test (role creation and the +resource-group permission gate). A multi-session isolation2 test covering the +dispatched / cross-session behaviour lives under `isolation2/` and is run +separately, against a cluster with resource groups enabled +(`gp_resource_manager=group`): + +```sh +make -C contrib/pg_aux_catalog installcheck-isolation2 +``` + +## Credits + +Based on [pg-sharding/cpg](https://github.com/pg-sharding/cpg) commit +`7b8c912`. Some tests are adapted from open-gpdb/gpdb commit `3ac99962ad2`. diff --git a/contrib/pg_aux_catalog/expected/pg_aux_catalog.out b/contrib/pg_aux_catalog/expected/pg_aux_catalog.out new file mode 100644 index 00000000000..32e52b9728d --- /dev/null +++ b/contrib/pg_aux_catalog/expected/pg_aux_catalog.out @@ -0,0 +1,46 @@ +-- Tests for the pg_aux_catalog extension: creation of the fixed-OID +-- mdb_admin role and the resource-group permission gate it enables. +CREATE EXTENSION pg_aux_catalog; +-- --------------------------------------------------------------------- +-- pg_create_mdb_admin_role() creates the mdb_admin role with its fixed OID. +-- --------------------------------------------------------------------- +SELECT pg_create_mdb_admin_role() AS mdb_admin_oid; + mdb_admin_oid +--------------- + 8067 +(1 row) + +-- The role exists with the fixed OID and is a non-login, non-superuser, +-- connection-limited role. +SELECT oid = 8067 AS has_fixed_oid, rolcanlogin, rolsuper, + rolcreaterole, rolcreatedb, rolconnlimit + FROM pg_authid WHERE rolname = 'mdb_admin'; + has_fixed_oid | rolcanlogin | rolsuper | rolcreaterole | rolcreatedb | rolconnlimit +---------------+-------------+----------+---------------+-------------+-------------- + t | f | f | f | f | 0 +(1 row) + +-- Creating it a second time is rejected. +SELECT pg_create_mdb_admin_role(); +ERROR: role with OID 8067 already exists +-- --------------------------------------------------------------------- +-- Resource-group permission gate: a role that is not a member of mdb_admin +-- is rejected on every entry point. These checks run before the "resource +-- group is enabled" check, so they are deterministic regardless of the +-- resource manager in use. +-- --------------------------------------------------------------------- +CREATE ROLE regress_rg_noadmin; +SET ROLE regress_rg_noadmin; +CREATE RESOURCE GROUP regress_rg_x WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +ALTER RESOURCE GROUP regress_rg_x SET cpu_max_percent 6; +ERROR: must be mdb_admin to alter resource groups +DROP RESOURCE GROUP regress_rg_x; +ERROR: must be mdb_admin to drop resource groups +RESET ROLE; +DROP ROLE regress_rg_noadmin; +-- --------------------------------------------------------------------- +-- Cleanup. +-- --------------------------------------------------------------------- +DROP ROLE mdb_admin; +DROP EXTENSION pg_aux_catalog; diff --git a/contrib/pg_aux_catalog/isolation2/expected/resgroup_mdb_admin.out b/contrib/pg_aux_catalog/isolation2/expected/resgroup_mdb_admin.out new file mode 100644 index 00000000000..c8fd232faea --- /dev/null +++ b/contrib/pg_aux_catalog/isolation2/expected/resgroup_mdb_admin.out @@ -0,0 +1,123 @@ +-- Tests permission checks for the mdb_admin role with +-- resource groups enabled. + +-- start_matchsubs +-- m/ERROR: cannot find process: \d+/ +-- s/\d+/XXX/g +-- end_matchsubs + +DROP ROLE IF EXISTS role_rg_admin; +DROP +DROP ROLE IF EXISTS role_rg_noadmin; +DROP +DROP ROLE IF EXISTS mdb_admin; +DROP +-- start_ignore +DROP RESOURCE GROUP rg_perm_admin1; +DROP RESOURCE GROUP rg_perm_admin2; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_revoke2; +DROP RESOURCE GROUP rg_perm_test; +-- end_ignore + +-- --------------------------------------------------------------------- +-- Setup. The mdb_admin role is not predefined in the catalog; it is +-- created here the same way the control plane provisions it at runtime. +-- --------------------------------------------------------------------- +CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10); +CREATE +CREATE ROLE mdb_admin; +CREATE +CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test; +CREATE +CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test; +CREATE +GRANT mdb_admin TO role_rg_admin; +GRANT + +-- --------------------------------------------------------------------- +-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups +-- (statements are dispatched to segments). +-- --------------------------------------------------------------------- +1: SET ROLE role_rg_admin; +SET +1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5); +CREATE +1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6; +ALTER +1: DROP RESOURCE GROUP rg_perm_admin1; +DROP + +-- 2. Even a member cannot ALTER or DROP the system admin_group. +1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99; +ERROR: must be superuser to alter resource group "admin_group" +1: DROP RESOURCE GROUP admin_group; +ERROR: must be superuser to drop resource group "admin_group" +1q: ... + +-- --------------------------------------------------------------------- +-- 3. A non-member is rejected on every entry point. +-- --------------------------------------------------------------------- +2: SET ROLE role_rg_noadmin; +SET +2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7; +ERROR: must be mdb_admin to alter resource groups +2: DROP RESOURCE GROUP rg_perm_test; +ERROR: must be mdb_admin to drop resource groups +2q: ... + +-- --------------------------------------------------------------------- +-- 4. pg_resgroup_move_query() honours the same permission check. +-- The first call (non-member) must fail with "must be mdb_admin". +-- The second call (member) gets past the permission gate and +-- fails on the pid lookup (masked by start_matchsubs above). +-- --------------------------------------------------------------------- +3: SET ROLE role_rg_noadmin; +SET +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +ERROR: must be mdb_admin to move query +3: RESET ROLE; +RESET +3: SET ROLE role_rg_admin; +SET +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +ERROR: cannot find process: XXX +3q: ... + +-- --------------------------------------------------------------------- +-- 5. Cross-session REVOKE takes effect on the granted session's +-- next statement (the privilege is re-checked per command, not +-- cached at SET ROLE time). +-- --------------------------------------------------------------------- +4: SET ROLE role_rg_admin; +SET +4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5); +CREATE +5: REVOKE mdb_admin FROM role_rg_admin; +REVOKE +4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5); +ERROR: must be mdb_admin to create resource groups +4: DROP RESOURCE GROUP rg_perm_revoke1; +ERROR: must be mdb_admin to drop resource groups +4q: ... +5q: ... + +-- --------------------------------------------------------------------- +-- Cleanup. Roles must be dropped before the resource group they +-- reference, otherwise DROP RESOURCE GROUP fails with +-- "resource group is used by at least one role". +-- --------------------------------------------------------------------- +RESET ROLE; +RESET +DROP ROLE role_rg_admin; +DROP +DROP ROLE role_rg_noadmin; +DROP +DROP ROLE mdb_admin; +DROP +DROP RESOURCE GROUP rg_perm_revoke1; +DROP +DROP RESOURCE GROUP rg_perm_test; +DROP diff --git a/contrib/pg_aux_catalog/isolation2/isolation2_schedule b/contrib/pg_aux_catalog/isolation2/isolation2_schedule new file mode 100644 index 00000000000..73b2a8a95a5 --- /dev/null +++ b/contrib/pg_aux_catalog/isolation2/isolation2_schedule @@ -0,0 +1 @@ +test: resgroup_mdb_admin diff --git a/contrib/pg_aux_catalog/isolation2/sql/resgroup_mdb_admin.sql b/contrib/pg_aux_catalog/isolation2/sql/resgroup_mdb_admin.sql new file mode 100644 index 00000000000..1b7ea19fb3e --- /dev/null +++ b/contrib/pg_aux_catalog/isolation2/sql/resgroup_mdb_admin.sql @@ -0,0 +1,91 @@ +-- Tests permission checks for the mdb_admin role with +-- resource groups enabled. + +-- start_matchsubs +-- m/ERROR: cannot find process: \d+/ +-- s/\d+/XXX/g +-- end_matchsubs + +DROP ROLE IF EXISTS role_rg_admin; +DROP ROLE IF EXISTS role_rg_noadmin; +DROP ROLE IF EXISTS mdb_admin; +-- start_ignore +DROP RESOURCE GROUP rg_perm_admin1; +DROP RESOURCE GROUP rg_perm_admin2; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_revoke2; +DROP RESOURCE GROUP rg_perm_test; +-- end_ignore + +-- --------------------------------------------------------------------- +-- Setup. mdb_admin is identified by its fixed OID, so it must be created +-- through contrib/pg_aux_catalog (a plain CREATE ROLE would assign a +-- different OID and the permission checks would not recognise its members). +-- --------------------------------------------------------------------- +CREATE RESOURCE GROUP rg_perm_test WITH (concurrency=2, cpu_max_percent=10); +CREATE EXTENSION IF NOT EXISTS pg_aux_catalog; +SELECT pg_create_mdb_admin_role(); +CREATE ROLE role_rg_admin RESOURCE GROUP rg_perm_test; +CREATE ROLE role_rg_noadmin RESOURCE GROUP rg_perm_test; +GRANT mdb_admin TO role_rg_admin; + +-- --------------------------------------------------------------------- +-- 1. Member of mdb_admin can CREATE/ALTER/DROP resource groups +-- (statements are dispatched to segments). +-- --------------------------------------------------------------------- +1: SET ROLE role_rg_admin; +1: CREATE RESOURCE GROUP rg_perm_admin1 WITH (concurrency=1, cpu_max_percent=5); +1: ALTER RESOURCE GROUP rg_perm_admin1 SET cpu_max_percent 6; +1: DROP RESOURCE GROUP rg_perm_admin1; + +-- 2. Even a member cannot ALTER or DROP the system admin_group. +1: ALTER RESOURCE GROUP admin_group SET cpu_max_percent 99; +1: DROP RESOURCE GROUP admin_group; +1q: + +-- --------------------------------------------------------------------- +-- 3. A non-member is rejected on every entry point. +-- --------------------------------------------------------------------- +2: SET ROLE role_rg_noadmin; +2: CREATE RESOURCE GROUP rg_perm_admin2 WITH (concurrency=1, cpu_max_percent=5); +2: ALTER RESOURCE GROUP rg_perm_test SET cpu_max_percent 7; +2: DROP RESOURCE GROUP rg_perm_test; +2q: + +-- --------------------------------------------------------------------- +-- 4. pg_resgroup_move_query() honours the same permission check. +-- The first call (non-member) must fail with "must be mdb_admin". +-- The second call (member) gets past the permission gate and +-- fails on the pid lookup (masked by start_matchsubs above). +-- --------------------------------------------------------------------- +3: SET ROLE role_rg_noadmin; +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +3: RESET ROLE; +3: SET ROLE role_rg_admin; +3: SELECT pg_resgroup_move_query(999999999, 'admin_group'); +3q: + +-- --------------------------------------------------------------------- +-- 5. Cross-session REVOKE takes effect on the granted session's +-- next statement (the privilege is re-checked per command, not +-- cached at SET ROLE time). +-- --------------------------------------------------------------------- +4: SET ROLE role_rg_admin; +4: CREATE RESOURCE GROUP rg_perm_revoke1 WITH (concurrency=1, cpu_max_percent=5); +5: REVOKE mdb_admin FROM role_rg_admin; +4: CREATE RESOURCE GROUP rg_perm_revoke2 WITH (concurrency=1, cpu_max_percent=5); +4: DROP RESOURCE GROUP rg_perm_revoke1; +4q: +5q: + +-- --------------------------------------------------------------------- +-- Cleanup. Roles must be dropped before the resource group they +-- reference, otherwise DROP RESOURCE GROUP fails with +-- "resource group is used by at least one role". +-- --------------------------------------------------------------------- +RESET ROLE; +DROP ROLE role_rg_admin; +DROP ROLE role_rg_noadmin; +DROP ROLE mdb_admin; +DROP RESOURCE GROUP rg_perm_revoke1; +DROP RESOURCE GROUP rg_perm_test; diff --git a/contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql b/contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql new file mode 100644 index 00000000000..a1e1b00fcce --- /dev/null +++ b/contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql @@ -0,0 +1,10 @@ +/* contrib/pg_aux_catalog/pg_aux_catalog--1.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION pg_aux_catalog" to load this file. \quit + +-- Create the mdb_admin role with fixed OID 8067 +CREATE FUNCTION pg_create_mdb_admin_role() +RETURNS OID +AS 'MODULE_PATHNAME', 'pg_create_mdb_admin_role' +LANGUAGE C PARALLEL SAFE STRICT; diff --git a/contrib/pg_aux_catalog/pg_aux_catalog.c b/contrib/pg_aux_catalog/pg_aux_catalog.c new file mode 100644 index 00000000000..91685561cca --- /dev/null +++ b/contrib/pg_aux_catalog/pg_aux_catalog.c @@ -0,0 +1,88 @@ +/*------------------------------------------------------------------------- + * + * pg_aux_catalog.c + * Extension for auxiliary catalog management + * + * contrib/pg_aux_catalog/pg_aux_catalog.c + * + *------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "catalog/oid_dispatch.h" +#include "catalog/pg_authid.h" +#include "commands/user.h" +#include "fmgr.h" +#include "miscadmin.h" +#include "nodes/makefuncs.h" +#include "nodes/parsenodes.h" +#include "utils/acl.h" +#include "utils/builtins.h" +#include "utils/syscache.h" + +PG_MODULE_MAGIC; + +/* Name of the mdb_admin role; its OID is MDB_ADMIN_ROLEID (see acl.h). */ +#define MDB_ADMIN_ROLE_NAME "mdb_admin" + +PG_FUNCTION_INFO_V1(pg_create_mdb_admin_role); + +/* + * Create the mdb_admin role with its fixed OID (MDB_ADMIN_ROLEID, 8067). + * + * The core privilege checks identify mdb_admin by this fixed OID (see acl.c + * and resgroupcmds.c), so the role must always be created with it. On a + * Cloudberry cluster the OID is dispatched to the segments so the role ends + * up with the same OID everywhere. Returns the new role's OID. + */ +Datum +pg_create_mdb_admin_role(PG_FUNCTION_ARGS) +{ + CreateRoleStmt stmt; + List *options = NIL; + Oid roleid; + + /* + * Only a superuser may establish the mdb_admin privilege role. Otherwise + * a CREATEROLE user could drop mdb_admin and re-create it (CreateRole only + * requires CREATEROLE), taking ownership of the fixed-OID role and + * granting the capability to itself. + */ + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to create the mdb_admin role"))); + + /* Check if a role with the fixed OID already exists. */ + if (SearchSysCacheExists1(AUTHOID, ObjectIdGetDatum(MDB_ADMIN_ROLEID))) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("role with OID %u already exists", MDB_ADMIN_ROLEID))); + + /* Check if a role named "mdb_admin" already exists. */ + if (SearchSysCacheExists1(AUTHNAME, CStringGetDatum(MDB_ADMIN_ROLE_NAME))) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("role \"%s\" already exists", MDB_ADMIN_ROLE_NAME))); + + /* Build options for CreateRole: connection limit = 0. */ + options = list_make1(makeDefElem("connectionlimit", + (Node *) makeInteger(0), -1)); + + /* Prepare the CreateRoleStmt. */ + memset(&stmt, 0, sizeof(stmt)); + stmt.type = T_CreateRoleStmt; + stmt.stmt_type = ROLESTMT_ROLE; + stmt.role = MDB_ADMIN_ROLE_NAME; + stmt.options = options; + + /* + * Request the fixed OID for the role. GetNewOidForAuthId() consumes and + * clears this override. + */ + next_aux_pg_authid_oid = MDB_ADMIN_ROLEID; + + roleid = CreateRole(NULL, &stmt); + + PG_RETURN_OID(roleid); +} diff --git a/contrib/pg_aux_catalog/pg_aux_catalog.control b/contrib/pg_aux_catalog/pg_aux_catalog.control new file mode 100644 index 00000000000..aff4205f7eb --- /dev/null +++ b/contrib/pg_aux_catalog/pg_aux_catalog.control @@ -0,0 +1,5 @@ +# pg_aux_catalog extension +comment = 'auxiliary catalog management (mdb_admin role creation)' +default_version = '1.0' +module_pathname = '$libdir/pg_aux_catalog' +relocatable = false diff --git a/contrib/pg_aux_catalog/sql/pg_aux_catalog.sql b/contrib/pg_aux_catalog/sql/pg_aux_catalog.sql new file mode 100644 index 00000000000..aa16ffe4e13 --- /dev/null +++ b/contrib/pg_aux_catalog/sql/pg_aux_catalog.sql @@ -0,0 +1,38 @@ +-- Tests for the pg_aux_catalog extension: creation of the fixed-OID +-- mdb_admin role and the resource-group permission gate it enables. + +CREATE EXTENSION pg_aux_catalog; + +-- --------------------------------------------------------------------- +-- pg_create_mdb_admin_role() creates the mdb_admin role with its fixed OID. +-- --------------------------------------------------------------------- +SELECT pg_create_mdb_admin_role() AS mdb_admin_oid; + +-- The role exists with the fixed OID and is a non-login, non-superuser, +-- connection-limited role. +SELECT oid = 8067 AS has_fixed_oid, rolcanlogin, rolsuper, + rolcreaterole, rolcreatedb, rolconnlimit + FROM pg_authid WHERE rolname = 'mdb_admin'; + +-- Creating it a second time is rejected. +SELECT pg_create_mdb_admin_role(); + +-- --------------------------------------------------------------------- +-- Resource-group permission gate: a role that is not a member of mdb_admin +-- is rejected on every entry point. These checks run before the "resource +-- group is enabled" check, so they are deterministic regardless of the +-- resource manager in use. +-- --------------------------------------------------------------------- +CREATE ROLE regress_rg_noadmin; +SET ROLE regress_rg_noadmin; +CREATE RESOURCE GROUP regress_rg_x WITH (concurrency=1, cpu_max_percent=5); +ALTER RESOURCE GROUP regress_rg_x SET cpu_max_percent 6; +DROP RESOURCE GROUP regress_rg_x; +RESET ROLE; +DROP ROLE regress_rg_noadmin; + +-- --------------------------------------------------------------------- +-- Cleanup. +-- --------------------------------------------------------------------- +DROP ROLE mdb_admin; +DROP EXTENSION pg_aux_catalog; diff --git a/pom.xml b/pom.xml index 0e000093399..6eaa095fa37 100644 --- a/pom.xml +++ b/pom.xml @@ -352,6 +352,9 @@ code or new licensing patterns. contrib/indexscan/indexscan.c contrib/indexscan/indexscan.sql.in + contrib/pg_aux_catalog/pg_aux_catalog.c + contrib/pg_aux_catalog/isolation2/isolation2_schedule + contrib/file_fdw/init_file contrib/file_fdw/data/** diff --git a/src/backend/catalog/oid_dispatch.c b/src/backend/catalog/oid_dispatch.c index 6f39a07857e..eaa2e099876 100644 --- a/src/backend/catalog/oid_dispatch.c +++ b/src/backend/catalog/oid_dispatch.c @@ -156,6 +156,18 @@ static MemoryContext oids_context = NULL; static bool preserve_oids_on_commit = false; +/* + * OID to assign to the next auxiliary pg_authid role created through + * GetNewOidForAuthId(), or InvalidOid for the normal allocation path. + * + * This is the GPDB analogue of upstream PostgreSQL's + * binary_upgrade_next_pg_authid_oid (which is disabled here in favour of the + * generic OID pre-assignment machinery). It lets contrib/pg_aux_catalog + * create roles such as mdb_admin with a fixed, well-known OID. It is reset + * to InvalidOid as soon as it is consumed. + */ +Oid next_aux_pg_authid_oid = InvalidOid; + /* * These will be used by the schema restoration process during binary upgrade, * so any new object must not use any Oid in this structure or else there will @@ -423,6 +435,7 @@ GetNewOrPreassignedOid(Relation relation, Oid indexId, AttrNumber oidcolumn, OidAssignment *searchkey) { Oid oid; + Oid forcedOid = searchkey->oid; searchkey->catalog = RelationGetRelid(relation); @@ -461,8 +474,18 @@ GetNewOrPreassignedOid(Relation relation, Oid indexId, AttrNumber oidcolumn, { MemoryContext oldcontext; - /* Assign a new oid, and memorize it in the list of OIDs to dispatch */ - oid = GetNewOidWithIndex(relation, indexId, oidcolumn); + /* + * Assign a new oid, and memorize it in the list of OIDs to dispatch. + * + * A caller may request a fixed, well-known OID by passing it in + * searchkey->oid (e.g. the mdb_admin auxiliary role, see + * GetNewOidForAuthId()). In that case use the requested OID instead + * of allocating a fresh one, but still record it for dispatch so the + * QEs end up with the same OID. + */ + oid = OidIsValid(forcedOid) + ? forcedOid + : GetNewOidWithIndex(relation, indexId, oidcolumn); oldcontext = MemoryContextSwitchTo(get_oids_context()); searchkey->oid = oid; @@ -479,7 +502,9 @@ GetNewOrPreassignedOid(Relation relation, Oid indexId, AttrNumber oidcolumn, } else { - oid = GetNewOidWithIndex(relation, indexId, oidcolumn); + oid = OidIsValid(forcedOid) + ? forcedOid + : GetNewOidWithIndex(relation, indexId, oidcolumn); } return oid; @@ -572,6 +597,25 @@ GetNewOidForAuthId(Relation relation, Oid indexId, AttrNumber oidcolumn, memset(&key, 0, sizeof(OidAssignment)); key.type = T_OidAssignment; key.objname = rolname; + + /* + * Allow auxiliary roles (such as mdb_admin, see contrib/pg_aux_catalog) + * to be created with a fixed, well-known OID. The OID is supplied through + * the next_aux_pg_authid_oid override, mirroring how upstream PostgreSQL + * assigns role OIDs during binary upgrade. We only honor it for OIDs in + * the auxiliary range to avoid clashing with normal OID allocation, and + * reset it immediately so it affects a single role only. + */ + if (OidIsValid(next_aux_pg_authid_oid)) + { + if (!IsAuxOid(next_aux_pg_authid_oid)) + elog(ERROR, "pre-assigned auxiliary role OID %u is out of the auxiliary OID range", + next_aux_pg_authid_oid); + + key.oid = next_aux_pg_authid_oid; + next_aux_pg_authid_oid = InvalidOid; + } + return GetNewOrPreassignedOid(relation, indexId, oidcolumn, &key); } diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c index 384675edb7f..b746a3db49b 100644 --- a/src/backend/commands/resgroupcmds.c +++ b/src/backend/commands/resgroupcmds.c @@ -34,6 +34,7 @@ #include "commands/resgroupcmds.h" #include "miscadmin.h" #include "nodes/pg_list.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/datetime.h" #include "utils/fmgroids.h" @@ -103,11 +104,11 @@ CreateResourceGroup(CreateResourceGroupStmt *stmt) int nResGroups; MemoryContext oldContext; - /* Permission check - only superuser can create groups. */ - if (!superuser()) + /* Permission check - only superuser or mdb_admin can create groups. */ + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to create resource groups"))); + errmsg("must be mdb_admin to create resource groups"))); /* * Check for an illegal name ('none' is used to signify no group in ALTER ROLE). @@ -269,11 +270,11 @@ DropResourceGroup(DropResourceGroupStmt *stmt) Oid groupid; ResourceGroupCallbackContext *callbackCtx; - /* Permission check - only superuser can drop resource groups. */ - if (!superuser()) + /* Permission check - only superuser or mdb_admin can drop resource groups. */ + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to drop resource groups"))); + errmsg("must be mdb_admin to drop resource groups"))); /* * Check the pg_resgroup relation to be certain the resource group already @@ -302,6 +303,13 @@ DropResourceGroup(DropResourceGroupStmt *stmt) */ groupid = ((Form_pg_resgroup) GETSTRUCT(tuple))->oid; + /* Permission check - only superuser can drop the admin/system resource groups. */ + if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to drop resource group \"%s\"", + stmt->name))); + /* cannot DROP default resource groups */ if (groupid == DEFAULTRESGROUP_OID || groupid == ADMINRESGROUP_OID @@ -375,11 +383,24 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) ResourceGroupCallbackContext *callbackCtx; MemoryContext oldContext; - /* Permission check - only superuser can alter resource groups. */ - if (!superuser()) + /* Permission check - only mdb_admin can alter resource groups. */ + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be mdb_admin to alter resource groups"))); + + /* + * Check the pg_resgroup relation to be certain the resource group already + * exists. + */ + groupid = get_resgroup_oid(stmt->name, false); + + /* Permission check - only superuser can alter the admin/system resource groups. */ + if (!superuser() && (groupid == ADMINRESGROUP_OID || groupid == SYSTEMRESGROUP_OID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to alter resource groups"))); + errmsg("must be superuser to alter resource group \"%s\"", + stmt->name))); /* Currently we only support to ALTER one limit at one time */ Assert(list_length(stmt->options) == 1); @@ -406,12 +427,6 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) checkResgroupCapLimit(limitType, value); } - /* - * Check the pg_resgroup relation to be certain the resource group already - * exists. - */ - groupid = get_resgroup_oid(stmt->name, false); - if (limitType == RESGROUP_LIMIT_TYPE_CONCURRENCY && value == 0 && groupid == ADMINRESGROUP_OID) @@ -500,7 +515,7 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) RESGROUP_DEFAULT_CPU_WEIGHT, ""); updateResgroupCapabilityEntry(pg_resgroupcapability_rel, - groupid, RESGROUP_LIMIT_TYPE_CPUSET, + groupid, RESGROUP_LIMIT_TYPE_CPUSET, 0, caps.cpuset); } else if (limitType == RESGROUP_LIMIT_TYPE_CPU) @@ -1007,7 +1022,7 @@ parseStmtOptions(CreateResourceGroupStmt *stmt, ResGroupCaps *caps) else mask |= 1 << type; - if (type == RESGROUP_LIMIT_TYPE_CPUSET) + if (type == RESGROUP_LIMIT_TYPE_CPUSET) { const char *cpuset = defGetString(defel); strlcpy(caps->cpuset, cpuset, sizeof(caps->cpuset)); @@ -1611,7 +1626,7 @@ checkCpuSetByRole(const char *cpuset) * ex: * cpuset = "1;4" * then we should assign '1' to corrdinator and '4' to segment - * + * * cpuset = "1" * assign '1' to both coordinator and segment */ diff --git a/src/backend/utils/resgroup/resgroup_helper.c b/src/backend/utils/resgroup/resgroup_helper.c index 00aaded168d..5acca0be5e7 100644 --- a/src/backend/utils/resgroup/resgroup_helper.c +++ b/src/backend/utils/resgroup/resgroup_helper.c @@ -21,6 +21,7 @@ #include "cdb/cdbvars.h" #include "commands/resgroupcmds.h" #include "storage/procarray.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/datetime.h" #include "utils/resgroup.h" @@ -464,10 +465,10 @@ pg_resgroup_move_query(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), (errmsg("resource group is not enabled")))); - if (!superuser()) + if (!is_member_of_role(GetUserId(), MDB_ADMIN_ROLEID)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg("must be superuser to move query")))); + (errmsg("must be mdb_admin to move query")))); if (Gp_role == GP_ROLE_DISPATCH) { diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 687799bec9f..a1bf330b762 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -208,6 +208,16 @@ FullTransactionIdAdvance(FullTransactionId *dest) #define FirstBinaryUpgradeReservedObjectId 9000 #define LastBinaryUpgradeReservedObjectId 9100 +/* + * Reserve a block of OIDs for auxiliary catalog objects (such as the + * mdb_admin role created by contrib/pg_aux_catalog). These need fixed, + * well-known OIDs that are stable across clusters, so they live in their own + * range below FirstBinaryUpgradeReservedObjectId. + */ +#define FirstAuxObjectId 8000 +#define LastAuxObjectId 9000 +#define IsAuxOid(oid) ((oid) >= FirstAuxObjectId && (oid) < LastAuxObjectId) + /* * VariableCache is a data structure in shared memory that is used to track * OID and XID assignment state. For largely historical reasons, there is diff --git a/src/include/catalog/oid_dispatch.h b/src/include/catalog/oid_dispatch.h index fbb7a14f59e..d9c543bee5d 100644 --- a/src/include/catalog/oid_dispatch.h +++ b/src/include/catalog/oid_dispatch.h @@ -16,6 +16,13 @@ #include "utils/relcache.h" #include "access/htup.h" +/* + * OID to assign to the next auxiliary pg_authid role created through + * GetNewOidForAuthId(), or InvalidOid for normal allocation. Set by + * contrib/pg_aux_catalog to create roles such as mdb_admin with a fixed OID. + */ +extern PGDLLIMPORT Oid next_aux_pg_authid_oid; + /* Functions used in master */ extern List *GetAssignedOidsForDispatch(void); diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 49068f04b2f..e3949dd8769 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -213,6 +213,14 @@ extern bool is_member_of_role_nosuper(Oid member, Oid role); extern bool is_admin_of_role(Oid member, Oid role); // -- non-upstream patch begin +/* + * Fixed, well-known OID of the mdb_admin role. The role is created by + * contrib/pg_aux_catalog (pg_create_mdb_admin_role()) with this OID, and the + * resource-group permission checks identify it by OID. It lives in the + * auxiliary OID range (see IsAuxOid()). + */ +#define MDB_ADMIN_ROLEID 8067 + extern bool mdb_admin_allow_bypass_owner_checks(Oid userId, Oid ownerId); extern void check_mdb_admin_is_member_of_role(Oid member, Oid role);