This is an automated email from the ASF dual-hosted git repository. leborchuk pushed a commit to branch PG14_ARCHIVE_REBASED in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit b2f60ef5f8491d71cb1772a20fa7afe4e985eeb0 Author: reshke <[email protected]> AuthorDate: Fri Sep 19 21:47:24 2025 +0500 MDB admin patch & tests (#4) * MDB admin patch & tests This patch introcudes new pseudo-pre-defined role "mdb_admin". Introduces 2 new function: 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); To check mdb admin belongship and role-to-role ownership transfer correctness. Our mdb_admin ACL model is the following: * Any roles user or/and roles can be granted with mdb_admin * mdb_admin memeber can tranfser ownershup of relations, namespaces and functions to other roles, if target role in neither: superuser, pg_read_server_files, pg_write_server_files nor pg_execute_server_program. This patch allows mdb admin to tranfers ownership on non-superuser objects * f --- src/backend/commands/functioncmds.c | 4 +- src/backend/storage/ipc/signalfuncs.c | 37 ++++++++++++---- src/backend/utils/adt/acl.c | 75 ++++++++++++++++++++++++++++++--- src/test/regress/expected/mdb_admin.out | 55 ++++++++---------------- src/test/regress/parallel_schedule | 3 +- src/test/regress/sql/mdb_admin.sql | 17 ++------ 6 files changed, 124 insertions(+), 67 deletions(-) diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 1ab3b36dd59..8a570fa6965 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -1526,7 +1526,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt) */ if (isLeakProof && !superuser()) { - Oid role = get_role_oid("mdb_admin", true /*if nodoby created mdb_admin role in this database*/); + Oid role = get_role_oid("mdb_admin", true); if (!is_member_of_role(GetUserId(), role)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), @@ -1857,7 +1857,7 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt) procForm->proleakproof = intVal(leakproof_item->arg); if (procForm->proleakproof && !superuser()) { - Oid role = get_role_oid("mdb_admin", true /*if nodoby created mdb_admin role in this database*/); + Oid role = get_role_oid("mdb_admin", true); if (!is_member_of_role(GetUserId(), role)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c index 7f8e420a6a5..753b94752d3 100644 --- a/src/backend/storage/ipc/signalfuncs.c +++ b/src/backend/storage/ipc/signalfuncs.c @@ -52,6 +52,7 @@ static int pg_signal_backend(int pid, int sig, char *msg) { PGPROC *proc = BackendPidGetProc(pid); + LocalPgBackendStatus *local_beentry; /* * BackendPidGetProc returns NULL if the pid isn't valid; but by the time @@ -72,14 +73,34 @@ pg_signal_backend(int pid, int sig, char *msg) return SIGNAL_BACKEND_ERROR; } - /* - * Only allow superusers to signal superuser-owned backends. Any process - * not advertising a role might have the importance of a superuser-owned - * backend, so treat it that way. - */ - if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) && - !superuser()) - return SIGNAL_BACKEND_NOSUPERUSER; + local_beentry = pgstat_fetch_stat_local_beentry_by_pid(pid); + + /* Only allow superusers to signal superuser-owned backends. */ + if (superuser_arg(proc->roleId) && !superuser()) + { + Oid role; + char * appname; + + if (local_beentry == NULL) { + return SIGNAL_BACKEND_NOSUPERUSER; + } + + role = get_role_oid("mdb_admin", true /*if nodoby created mdb_admin role in this database*/); + appname = local_beentry->backendStatus.st_appname; + + // only allow mdb_admin to kill su queries + if (!is_member_of_role(GetUserId(), role)) { + return SIGNAL_BACKEND_NOSUPERUSER; + } + + if (local_beentry->backendStatus.st_backendType == B_AUTOVAC_WORKER) { + // ok + } else if (appname != NULL && strcmp(appname, "MDB") == 0) { + // ok + } else { + return SIGNAL_BACKEND_NOSUPERUSER; + } + } /* Users can signal backends they have role membership in. */ if (!has_privs_of_role(GetUserId(), proc->roleId) && diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 906480c5137..ae1fd5802c6 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -5129,6 +5129,60 @@ mdb_admin_allow_bypass_owner_checks(Oid userId, Oid ownerId) // -- non-upstream patch end +// -- non-upstream patch begin +/* + * Is userId allowed to bypass ownership check + * and tranfer onwership to ownerId role? + */ +bool +mdb_admin_allow_bypass_owner_checks(Oid userId, Oid ownerId) +{ + Oid mdb_admin_roleoid; + /* + * Never allow nobody to grant objects to + * superusers. + * This can result in various CVE. + * For paranoic reasons, check this even before + * membership of mdb_admin role. + */ + if (superuser_arg(ownerId)) { + return false; + } + + mdb_admin_roleoid = get_role_oid("mdb_admin", true /* superuser suggested to be mdb_admin*/); + /* Is userId actually member of mdb admin? */ + if (!is_member_of_role(userId, mdb_admin_roleoid)) { + /* if no, disallow. */ + return false; + } + + /* + * Now, we need to check if ownerId + * is some dangerous role to trasfer membership to. + * + * For now, we check that ownerId does not have + * priviledge to execute server program or/and + * read/write server files. + */ + + if (has_privs_of_role(ownerId, ROLE_PG_READ_SERVER_FILES)) { + return false; + } + + if (has_privs_of_role(ownerId, ROLE_PG_WRITE_SERVER_FILES)) { + return false; + } + + if (has_privs_of_role(ownerId, ROLE_PG_EXECUTE_SERVER_PROGRAM)) { + return false; + } + + /* All checks passed, hope will not be hacked here (again) */ + return true; +} + +// -- non-upstream patch end + /* * Is member a member of role (directly or indirectly)? * @@ -5173,7 +5227,7 @@ check_is_member_of_role(Oid member, Oid role) * check_mdb_admin_is_member_of_role * is_member_of_role with a standard permission-violation error if not in usual case * Is case `member` in mdb_admin we check that role is neither of superuser, pg_read/write - * server files nor pg_execute_server_program or pg_read/write all data + * server files nor pg_execute_server_program */ void check_mdb_admin_is_member_of_role(Oid member, Oid role) @@ -5184,10 +5238,9 @@ check_mdb_admin_is_member_of_role(Oid member, Oid role) return; } - mdb_admin_roleoid = get_role_oid("mdb_admin", true /*if nodoby created mdb_admin role in this database*/); + mdb_admin_roleoid = get_role_oid("mdb_admin", true /* superuser suggested to be mdb_admin*/); /* Is userId actually member of mdb admin? */ if (is_member_of_role(member, mdb_admin_roleoid)) { - /* role is mdb admin */ if (superuser_arg(role)) { ereport(ERROR, @@ -5196,10 +5249,22 @@ check_mdb_admin_is_member_of_role(Oid member, Oid role) GetUserNameFromId(role, false)))); } - if (has_privs_of_unwanted_system_role(role)) { + if (has_privs_of_role(role, ROLE_PG_READ_SERVER_FILES)) { + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("cannot transfer ownership to pg_read_server_files role in Cloud"))); + } + + if (has_privs_of_role(role, ROLE_PG_WRITE_SERVER_FILES)) { + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("cannot transfer ownership to pg_write_server_files role in Cloud"))); + } + + if (has_privs_of_role(role, ROLE_PG_EXECUTE_SERVER_PROGRAM)) { ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("forbidden to transfer ownership to this system role in Cloud"))); + errmsg("cannot transfer ownership to pg_execute_server_program role in Cloud"))); } } else { /* if no, check membership transfer in usual way. */ diff --git a/src/test/regress/expected/mdb_admin.out b/src/test/regress/expected/mdb_admin.out index e4dfc436802..5fc2dab10cb 100644 --- a/src/test/regress/expected/mdb_admin.out +++ b/src/test/regress/expected/mdb_admin.out @@ -1,6 +1,7 @@ CREATE ROLE regress_mdb_admin_user1; CREATE ROLE regress_mdb_admin_user2; CREATE ROLE regress_mdb_admin_user3; +CREATE ROLE mdb_admin; CREATE ROLE regress_superuser WITH SUPERUSER; GRANT mdb_admin TO regress_mdb_admin_user1; GRANT CREATE ON DATABASE regression TO regress_mdb_admin_user2; @@ -23,7 +24,7 @@ ALTER VIEW regress_mdb_admin_view OWNER TO regress_mdb_admin_user3; ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO regress_mdb_admin_user3; ALTER TABLE regress_mdb_admin_table OWNER TO regress_mdb_admin_user3; ALTER SCHEMA regress_mdb_admin_schema OWNER TO regress_mdb_admin_user3; --- mdb admin fails to transfer ownership to superusers and particular system roles +-- mdb admin fails to transfer ownership to superusers and system roles ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO regress_superuser; ERROR: cannot transfer ownership to superuser "regress_superuser" ALTER VIEW regress_mdb_admin_view OWNER TO regress_superuser; @@ -35,55 +36,35 @@ ERROR: cannot transfer ownership to superuser "regress_superuser" ALTER SCHEMA regress_mdb_admin_schema OWNER TO regress_superuser; ERROR: cannot transfer ownership to superuser "regress_superuser" ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO pg_execute_server_program; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_execute_server_program role in Cloud ALTER VIEW regress_mdb_admin_view OWNER TO pg_execute_server_program; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_execute_server_program role in Cloud ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_execute_server_program; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_execute_server_program role in Cloud ALTER TABLE regress_mdb_admin_table OWNER TO pg_execute_server_program; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_execute_server_program role in Cloud ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_execute_server_program; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_execute_server_program role in Cloud ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO pg_write_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_write_server_files role in Cloud ALTER VIEW regress_mdb_admin_view OWNER TO pg_write_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_write_server_files role in Cloud ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_write_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_write_server_files role in Cloud ALTER TABLE regress_mdb_admin_table OWNER TO pg_write_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_write_server_files role in Cloud ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_write_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_write_server_files role in Cloud ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO pg_read_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_read_server_files role in Cloud ALTER VIEW regress_mdb_admin_view OWNER TO pg_read_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_read_server_files role in Cloud ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_read_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_read_server_files role in Cloud ALTER TABLE regress_mdb_admin_table OWNER TO pg_read_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_read_server_files role in Cloud ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_read_server_files; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO pg_write_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER VIEW regress_mdb_admin_view OWNER TO pg_write_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_write_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER TABLE regress_mdb_admin_table OWNER TO pg_write_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_write_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO pg_read_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER VIEW regress_mdb_admin_view OWNER TO pg_read_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_read_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER TABLE regress_mdb_admin_table OWNER TO pg_read_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud -ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_read_all_data; -ERROR: forbidden to transfer ownership to this system role in Cloud +ERROR: cannot transfer ownership to pg_read_server_files role in Cloud -- end tests RESET SESSION AUTHORIZATION; -- @@ -97,4 +78,4 @@ DROP SCHEMA regress_mdb_admin_schema; DROP ROLE regress_mdb_admin_user1; DROP ROLE regress_mdb_admin_user2; DROP ROLE regress_mdb_admin_user3; -DROP ROLE regress_superuser; +DROP ROLE mdb_admin; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 5adb7d9df01..f458be0bcd8 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -6,7 +6,8 @@ # ---------- # mdb admin simple checks -test: test_setup + +test: mdb_admin # run tablespace by itself, and first, because it forces a checkpoint; # we'd prefer not to have checkpoints later in the tests because that diff --git a/src/test/regress/sql/mdb_admin.sql b/src/test/regress/sql/mdb_admin.sql index b6b048e5692..65e294769ee 100644 --- a/src/test/regress/sql/mdb_admin.sql +++ b/src/test/regress/sql/mdb_admin.sql @@ -1,6 +1,7 @@ CREATE ROLE regress_mdb_admin_user1; CREATE ROLE regress_mdb_admin_user2; CREATE ROLE regress_mdb_admin_user3; +CREATE ROLE mdb_admin; CREATE ROLE regress_superuser WITH SUPERUSER; @@ -31,7 +32,7 @@ ALTER TABLE regress_mdb_admin_table OWNER TO regress_mdb_admin_user3; ALTER SCHEMA regress_mdb_admin_schema OWNER TO regress_mdb_admin_user3; --- mdb admin fails to transfer ownership to superusers and particular system roles +-- mdb admin fails to transfer ownership to superusers and system roles ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO regress_superuser; ALTER VIEW regress_mdb_admin_view OWNER TO regress_superuser; @@ -57,18 +58,6 @@ ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_read_se ALTER TABLE regress_mdb_admin_table OWNER TO pg_read_server_files; ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_read_server_files; -ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO pg_write_all_data; -ALTER VIEW regress_mdb_admin_view OWNER TO pg_write_all_data; -ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_write_all_data; -ALTER TABLE regress_mdb_admin_table OWNER TO pg_write_all_data; -ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_write_all_data; - -ALTER FUNCTION regress_mdb_admin_add (integer, integer) OWNER TO pg_read_all_data; -ALTER VIEW regress_mdb_admin_view OWNER TO pg_read_all_data; -ALTER TABLE regress_mdb_admin_schema.regress_mdb_admin_table OWNER TO pg_read_all_data; -ALTER TABLE regress_mdb_admin_table OWNER TO pg_read_all_data; -ALTER SCHEMA regress_mdb_admin_schema OWNER TO pg_read_all_data; - -- end tests RESET SESSION AUTHORIZATION; @@ -84,4 +73,4 @@ DROP SCHEMA regress_mdb_admin_schema; DROP ROLE regress_mdb_admin_user1; DROP ROLE regress_mdb_admin_user2; DROP ROLE regress_mdb_admin_user3; -DROP ROLE regress_superuser; +DROP ROLE mdb_admin; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
