Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using event triggers without SUPERUSER #67

Open
steve-chavez opened this issue Dec 7, 2023 · 9 comments · May be fixed by #98
Open

Allow using event triggers without SUPERUSER #67

steve-chavez opened this issue Dec 7, 2023 · 9 comments · May be fixed by #98
Labels
enhancement New feature or request

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Dec 7, 2023

Problem

Currently there's no way to use event triggers. RDS allows it though, see https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/PostgreSQL.Concepts.General.FeatureSupport.EventTriggers.html. Caveat: the user must delete event triggers when migrating to a new major postgres version.

References:

Solution

Allow event triggers for non superusers.

@steve-chavez steve-chavez added the enhancement New feature or request label Dec 7, 2023
@steve-chavez steve-chavez self-assigned this Dec 7, 2023
@steve-chavez steve-chavez removed their assignment May 16, 2024
@steve-chavez
Copy link
Member Author

I gave this a shot but I was surprised to find privileged_role is not used for creating privileged extensions. There's a privileged_extensions_superuser that's used for switching to superuser.

@soedirgo Which role should we switch to create the event trigger? Why privileged_role wasn't used for extensions?

@steve-chavez
Copy link
Member Author

Here it says privileged_role but privileged_extensions_superuser is used for the switch:

supautils/src/supautils.c

Lines 284 to 286 in fa39431

// Allow `privileged_role` (in addition to superusers) to
// set bypassrls & replication attributes.
switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser);

@soedirgo What's the relationship between these 2 roles?

@soedirgo
Copy link
Member

privileged_role is named as such because from the user's perspective it's the role that can do certain superuser-only actions (hence "privileged"). But under the hood some of it is achieved by switching to a superuser when performing the action.

Agree that privileged_extensions_superuser is confusing though - we should rename them to supautils.superuser in the future, since it's not only used for creating privileged_extensions.

@soedirgo
Copy link
Member

Before you go ahead with the implementation, keep in mind that simply switching to a superuser to perform the CREATE EVENT TRIGGER, similar to how we do it for FDWs etc., wouldn't work since it opens a door for privilege escalation.

@steve-chavez
Copy link
Member Author

privileged_role is named as such because from the user's perspective it's the role that can do certain superuser-only actions (hence "privileged"). But under the hood some of it is achieved by switching to a superuser when performing the action.

So we could say that the privileged_role is a proxy role for the supautils.superuser right? (this allows the privileged_role to do certain restricted superuser actions)

Agree that privileged_extensions_superuser is confusing though - we should rename them to supautils.superuser in the future, since it's not only used for creating privileged_extensions.

Clarifying that on #99.

@steve-chavez
Copy link
Member Author

steve-chavez commented Jan 14, 2025

keep in mind that simply switching to a superuser to perform the CREATE EVENT TRIGGER, similar to how we do it for FDWs etc., wouldn't work since it opens a door for privilege escalation.

Checking how this works on RDS:

CREATE OR REPLACE FUNCTION become_super()
    RETURNS event_trigger
    LANGUAGE plpgsql AS
$$
BEGIN
    RAISE NOTICE 'Transforming % to superuser', current_user;
    ALTER ROLE current_user SUPERUSER;
END;
$$;

CREATE EVENT TRIGGER event_trigger_2
    ON ddl_command_end
EXECUTE PROCEDURE become_super();

postgres=> \dy
                            List of event triggers
      Name       |      Event      |  Owner   | Enabled |   Function   | Tags 
-----------------+-----------------+----------+---------+--------------+------
 event_trigger_2 | ddl_command_end | rdsadmin | enabled | become_super | 
postgres=> select current_user;
 current_user 
--------------
 postgres
(1 row)

postgres=> create table foo();
NOTICE:  Transforming postgres to superuser
ERROR:  permission denied to alter role
DETAIL:  Only roles with the SUPERUSER attribute may change the SUPERUSER attribute.
CONTEXT:  SQL statement "ALTER ROLE current_user SUPERUSER"
PL/pgSQL function become_super() line 4 at SQL statement

So the event trigger SQL is ran as the current_user, not the owner.


Now, using the implementation on #98:

$ supautils-with-pg-14 psql -U privileged_role

CREATE OR REPLACE FUNCTION become_super()
    RETURNS event_trigger
    LANGUAGE plpgsql AS
$$
BEGIN
    RAISE NOTICE 'Transforming % to superuser', current_user;
    ALTER ROLE current_user SUPERUSER;
END;
$$;

CREATE EVENT TRIGGER event_trigger_2
    ON ddl_command_end
EXECUTE PROCEDURE become_super();

postgres=> \dy
                            List of event triggers
      Name       |      Event      |  Owner   | Enabled |   Function   | Tags 
-----------------+-----------------+----------+---------+--------------+------
 event_trigger_2 | ddl_command_end | postgres | enabled | become_super | 
(1 row)

postgres=> select current_user;
  current_user   
-----------------
 privileged_role
(1 row)

postgres=> create table foo();
2025-01-14 18:01:07.922 -05 [435306] ERROR:  permission denied to alter role
2025-01-14 18:01:07.922 -05 [435306] DETAIL:  Only superusers can alter privileged roles.
2025-01-14 18:01:07.922 -05 [435306] CONTEXT:  SQL statement "ALTER ROLE privileged_role SUPERUSER"
        PL/pgSQL function become_super() line 4 at SQL statement
