Skip to content

Commit 2e308bb

Browse files
committed
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
1 parent 25f28fa commit 2e308bb

File tree

24 files changed

+494
-33
lines changed

24 files changed

+494
-33
lines changed

contrib/pax_storage/src/test/regress/expected/create_function_3.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,10 @@ SET SESSION AUTHORIZATION regress_unpriv_user;
166166
SET search_path TO temp_func_test, public;
167167
ALTER FUNCTION functest_E_1(int) NOT LEAKPROOF;
168168
ALTER FUNCTION functest_E_2(int) LEAKPROOF;
169-
ERROR: only superuser can define a leakproof function
169+
ERROR: only superuser or mdb_admin can define a leakproof function
170170
CREATE FUNCTION functest_E_3(int) RETURNS bool LANGUAGE 'sql'
171171
LEAKPROOF AS 'SELECT $1 < 200'; -- fail
172-
ERROR: only superuser can define a leakproof function
172+
ERROR: only superuser or mdb_admin can define a leakproof function
173173
RESET SESSION AUTHORIZATION;
174174
--
175175
-- CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT

contrib/pax_storage/src/test/regress/expected/create_function_3_optimizer.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,10 @@ SET SESSION AUTHORIZATION regress_unpriv_user;
166166
SET search_path TO temp_func_test, public;
167167
ALTER FUNCTION functest_E_1(int) NOT LEAKPROOF;
168168
ALTER FUNCTION functest_E_2(int) LEAKPROOF;
169-
ERROR: only superuser can define a leakproof function
169+
ERROR: only superuser or mdb_admin can define a leakproof function
170170
CREATE FUNCTION functest_E_3(int) RETURNS bool LANGUAGE 'sql'
171171
LEAKPROOF AS 'SELECT $1 < 200'; -- fail
172-
ERROR: only superuser can define a leakproof function
172+
ERROR: only superuser or mdb_admin can define a leakproof function
173173
RESET SESSION AUTHORIZATION;
174174
--
175175
-- CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT

src/backend/catalog/namespace.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2971,7 +2971,6 @@ LookupExplicitNamespace(const char *nspname, bool missing_ok)
29712971
{
29722972
Oid namespaceId;
29732973
AclResult aclresult;
2974-
29752974
/* check for pg_temp alias */
29762975
if (strcmp(nspname, "pg_temp") == 0)
29772976
{
@@ -2989,7 +2988,24 @@ LookupExplicitNamespace(const char *nspname, bool missing_ok)
29892988
if (missing_ok && !OidIsValid(namespaceId))
29902989
return InvalidOid;
29912990

2992-
aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(), ACL_USAGE);
2991+
HeapTuple tuple;
2992+
Oid ownerId;
2993+
2994+
tuple = SearchSysCache1(NAMESPACEOID, ObjectIdGetDatum(namespaceId));
2995+
if (!HeapTupleIsValid(tuple))
2996+
ereport(ERROR,
2997+
(errcode(ERRCODE_UNDEFINED_SCHEMA),
2998+
errmsg("schema with OID %u does not exist", namespaceId)));
2999+
3000+
ownerId = ((Form_pg_namespace) GETSTRUCT(tuple))->nspowner;
3001+
3002+
ReleaseSysCache(tuple);
3003+
3004+
if (!mdb_admin_allow_bypass_owner_checks(GetUserId(), ownerId)) {
3005+
aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(), ACL_USAGE);
3006+
} else {
3007+
aclresult = ACLCHECK_OK;
3008+
}
29933009
if (aclresult != ACLCHECK_OK)
29943010
aclcheck_error(aclresult, OBJECT_SCHEMA,
29953011
nspname);

