Thread: Allow database owners to CREATE EVENT TRIGGER

Allow database owners to CREATE EVENT TRIGGER

From
Steve Chavez
Date:
Hello hackers,

Currently PostgreSQL only allows creating event triggers for superusers, this prevents usage on PostgreSQL service providers, which do not grant superuser access.

This patch allows database owners to create event triggers, while preventing privilege escalation.

Unlike superuser event triggers, which execute functions for every role, database owner event triggers are only executed for non-superusers.
This is necessary to prevent privesc. i.e. a superuser tripping on an event trigger containing an `ALTER ROLE dbowner SUPERUSER`.

For skipping dbowner event triggers for superusers:

- A restriction is added for superuser event triggers, the event trigger function must be owned by a superuser.
  + While this is a breaking change, I think it's minor as the usual flow is to "login as superuser" -> "create an evtrig function" -> "create the evtrig". This is also proved by the existing tests, which barely change.
- A restriction is added for dbowner event triggers, the event trigger function must not be owned by a superuser.

This way we can filter dbowner event trigger functions inside `EventTriggerInvoke`.

Tests are included in the patch, I've added a dedicated regression file for easier review. Only a couple of error messages of the existing event trigger regression tests are changed.

Any feedback is welcomed. I haven't added docs yet but I'll gladly add them if the community thinks this patch makes sense.


Best regards,
Steve Chavez
Attachment

Re: Allow database owners to CREATE EVENT TRIGGER

From
Aleksander Alekseev
Date:
Hi,

> Unlike superuser event triggers, which execute functions for every role, database owner event triggers are only
executedfor non-superusers.
 

Even if we forget about the security aspect for a moment, personally I
have mixed feelings about the idea of adding a new type of event
trigger that looks like a regular one but works differently depending
on who creates them. Also what will happen if I promote a user to a
superuser or vice versa? All this doesn't strike me as a great UI.

Maybe you could explain your particular use case?

-- 
Best regards,
Aleksander Alekseev



Re: Allow database owners to CREATE EVENT TRIGGER

From
"David G. Johnston"
Date:
On Wednesday, March 5, 2025, Aleksander Alekseev <aleksander@timescale.com> wrote:
> Unlike superuser event triggers, which execute functions for every role, database owner event triggers are only executed for non-superusers.
 
All this doesn't strike me as a great UI.

Yeah. Seems better to make “execute_for” an attribute of the trigger and allow both superusers and non-superusers to create them.  Then enforce that non-superusers must specify the more limited value.

Though it would seem nice to be able to exclude the pseudo-admin roles these service providers create as well.

David J.

Re: Allow database owners to CREATE EVENT TRIGGER

From
Steve Chavez
Date:
Many thanks for the feedback.

> an attribute of the trigger and allow both superusers and non-superusers to create them.  

The above is the crux of the issue. Superuser evtrigs can target every role but non-superusers evtrigs must apply only to a restricted set of roles to avoid privilege escalation.

With an explicit attribute, I guess the SQL syntax should be like:

> Seems better to make “execute_for” an attribute of the trigger 

CREATE EVENT TRIGGER name ... FOR role1, role2;

Now say a new role is created and has usage/create on this database and we want the evtrig to apply to it. We would need to manually update the list of roles, it won't be done automatically. That is a problem if, for example, we need to enforce an audit trail through event triggers.

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;

I welcome any alternative ideas.

Best regards,
Steve Chavez

On Wed, 5 Mar 2025 at 08:54, David G. Johnston <david.g.johnston@gmail.com> wrote:
On Wednesday, March 5, 2025, Aleksander Alekseev <aleksander@timescale.com> wrote:
> Unlike superuser event triggers, which execute functions for every role, database owner event triggers are only executed for non-superusers.
 
All this doesn't strike me as a great UI.

Yeah. Seems better to make “execute_for” an attribute of the trigger and allow both superusers and non-superusers to create them.  Then enforce that non-superusers must specify the more limited value.

Though it would seem nice to be able to exclude the pseudo-admin roles these service providers create as well.

David J.

Re: Allow database owners to CREATE EVENT TRIGGER

From
"David G. Johnston"
Date:
On Wed, Mar 5, 2025 at 7:55 AM Steve Chavez <steve@supabase.io> wrote:
How about requiring explicit non-superuser execution for the database owner evtrig? It would be like:

CREATE EVENT TRIGGER name ... FOR NOSUPERUSER;

This is more what I was thinking - which works for a boolean.  The ability to have an exclusion list would make sense, or maybe just a predefined role pg_bypass_eventtriggers.