2025-01-14 18:01:07.922 -05 [435306] STATEMENT:  create table foo();
NOTICE:  Transforming privileged_role to superuser
ERROR:  permission denied to alter role
DETAIL:  Only superusers can alter privileged roles.
CONTEXT:  SQL statement "ALTER ROLE privileged_role SUPERUSER"
PL/pgSQL function become_super() line 4 at SQL statement

So the behavior is the same as RDS, the event trigger is ran as the current user not the owner.


Before you go ahead with the implementation, keep in mind that simply switching to a superuser to perform the CREATE EVENT TRIGGER, similar to how we do it for FDWs etc., wouldn't work since it opens a door for privilege escalation.

@soedirgo So is the above true at all? If the event trigger is ran as the current user, then no privilege escalation can happen? (since the user will execute the SQL with its current privileges) Or am I'm missing something?

(Ideally we would have a clear test case for this, it took me a long time to parse the pg email thread and I didn't seem to gain much as compared to just messing with RDS)

@soedirgo
Copy link
Member

Here's an example using the implementation in #98

$ supautils-with-pg-14 bash

$ psql -U privileged_role
postgres=> CREATE OR REPLACE FUNCTION become_super()
    RETURNS event_trigger
    LANGUAGE plpgsql AS
$$
BEGIN
    RAISE NOTICE 'Transforming privileged_role to superuser';
    ALTER ROLE privileged_role SUPERUSER;
END;
$$;

CREATE EVENT TRIGGER event_trigger_1
    ON ddl_command_end
EXECUTE PROCEDURE become_super();
CREATE FUNCTION
CREATE EVENT TRIGGER
postgres=> 
\q

$ psql -U postgres
postgres=# create table foo();
NOTICE:  Transforming privileged_role to superuser
CREATE TABLE
postgres=# select rolsuper from pg_roles where rolname = 'privileged_role';
 rolsuper 
----------
 t
(1 row)

The privesc happens not when the malicious role trips over the event trigger, it happens when the internal admin role trips over it. Replace privileged_role with postgres and postgres with supabase_admin on our platform.

@steve-chavez
Copy link
Member Author

steve-chavez commented Jan 21, 2025

The privesc happens not when the malicious role trips over the event trigger, it happens when the internal admin role trips over it. Replace privileged_role with postgres and postgres with supabase_admin on our platform.

Thanks for clarifying.

Do we expect something to be run directly by the postgres user?

  • Answering my own question: yes, we actually switch to superuser for creating extensions. So on the above example it actually becomes easier to reproduce the problem as we don't need login as the postgres (superuser) role:

    $ psql -U privileged_role
    postgres=> CREATE OR REPLACE FUNCTION become_super()
        RETURNS event_trigger
        LANGUAGE plpgsql AS
    $$
    BEGIN
        RAISE NOTICE 'Transforming privileged_role to superuser';
        ALTER ROLE privileged_role SUPERUSER;
    END;
    $$;
    
    CREATE EVENT TRIGGER event_trigger_1
        ON ddl_command_end
    EXECUTE PROCEDURE become_super();
    CREATE FUNCTION
    CREATE EVENT TRIGGER
    
    postgres=> create extension citext;
    NOTICE:  Transforming privileged_role to superuser
    CREATE EXTENSION
    
    postgres=> select rolname from pg_roles where rolsuper is true;
         rolname     
    -----------------
     postgres
     privileged_role
    (2 rows)
  • RDS doesn't have this issue. They don't allow the user to run anything with the rdsadmin role plus they don't switch to superuser for creating extensions: they patch the extensions to become "trusted" - hence no superuser switch needed.

    • Maybe we could try to switch to this model later on. Before we didn't have a Nixified supabase/postgres that could easily allow us to patch extensions (and add "trusted" to their control files). This way we would avoid superuser switching.

Possible fixes

  • On pg 17, we have the GUC event_triggers, which we could be set to false for the superuser role. This would make the event triggers not fire for superuser, but of course it's only for pg 17 so not a solution.

    • It's not simple to recreate the behavior of pg 17 event_triggers as an extension, this to disable them in superuser. It would involve recreating standard_ProcessUtility internals.
  • One simple thing that occurred to me was to enforce setting a role to the event trigger function:

    CREATE OR REPLACE FUNCTION become_super()
        RETURNS event_trigger
        LANGUAGE plpgsql AS
    $$
    BEGIN
        RAISE NOTICE 'Transforming privileged_role to superuser';
        ALTER ROLE privileged_role SUPERUSER;
    END;
    $$ SET role = 'privileged_role';
    • but this makes extension creation fail somehow
      postgres=> create extension citext ;
      2025-01-20 19:40:06.385 -05 [492131] ERROR:  cannot set parameter "role" within security-definer function
      2025-01-20 19:40:06.385 -05 [492131] STATEMENT:  create extension citext ;
      ERROR:  cannot set parameter "role" within security-definer function
      
      -- not sure what's happening here
  • Analyzing the event trigger function body. This would be too complex, even more so in the presence of plpgsql which can do dynamic SQL.

@steve-chavez
Copy link
Member Author

steve-chavez commented Jan 21, 2025

Thinking more about privesc. The essential problem here is that once a user creates an event trigger, it runs globally for every role. And this impacts superuser and also reserved_roles (roles that ran migrations in behalf of services) , which shouldn't run any user supplied code. So what we need is to skip event triggers for supautils.reserved_roles + supautils.superuser.

This would imply that create extension would not fire event triggers (since they are ran as supautils.superuser), but maybe that's a reasonable limitation that can be documented.

(It also occurs me that we an extra check would be good: there should only be one superuser per database cluster. This also seems like a best practice.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants