From 5d35438273c4523a4dc4b48c3bd575e64310d3d4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 4 Jan 2016 12:21:31 -0500 Subject: [PATCH] Adjust behavior of row_security GUC to match the docs. Some time back we agreed that row_security=off should not be a way to bypass RLS entirely, but only a way to get an error if it was being applied. However, the code failed to act that way for table owners. Per discussion, this is a must-fix bug for 9.5.0. Adjust the logic in rls.c to behave as expected; also, modify the error message to be more consistent with the new interpretation. The regression tests need minor corrections as well. Also update the comments about row_security in ddl.sgml to be correct. (The official description of the GUC in config.sgml is already correct.) I failed to resist the temptation to do some other very minor cleanup as well, such as getting rid of a duplicate extern declaration. --- doc/src/sgml/ddl.sgml | 34 ++++++++----- src/backend/utils/misc/rls.c | 50 ++++++++++--------- src/test/regress/expected/rowsecurity.out | 61 ++++++++--------------- src/test/regress/sql/rowsecurity.sql | 25 ++++------ 4 files changed, 78 insertions(+), 92 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 584a618e9d..a4b1a35bba 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1572,11 +1572,7 @@ REVOKE ALL ON accounts FROM PUBLIC; bypass the row security system when accessing a table. Table owners normally bypass row security as well, though a table owner can choose to be subject to row security with ALTER - TABLE ... FORCE ROW LEVEL SECURITY. Even in a table with that option - selected, the table owner will bypass row security if the - configuration parameter is set - to off; this setting is typically used for purposes such as - backup and restore. + TABLE ... FORCE ROW LEVEL SECURITY. @@ -1606,14 +1602,6 @@ REVOKE ALL ON accounts FROM PUBLIC; of all roles that they are a member of. - - Referential integrity checks, such as unique or primary key constraints - and foreign key references, always bypass row security to ensure that - data integrity is maintained. Care must be taken when developing - schemas and row level policies to avoid covert channel leaks of - information through such referential integrity checks. - - As a simple example, here is how to create a policy on the account relation to allow only members of @@ -1773,6 +1761,26 @@ postgres=> update passwd set pwhash = 'abc'; UPDATE 1 + + Referential integrity checks, such as unique or primary key constraints + and foreign key references, always bypass row security to ensure that + data integrity is maintained. Care must be taken when developing + schemas and row level policies to avoid covert channel leaks of + information through such referential integrity checks. + + + + In some contexts it is important to be sure that row security is + not being applied. For example, when taking a backup, it could be + disastrous if row security silently caused some rows to be omitted + from the backup. In such a situation, you can set the + configuration parameter + to off. This does not in itself bypass row security; + what it does is throw an error if any query's results would get filtered + by a policy. The reason for the error can then be investigated and + fixed. + + For additional details see and . diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c index b6c1d75fad..c33f29e9f2 100644 --- a/src/backend/utils/misc/rls.c +++ b/src/backend/utils/misc/rls.c @@ -17,18 +17,17 @@ #include "access/htup.h" #include "access/htup_details.h" #include "access/transam.h" -#include "catalog/pg_class.h" #include "catalog/namespace.h" +#include "catalog/pg_class.h" #include "miscadmin.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/elog.h" +#include "utils/lsyscache.h" #include "utils/rls.h" #include "utils/syscache.h" -extern int check_enable_rls(Oid relid, Oid checkAsUser, bool noError); - /* * check_enable_rls * @@ -52,20 +51,21 @@ extern int check_enable_rls(Oid relid, Oid checkAsUser, bool noError); int check_enable_rls(Oid relid, Oid checkAsUser, bool noError) { + Oid user_id = checkAsUser ? checkAsUser : GetUserId(); HeapTuple tuple; Form_pg_class classform; bool relrowsecurity; bool relforcerowsecurity; - Oid user_id = checkAsUser ? checkAsUser : GetUserId(); + bool amowner; /* Nothing to do for built-in relations */ - if (relid < FirstNormalObjectId) + if (relid < (Oid) FirstNormalObjectId) return RLS_NONE; + /* Fetch relation's relrowsecurity and relforcerowsecurity flags */ tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); if (!HeapTupleIsValid(tuple)) return RLS_NONE; - classform = (Form_pg_class) GETSTRUCT(tuple); relrowsecurity = classform->relrowsecurity; @@ -88,41 +88,45 @@ check_enable_rls(Oid relid, Oid checkAsUser, bool noError) return RLS_NONE_ENV; /* - * Table owners generally bypass RLS, except if row_security=true and the - * table has been set (by an owner) to FORCE ROW SECURITY, and this is not - * a referential integrity check. + * Table owners generally bypass RLS, except if the table has been set (by + * an owner) to FORCE ROW SECURITY, and this is not a referential + * integrity check. * * Return RLS_NONE_ENV to indicate that this decision depends on the * environment (in this case, the user_id). */ - if (pg_class_ownercheck(relid, user_id)) + amowner = pg_class_ownercheck(relid, user_id); + if (amowner) { /* - * If row_security=true and FORCE ROW LEVEL SECURITY has been set on - * the relation then we return RLS_ENABLED to indicate that RLS should - * still be applied. If we are in a SECURITY_NOFORCE_RLS context or if - * row_security=false then we return RLS_NONE_ENV. + * If FORCE ROW LEVEL SECURITY has been set on the relation then we + * should return RLS_ENABLED to indicate that RLS should be applied. + * If not, or if we are in an InNoForceRLSOperation context, we return + * RLS_NONE_ENV. * - * The SECURITY_NOFORCE_RLS indicates that we should not apply RLS even - * if the table has FORCE RLS set- IF the current user is the owner. - * This is specifically to ensure that referential integrity checks are - * able to still run correctly. + * InNoForceRLSOperation indicates that we should not apply RLS even + * if the table has FORCE RLS set - IF the current user is the owner. + * This is specifically to ensure that referential integrity checks + * are able to still run correctly. * * This is intentionally only done after we have checked that the user * is the table owner, which should always be the case for referential * integrity checks. */ - if (row_security && relforcerowsecurity && !InNoForceRLSOperation()) - return RLS_ENABLED; - else + if (!relforcerowsecurity || InNoForceRLSOperation()) return RLS_NONE_ENV; } - /* row_security GUC says to bypass RLS, but user lacks permission */ + /* + * We should apply RLS. However, the user may turn off the row_security + * GUC to get a forced error instead. + */ if (!row_security && !noError) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("insufficient privilege to bypass row-level security"))); + errmsg("query would be affected by row-level security policy for table \"%s\"", + get_rel_name(relid)), + amowner ? errhint("To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY.") : 0)); /* RLS should be fully enabled for this relation. */ return RLS_ENABLED; diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 8d925dc160..4aaa88f2c3 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -2728,8 +2728,8 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; -- Check COPY TO as user with permissions. SET SESSION AUTHORIZATION rls_regress_user1; SET row_security TO OFF; -COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls -ERROR: insufficient privilege to bypass row-level security +COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS +ERROR: query would be affected by row-level security policy for table "copy_t" SET row_security TO ON; COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok 0,cfcd208495d565ef66e7dff9f98764da @@ -2769,8 +2769,8 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok -- Check COPY TO as user without permissions. SET row_security TO OFF; SET SESSION AUTHORIZATION rls_regress_user2; SET row_security TO OFF; -COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls -ERROR: insufficient privilege to bypass row-level security +COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS +ERROR: query would be affected by row-level security policy for table "copy_t" SET row_security TO ON; COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - permission denied ERROR: permission denied for relation copy_t @@ -2793,8 +2793,8 @@ COPY copy_rel_to TO STDOUT WITH DELIMITER ','; -- Check COPY TO as user with permissions. SET SESSION AUTHORIZATION rls_regress_user1; SET row_security TO OFF; -COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls -ERROR: insufficient privilege to bypass row-level security +COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS +ERROR: query would be affected by row-level security policy for table "copy_rel_to" SET row_security TO ON; COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok -- Check COPY TO as user with permissions and BYPASSRLS @@ -2822,8 +2822,8 @@ COPY copy_t FROM STDIN; --ok -- Check COPY FROM as user with permissions. SET SESSION AUTHORIZATION rls_regress_user1; SET row_security TO OFF; -COPY copy_t FROM STDIN; --fail - insufficient privilege to bypass rls. -ERROR: insufficient privilege to bypass row-level security +COPY copy_t FROM STDIN; --fail - would be affected by RLS. +ERROR: query would be affected by row-level security policy for table "copy_t" SET row_security TO ON; COPY copy_t FROM STDIN; --fail - COPY FROM not supported by RLS. ERROR: COPY FROM not supported with row-level security @@ -3181,8 +3181,7 @@ SET SESSION AUTHORIZATION rls_regress_user0; DROP TABLE r1; DROP TABLE r2; -- --- FORCE ROW LEVEL SECURITY applies RLS to owners but --- only when row_security = on +-- FORCE ROW LEVEL SECURITY applies RLS to owners too -- SET SESSION AUTHORIZATION rls_regress_user0; SET row_security = on; @@ -3215,30 +3214,16 @@ TABLE r1; (0 rows) SET row_security = off; --- Shows all rows +-- these all fail, would be affected by RLS TABLE r1; - a ----- - 10 - 20 -(2 rows) - --- Update all rows +ERROR: query would be affected by row-level security policy for table "r1" +HINT: To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY. UPDATE r1 SET a = 1; -TABLE r1; - a ---- - 1 - 1 -(2 rows) - --- Delete all rows +ERROR: query would be affected by row-level security policy for table "r1" +HINT: To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY. DELETE FROM r1; -TABLE r1; - a ---- -(0 rows) - +ERROR: query would be affected by row-level security policy for table "r1" +HINT: To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY. DROP TABLE r1; -- -- FORCE ROW LEVEL SECURITY does not break RI @@ -3349,14 +3334,10 @@ TABLE r1; (0 rows) SET row_security = off; --- Rows shown now +-- fail, would be affected by RLS TABLE r1; - a ----- - 10 - 20 -(2 rows) - +ERROR: query would be affected by row-level security policy for table "r1" +HINT: To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY. SET row_security = on; -- Error INSERT INTO r1 VALUES (10), (20) RETURNING *; @@ -3377,7 +3358,7 @@ ALTER TABLE r1 FORCE ROW LEVEL SECURITY; -- Works fine UPDATE r1 SET a = 30; -- Show updated rows -SET row_security = off; +ALTER TABLE r1 NO FORCE ROW LEVEL SECURITY; TABLE r1; a ---- @@ -3393,7 +3374,7 @@ TABLE r1; 10 (1 row) -SET row_security = on; +ALTER TABLE r1 FORCE ROW LEVEL SECURITY; -- Error UPDATE r1 SET a = 30 RETURNING *; ERROR: new row violates row-level security policy for table "r1" diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index decde90730..b5f5bcf8de 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -1014,7 +1014,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; -- Check COPY TO as user with permissions. SET SESSION AUTHORIZATION rls_regress_user1; SET row_security TO OFF; -COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls +COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS SET row_security TO ON; COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok @@ -1028,7 +1028,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok -- Check COPY TO as user without permissions. SET row_security TO OFF; SET SESSION AUTHORIZATION rls_regress_user2; SET row_security TO OFF; -COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls +COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS SET row_security TO ON; COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - permission denied @@ -1054,7 +1054,7 @@ COPY copy_rel_to TO STDOUT WITH DELIMITER ','; -- Check COPY TO as user with permissions. SET SESSION AUTHORIZATION rls_regress_user1; SET row_security TO OFF; -COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls +COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - would be affected by RLS SET row_security TO ON; COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok @@ -1092,7 +1092,7 @@ COPY copy_t FROM STDIN; --ok -- Check COPY FROM as user with permissions. SET SESSION AUTHORIZATION rls_regress_user1; SET row_security TO OFF; -COPY copy_t FROM STDIN; --fail - insufficient privilege to bypass rls. +COPY copy_t FROM STDIN; --fail - would be affected by RLS. SET row_security TO ON; COPY copy_t FROM STDIN; --fail - COPY FROM not supported by RLS. @@ -1315,8 +1315,7 @@ DROP TABLE r1; DROP TABLE r2; -- --- FORCE ROW LEVEL SECURITY applies RLS to owners but --- only when row_security = on +-- FORCE ROW LEVEL SECURITY applies RLS to owners too -- SET SESSION AUTHORIZATION rls_regress_user0; SET row_security = on; @@ -1342,16 +1341,10 @@ DELETE FROM r1; TABLE r1; SET row_security = off; --- Shows all rows +-- these all fail, would be affected by RLS TABLE r1; - --- Update all rows UPDATE r1 SET a = 1; -TABLE r1; - --- Delete all rows DELETE FROM r1; -TABLE r1; DROP TABLE r1; @@ -1469,7 +1462,7 @@ INSERT INTO r1 VALUES (10), (20); TABLE r1; SET row_security = off; --- Rows shown now +-- fail, would be affected by RLS TABLE r1; SET row_security = on; @@ -1497,7 +1490,7 @@ ALTER TABLE r1 FORCE ROW LEVEL SECURITY; UPDATE r1 SET a = 30; -- Show updated rows -SET row_security = off; +ALTER TABLE r1 NO FORCE ROW LEVEL SECURITY; TABLE r1; -- reset value in r1 for test with RETURNING UPDATE r1 SET a = 10; @@ -1505,7 +1498,7 @@ UPDATE r1 SET a = 10; -- Verify row reset TABLE r1; -SET row_security = on; +ALTER TABLE r1 FORCE ROW LEVEL SECURITY; -- Error UPDATE r1 SET a = 30 RETURNING *;