Re: [HACKERS] Some notes about the index-functions security vulnerability
Added to TODO: * Prevent malicious functions from being executed with the permissions of unsuspecting users Index functions are safe, so VACUUM and ANALYZE are safe too. Triggers, CHECK and DEFAULT expressions, and rules are still vulnerable. http://archives.postgresql.org/pgsql-hackers/2008-01/msg00268.php --- Tom Lane wrote: Now that the dust has settled, I want to post some notes about CVE-2007-6600, which is to my mind the most important of the five security problems fixed in our recent security updates. There are some unfinished issues here. Itagaki Takahiro originally identified the issue. The crux of it is that VACUUM FULL and ANALYZE need to execute functions in index definitions (both expression index columns and partial index predicates). Up to now this has just happened without any special steps being taken, which means that such functions were executed with the privileges of whoever is doing VACUUM/ANALYZE, who is very likely to be a superuser. Now CREATE INDEX requires such functions to be marked IMMUTABLE, which makes them unable to write anything, so the damage is seemingly limited; but it's easy to get around that. Hence, a nefarious user need only put some trojan-horse code into a PL-language function, use the function in an index on one of his tables, and wait for the next routine vacuuming in order to get his code executed as superuser. There are a whole bunch of related scenarios involving trojan-horse code in triggers, view definitions, etc. pgsql-core wasted quite a lot of time (months, actually :-() trying to devise an all-encompassing solution for all of them. However, those other scenarios have been publicly known for years, and haven't seemed to cause a lot of problems in practice, in part because it requires intentional use of a table or view in order to expose yourself to subversion. The index function attack is more nasty than these because it can subvert a superuser during required routine maintenance (including autovacuum). Moreover we couldn't find any way to deal with these other issues that doesn't involve nontrivial semantic incompatibilities, which wouldn't be suitable for back-patching. So the decision was to deal with only the index function problem as a security exercise, and after that try to get people to think some more about plugging those other holes in a future release. Takahiro-san's initial suggestion for fixing this was to try to make the marking of a function as IMMUTABLE into an air-tight guarantee that it couldn't modify the database. Right now it is not air-tight for a number of reasons: you can alter the volatility marking of a function after-the-fact, you can call a volatile function from an immutable one, etc. I originally argued against this fix on the grounds that making a planner hint into a security classification was a bad idea, since people routinely want to lie to the planner, and often have good reasons for it. But there is a better argument: even if you guarantee that a function can't write the database, it'll still be able to read the database and thereby read data the user shouldn't be able to get at. At that point you are reduced to hoping that the user cannot think of any covert channel by which to transmit the interesting info; and there are *always* covert channels, eg timing or CPU usage. We'd have to try to restrict IMMUTABLE functions so that they could not read the DB either, which seems impractical, as well as likely to break a lot of existing applications. So the direction we've pursued instead is to arrange for index expressions to be evaluated as if they were being executed by the table owner, that is, there's an implicit SECURITY DEFINER property attached to them. Up to now I think we've always thought of SECURITY DEFINER functions as being a mechanism for increasing one's privilege level. However, in this context we want to use them as a mechanism for *decreasing* privilege level, and if we want to use them that way then the privilege loss has to be air-tight. The problem there is that so far it's been possible for a SECURITY DEFINER function to execute SET SESSION AUTHORIZATION or SET ROLE and thereby regain whatever privileges are held at the outermost level. The patch as applied disallows both these operations inside a security-definer context. One reason for doing this restrictive fix is that GUC currently isn't being told about fmgr_security_definer's manipulations of CurrentUserId. There was actually a separate bug here: if you did SET ROLE inside a sec-def function and then exited without any error, SHOW ROLE continued to report the SET value as the current role, even though in reality the session had reverted to the previous CurrentUserId. Worse yet, a subsequent ABORT could cause GUC's idea of
Re: [HACKERS] Some notes about the index-functions security vulnerability
On 1/8/08, Tom Lane [EMAIL PROTECTED] wrote: The other issue that ought to be on the TODO radar is that we've only plugged the hole for the very limited case of maintenance operations that are likely to be executed by superusers. If user A modifies user B's table (via INSERT/UPDATE/DELETE), there are a lot of bits of code that are controlled by B but will be executed with A's permissions; so A must trust B a whole lot. This general issue has been understood for quite some time, I think, but maybe it's time to make a serious push to solve it. High-level brain dump, does not cover all the use cases I'm sure... Given Invoker executing code created by Definer, there are basically 3 situations: 1) Definer does not trust Invoker, but Invoker trusts Definer. - use Invoker's permission set This is probably the case for most system/library generic functions, such as the various trigger templates They are typically owned by a superuser 2) Invoker does not trust Definer, but Definer trusts Invoker. - use Definer's permission set This case covers most triggers, since they are there to maintain Definer's data, and Invoker's input is inherently controlled. 3) Neither trusts the other. - use the intersection of Invoker's and Definer's permission sets This is essentially the case for any arbitrary functions floating around, where Invoker's input is not inherently controlled, and Definer is an unknown entity. Situation 1 is covered by SECURITY INVOKER, and 2 is covered by SECURITY DEFINER. Suppose another function option is added for situation 3, SECURITY INTERSECTION. Also suppose there is a new role option, TRUSTED (needs a better name). * A function is created with SECURITY INTERSECTION by default. * A function's owner can choose SECURITY DEFINER. * Only a role with TRUSTED can choose SECURITY INVOKER. * Only the superuser has TRUSTED by default. The idea here is that by default, neither Invoker nor Definer need to be terribly concerned. If Definer is creating the function specifically to operate on its own data, and is checking input appropriately, SECURITY DEFINER will allow it to work. If Definer is creating the function for generic use purposes, Invoker will want to apply it to its own data, and SECURITY INVOKER is appropriate for that. A Definer's trustworthiness for all Invokers is determined by the superuser via the TRUSTED role option. Offhand I can cite the following ways in which B could exploit A's privileges: * triggers Ideally Invoker's permission set would be replaced by the trigger owner's for the duration of the call. However it doesn't look like there actually is an owner concept for triggers, despite there being a TRIGGER permission for the associated table. The next appropriate option is to assign the table owner's permission set to Invoker. In the case of functions marked SECURITY INVOKER, this leaves a hole: a role that has TRIGGER permission on the table can elevate its permissions to that of the table owner's when calling that function. If the role with TRIGGER permission is not TRUSTED, it can only create new functions with SECURITY INTERSECTION, which will result in executing with its own permissions at best. This seems reasonable. * functions in indexes * functions in CHECK constraints * functions in DEFAULT expressions * functions in rules (including VIEW definitions) Replace the Invoker's permission set with the table owner's for the duration of the call. These all require you to be the owner of the associated object, so there is no potential hole as with triggers. The first three of these are probably not too difficult to solve: we could switch privilege state to the table owner before executing such functions, because the backend knows perfectly well when it's doing each of those things. But default expressions and rules get intertwined freely with query fragments supplied by the calling user, and it's not so easy to see how to know what to execute as which user. I'll just wave my hands wildly here and say functions in expressions supplied by the Invoker magically avoid being called with the object owner's permission set instead. I don't know how, they just do :) What this doesn't allow is actually executing things like VIEW expressions using the calling user's permission set. I don't have an actual use case for that, but I feel it's a problem somehow. I've also completely avoided things like CURRENT_USER by talking about permission sets only. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Some notes about the index-functions security vulnerability
On Wed, 2008-01-09 at 00:22 -0500, Tom Lane wrote: pgsql-core wasted quite a lot of time Core's efforts are appreciated by all, so not time wasted. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com ---(end of broadcast)--- TIP 6: explain analyze is your friend
[HACKERS] Some notes about the index-functions security vulnerability
Now that the dust has settled, I want to post some notes about CVE-2007-6600, which is to my mind the most important of the five security problems fixed in our recent security updates. There are some unfinished issues here. Itagaki Takahiro originally identified the issue. The crux of it is that VACUUM FULL and ANALYZE need to execute functions in index definitions (both expression index columns and partial index predicates). Up to now this has just happened without any special steps being taken, which means that such functions were executed with the privileges of whoever is doing VACUUM/ANALYZE, who is very likely to be a superuser. Now CREATE INDEX requires such functions to be marked IMMUTABLE, which makes them unable to write anything, so the damage is seemingly limited; but it's easy to get around that. Hence, a nefarious user need only put some trojan-horse code into a PL-language function, use the function in an index on one of his tables, and wait for the next routine vacuuming in order to get his code executed as superuser. There are a whole bunch of related scenarios involving trojan-horse code in triggers, view definitions, etc. pgsql-core wasted quite a lot of time (months, actually :-() trying to devise an all-encompassing solution for all of them. However, those other scenarios have been publicly known for years, and haven't seemed to cause a lot of problems in practice, in part because it requires intentional use of a table or view in order to expose yourself to subversion. The index function attack is more nasty than these because it can subvert a superuser during required routine maintenance (including autovacuum). Moreover we couldn't find any way to deal with these other issues that doesn't involve nontrivial semantic incompatibilities, which wouldn't be suitable for back-patching. So the decision was to deal with only the index function problem as a security exercise, and after that try to get people to think some more about plugging those other holes in a future release. Takahiro-san's initial suggestion for fixing this was to try to make the marking of a function as IMMUTABLE into an air-tight guarantee that it couldn't modify the database. Right now it is not air-tight for a number of reasons: you can alter the volatility marking of a function after-the-fact, you can call a volatile function from an immutable one, etc. I originally argued against this fix on the grounds that making a planner hint into a security classification was a bad idea, since people routinely want to lie to the planner, and often have good reasons for it. But there is a better argument: even if you guarantee that a function can't write the database, it'll still be able to read the database and thereby read data the user shouldn't be able to get at. At that point you are reduced to hoping that the user cannot think of any covert channel by which to transmit the interesting info; and there are *always* covert channels, eg timing or CPU usage. We'd have to try to restrict IMMUTABLE functions so that they could not read the DB either, which seems impractical, as well as likely to break a lot of existing applications. So the direction we've pursued instead is to arrange for index expressions to be evaluated as if they were being executed by the table owner, that is, there's an implicit SECURITY DEFINER property attached to them. Up to now I think we've always thought of SECURITY DEFINER functions as being a mechanism for increasing one's privilege level. However, in this context we want to use them as a mechanism for *decreasing* privilege level, and if we want to use them that way then the privilege loss has to be air-tight. The problem there is that so far it's been possible for a SECURITY DEFINER function to execute SET SESSION AUTHORIZATION or SET ROLE and thereby regain whatever privileges are held at the outermost level. The patch as applied disallows both these operations inside a security-definer context. One reason for doing this restrictive fix is that GUC currently isn't being told about fmgr_security_definer's manipulations of CurrentUserId. There was actually a separate bug here: if you did SET ROLE inside a sec-def function and then exited without any error, SHOW ROLE continued to report the SET value as the current role, even though in reality the session had reverted to the previous CurrentUserId. Worse yet, a subsequent ABORT could cause GUC's idea of the setting to become the reality. The thinking among core was that we'd be happy with leaving SET SESSION AUTHORIZATION disabled forever, but it would be nice to allow SET ROLE, with the modified semantics that the set of accessible roles would be determined by the innermost security-definer function's owner, rather than the session authorization; and that the effects of SET ROLE would roll back at function exit. To implement that we'd need to redo the interface between GUC and miscinit.c's tracking of privilege state, but
