From b8ad8ca34ad473813b072cc871ad33c48ba68b42 Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Mon, 28 Sep 2015 13:25:28 -0400 Subject: [PATCH] Include policies based on ACLs needed When considering which policies should be included, rather than look at individual bits of the query (eg: if a RETURNING clause exists, or if a WHERE clause exists which is referencing the table, or if it's a FOR SHARE/UPDATE query), consider any case where we've determined the user needs SELECT rights on the relation to be a case where we apply SELECT policies, and any case where we've deteremind that the user needs UPDATE rights on the relation to be a case where we apply UPDATE policies. This simplifies the logic and addresses concerns that a user could use UPDATE or DELETE with a WHERE clauses to determine if rows exist, or they could lock rows which they are not actually allowed to modify through UPDATE policies. Back-patch to 9.5 where RLS was added. --- src/backend/rewrite/rowsecurity.c | 76 +++++++++----- src/test/regress/expected/rowsecurity.out | 166 ++++++++++++++++++------------ 2 files changed, 150 insertions(+), 92 deletions(-) diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index c20a178..1a51c20 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -187,28 +187,55 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, hasSubLinks); /* - * For the target relation, when there is a returning list, we need to - * collect up CMD_SELECT policies and add them via add_security_quals. - * This is because, for the RETURNING case, we have to filter any records - * which are not visible through an ALL or SELECT USING policy. + * For a SELECT, if UPDATE privileges are required (eg: the user has + * specified FOR [KEY] UPDATE/SHARE), then also add the UPDATE USING quals. * - * We don't need to worry about the non-target relation case because we are - * checking the ALL and SELECT policies for those relations anyway (see - * above). + * This way, we filter out any records from the SELECT FOR SHARE/UPDATE + * which the user does not have access to via the UPDATE USING policies, + * similar to how we require normal UPDATE rights for these queries. */ - if (root->returningList != NIL && - (commandType == CMD_UPDATE || commandType == CMD_DELETE)) + if (commandType == CMD_SELECT && rte->requiredPerms & ACL_UPDATE) { - List *returning_permissive_policies; - List *returning_restrictive_policies; + List *update_permissive_policies; + List *update_restrictive_policies; + + get_policies_for_relation(rel, CMD_UPDATE, user_id, + &update_permissive_policies, + &update_restrictive_policies); + + add_security_quals(rt_index, + update_permissive_policies, + update_restrictive_policies, + securityQuals, + hasSubLinks); + } + + /* + * Similar to above, during an UPDATE or DELETE, if SELECT rights are also + * required (eg: when a RETURNING clause exists, or the user has provided + * a WHERE clause which involves columns from the relation), we collect up + * CMD_SELECT policies and add them via add_security_quals. + * + * This way, we filter out any records which are not visible through an ALL + * or SELECT USING policy. + * + * We don't need to worry about the non-target relation case here because + * we are checking the ALL and SELECT policies for those relations anyway + * (see above). + */ + if ((commandType == CMD_UPDATE || commandType == CMD_DELETE) && + rte->requiredPerms & ACL_SELECT) + { + List *select_permissive_policies; + List *select_restrictive_policies; get_policies_for_relation(rel, CMD_SELECT, user_id, - &returning_permissive_policies, - &returning_restrictive_policies); + &select_permissive_policies, + &select_restrictive_policies); add_security_quals(rt_index, - returning_permissive_policies, - returning_restrictive_policies, + select_permissive_policies, + select_restrictive_policies, securityQuals, hasSubLinks); } @@ -261,21 +288,22 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, hasSubLinks); /* - * Get and add ALL/SELECT policies, if there is a RETURNING clause, - * also as WCO policies, again, to avoid silently dropping data. + * Get and add ALL/SELECT policies, if SELECT rights are required + * for this relation, also as WCO policies, again, to avoid + * silently dropping data. See above. */ - if (root->returningList != NIL) + if (rte->requiredPerms & ACL_SELECT) { - List *conflict_returning_permissive_policies = NIL; - List *conflict_returning_restrictive_policies = NIL; + List *conflict_select_permissive_policies = NIL; + List *conflict_select_restrictive_policies = NIL; get_policies_for_relation(rel, CMD_SELECT, user_id, - &conflict_returning_permissive_policies, - &conflict_returning_restrictive_policies); + &conflict_select_permissive_policies, + &conflict_select_restrictive_policies); add_with_check_options(rel, rt_index, WCO_RLS_CONFLICT_CHECK, - conflict_returning_permissive_policies, - conflict_returning_restrictive_policies, + conflict_select_permissive_policies, + conflict_select_restrictive_policies, withCheckOptions, hasSubLinks); } diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 9d3540f..b4e64ab 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -1043,28 +1043,34 @@ EXPLAIN (COSTS OFF) EXECUTE p2(2); -- SET SESSION AUTHORIZATION rls_regress_user1; EXPLAIN (COSTS OFF) UPDATE t1 SET b = b || b WHERE f_leak(b); - QUERY PLAN -------------------------------------------- + QUERY PLAN +------------------------------------------------- Update on t1 t1_3 Update on t1 t1_3 Update on t2 t1 Update on t3 t1 -> Subquery Scan on t1 Filter: f_leak(t1.b) - -> LockRows - -> Seq Scan on t1 t1_4 - Filter: ((a % 2) = 0) + -> Subquery Scan on t1_4 + Filter: ((t1_4.a % 2) = 0) + -> LockRows + -> Seq Scan on t1 t1_5 + Filter: ((a % 2) = 0) -> Subquery Scan on t1_1 Filter: f_leak(t1_1.b) - -> LockRows - -> Seq Scan on t2 - Filter: ((a % 2) = 0) + -> Subquery Scan on t1_6 + Filter: ((t1_6.a % 2) = 0) + -> LockRows + -> Seq Scan on t2 + Filter: ((a % 2) = 0) -> Subquery Scan on t1_2 Filter: f_leak(t1_2.b) - -> LockRows - -> Seq Scan on t3 - Filter: ((a % 2) = 0) -(19 rows) + -> Subquery Scan on t1_7 + Filter: ((t1_7.a % 2) = 0) + -> LockRows + -> Seq Scan on t3 + Filter: ((a % 2) = 0) +(25 rows) UPDATE t1 SET b = b || b WHERE f_leak(b); NOTICE: f_leak => bbb @@ -1073,15 +1079,17 @@ NOTICE: f_leak => bcd NOTICE: f_leak => def NOTICE: f_leak => yyy EXPLAIN (COSTS OFF) UPDATE only t1 SET b = b || '_updt' WHERE f_leak(b); - QUERY PLAN -------------------------------------------- + QUERY PLAN +------------------------------------------------- Update on t1 t1_1 -> Subquery Scan on t1 Filter: f_leak(t1.b) - -> LockRows - -> Seq Scan on t1 t1_2 - Filter: ((a % 2) = 0) -(6 rows) + -> Subquery Scan on t1_2 + Filter: ((t1_2.a % 2) = 0) + -> LockRows + -> Seq Scan on t1 t1_3 + Filter: ((a % 2) = 0) +(8 rows) UPDATE only t1 SET b = b || '_updt' WHERE f_leak(b); NOTICE: f_leak => bbbbbb @@ -1129,18 +1137,20 @@ NOTICE: f_leak => yyyyyy -- updates with from clause EXPLAIN (COSTS OFF) UPDATE t2 SET b=t2.b FROM t3 WHERE t2.a = 3 and t3.a = 2 AND f_leak(t2.b) AND f_leak(t3.b); - QUERY PLAN ---------------------------------------------------------------- + QUERY PLAN +--------------------------------------------------------------------- Update on t2 t2_1 -> Nested Loop -> Subquery Scan on t2 Filter: f_leak(t2.b) - -> LockRows - -> Seq Scan on t2 t2_2 - Filter: ((a = 3) AND ((a % 2) = 1)) + -> Subquery Scan on t2_2 + Filter: ((t2_2.a % 2) = 1) + -> LockRows + -> Seq Scan on t2 t2_3 + Filter: ((a = 3) AND ((a % 2) = 1)) -> Seq Scan on t3 Filter: (f_leak(b) AND (a = 2)) -(9 rows) +(11 rows) UPDATE t2 SET b=t2.b FROM t3 WHERE t2.a = 3 and t3.a = 2 AND f_leak(t2.b) AND f_leak(t3.b); @@ -1150,8 +1160,8 @@ NOTICE: f_leak => zzz NOTICE: f_leak => yyyyyy EXPLAIN (COSTS OFF) UPDATE t1 SET b=t1.b FROM t2 WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b); - QUERY PLAN ---------------------------------------------------------------- + QUERY PLAN +--------------------------------------------------------------------- Update on t1 t1_3 Update on t1 t1_3 Update on t2 t1 @@ -1159,9 +1169,11 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b); -> Nested Loop -> Subquery Scan on t1 Filter: f_leak(t1.b) - -> LockRows - -> Seq Scan on t1 t1_4 - Filter: ((a = 3) AND ((a % 2) = 0)) + -> Subquery Scan on t1_4 + Filter: ((t1_4.a % 2) = 0) + -> LockRows + -> Seq Scan on t1 t1_5 + Filter: ((a = 3) AND ((a % 2) = 0)) -> Subquery Scan on t2 Filter: f_leak(t2.b) -> Seq Scan on t2 t2_3 @@ -1169,9 +1181,11 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b); -> Nested Loop -> Subquery Scan on t1_1 Filter: f_leak(t1_1.b) - -> LockRows - -> Seq Scan on t2 t2_4 - Filter: ((a = 3) AND ((a % 2) = 0)) + -> Subquery Scan on t1_6 + Filter: ((t1_6.a % 2) = 0) + -> LockRows + -> Seq Scan on t2 t2_4 + Filter: ((a = 3) AND ((a % 2) = 0)) -> Subquery Scan on t2_1 Filter: f_leak(t2_1.b) -> Seq Scan on t2 t2_5 @@ -1179,14 +1193,16 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b); -> Nested Loop -> Subquery Scan on t1_2 Filter: f_leak(t1_2.b) - -> LockRows - -> Seq Scan on t3 - Filter: ((a = 3) AND ((a % 2) = 0)) + -> Subquery Scan on t1_7 + Filter: ((t1_7.a % 2) = 0) + -> LockRows + -> Seq Scan on t3 + Filter: ((a = 3) AND ((a % 2) = 0)) -> Subquery Scan on t2_2 Filter: f_leak(t2_2.b) -> Seq Scan on t2 t2_6 Filter: ((a = 3) AND ((a % 2) = 1)) -(34 rows) +(40 rows) UPDATE t1 SET b=t1.b FROM t2 WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b); @@ -1198,20 +1214,22 @@ WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b); -> Nested Loop -> Subquery Scan on t2 Filter: f_leak(t2.b) - -> LockRows - -> Seq Scan on t2 t2_2 - Filter: ((a = 3) AND ((a % 2) = 1)) + -> Subquery Scan on t2_2 + Filter: ((t2_2.a % 2) = 1) + -> LockRows + -> Seq Scan on t2 t2_3 + Filter: ((a = 3) AND ((a % 2) = 1)) -> Subquery Scan on t1 Filter: f_leak(t1.b) -> Result -> Append -> Seq Scan on t1 t1_1 Filter: ((a = 3) AND ((a % 2) = 0)) - -> Seq Scan on t2 t2_3 + -> Seq Scan on t2 t2_4 Filter: ((a = 3) AND ((a % 2) = 0)) -> Seq Scan on t3 Filter: ((a = 3) AND ((a % 2) = 0)) -(17 rows) +(19 rows) UPDATE t2 SET b=t2.b FROM t1 WHERE t1.a = 3 and t2.a = 3 AND f_leak(t1.b) AND f_leak(t2.b); @@ -1349,39 +1367,47 @@ SELECT * FROM t1 ORDER BY a,b; SET SESSION AUTHORIZATION rls_regress_user1; SET row_security TO ON; EXPLAIN (COSTS OFF) DELETE FROM only t1 WHERE f_leak(b); - QUERY PLAN -------------------------------------------- + QUERY PLAN +------------------------------------------------- Delete on t1 t1_1 -> Subquery Scan on t1 Filter: f_leak(t1.b) - -> LockRows - -> Seq Scan on t1 t1_2 - Filter: ((a % 2) = 0) -(6 rows) + -> Subquery Scan on t1_2 + Filter: ((t1_2.a % 2) = 0) + -> LockRows + -> Seq Scan on t1 t1_3 + Filter: ((a % 2) = 0) +(8 rows) EXPLAIN (COSTS OFF) DELETE FROM t1 WHERE f_leak(b); - QUERY PLAN -------------------------------------------- + QUERY PLAN +------------------------------------------------- Delete on t1 t1_3 Delete on t1 t1_3 Delete on t2 t1 Delete on t3 t1 -> Subquery Scan on t1 Filter: f_leak(t1.b) - -> LockRows - -> Seq Scan on t1 t1_4 - Filter: ((a % 2) = 0) + -> Subquery Scan on t1_4 + Filter: ((t1_4.a % 2) = 0) + -> LockRows + -> Seq Scan on t1 t1_5 + Filter: ((a % 2) = 0) -> Subquery Scan on t1_1 Filter: f_leak(t1_1.b) - -> LockRows - -> Seq Scan on t2 - Filter: ((a % 2) = 0) + -> Subquery Scan on t1_6 + Filter: ((t1_6.a % 2) = 0) + -> LockRows + -> Seq Scan on t2 + Filter: ((a % 2) = 0) -> Subquery Scan on t1_2 Filter: f_leak(t1_2.b) - -> LockRows - -> Seq Scan on t3 - Filter: ((a % 2) = 0) -(19 rows) + -> Subquery Scan on t1_7 + Filter: ((t1_7.a % 2) = 0) + -> LockRows + -> Seq Scan on t3 + Filter: ((a % 2) = 0) +(25 rows) DELETE FROM only t1 WHERE f_leak(b) RETURNING oid, *, t1; NOTICE: f_leak => bbbbbb_updt @@ -1452,10 +1478,11 @@ EXPLAIN (COSTS OFF) UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b); -> Subquery Scan on b1 Filter: f_leak(b1.b) -> Subquery Scan on b1_2 + Filter: ((b1_2.a % 2) = 0) -> LockRows -> Seq Scan on b1 b1_3 Filter: ((a > 0) AND (a = 4) AND ((a % 2) = 0)) -(7 rows) +(8 rows) UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b); NOTICE: f_leak => a87ff679a2f3e71d9181a67b7542122c @@ -1466,10 +1493,11 @@ EXPLAIN (COSTS OFF) DELETE FROM bv1 WHERE a = 6 AND f_leak(b); -> Subquery Scan on b1 Filter: f_leak(b1.b) -> Subquery Scan on b1_2 + Filter: ((b1_2.a % 2) = 0) -> LockRows -> Seq Scan on b1 b1_3 Filter: ((a > 0) AND (a = 6) AND ((a % 2) = 0)) -(7 rows) +(8 rows) DELETE FROM bv1 WHERE a = 6 AND f_leak(b); NOTICE: f_leak => 1679091c5a880faf6fb5e6087eb1b2dc @@ -2743,15 +2771,17 @@ SELECT * FROM current_check; -- Plan should be a subquery TID scan EXPLAIN (COSTS OFF) UPDATE current_check SET payload = payload WHERE CURRENT OF current_check_cursor; - QUERY PLAN ---------------------------------------------------------------- + QUERY PLAN +--------------------------------------------------------------------- Update on current_check current_check_1 -> Subquery Scan on current_check - -> LockRows - -> Tid Scan on current_check current_check_2 - TID Cond: CURRENT OF current_check_cursor - Filter: (currentid = 4) -(6 rows) + -> Subquery Scan on current_check_2 + Filter: ((current_check_2.currentid % 2) = 0) + -> LockRows + -> Tid Scan on current_check current_check_3 + TID Cond: CURRENT OF current_check_cursor + Filter: (currentid = 4) +(8 rows) -- Similarly can only delete row 4 FETCH ABSOLUTE 1 FROM current_check_cursor; -- 1.9.1