From b3c3d1b478c5bef5f7a0a71d445e0fd465a66426 Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Sun, 26 Sep 2021 17:42:38 -0700 Subject: [PATCH v1 2/2] Allow regular users to create event triggers This is made safe by conditioning event triggers on the owning role and making them fire only when the trigger owner is a direct or indirect member of the role which performed the event. As such, an event trigger owner cannot trap users into performing actions unless those users are ones the event owner could change role to anyway. For superuser owners, nothing changes, because they can change role to anybody and hence their event triggers still fire for everybody. For non-superuser owners, the behavior does change, but non-superuser owners of event triggers seem a sufficiently unusual situation as to not need to preserve the old behavior. --- doc/src/sgml/event-trigger.sgml | 7 ++++++ doc/src/sgml/ref/create_event_trigger.sgml | 11 +++++----- src/backend/commands/event_trigger.c | 21 ++++++------------ src/backend/utils/cache/evtcache.c | 1 + src/include/utils/evtcache.h | 1 + src/test/regress/expected/event_trigger.out | 22 ++++++++++++++++--- src/test/regress/sql/event_trigger.sql | 24 ++++++++++++++++++++- 7 files changed, 64 insertions(+), 23 deletions(-) diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 60366a950e..f003bedc29 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -119,6 +119,13 @@ to intercept. A common use of such triggers is to restrict the range of DDL operations which users may perform. + + + A trigger will only fire if the trigger owner is a direct or indirect + member of the role which performed the event. (Note that + PostgreSQL prior to version 15.0 did not + condition trigger firing on the permissions of the trigger owner.) + diff --git a/doc/src/sgml/ref/create_event_trigger.sgml b/doc/src/sgml/ref/create_event_trigger.sgml index 22c8119198..c2ae58ea21 100644 --- a/doc/src/sgml/ref/create_event_trigger.sgml +++ b/doc/src/sgml/ref/create_event_trigger.sgml @@ -33,11 +33,12 @@ CREATE EVENT TRIGGER name CREATE EVENT TRIGGER creates a new event trigger. - Whenever the designated event occurs and the WHEN condition - associated with the trigger, if any, is satisfied, the trigger function - will be executed. For a general introduction to event triggers, see - . The user who creates an event trigger - becomes its owner. + Whenever the designated event occurs, the trigger function will be executed + if the WHEN condition associated with the trigger, if + any, is satisfied, and if the owner of the trigger is a direct or indirect + member of the role which performed the event. For a general introduction to + event triggers, see . The user who creates + an event trigger becomes its owner. diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index 93e211cecf..62d1c32145 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -114,18 +114,6 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) ListCell *lc; List *tags = NULL; - /* - * 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()) - 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."))); - /* Validate event name. */ if (strcmp(stmt->eventname, "ddl_command_start") != 0 && strcmp(stmt->eventname, "ddl_command_end") != 0 && @@ -612,8 +600,13 @@ EventTriggerCommonSetup(Node *parsetree, if (filter_event_trigger(tag, item)) { - /* We must plan to fire this trigger. */ - runlist = lappend_oid(runlist, item->fnoid); + /* + * We must plan to fire this trigger only if the event trigger + * owner is a member of the current role, else the trigger owner + * can execute with privilege it could not do directly. + */ + if (is_member_of_role(item->fnowner, GetUserId())) + runlist = lappend_oid(runlist, item->fnoid); } } diff --git a/src/backend/utils/cache/evtcache.c b/src/backend/utils/cache/evtcache.c index 460b720a65..ab86fcfa31 100644 --- a/src/backend/utils/cache/evtcache.c +++ b/src/backend/utils/cache/evtcache.c @@ -173,6 +173,7 @@ BuildEventTriggerCache(void) /* Allocate new cache item. */ item = palloc0(sizeof(EventTriggerCacheItem)); item->fnoid = form->evtfoid; + item->fnowner = form->evtowner; item->enabled = form->evtenabled; /* Decode and sort tags array. */ diff --git a/src/include/utils/evtcache.h b/src/include/utils/evtcache.h index 58ddb71cb1..1e968129c6 100644 --- a/src/include/utils/evtcache.h +++ b/src/include/utils/evtcache.h @@ -28,6 +28,7 @@ typedef enum typedef struct { Oid fnoid; /* function to be called */ + Oid fnowner; /* event trigger owner */ char enabled; /* as SESSION_REPLICATION_ROLE_* */ Bitmapset *tagset; /* command tags, or NULL if empty */ } EventTriggerCacheItem; diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index d56c75de84..938c459203 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -29,6 +29,23 @@ ERROR: unrecognized event name "elephant_bootstrap" -- OK create event trigger regress_event_trigger on ddl_command_start execute procedure test_event_trigger(); +create role test_evt_role1 nosuperuser; +create role test_evt_role2 nosuperuser; +alter event trigger regress_event_trigger owner to test_evt_role1; +set session authorization test_evt_role1; +-- OK, should fire test_event_trigger +create table test_evt_table1 (col text); +NOTICE: test_event_trigger: ddl_command_start CREATE TABLE +reset session authorization; +set session authorization test_evt_role2; +-- OK, should bypass test_event_trigger +create table test_evt_table2 (col text); +grant test_evt_role2 to test_evt_role1; +-- Ok, should fire test_event_trigger now +create table test_evt_table3 (col text); +NOTICE: test_event_trigger: ddl_command_start CREATE TABLE +reset session authorization; +alter event trigger regress_event_trigger owner to current_user; -- OK create event trigger regress_event_trigger_end on ddl_command_end execute function test_event_trigger(); @@ -89,8 +106,7 @@ create role regress_evt_user; 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. +drop event trigger regress_event_trigger_noperms; reset role; -- test enabling and disabling alter event trigger regress_event_trigger disable; @@ -173,7 +189,7 @@ NOTICE: test_event_trigger: ddl_command_end CREATE USER MAPPING alter default privileges for role regress_evt_user revoke delete on tables from regress_evt_user; NOTICE: test_event_trigger: ddl_command_end ALTER DEFAULT PRIVILEGES --- alter owner to non-superuser should fail +-- alter owner to non-superuser should work alter event trigger regress_event_trigger owner to regress_evt_user; -- alter owner to superuser should work alter role regress_evt_user superuser; diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql index ef79b1059d..e12315b70c 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -29,6 +29,27 @@ create event trigger regress_event_trigger on elephant_bootstrap create event trigger regress_event_trigger on ddl_command_start execute procedure test_event_trigger(); +create role test_evt_role1 nosuperuser; +create role test_evt_role2 nosuperuser; + +alter event trigger regress_event_trigger owner to test_evt_role1; + +set session authorization test_evt_role1; +-- OK, should fire test_event_trigger +create table test_evt_table1 (col text); +reset session authorization; + +set session authorization test_evt_role2; +-- OK, should bypass test_event_trigger +create table test_evt_table2 (col text); + +grant test_evt_role2 to test_evt_role1; +-- Ok, should fire test_event_trigger now +create table test_evt_table3 (col text); + +reset session authorization; +alter event trigger regress_event_trigger owner to current_user; + -- OK create event trigger regress_event_trigger_end on ddl_command_end execute function test_event_trigger(); @@ -90,6 +111,7 @@ create role regress_evt_user; set role regress_evt_user; create event trigger regress_event_trigger_noperms on ddl_command_start execute procedure test_event_trigger(); +drop event trigger regress_event_trigger_noperms; reset role; -- test enabling and disabling @@ -143,7 +165,7 @@ create user mapping for regress_evt_user server useless_server; alter default privileges for role regress_evt_user revoke delete on tables from regress_evt_user; --- alter owner to non-superuser should fail +-- alter owner to non-superuser should work alter event trigger regress_event_trigger owner to regress_evt_user; -- alter owner to superuser should work -- 2.21.1 (Apple Git-122.3)