Thread: Security leak with trigger functions?
Permissions on a trigger function seem not to be checked, and I can execute a function for which I have no privileges. I consider this a security leak - or am I missing something? Here is a complete example: As superuser, create a trigger function that selects from pg_authid with SECURITY INVOKER, and REVOKE EXECUTE from public: test=# \c test postgres You are now connected to database "test" as user "postgres". test=# CREATE OR REPLACE FUNCTION insert_oid() RETURNS trigger AS test-# $$BEGIN SELECT oid INTO NEW.useroid FROM pg_catalog.pg_authid WHERE rolname = user; RETURN NEW; END;$$ test-# LANGUAGE plpgsql STABLE STRICT SECURITY DEFINER; CREATE FUNCTION test=# REVOKE EXECUTE ON FUNCTION insert_oid() FROM public; REVOKE test=# SELECT proacl FROM pg_catalog.pg_proc WHERE proname = 'insert_oid'; proacl -----------------------{postgres=X/postgres} (1 row) As normal user, try to execute the function or select from pg_catalog.pg_authid directly; both fail as expected: test=# \c test laurenz You are now connected to database "test" as user "laurenz". test=> SELECT insert_oid(); ERROR: permission denied for function insert_oid test=> SELECT oid FROM pg_catalog.pg_authid WHERE rolname = user; ERROR: permission denied for relation pg_authid Create a temporary table, define a trigger BEFORE INSERT using the function that we cannot execute: test=> CREATE TEMP TABLE lautest (id integer PRIMARY KEY, useroid oid NOT NULL); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "lautest_pkey" for table "lautest" CREATE TABLE test=> CREATE TRIGGER insert_oid BEFORE INSERT ON lautest FOR EACH ROW EXECUTE PROCEDURE insert_oid(); CREATE TRIGGER Insert a row into the table. The trigger function is executed, and I have selected a value from pg_authid! test=> INSERT INTO lautest (id) VALUES (1); INSERT 0 1 test=> SELECT * FROM lautest;id | useroid ----+--------- 1 | 10 (1 row) Yours, Laurenz Albe
"Albe Laurenz" <all@adv.magwien.gv.at> writes: > Permissions on a trigger function seem not to be checked, > and I can execute a function for which I have no privileges. Only if it's a trigger function, but I agree this is not very good. The question in my mind is what privilege to check and when. I believe the check probably ought to be whether the table owner can call the function (certainly not the person doing the command that invokes the trigger). However, that raises the question of whether having a separate TRIGGER privilege to attach triggers to tables isn't itself a security hole: it means someone who might not himself have call privileges could cause other people to call a function. We just removed the separate RULE privilege, and said you must be table owner to put a rule on a table, for exactly analogous reasons. The other question is when to check it. If we check only at CREATE TRIGGER time then there's the problem that revoking execute privilege on a trigger function would not do what you'd expect. OTOH checking at every trigger call seems quite unpleasant. Would it be all right to check all the triggers once at statement start (eg, in ExecBSInsertTriggers) whether or not they actually get called? BTW, I just noticed another bug in the trigger stuff: in a statement with multiple target relations, such as UPDATE across an inheritance tree, ExecBSUpdateTriggers and friends get called only on the first (parent) target relation. Hence any statement-level triggers on child tables aren't invoked. This probably isn't very critical today, but it would be if we use those functions to enforce permissions checks. regards, tom lane
Tom Lane wrote: > The question in my mind is what privilege to check and when. By extrapolation of the SQL standard, I'd say we'd need to check the EXECUTE privilege of the function at run time. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes: > Tom Lane wrote: >> The question in my mind is what privilege to check and when. > By extrapolation of the SQL standard, I'd say we'd need to check the > EXECUTE privilege of the function at run time. Certainly EXECUTE privilege is what to check, but whose privilege? regards, tom lane
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > Tom Lane wrote: > >> The question in my mind is what privilege to check and when. > > > > By extrapolation of the SQL standard, I'd say we'd need to check > > the EXECUTE privilege of the function at run time. > > Certainly EXECUTE privilege is what to check, but whose privilege? SQL allows a trigger action to be a more or less random list of statements, which are checked at trigger run time against the privileges of the owner of the trigger. ("The authorization identifier of the owner of the schema that includes the trigger descriptor of TR is pushed onto the authorization stack.") PostgreSQL only allows a trigger action of "call this function", so in the SQL standard context that would mean we'd need to check the EXECUTE privilege of the owner of the trigger. The trick is figuring out who the owner is. If it's the owner of the table, then TRIGGER privilege is effectively total control over the owner of the table. If it's whoever created the trigger, it might be useful, but I don't see how that is compatible with the intent of the SQL standard. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter, > PostgreSQL only allows a trigger action of "call this function", so in > the SQL standard context that would mean we'd need to check the EXECUTE > privilege of the owner of the trigger. The trick is figuring out who > the owner is. If it's the owner of the table, then TRIGGER privilege > is effectively total control over the owner of the table. If it's > whoever created the trigger, it might be useful, but I don't see how > that is compatible with the intent of the SQL standard. If that's the case, then a separate TRIGGER priveledge would seem to be superfluous. One thing to think about, though; our model allows granting ALTER privelidge on a table to roles other than the table owner. It would seem kind of inconsistent to be able to grant non-owner roles the ability to drop a column, but restrict only the owner to adding a trigger. For one thing, if you have a non-owner role which has ALTER permission and wants to add an FK, how would that work? -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
Josh Berkus <josh@agliodbs.com> writes: >> ... we'd need to check the EXECUTE >> privilege of the owner of the trigger. The trick is figuring out who >> the owner is. If it's the owner of the table, then TRIGGER privilege >> is effectively total control over the owner of the table. > If that's the case, then a separate TRIGGER priveledge would seem to be > superfluous. Yeah, you could make a good case for removing TRIGGER privilege and making it be an ownership check, as we just did for RULE privilege. > One thing to think about, though; our model allows granting ALTER > privelidge on a table to roles other than the table owner. Huh? ALTER requires ownership. regards, tom lane
Peter Eisentraut wrote: > Tom Lane wrote: >> Peter Eisentraut <peter_e@gmx.net> writes: >>> Tom Lane wrote: >>>> The question in my mind is what privilege to check and when. >>> >>> By extrapolation of the SQL standard, I'd say we'd need to check >>> the EXECUTE privilege of the function at run time. >> >> Certainly EXECUTE privilege is what to check, but whose privilege? > > PostgreSQL only allows a trigger action of "call this function", so in > the SQL standard context that would mean we'd need to check the EXECUTE > privilege of the owner of the trigger. The trick is figuring out who > the owner is. If it's the owner of the table, then TRIGGER privilege > is effectively total control over the owner of the table. If it's > whoever created the trigger, it might be useful, but I don't see how > that is compatible with the intent of the SQL standard. Looking at pg_trigger I have the impression that there is no such thing as an 'owner of a trigger', and consequently the owner of the trigger would automatically be the table owner. I understand the reservations about the TRIGGER privilege, but I think that it is obvious anyway that anybody who can add a trigger can basically do everything with the table. When adding a trigger, I would check if both the table owner and the user who adds the trigger have EXECUTE privilege on the function. That doesn't seem too restrictive to me. For trigger execution, I see two options: 1) Check for EXECUTE privilege of the table owner at statement begin time, as Tom Lane suggested. We cannot be sure if thetrigger would actually be executed, right? Should there be an error message even when the trigger is not fired? Or shouldthe trigger be silently disabled? 2) Whenever EXECUTE on a function is revoked, disable triggers on all tables whose owners have now no longer execute privilege. This should probably not be silent and require something like a CASCADE option for REVOKE... Also, there'd haveto be an update whenever table ownership is changed... Seems quite difficult, but would save checking at runtime. Yours, Laurenz Albe
Albe Laurenz wrote: > Looking at pg_trigger I have the impression that there is no such thing > as an 'owner of a trigger', and consequently the owner of the trigger > would automatically be the table owner. > > I understand the reservations about the TRIGGER privilege, but I think > that it is obvious anyway that anybody who can add a trigger can > basically do everything with the table. > > Isn't the problem that they can do more than just things with the table? If the trigger runs as the owner of the table it can do *anything* the owner can do. So if we allow the alter privilege to include ability to place a trigger then that privilege includes everything the owner can do (including granting/revoking other privileges). Surely that is not what was intended. Arguably we should invent a concept of an explicit trigger owner. cheers andrew
On Fri, Dec 15, 2006 at 11:52:33AM -0500, Andrew Dunstan wrote: > Isn't the problem that they can do more than just things with the table? > If the trigger runs as the owner of the table it can do *anything* the > owner can do. So if we allow the alter privilege to include ability to > place a trigger then that privilege includes everything the owner can do > (including granting/revoking other privileges). Surely that is not what > was intended. Arguably we should invent a concept of an explicit trigger > owner. I thought the problem was the other way round. That some person created a function as SECURITY DEFINER but restricted EXECUTE permissions. And now anybody can create a table and use that function as a trigger and it will be executed even though neither the owner of the table nor the person executing the trigger has EXECUTE permissions. Triggers don't have owners because like you said, the table owner controls them. The point is that there's no check that the table owner is actually allowed to execute the function being used as trigger. The trigger never runs as the owner of the table AIUI, only ever as the definer of the function or as session user. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Martijn van Oosterhout wrote: > On Fri, Dec 15, 2006 at 11:52:33AM -0500, Andrew Dunstan wrote: > >> Isn't the problem that they can do more than just things with the table? >> If the trigger runs as the owner of the table it can do *anything* the >> owner can do. So if we allow the alter privilege to include ability to >> place a trigger then that privilege includes everything the owner can do >> (including granting/revoking other privileges). Surely that is not what >> was intended. Arguably we should invent a concept of an explicit trigger >> owner. >> > > I thought the problem was the other way round. That some person created > a function as SECURITY DEFINER but restricted EXECUTE permissions. And > now anybody can create a table and use that function as a trigger and > it will be executed even though neither the owner of the table nor the > person executing the trigger has EXECUTE permissions. > > Triggers don't have owners because like you said, the table owner > controls them. The point is that there's no check that the table owner > is actually allowed to execute the function being used as trigger. > > The trigger never runs as the owner of the table AIUI, only ever as the > definer of the function or as session user. > > > OK, sorry for the confusion. cheers andrew
Martijn van Oosterhout <kleptog@svana.org> writes: > The trigger never runs as the owner of the table AIUI, only ever as the > definer of the function or as session user. Yeah. This might itself be seen as a bug: I think you could make a reasonable case that the default behavior ought to be to run as the table owner (but still overridable if trigger function is SECURITY DEFINER, of course). In the current situation a table owner can use a trigger function as a trojan horse against anyone modifying the table. And then there's the question of functions run as a result of rule definitions. That's a lot harder to fix, but (I think) triggers would be relatively easy to do something about. regards, tom lane
Tom Lane wrote: > Martijn van Oosterhout <kleptog@svana.org> writes: >> The trigger never runs as the owner of the table AIUI, only ever as the >> definer of the function or as session user. > > Yeah. This might itself be seen as a bug: I think you could make a > reasonable case that the default behavior ought to be to run as the > table owner (but still overridable if trigger function is SECURITY > DEFINER, of course). In the current situation a table owner can use > a trigger function as a trojan horse against anyone modifying the > table. Is this true for on-select rules too? In that case, couldn't any user run his code as postmaster by creating an appropriate on-select rule and waiting until somebody/cron backups the database using pg_dump? Or is pg_dump smart enough to skip dumping tables with on-select rules? greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes: > Is this true for on-select rules too? In that case, couldn't any > user run his code as postmaster by creating an appropriate on-select > rule and waiting until somebody/cron backups the database using pg_dump? I don't see any issue for views' on-select rules; they wouldn't get executed during either dump or reload. It does seem like there are some other potential hazards once you start thinking this way: * Datatype I/O functions: the output function will be run as superuser during pg_dump, and the input function during restore. I think this is not an attack spot today because I/O functions can only be written in C, but we'd have to think about the consequences before allowing I/O functions in trusted P/L languages. (Perhaps arrange for I/O functions to be run as if setuid to their owner? Could be expensive...) * Functions associated with indexes would get run during restore: both the datatype-related index support functions, and any functions used in functional indexes. This might be OK because we require such functions to be immutable, but I do not think the link from "immutable" to "can't write database" is currently air-tight. * Functions in CHECK constraints (either table or domain constraints) would be executed during restores. There is not an immutability constraint for these currently, although arguably it'd be reasonable to require? * Trigger functions: not executed during pg_dump, nor during a full restore, but they *would* be executed during a data-only restore if you'd not used --disable-triggers. * ON INSERT rules: likewise, executed during data-only restores, possibly resulting in execution of user-defined functions. During restores, we normally set the userid to be the table owner while loading data into a particular table, which would mostly close these holes except that I think a function can revert the session authorization to be whatever the outermost user id is. Probably we need to tighten up the conditions under which a SET SESSION AUTHORIZATION can be reverted within a function. regards, tom lane
Added to TODO: > * Tighten trigger permission checks > > http://archives.postgresql.org/pgsql-hackers/2006-12/msg00564.php and: > * Tighten function permission checks > > http://archives.postgresql.org/pgsql-hackers/2006-12/msg00568.php > --------------------------------------------------------------------------- Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: > > Is this true for on-select rules too? In that case, couldn't any > > user run his code as postmaster by creating an appropriate on-select > > rule and waiting until somebody/cron backups the database using pg_dump? > > I don't see any issue for views' on-select rules; they wouldn't get > executed during either dump or reload. > > It does seem like there are some other potential hazards once you start > thinking this way: > > * Datatype I/O functions: the output function will be run as superuser > during pg_dump, and the input function during restore. I think this is > not an attack spot today because I/O functions can only be written in > C, but we'd have to think about the consequences before allowing I/O > functions in trusted P/L languages. (Perhaps arrange for I/O functions > to be run as if setuid to their owner? Could be expensive...) > > * Functions associated with indexes would get run during restore: > both the datatype-related index support functions, and any functions > used in functional indexes. This might be OK because we require > such functions to be immutable, but I do not think the link from > "immutable" to "can't write database" is currently air-tight. > > * Functions in CHECK constraints (either table or domain constraints) > would be executed during restores. There is not an immutability > constraint for these currently, although arguably it'd be reasonable > to require? > > * Trigger functions: not executed during pg_dump, nor during a full > restore, but they *would* be executed during a data-only restore if > you'd not used --disable-triggers. > > * ON INSERT rules: likewise, executed during data-only restores, > possibly resulting in execution of user-defined functions. > > During restores, we normally set the userid to be the table owner while > loading data into a particular table, which would mostly close these > holes except that I think a function can revert the session > authorization to be whatever the outermost user id is. Probably we need > to tighten up the conditions under which a SET SESSION AUTHORIZATION can > be reverted within a function. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On 12/14/2006 01:17 PM, Peter Eisentraut wrote: > Tom Lane wrote: >> Peter Eisentraut <peter_e@gmx.net> writes: >>> By extrapolation of the SQL standard, I'd say we'd need to check >>> the EXECUTE privilege of the function at run time. >> >> Certainly EXECUTE privilege is what to check, but whose privilege? > > ... > ("The authorization identifier of the owner of the schema that includes > the trigger descriptor of TR is pushed onto the authorization stack.") > > PostgreSQL only allows a trigger action of "call this function", so in > the SQL standard context that would mean we'd need to check the EXECUTE > privilege of the owner of the trigger. The trick is figuring out who > the owner is. If it's the owner of the table, then TRIGGER privilege > is effectively total control over the owner of the table. If it's > whoever created the trigger, it might be useful, but I don't see how > that is compatible with the intent of the SQL standard. Hmm, it's been not quite a dozen years, have there been later threads that followed up on this discussion? Is it still accurate to describe the status quo as: - Triggers can be created only by a role with TRIGGER on the relation and EXECUTE on the function, checked at trigger creation time. - Once created, triggers are executed with no local userid change (so, just as whatever role issued whatever DML fired the trigger), and without a check for EXECUTE on the function at that time, except: - Internal trigger functions implementing RI checks will execute their queries as "the owner of whichever table the query touches (the referenced table for verification of FK inserts, the referencing table when cascading a PK change)." https://www.postgresql.org/message-id/5761.1509733215%40sss.pgh.pa.us It seems to me that the 2006 conversation was complicated by the fact that PostgreSQL triggers don't have owners of their own, and don't have schema-qualified names, as triggers in the SQL standard do. So the conversation was going in the direction of running as the owner of the relation, but for the obvious consequence that it would turn TRIGGER permission into an equivalent of the relation owner's identity. If the idea of changing the current behavior at all could be thinkable, could one think about changing it more along SQL standard lines, by letting a trigger itself have a schema, and executing as the owner of the schema? This seems to me not too different from just letting a trigger have an owner, and executing as that, which could seem less roundabout. OTOH, going through the schema has the advantage of following the standard, and may allow a little more flexibility in setting up delegated authority; a schema with a certain owner can be created, CREATE(*) on that schema can be selectively granted, enabling grantees to create trigger declarations in that schema, applying to any relations they have TRIGGER permission for. Either way, TRIGGER permission is left with something useful to mean, unlike the execute-as-relation-owner case, where the meaning of TRIGGER sort of evaporates. (*) a different permission might be better here, as many existing schemas probably grant CREATE to roles today without intending that they be able to create triggers that execute as the schema owner. TRIGGER might work, having no existing meaning on a schema. I think the execute-as-relation-owner idea might not easily fit situations where different roles with cross-cutting concerns all have their own reasons to put triggers on a given relation, but should not otherwise have the same permissions or run with the same id. I think such situations could easily be accommodated in a model where triggers have schemas and execute as the schema owner. I could imagine a graceful migration plan: 1. Give pg_trigger a tgnamespace column, but allow it to be null or InvalidOid, meaning a trigger that should have the current behavior, executed with no local userid change. Conversely, a trigger with a valid tgnamespace gets executed as the owner of that schema. 2. pg_upgrade preserves existing triggers as triggers with no namespace. The grammar is altered to allow a schema when creating a trigger (and to allow ALTER TRIGGER SET SCHEMA). 3. Permission checks for schemaless triggers stay as they are now: EXECUTE on the function checked at for the trigger's creator at creation time, but not at run time. For triggers in schemas, the schema owner is who must have EXECUTE on the function. 4. It could still be possible to avoid runtime permission checks, the existence of EXECUTE for the right role being checked at trigger creation time, if REVOKE EXECUTE on a trigger function includes a check through pg_depend for triggers whose execute right would be lost. Those would be (a) any triggers with schemas owned by a role having EXECUTE revoked, and (b) any schema-less triggers at all (as there's no telling whose permission might have been checked at the time they were created). The simple check in 4(b) could be implemented on its own, without any of the rest, as a simple way to plug the "security leak" in the status quo. Thoughts? -Chap
On 1/22/18 16:04, Chapman Flack wrote: >> PostgreSQL only allows a trigger action of "call this function", so in >> the SQL standard context that would mean we'd need to check the EXECUTE >> privilege of the owner of the trigger. The trick is figuring out who >> the owner is. If it's the owner of the table, then TRIGGER privilege >> is effectively total control over the owner of the table. If it's >> whoever created the trigger, it might be useful, but I don't see how >> that is compatible with the intent of the SQL standard. > > Hmm, it's been not quite a dozen years, have there been later threads > that followed up on this discussion? No, I don't think anything has changed here. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services