From 84965d3ad70c4794f93473b939f5437c0d154826 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Wed, 26 Feb 2025 20:17:36 -0500 Subject: [PATCH] Allow database owners to CREATE EVENT TRIGGER --- src/backend/commands/dbcommands.c | 22 ++++ src/backend/commands/event_trigger.c | 38 ++++-- src/backend/utils/cache/lsyscache.c | 23 ++++ src/include/commands/dbcommands.h | 1 + src/include/utils/lsyscache.h | 1 + src/test/regress/expected/event_trigger.out | 2 +- src/test/regress/parallel_schedule | 2 +- .../regress/sql/event_trigger_dbowner.sql | 117 ++++++++++++++++++ 8 files changed, 197 insertions(+), 9 deletions(-) create mode 100644 src/test/regress/sql/event_trigger_dbowner.sql diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 1753b289074..4cd31dc1d90 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -3201,6 +3201,28 @@ get_database_name(Oid dbid) return result; } +/* + * get_database_owner - given a database OID, look up the owner OID + * + * If the owner is not found, it will return InvalidOid + */ +Oid +get_database_owner(Oid dbid) +{ + HeapTuple dbtuple; + Oid dbowner; + + dbtuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(dbid)); + if (HeapTupleIsValid(dbtuple)) + { + dbowner = ((Form_pg_database) GETSTRUCT(dbtuple))->datdba; + ReleaseSysCache(dbtuple); + } + else + dbowner = InvalidOid; + + return dbowner; +} /* * While dropping a database the pg_database row is marked invalid, but the diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index edc2c988e29..62e7c7326f4 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -34,6 +34,7 @@ #include "catalog/pg_trigger.h" #include "catalog/pg_ts_config.h" #include "catalog/pg_type.h" +#include "commands/dbcommands.h" #include "commands/event_trigger.h" #include "commands/extension.h" #include "commands/trigger.h" @@ -125,18 +126,17 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) Oid evtowner = GetUserId(); ListCell *lc; List *tags = NULL; + bool is_database_owner = (evtowner == get_database_owner(MyDatabaseId)); + bool owner_is_super = superuser_arg(evtowner); + bool function_is_owned_by_super; - /* - * It would be nice to allow database owners or even regular users to do - * this, but there are obvious privilege escalation risks which would have - * to somehow be plugged first. - */ - if (!superuser()) + if (!owner_is_super && !is_database_owner){ ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to create event trigger \"%s\"", stmt->trigname), - errhint("Must be superuser to create an event trigger."))); + errhint("Must be the database owner or superuser to create an event trigger."))); + } /* Validate event name. */ if (strcmp(stmt->eventname, "ddl_command_start") != 0 && @@ -200,6 +200,24 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) errmsg("function %s must return type %s", NameListToString(stmt->funcname), "event_trigger"))); + function_is_owned_by_super = superuser_arg(get_func_owner(funcoid)); + + if(owner_is_super && !function_is_owned_by_super){ + ereport(ERROR, ( + errcode(ERRCODE_INVALID_OBJECT_DEFINITION) + , errmsg("A superuser event trigger must execute a superuser owned function") + , errdetail("The current user \"%s\" is a superuser and the function \"%s\" is owned by a non-superuser", GetUserNameFromId(evtowner, false), NameListToString(stmt->funcname)) + )); + } + + if(!owner_is_super && function_is_owned_by_super){ + ereport(ERROR, ( + errcode(ERRCODE_INVALID_OBJECT_DEFINITION) + , errmsg("A database owner event trigger must not execute a superuser owned function") + , errdetail("The function \"%s\" is owned by a superuser", NameListToString(stmt->funcname)) + )); + } + /* Insert catalog entries. */ return insert_event_trigger_tuple(stmt->trigname, stmt->eventname, evtowner, funcoid, tags); @@ -1087,9 +1105,15 @@ EventTriggerInvoke(List *fn_oid_list, EventTriggerData *trigdata) Oid fnoid = lfirst_oid(lc); FmgrInfo flinfo; PgStat_FunctionCallUsage fcusage; + bool function_is_owned_by_super = superuser_arg(get_func_owner(fnoid)); elog(DEBUG1, "EventTriggerInvoke %u", fnoid); + if (superuser() && !function_is_owned_by_super) { + elog(DEBUG1, "Non-superuser owned event trigger function \"%s\" is skipped for superuser \"%s\"", get_func_name(fnoid), GetUserNameFromId(GetUserId(), false)); + continue; + } + /* * We want each event trigger to be able to see the results of the * previous event trigger's action. Caller is responsible for any diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index 7bd476f3de7..3fc0eec9783 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -1650,6 +1650,29 @@ get_func_name(Oid funcid) return NULL; } +/* + * get_func_owner + * returns the name of the function owner the given funcid + */ +Oid +get_func_owner(Oid funcid) +{ + HeapTuple tp; + + tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid)); + if (HeapTupleIsValid(tp)) + { + Form_pg_proc functup = (Form_pg_proc) GETSTRUCT(tp); + Oid result; + + result = functup->proowner; + ReleaseSysCache(tp); + return result; + } + else + return InvalidOid; +} + /* * get_func_namespace * diff --git a/src/include/commands/dbcommands.h b/src/include/commands/dbcommands.h index 524ac6d97e8..4d572bb53ed 100644 --- a/src/include/commands/dbcommands.h +++ b/src/include/commands/dbcommands.h @@ -30,6 +30,7 @@ extern ObjectAddress AlterDatabaseOwner(const char *dbname, Oid newOwnerId); extern Oid get_database_oid(const char *dbname, bool missing_ok); extern char *get_database_name(Oid dbid); +extern Oid get_database_owner(Oid dbid); extern bool have_createdb_privilege(void); extern void check_encoding_locale_matches(int encoding, const char *collate, const char *ctype); diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index 6fab7aa6009..b9d560a6ef9 100644 --- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -122,6 +122,7 @@ extern Oid get_negator(Oid opno); extern RegProcedure get_oprrest(Oid opno); extern RegProcedure get_oprjoin(Oid opno); extern char *get_func_name(Oid funcid); +extern Oid get_func_owner(Oid funcid); extern Oid get_func_namespace(Oid funcid); extern Oid get_func_rettype(Oid funcid); extern int get_func_nargs(Oid funcid); diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index 7b2198eac6f..f951031b4c8 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -90,7 +90,7 @@ set role regress_evt_user; create event trigger regress_event_trigger_noperms on ddl_command_start execute procedure test_event_trigger(); ERROR: permission denied to create event trigger "regress_event_trigger_noperms" -HINT: Must be superuser to create an event trigger. +HINT: Must be the database owner or superuser to create an event trigger. reset role; -- test enabling and disabling alter event trigger regress_event_trigger disable; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 37b6d21e1f9..6b322c76b00 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -124,7 +124,7 @@ test: partition_join partition_prune reloptions hash_part indexing partition_agg # event_trigger depends on create_am and cannot run concurrently with # any test that runs DDL # oidjoins is read-only, though, and should run late for best coverage -test: oidjoins event_trigger +test: oidjoins event_trigger event_trigger_dbowner # event_trigger_login cannot run concurrently with any other tests because # on-login event handling could catch connection of a concurrent test. diff --git a/src/test/regress/sql/event_trigger_dbowner.sql b/src/test/regress/sql/event_trigger_dbowner.sql new file mode 100644 index 00000000000..00b62045e47 --- /dev/null +++ b/src/test/regress/sql/event_trigger_dbowner.sql @@ -0,0 +1,117 @@ +-- set up +create role regress_non_owner; +create role regress_owner login nosuperuser; +create database regress_owner owner regress_owner; +\connect regress_owner +grant create on schema public to regress_non_owner; +create function regress_super_show_cur_user() + returns event_trigger + language plpgsql as +$$ +begin + raise notice 'the superuser event trigger is executed'; +end; +$$; +\echo + +-- a database owner can create event triggers +set role regress_owner; +\echo + +create function regress_show_cur_user() + returns event_trigger + language plpgsql as +$$ +begin + raise notice 'the event trigger is executed for %', current_user; +end; +$$; +\echo + +create event trigger regress_evtrig_1 on ddl_command_end +execute procedure regress_show_cur_user(); +\echo + +reset role; +\echo + +-- a database owner cannot create event triggers when the function is superuser owned +set role regress_owner; +\echo + +create event trigger regress_evtrig_2 on ddl_command_end +execute procedure regress_super_show_cur_user(); +\echo + +reset role; +\echo + +-- a superuser cannot create event triggers when the function is not superuser owned +create event trigger regress_evtrig_3 on ddl_command_end +execute procedure regress_show_cur_user(); +\echo + +-- a non-database owner cannot create event triggers +set role regress_non_owner; +\echo + +create event trigger regress_evtrig_4 on ddl_command_end +execute procedure regress_show_cur_user(); +\echo + +reset role; +\echo + +-- database owner event trigger will fire for database owners +set role regress_owner; +create table regress_foo(); +reset role; +\echo + +-- database owner event trigger will fire for non-database owners +set role regress_non_owner; +create table regress_bar(); +reset role; +\echo + +-- database owner event trigger will not fire for superusers +create table regress_qux(); +\echo + +-- superuser event triggers will fire for all roles +create event trigger regress_evtrig_5 on ddl_command_end +execute procedure regress_super_show_cur_user(); +\echo + +-- will fire for superusers +create table regress_super_foo(); +\echo + +-- will fire for db owners +set role regress_owner; +create table regress_foo_2(); +reset role; +\echo + +-- will fire for non-db-owners +set role regress_owner; +create table regress_bar_2(); +reset role; +\echo + +-- clean up +drop event trigger regress_evtrig_5; +drop table regress_foo; +drop table regress_foo_2; +drop table regress_bar; +drop table regress_bar_2; +drop table regress_qux; +drop table regress_super_foo; +revoke create on schema public from regress_non_owner; +drop role regress_non_owner; +drop event trigger regress_evtrig_1; +drop function regress_show_cur_user(); +drop function regress_super_show_cur_user(); +\connect regression +drop database regress_owner; +drop role regress_owner; -- 2.40.1