Re: Allow database owners to CREATE EVENT TRIGGER

Hello hackers,

I've now modified my patch to allow regular users (not just database
owners) to create event triggers.

As mentioned by Tom before, this now relies on role membership to allowlist
the target roles of the event trigger.

A summary of the changes:

- EventTriggerCacheItem now has an owneroid
- EventTriggerCommonSetup now returns a list of EventTriggerCacheItem
instead of a list of function oids
- The cache items are used in EventTriggerInvoke to discriminate the role
memberships using the `is_member_of_role_nosuper` function
- The event_trigger.sql tests are minimally modified.
- A new file event_trigger_nosuper.sql is added for the new tests.

Any feedback is welcomed. I'll also gladly modify the docs if the patch
looks good.

Best regards,
Steve Chavez

On Sat, 8 Mar 2025 at 00:03, Steve Chavez <steve(at)supabase(dot)io> wrote:

> Hi Tom,
>
> > Well, no. That still allows the database owner to commandeer any
> > non-superuser role. Even if we tightened "nosuperuser" to mean
> > "not superuser and not any built-in role", I don't think it will fly.
>
> Why would the predefined roles be taken into consideration here? The docs
> on https://www.postgresql.org/docs/current/predefined-roles.html say:
>
> " pg_read_server_files, pg_write_server_files and
> pg_execute_server_program..."
> " ..they could be used to gain superuser-level access, therefore great
> care should be taken when granting these roles to users."
>
> If a dbowner event trigger does `GRANT pg_read_server_files TO
> current_user;` inside it will fail with `ERROR: permission denied to grant
> role "pg_read_server_files"`.
> The only way for that to succeed is for a superuser to explicitly grant
> access to `pg_read_server_files` before, and that would have to be a
> conscious operation.
>
> I would appreciate any clarification here.
>
> > maybe say that an event trigger fires for queries that are run by a role
> - that the trigger's owning role is a member of?
>
> The role membership approach would work but it seems insufficient. For
> example consider `pgaudit` which installs event triggers and requires
> superuser.
>
> Let's assume `pgaudit` would try to adopt this new feature. Then it would
> need to provide some special role like `pgaudit_admin`, create the event
> triggers under this role,
> and users of this extension would have to manually grant membership to
> `pgaudit_admin` for the audit event triggers to fire.
> That is a problem because that's easy to forget when creating new roles
> and the audit event triggers won't be "enforced".
> So in that case I guess `pgaudit` developers would keep requiring
> superuser and not bother to adopt this new feature.
>
> From a PoLP perspective it would be a desirable side-effect of this
> feature to stop requiring superuser for certain extensions too.
>
> Best regards,
> Steve Chavez
>
> On Fri, 7 Mar 2025 at 15:19, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Steve Chavez <steve(at)supabase(dot)io> writes:
>> > This is why I thought the database owner is the right role to allow
>> evtrig
>> > creation since it won't need an explicit list of roles.
>>
>> > How about requiring explicit non-superuser execution for the database
>> owner
>> > evtrig? It would be like:
>> > CREATE EVENT TRIGGER name ... FOR NOSUPERUSER;
>>
>> Well, no. That still allows the database owner to commandeer any
>> non-superuser role. Even if we tightened "nosuperuser" to mean
>> "not superuser and not any built-in role", I don't think it will fly.
>>
>> Here is the real problem: database owners are not specially
>> privileged in Postgres. Yeah, they can drop their DB, but they
>> don't have automatic permissions to mess with other people's
>> objects inside the DB. (Much the same can be said of schema
>> owners.) So any proposal that effectively gives DB owners
>> such privileges is going to fail. I realize that some other
>> DBMSes assign more privileges to schema or DB owners, but we
>> don't and I don't think we're open to changing that.
>>
>> I think you need to be thinking of this in terms of "what sort
>> of feature can we add that can be allowed to any SQL user?"
>> The notion I proposed earlier that an event trigger only fires
>> on queries executed by roles the trigger's owner belongs to
>> is (AFAICS) safe to allow to anyone. If that's not good enough
>> for your notion of what a DB owner should be able to do, the
>> answer is to grant the DB owner membership in every role that
>> uses her database. That's effectively what the feature you're
>> suggesting would do anyway.
>>
>> regards, tom lane
>>
>