feat: execute event triggers under superuser

Makefile

@@ -16,7 +16,8 @@ PG_CFLAGS += --coverage
 endif
 
 MODULE_big = supautils
-OBJS = src/supautils.o src/privileged_extensions.o src/drop_trigger_grants.o src/constrained_extensions.o src/extensions_parameter_overrides.o src/policy_grants.o src/utils.o
+SRC = $(wildcard src/*.c)
+OBJS = $(patsubst src/%.c, src/%.o, $(SRC))
 
 PG_VERSION = $(strip $(shell $(PG_CONFIG) --version | $(GREP) -oP '(?<=PostgreSQL )[0-9]+'))
 PG_GE16 = $(shell test $(PG_VERSION) -ge 16; echo $$?)

src/event_triggers.c

@@ -0,0 +1,46 @@
+#include "pg_prelude.h"
+#include "event_triggers.h"
+
+PG_FUNCTION_INFO_V1(noop);
+Datum noop(__attribute__ ((unused)) PG_FUNCTION_ARGS) { PG_RETURN_VOID();}
+
+void
+force_noop(FmgrInfo *finfo)
+{
+    finfo->fn_addr   = (PGFunction) noop;
+    finfo->fn_oid    = InvalidOid;           /* not a known function OID anymore */
+    finfo->fn_nargs  = 0;                    /* no arguments for noop */
+    finfo->fn_strict = false;
+    finfo->fn_retset = false;
+    finfo->fn_stats  = 0;                    /* no stats collection */
+    finfo->fn_extra  = NULL;                 /* clear out old context data */
+    finfo->fn_mcxt   = CurrentMemoryContext;
+    finfo->fn_expr   = NULL;                 /* no parse tree */
+}
+
+Oid get_function_owner(func_owner_search search){
+  // Lookup function name OID. Note that for event trigger functions, there's no arguments.
+  Oid func_oid = InvalidOid;
+
+  switch(search.as){
+  case FO_SEARCH_NAME:
+    func_oid = LookupFuncName(search.val.funcname, 0, NULL, false);
+    break;
+  case FO_SEARCH_FINFO:
+    func_oid = search.val.finfo->fn_oid;
+    break;
+  }
+
+  HeapTuple proc_tup = SearchSysCache1(PROCOID, ObjectIdGetDatum(func_oid));
+  if (!HeapTupleIsValid(proc_tup))
+      ereport(ERROR,
+              (errmsg("cache lookup failed for function %u", func_oid)));
+
+  Form_pg_proc procForm = (Form_pg_proc) GETSTRUCT(proc_tup);
+  Oid func_owner = procForm->proowner;
+
+  ReleaseSysCache(proc_tup);
+
+  return func_owner;
+}
+

src/event_triggers.h

@@ -0,0 +1,21 @@
+#ifndef EVENT_TRIGGERS_H
+#define EVENT_TRIGGERS_H
+
+typedef enum {
+  FO_SEARCH_NAME,
+  FO_SEARCH_FINFO
+} func_owner_search_type;
+
+typedef struct {
+    func_owner_search_type as;
+    union {
+        List     *funcname;
+        FmgrInfo *finfo;
+    } val;
+} func_owner_search;
+
+extern Oid get_function_owner(func_owner_search search);
+
+extern void force_noop(FmgrInfo *finfo);
+
+#endif

src/pg_prelude.h

@@ -8,12 +8,14 @@
 #include <common/jsonapi.h>
 #include <catalog/namespace.h>
 #include <catalog/pg_authid.h>
+#include <catalog/pg_proc.h>
 #include <commands/defrem.h>
 #include <executor/spi.h>
 #include <fmgr.h>
 #include <miscadmin.h>
 #include <nodes/makefuncs.h>
 #include <nodes/pg_list.h>
+#include <parser/parse_func.h>
 #include <tcop/utility.h>
 #include <tsearch/ts_locale.h>
 #include <utils/acl.h>
@@ -24,6 +26,7 @@
 #include <utils/json.h>
 #include <utils/jsonb.h>
 #include <utils/jsonfuncs.h>
+#include <utils/syscache.h>
 #include <utils/lsyscache.h>
 #include <utils/memutils.h>
 #include <utils/regproc.h>

src/supautils.c

@@ -5,6 +5,7 @@
 #include "policy_grants.h"
 #include "privileged_extensions.h"
 #include "utils.h"