src/backend/commands/alter.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
10851085
if (!superuser())
10861086
{
10871087
/* must be owner */
1088-
if (!has_privs_of_role(GetUserId(), old_ownerId))
1088+
if (!has_privs_of_role(GetUserId(), old_ownerId)
1089+
&& !mdb_admin_allow_bypass_owner_checks(GetUserId(), old_ownerId))
10891090
{
10901091
char *objname;
10911092
char namebuf[NAMEDATALEN];
@@ -1105,14 +1106,13 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
11051106
aclcheck_error(ACLCHECK_NOT_OWNER, get_object_type(classId, objectId),
11061107
objname);
11071108
}
1108-
/* Must be able to become new owner */
1109-
check_is_member_of_role(GetUserId(), new_ownerId);
1109+
1110+
check_mdb_admin_is_member_of_role(GetUserId(), new_ownerId);
11101111

11111112
/* New owner must have CREATE privilege on namespace */
11121113
if (OidIsValid(namespaceId))
11131114
{
11141115
AclResult aclresult;
1115-
11161116
aclresult = pg_namespace_aclcheck(namespaceId, new_ownerId,
11171117
ACL_CREATE);
11181118
if (aclresult != ACLCHECK_OK)

src/backend/commands/functioncmds.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,9 +1525,13 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
15251525
* by security barrier views or row-level security policies.
15261526
*/
15271527
if (isLeakProof && !superuser())
1528-
ereport(ERROR,
1529-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1530-
errmsg("only superuser can define a leakproof function")));
1528+
{
1529+
Oid role = get_role_oid("mdb_admin", true);
1530+
if (!is_member_of_role(GetUserId(), role))
1531+
ereport(ERROR,
1532+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1533+
errmsg("only superuser or mdb_admin can define a leakproof function")));
1534+
}
15311535

15321536
if (transformDefElem)
15331537
{
@@ -1852,9 +1856,13 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
18521856
{
18531857
procForm->proleakproof = intVal(leakproof_item->arg);
18541858
if (procForm->proleakproof && !superuser())
1855-
ereport(ERROR,
1856-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1857-
errmsg("only superuser can define a leakproof function")));
1859+
{
1860+
Oid role = get_role_oid("mdb_admin", true);
1861+
if (!is_member_of_role(GetUserId(), role))
1862+
ereport(ERROR,
1863+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1864+
errmsg("only superuser or mdb_admin can define a leakproof function")));
1865+
}
18581866
}
18591867
if (cost_item)
18601868
{

src/backend/commands/schemacmds.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -598,12 +598,12 @@ AlterSchemaOwner_internal(HeapTuple tup, Relation rel, Oid newOwnerId)
598598
AclResult aclresult;
599599

600600
/* Otherwise, must be owner of the existing object */
601-
if (!pg_namespace_ownercheck(nspForm->oid, GetUserId()))
601+
if (!mdb_admin_allow_bypass_owner_checks(GetUserId(), nspForm->nspowner)
602+
&& !pg_namespace_ownercheck(nspForm->oid, GetUserId()))
602603
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SCHEMA,
603604
NameStr(nspForm->nspname));
604605

605-
/* Must be able to become new owner */
606-
check_is_member_of_role(GetUserId(), newOwnerId);
606+
check_mdb_admin_is_member_of_role(GetUserId(), newOwnerId);
607607

608608
/*
609609
* must have create-schema rights
@@ -614,8 +614,13 @@ AlterSchemaOwner_internal(HeapTuple tup, Relation rel, Oid newOwnerId)
614614
* schemas. Because superusers will always have this right, we need
615615
* no special case for them.
616616
*/
617-
aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(),
617+
if (mdb_admin_allow_bypass_owner_checks(GetUserId(), nspForm->nspowner)) {
618+
aclresult = ACLCHECK_OK;
619+
} else {
620+
aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(),
618621
ACL_CREATE);
622+
}
623+
619624
if (aclresult != ACLCHECK_OK)
620625
aclcheck_error(aclresult, OBJECT_DATABASE,
621626
get_database_name(MyDatabaseId));

src/backend/commands/tablecmds.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15704,13 +15704,14 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock
1570415704
AclResult aclresult;
1570515705

1570615706
/* Otherwise, must be owner of the existing object */
15707-
if (!pg_class_ownercheck(relationOid, GetUserId()))
15707+
if (!mdb_admin_allow_bypass_owner_checks(GetUserId(), tuple_class->relowner)
15708+
&& !pg_class_ownercheck(relationOid, GetUserId()))
1570815709
aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relationOid)),
1570915710
RelationGetRelationName(target_rel));
1571015711

15711-
/* Must be able to become new owner */
15712-
check_is_member_of_role(GetUserId(), newOwnerId);
1571315712