David J.

Re: Allow database owners to CREATE EVENT TRIGGER

From
Tom Lane
Date:
Steve Chavez <steve@supabase.io> writes:
> Currently PostgreSQL only allows creating event triggers for superusers,
> this prevents usage on PostgreSQL service providers, which do not grant
> superuser access.
> This patch allows database owners to create event triggers, while
> preventing privilege escalation.

I'm pretty down on this, at least in the form presented.  While
you may have managed to keep the DB owner from sabotaging superusers,
the proposed feature still allows owning every other special role,
for example pg_write_server_files (which is something that's pretty
trivially exploitable to get superuser).  Since we've generally been
working towards not requiring superuser for most routine admin tasks,
that problem is going to get worse not better over time.  I don't
want to see us add a feature that creates a security reason to
avoid using those special roles in favor of using a superuser.

Or in other words: not-superuser to superuser is far from the only
type of privilege escalation that we need to prevent.

            regards, tom lane



Re: Allow database owners to CREATE EVENT TRIGGER

From
Tom Lane
Date:
I wrote:
> Or in other words: not-superuser to superuser is far from the only
> type of privilege escalation that we need to prevent.

After reflecting on that for a moment: 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?  That changes nothing for superuser-owned
triggers.

            regards, tom lane



Re: Allow database owners to CREATE EVENT TRIGGER

From
Isaac Morland
Date:
On Wed, 5 Mar 2025 at 10:28, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
> Or in other words: not-superuser to superuser is far from the only
> type of privilege escalation that we need to prevent.

After reflecting on that for a moment: 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?  That changes nothing for superuser-owned
triggers.

Can somebody remind me why triggers don't run as their owner in the first place?

It would make triggers way more useful, and eliminate the whole issue of trigger owners escalating to whomever tries to access the object on which the trigger is defined.

Re: Allow database owners to CREATE EVENT TRIGGER

From
Tom Lane
Date:
Steve Chavez <steve@supabase.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



Re: Allow database owners to CREATE EVENT TRIGGER

From
Steve Chavez
Date:
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@sss.pgh.pa.us> wrote:
Steve Chavez <steve@supabase.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

Re: Allow database owners to CREATE EVENT TRIGGER

From
Steve Chavez
Date:

On Sat, 8 Mar 2025 at 00:03, Steve Chavez <steve@supabase.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@sss.pgh.pa.us> wrote:
Steve Chavez <steve@supabase.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
Attachment

Re: Allow database owners to CREATE EVENT TRIGGER

From
"David G. Johnston"
Date:
On Sunday, April 20, 2025, Steve Chavez <steve@supabase.io> wrote:

Expected output for the new script was not included in the commit.

Also, this looks unconventional…
EventTriggerCacheItem *item = (EventTriggerCacheItem*) lfirst_oid(lc);

David J.


Re: Allow database owners to CREATE EVENT TRIGGER

From
Steve Chavez
Date:
Sorry, attached the output file.

Attachment

Re: Allow database owners to CREATE EVENT TRIGGER

From
Steve Chavez
Date:
> Also, this looks unconventional…
> EventTriggerCacheItem *item = (EventTriggerCacheItem*) lfirst_oid(lc);

Just noticed the mistake there, I would have expected a compilation error. New patch attached with the following change:

  EventTriggerCacheItem *item = lfirst(lc);

On Sun, 20 Apr 2025 at 22:55, Steve Chavez <steve@supabase.io> wrote:
Sorry, attached the output file.

Attachment

Re: Allow database owners to CREATE EVENT TRIGGER

From
"David G. Johnston"
Date:
On Sunday, April 20, 2025, Steve Chavez <steve@supabase.io> wrote:
> Also, this looks unconventional…
> EventTriggerCacheItem *item = (EventTriggerCacheItem*) lfirst_oid(lc);

Just noticed the mistake there, I would have expected a compilation error. New patch attached with the following change:

  EventTriggerCacheItem *item = lfirst(lc);

On Sun, 20 Apr 2025 at 22:55, Steve Chavez <steve@supabase.io> wrote:
Sorry, attached the output file.


You can remove role member_1 and trigger..1 and “create table foo” from the nosuper script without any loss of test coverage.  Or member2 trigger2 table_bar along with the alter event trigger command which doesn’t need to be exercised here.  Ownership is all that matters.  Whether come to directly or via alter.

Actually, leave the other member around, but not granted ownership, and both create tables, to demonstrate that a non-superuser and non-owner is unaffected by the trigger.

David J.