diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 514612857ad..65fabb7f3b4 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -4589,10 +4589,17 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid, { /* Get index's table for permission check */ RangeTblEntry *rte; + Oid userid; rte = planner_rt_fetch(index->rel->relid, root); Assert(rte->rtekind == RTE_RELATION); + /* + * Use checkAsUser if it's set, in case we're + * accessing the table via a view. + */ + userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); + /* * For simplicity, we insist on the whole * table being selectable, rather than trying @@ -4604,7 +4611,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid, */ vardata->acl_ok = rte->securityQuals == NIL && - (pg_class_aclcheck(rte->relid, GetUserId(), + (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) == ACLCHECK_OK); } else @@ -4667,16 +4674,21 @@ examine_simple_variable(PlannerInfo *root, Var *var, if (HeapTupleIsValid(vardata->statsTuple)) { + Oid userid; + /* * Check if user has permission to read this column. We require * all rows to be accessible, so there must be no securityQuals - * from security barrier views or RLS policies. + * from security barrier views or RLS policies. Use checkAsUser + * if it's set, in case we're accessing the table via a view. */ + userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); + vardata->acl_ok = rte->securityQuals == NIL && - ((pg_class_aclcheck(rte->relid, GetUserId(), + ((pg_class_aclcheck(rte->relid, userid, ACL_SELECT) == ACLCHECK_OK) || - (pg_attribute_aclcheck(rte->relid, var->varattno, GetUserId(), + (pg_attribute_aclcheck(rte->relid, var->varattno, userid, ACL_SELECT) == ACLCHECK_OK)); } else diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 256890ab72a..0ddbd8e89fd 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -185,7 +185,7 @@ SELECT * FROM atest1; -- ok (2 rows) -- test leaky-function protections in selfuncs --- regress_priv_user1 will own a table and provide a view for it. +-- regress_priv_user1 will own a table and provide views for it. SET SESSION AUTHORIZATION regress_priv_user1; CREATE TABLE atest12 as SELECT x AS a, 10001 - x AS b FROM generate_series(1,10000) x; @@ -197,10 +197,13 @@ CREATE FUNCTION leak(integer,integer) RETURNS boolean LANGUAGE plpgsql immutable; CREATE OPERATOR <<< (procedure = leak, leftarg = integer, rightarg = integer, restrict = scalarltsel); --- view with leaky operator +-- views with leaky operator CREATE VIEW atest12v AS SELECT * FROM atest12 WHERE b <<< 5; +CREATE VIEW atest12sbv WITH (security_barrier=true) AS + SELECT * FROM atest12 WHERE b <<< 5; GRANT SELECT ON atest12v TO PUBLIC; +GRANT SELECT ON atest12sbv TO PUBLIC; -- This plan should use nestloop, knowing that few rows will be selected. EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y WHERE x.a = y.b; QUERY PLAN @@ -225,6 +228,20 @@ EXPLAIN (COSTS OFF) SELECT * FROM atest12 x, atest12 y Index Cond: (a = y.b) (5 rows) +-- This should also be a nestloop, but the security barrier forces the inner +-- scan to be materialized +EXPLAIN (COSTS OFF) SELECT * FROM atest12sbv x, atest12sbv y WHERE x.a = y.b; + QUERY PLAN +------------------------------------------- + Nested Loop + Join Filter: (atest12.a = atest12_1.b) + -> Seq Scan on atest12 + Filter: (b <<< 5) + -> Materialize + -> Seq Scan on atest12 atest12_1 + Filter: (b <<< 5) +(7 rows) + -- Check if regress_priv_user2 can break security. SET SESSION AUTHORIZATION regress_priv_user2; CREATE FUNCTION leak2(integer,integer) RETURNS boolean @@ -235,24 +252,64 @@ CREATE OPERATOR >>> (procedure = leak2, leftarg = integer, rightarg = integer, -- This should not show any "leak" notices before failing. EXPLAIN (COSTS OFF) SELECT * FROM atest12 WHERE a >>> 0; ERROR: permission denied for table atest12 --- This plan should use hashjoin, as it will expect many rows to be selected. +-- These plans should continue to use a nestloop, since they execute with the +-- privileges of the view owner. EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y WHERE x.a = y.b; + QUERY PLAN +------------------------------------------------- + Nested Loop + -> Seq Scan on atest12 atest12_1 + Filter: (b <<< 5) + -> Index Scan using atest12_a_idx on atest12 + Index Cond: (a = atest12_1.b) + Filter: (b <<< 5) +(6 rows) + +EXPLAIN (COSTS OFF) SELECT * FROM atest12sbv x, atest12sbv y WHERE x.a = y.b; QUERY PLAN ------------------------------------------- - Hash Join - Hash Cond: (atest12.a = atest12_1.b) + Nested Loop + Join Filter: (atest12.a = atest12_1.b) -> Seq Scan on atest12 Filter: (b <<< 5) - -> Hash + -> Materialize -> Seq Scan on atest12 atest12_1 Filter: (b <<< 5) (7 rows) +-- A non-security barrier view does not guard against information leakage. +EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y + WHERE x.a = y.b and abs(y.a) <<< 5; + QUERY PLAN +------------------------------------------------- + Nested Loop + -> Seq Scan on atest12 atest12_1 + Filter: ((b <<< 5) AND (abs(a) <<< 5)) + -> Index Scan using atest12_a_idx on atest12 + Index Cond: (a = atest12_1.b) + Filter: (b <<< 5) +(6 rows) + +-- But a security barrier view isolates the leaky operator. +EXPLAIN (COSTS OFF) SELECT * FROM atest12sbv x, atest12sbv y + WHERE x.a = y.b and abs(y.a) <<< 5; + QUERY PLAN +------------------------------------- + Nested Loop + Join Filter: (atest12_1.a = y.b) + -> Subquery Scan on y + Filter: (abs(y.a) <<< 5) + -> Seq Scan on atest12 + Filter: (b <<< 5) + -> Seq Scan on atest12 atest12_1 + Filter: (b <<< 5) +(8 rows) + -- Now regress_priv_user1 grants sufficient access to regress_priv_user2. SET SESSION AUTHORIZATION regress_priv_user1; GRANT SELECT (a, b) ON atest12 TO PUBLIC; SET SESSION AUTHORIZATION regress_priv_user2; --- Now regress_priv_user2 will also get a good row estimate. +-- regress_priv_user2 should continue to get a good row estimate. EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y WHERE x.a = y.b; QUERY PLAN ------------------------------------------------- diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index d2cfcb61c77..f15d1f37737 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -129,7 +129,7 @@ SELECT * FROM atest1; -- ok -- test leaky-function protections in selfuncs --- regress_priv_user1 will own a table and provide a view for it. +-- regress_priv_user1 will own a table and provide views for it. SET SESSION AUTHORIZATION regress_priv_user1; CREATE TABLE atest12 as @@ -144,10 +144,13 @@ CREATE FUNCTION leak(integer,integer) RETURNS boolean CREATE OPERATOR <<< (procedure = leak, leftarg = integer, rightarg = integer, restrict = scalarltsel); --- view with leaky operator +-- views with leaky operator CREATE VIEW atest12v AS SELECT * FROM atest12 WHERE b <<< 5; +CREATE VIEW atest12sbv WITH (security_barrier=true) AS + SELECT * FROM atest12 WHERE b <<< 5; GRANT SELECT ON atest12v TO PUBLIC; +GRANT SELECT ON atest12sbv TO PUBLIC; -- This plan should use nestloop, knowing that few rows will be selected. EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y WHERE x.a = y.b; @@ -156,6 +159,10 @@ EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y WHERE x.a = y.b; EXPLAIN (COSTS OFF) SELECT * FROM atest12 x, atest12 y WHERE x.a = y.b and abs(y.a) <<< 5; +-- This should also be a nestloop, but the security barrier forces the inner +-- scan to be materialized +EXPLAIN (COSTS OFF) SELECT * FROM atest12sbv x, atest12sbv y WHERE x.a = y.b; + -- Check if regress_priv_user2 can break security. SET SESSION AUTHORIZATION regress_priv_user2; @@ -168,15 +175,25 @@ CREATE OPERATOR >>> (procedure = leak2, leftarg = integer, rightarg = integer, -- This should not show any "leak" notices before failing. EXPLAIN (COSTS OFF) SELECT * FROM atest12 WHERE a >>> 0; --- This plan should use hashjoin, as it will expect many rows to be selected. +-- These plans should continue to use a nestloop, since they execute with the +-- privileges of the view owner. EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y WHERE x.a = y.b; +EXPLAIN (COSTS OFF) SELECT * FROM atest12sbv x, atest12sbv y WHERE x.a = y.b; + +-- A non-security barrier view does not guard against information leakage. +EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y + WHERE x.a = y.b and abs(y.a) <<< 5; + +-- But a security barrier view isolates the leaky operator. +EXPLAIN (COSTS OFF) SELECT * FROM atest12sbv x, atest12sbv y + WHERE x.a = y.b and abs(y.a) <<< 5; -- Now regress_priv_user1 grants sufficient access to regress_priv_user2. SET SESSION AUTHORIZATION regress_priv_user1; GRANT SELECT (a, b) ON atest12 TO PUBLIC; SET SESSION AUTHORIZATION regress_priv_user2; --- Now regress_priv_user2 will also get a good row estimate. +-- regress_priv_user2 should continue to get a good row estimate. EXPLAIN (COSTS OFF) SELECT * FROM atest12v x, atest12v y WHERE x.a = y.b; -- But not for this, due to lack of table-wide permissions needed