+#include "event_triggers.h"
 
 #define EREPORT_RESERVED_MEMBERSHIP(name)                                   \
     ereport(ERROR,                                                          \
@@ -91,23 +92,6 @@ static bool supautils_needs_fmgr_hook(Oid functionId) {
     return false;
 }
 
-PG_FUNCTION_INFO_V1(noop);
-Datum noop(__attribute__ ((unused)) PG_FUNCTION_ARGS) { PG_RETURN_VOID();}
-
-static void
-force_noop(FmgrInfo *finfo)
-{
-    finfo->fn_addr   = (PGFunction) noop;
-    finfo->fn_oid    = InvalidOid;           /* not a known function OID anymore */
-    finfo->fn_nargs  = 0;                    /* no arguments for noop */
-    finfo->fn_strict = false;
-    finfo->fn_retset = false;
-    finfo->fn_stats  = 0;                    /* no stats collection */
-    finfo->fn_extra  = NULL;                 /* clear out old context data */
-    finfo->fn_mcxt   = CurrentMemoryContext;
-    finfo->fn_expr   = NULL;                 /* no parse tree */
-}
-
 // This function will fire twice: once before execution of the database function (event=FHET_START)
 // and once after execution has finished or failed (event=FHET_END/FHET_ABORT).
 static void supautils_fmgr_hook(FmgrHookEventType event, FmgrInfo *flinfo, Datum *private) {
@@ -116,8 +100,10 @@ static void supautils_fmgr_hook(FmgrHookEventType event, FmgrInfo *flinfo, Datum
     case FHET_START: {
         const char *current_role_name = GetUserNameFromId(GetUserId(), false);
         if (superuser() || is_reserved_role(current_role_name, false)) {
+            bool function_is_owned_by_super = superuser_arg(get_function_owner((func_owner_search){ .as = FO_SEARCH_FINFO, .val.finfo = flinfo }));
+            if (!function_is_owned_by_super)
             // we can't skip execution directly inside the fmgr_hook (although we can abort it with ereport)
-            // so instead we change the event trigger function to a noop function
+            // so instead we use the workaround of changing the event trigger function to a noop function
             force_noop(flinfo);
         }
 
@@ -802,9 +788,6 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
       if (!IsTransactionState()) {
           break;
       }
-      if (superuser()) {
-          break;
-      }
 
       if (!is_current_role_privileged()) {
           break;
@@ -814,15 +797,34 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
           bool already_switched_to_superuser = false;
           const Oid current_user_id = GetUserId();
 
+          CreateEventTrigStmt *stmt = (CreateEventTrigStmt *)utility_stmt;
+          const char *current_role_name = GetUserNameFromId(current_user_id, false);
+
+          bool current_user_is_super = superuser_arg(current_user_id);
+          Oid  function_owner = get_function_owner((func_owner_search){FO_SEARCH_NAME, {stmt->funcname}});
+          bool function_is_owned_by_super = superuser_arg(function_owner);
+
+          if(!current_user_is_super && function_is_owned_by_super){
+            ereport(ERROR, (
+              errmsg("Non-superuser owned event trigger must execute a non-superuser owned function")
+            , errdetail("The current user \"%s\" is not a superuser and the function \"%s\" is owned by a superuser", current_role_name, NameListToString(stmt->funcname))
+            ));
+          }
+
+          if(current_user_is_super && !function_is_owned_by_super){
+            ereport(ERROR, (
+              errmsg("Superuser owned 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", current_role_name, NameListToString(stmt->funcname))
+            ));
+          }
+
           switch_to_superuser(supautils_superuser, &already_switched_to_superuser);
 
           run_process_utility_hook(prev_hook);
 
-          CreateEventTrigStmt *stmt = (CreateEventTrigStmt *)utility_stmt;
-          const char *current_role_name = GetUserNameFromId(current_user_id, false);
-
-          // Change event trigger owner to the current role (which is a privileged role)
-          alter_owner(stmt->trigname, current_role_name, ALT_EVTRIG);
+          if (!current_user_is_super)
+            // Change event trigger owner to the current role (which is a privileged role)
+            alter_owner(stmt->trigname, current_role_name, ALT_EVTRIG);
 
           if (!already_switched_to_superuser) {
               switch_to_original_role();

test/expected/event_triggers.out.in

@@ -1,23 +1,18 @@
-create or replace function show_current_user()
+-- create a function owned by a non-superuser
+set role privileged_role;
+\echo
+
+create function show_current_user()
     returns event_trigger
     language plpgsql as
 $$
 begin
     raise notice 'the event trigger is executed for %', current_user;
 end;
 $$;
-create or replace function become_super()
-    returns event_trigger
-    language plpgsql as
-$$
-begin
-    raise notice 'transforming % to superuser', current_user;
-    alter role rolecreator superuser;
-end;
-$$;
 \echo
 
--- A role other than privileged_role shouldn't be able to create the event trigger
+-- A role other than privileged_role shouldn't be able to create an event trigger
 set role rolecreator;
 \echo
 
@@ -47,18 +42,28 @@ create table privileged_stuff();
 NOTICE:  the event trigger is executed for privileged_role
 \echo
 
-set role rolecreator;
+-- A superuser is not able to create event trigger using non-superuser function
+set role postgres;
+\echo
+
+create event trigger event_trigger_2 on ddl_command_end
+execute procedure show_current_user();
+ERROR:  Superuser owned event trigger must execute a superuser owned function
+DETAIL:  The current user "postgres" is a superuser and the function "show_current_user" is owned by a non-superuser
 \echo
 
 -- A role other than privileged_role should execute the event trigger function
+set role rolecreator;
+\echo
+
 create function dummy() returns text as $$ select 'dummy'; $$ language sql;
 NOTICE:  the event trigger is executed for rolecreator
 \echo
 
+-- A reserved_role shouldn't execute the event trigger function
 set role supabase_storage_admin;
 \echo
 
--- A reserved_role shouldn't execute the event trigger function
 create table storage_stuff();
 \echo
 
@@ -72,17 +77,37 @@ set role postgres;
 create table super_stuff();
 \echo
 
+-- extensions won't execute the event trigger function (since they're executed by superuser under our implementation)
+set role rolecreator;
+\echo
+
+create extension postgres_fdw;
+drop extension postgres_fdw;
+\echo
+
 -- privesc shouldn't happen due to superuser tripping over a user-defined event trigger
+set role privileged_role;
+\echo
+
+create or replace function become_super()
+    returns event_trigger
+    language plpgsql as
+$$
+begin
+    raise notice 'transforming % to superuser', current_user;
+    alter role rolecreator superuser;
+end;
+$$;
+NOTICE:  the event trigger is executed for privileged_role
+\echo
+
 create event trigger event_trigger_2 on ddl_command_end
 execute procedure become_super();
 \echo
 
--- the event trigger is owned by the superuser
-select evtowner::regrole from pg_event_trigger where evtname = 'event_trigger_2';
- evtowner 
-----------
- postgres
-(1 row)
+-- when switching to super, the become_super evtrig won't fire
+set role postgres;
+\echo
 
 create table super_duper_stuff();
 select count(*) = 1 as only_one_super from pg_roles where rolsuper;
@@ -113,14 +138,6 @@ CONTEXT:  SQL statement "alter role rolecreator superuser"
 PL/pgSQL function become_super() line 4 at SQL statement
 \echo
 
--- limitation: create extension won't fire event triggers due to implementation details (we switch to superuser temporarily to create them and we don't fire evtrigs for superusers)
-set role rolecreator;
-\echo
-
-create extension postgres_fdw;
-drop extension postgres_fdw;
-\echo
-
 -- a non-privileged role can't alter event triggers
 set role rolecreator;
 alter event trigger event_trigger_1 disable;
@@ -132,34 +149,13 @@ set role privileged_role;
 alter event trigger event_trigger_1 disable;
 \echo
 
--- but it cannot alter a superuser owned event trigger
-alter event trigger event_trigger_2 disable;
-ERROR:  must be owner of event trigger event_trigger_2
-\echo
-
--- only the superuser can alter its own event triggers
-set role postgres;
-alter event trigger event_trigger_2 disable;
-\echo
-
 -- a non-privileged role can't drop the event triggers
 set role rolecreator;
 drop event trigger event_trigger_1;
 ERROR:  must be owner of event trigger event_trigger_1
-drop event trigger event_trigger_2;
-ERROR:  must be owner of event trigger event_trigger_2
 \echo
 
 -- the privileged role can drop its own event triggers
 set role privileged_role;
 drop event trigger event_trigger_1;
-\echo
-
--- but it cannot drop a superuser owned event trigger
-drop event trigger event_trigger_2;
-ERROR:  must be owner of event trigger event_trigger_2
-\echo
-
--- only the superuser can drop its own event triggers
-set role postgres;
 drop event trigger event_trigger_2;