15713+
check_mdb_admin_is_member_of_role(GetUserId(), newOwnerId);
15714+
1571415715
/* New owner must have CREATE privilege on namespace */
1571515716
aclresult = pg_namespace_aclcheck(namespaceOid, newOwnerId,
1571615717
ACL_CREATE);
@@ -20791,15 +20792,16 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid,
2079120792
Form_pg_class classform;
2079220793
AclResult aclresult;
2079320794
char relkind;
20794-
20795+
2079520796
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
2079620797
if (!HeapTupleIsValid(tuple))
2079720798
return; /* concurrently dropped */
2079820799
classform = (Form_pg_class) GETSTRUCT(tuple);
2079920800
relkind = classform->relkind;
2080020801

2080120802
/* Must own relation. */
20802-
if (!pg_class_ownercheck(relid, GetUserId()))
20803+
if (!mdb_admin_allow_bypass_owner_checks(GetUserId(), classform->relowner)
20804+
&& !pg_class_ownercheck(relid, GetUserId()))
2080320805
aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relid)), rv->relname);
2080420806

2080520807
/* No system table modifications unless explicitly allowed. */

src/backend/storage/ipc/signalfuncs.c

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ static int
5252
pg_signal_backend(int pid, int sig, char *msg)
5353
{
5454
PGPROC *proc = BackendPidGetProc(pid);
55+
LocalPgBackendStatus *local_beentry;
5556

5657
/*
5758
* BackendPidGetProc returns NULL if the pid isn't valid; but by the time
@@ -72,9 +73,34 @@ pg_signal_backend(int pid, int sig, char *msg)
7273
return SIGNAL_BACKEND_ERROR;
7374
}
7475

76+
local_beentry = pgstat_fetch_stat_local_beentry_by_pid(pid);
77+
7578
/* Only allow superusers to signal superuser-owned backends. */
7679
if (superuser_arg(proc->roleId) && !superuser())
77-
return SIGNAL_BACKEND_NOSUPERUSER;
80+
{
81+
Oid role;
82+
char * appname;
83+
84+
if (local_beentry == NULL) {
85+
return SIGNAL_BACKEND_NOSUPERUSER;
86+
}
87+
88+
role = get_role_oid("mdb_admin", true /*if nodoby created mdb_admin role in this database*/);
89+
appname = local_beentry->backendStatus.st_appname;
90+
91+
// only allow mdb_admin to kill su queries
92+
if (!is_member_of_role(GetUserId(), role)) {
93+
return SIGNAL_BACKEND_NOSUPERUSER;
94+
}
95+
96+
if (local_beentry->backendStatus.st_backendType == B_AUTOVAC_WORKER) {
97+
// ok
98+
} else if (appname != NULL && strcmp(appname, "MDB") == 0) {
99+
// ok
100+
} else {
101+
return SIGNAL_BACKEND_NOSUPERUSER;
102+
}
103+
}
78104

79105
/* Users can signal backends they have role membership in. */
80106
if (!has_privs_of_role(GetUserId(), proc->roleId) &&

src/backend/utils/activity/backend_status.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,6 +1102,22 @@ pgstat_fetch_stat_local_beentry(int beid)
11021102
return &localBackendStatusTable[beid - 1];
11031103
}
11041104

1105+
/* -- mdb admin patch -- */
1106+
LocalPgBackendStatus *
1107+
pgstat_fetch_stat_local_beentry_by_pid(int pid)
1108+
{
1109+
pgstat_read_current_status();
1110+
1111+
for (int i = 1; i <= localNumBackends; ++i) {
1112+
if (localBackendStatusTable[i - 1].backendStatus.st_procpid == pid) {
1113+
return &localBackendStatusTable[i - 1];
1114+
}
1115+
}
1116+
1117+
return NULL;
1118+
}
1119+
1120+
/* -- mdb admin patch end -- */
11051121

11061122
/* ----------
11071123
* pgstat_fetch_stat_numbackends() -

src/backend/utils/adt/acl.c

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5012,6 +5012,60 @@ has_privs_of_role(Oid member, Oid role)
50125012
}
50135013

50145014

5015+
// -- non-upstream patch begin
5016+
/*
5017+
* Is userId allowed to bypass ownership check
5018+
* and tranfer onwership to ownerId role?
5019+
*/
5020+
bool
5021+
mdb_admin_allow_bypass_owner_checks(Oid userId, Oid ownerId)
5022+
{
5023+
Oid mdb_admin_roleoid;
5024+
/*
5025+
* Never allow nobody to grant objects to
5026+
* superusers.
5027+
* This can result in various CVE.
5028+
* For paranoic reasons, check this even before
5029+
* membership of mdb_admin role.
5030+
*/
5031+
if (superuser_arg(ownerId)) {
5032+
return false;
5033+
}
5034+
5035+
mdb_admin_roleoid = get_role_oid("mdb_admin", true /* superuser suggested to be mdb_admin*/);
5036+
/* Is userId actually member of mdb admin? */
5037+
if (!is_member_of_role(userId, mdb_admin_roleoid)) {
5038+
/* if no, disallow. */
5039+
return false;
5040+
}
5041+
5042+
/*
5043+
* Now, we need to check if ownerId
5044+
* is some dangerous role to trasfer membership to.
5045+
*
5046+
* For now, we check that ownerId does not have
5047+
* priviledge to execute server program or/and
5048+
* read/write server files.
5049+
*/
5050+
5051+
if (has_privs_of_role(ownerId, ROLE_PG_READ_SERVER_FILES)) {
5052+
return false;
5053+
}
5054+
5055+
if (has_privs_of_role(ownerId, ROLE_PG_WRITE_SERVER_FILES)) {
5056+
return false;
5057+
}
5058+
5059+
if (has_privs_of_role(ownerId, ROLE_PG_EXECUTE_SERVER_PROGRAM)) {
5060+
return false;
5061+
}
5062+
5063+
/* All checks passed, hope will not be hacked here (again) */
5064+
return true;
5065+
}
5066+
5067+
// -- non-upstream patch end
5068+
50155069
/*
50165070
* Is member a member of role (directly or indirectly)?
50175071
*
@@ -5051,6 +5105,64 @@ check_is_member_of_role(Oid member, Oid role)
50515105
GetUserNameFromId(role, false))));
50525106
}
50535107

5108+
// -- mdb admin patch
5109+
/*
5110+
* check_mdb_admin_is_member_of_role
5111+
* is_member_of_role with a standard permission-violation error if not in usual case
5112+
* Is case `member` in mdb_admin we check that role is neither of superuser, pg_read/write
5113+
* server files nor pg_execute_server_program
5114+
*/
5115+
void
5116+
check_mdb_admin_is_member_of_role(Oid member, Oid role)
5117+
{
5118+
Oid mdb_admin_roleoid;
5119+
/* fast path - if we are superuser, its ok */
5120+
if (superuser_arg(member)) {
5121+
return;
5122+
}
5123+
5124+
mdb_admin_roleoid = get_role_oid("mdb_admin", true /* superuser suggested to be mdb_admin*/);
5125+
/* Is userId actually member of mdb admin? */
5126+
if (is_member_of_role(member, mdb_admin_roleoid)) {
5127+
/* role is mdb admin */
5128+
if (superuser_arg(role)) {
5129+
ereport(ERROR,
5130+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
5131+
errmsg("cannot transfer ownership to superuser \"%s\"",
5132+
GetUserNameFromId(role, false))));
5133+
}
5134+
5135+
if (has_privs_of_role(role, ROLE_PG_READ_SERVER_FILES)) {
5136+
ereport(ERROR,
5137+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
5138+
errmsg("cannot transfer ownership to pg_read_server_files role in Cloud")));
5139+
}
5140+
5141+
if (has_privs_of_role(role, ROLE_PG_WRITE_SERVER_FILES)) {
5142+
ereport(ERROR,
5143+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
5144+
errmsg("cannot transfer ownership to pg_write_server_files role in Cloud")));
5145+
}
5146+
5147+
if (has_privs_of_role(role, ROLE_PG_EXECUTE_SERVER_PROGRAM)) {
5148+
ereport(ERROR,
5149+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
5150+
errmsg("cannot transfer ownership to pg_execute_server_program role in Cloud")));
5151+
}
5152+
} else {
5153+
/* if no, check membership transfer in usual way. */
5154+
5155+
if (!is_member_of_role(member, role)) {
5156+
ereport(ERROR,
5157+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
5158+
errmsg("must be member of role \"%s\"",
5159+
GetUserNameFromId(role, false))));
5160+
}
5161+
}
5162+
}
5163+
5164+
// -- mdb admin patch
5165+
50545166
/*
50555167
* Is member a member of role, not considering superuserness?
50565168
*

0 commit comments

Comments
 (0)