diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 1a7cdb7f7cb815fe524e11280ae1df2b87c8ba94..9fa0ce6976b45de0bf1d45aab9ec1a7a6cc56185 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -4505,18 +4505,28 @@ 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 * to identify which column(s) the index - * depends on. + * depends on. Also require all rows to be + * selectable --- there must be no + * securityQuals from security barrier views. */ vardata->acl_ok = - (pg_class_aclcheck(rte->relid, GetUserId(), + rte->securityQuals == NIL && + (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) == ACLCHECK_OK); } else @@ -4579,12 +4589,22 @@ examine_simple_variable(PlannerInfo *root, Var *var, if (HeapTupleIsValid(vardata->statsTuple)) { - /* check if user has permission to read this column */ + 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. 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 = - (pg_class_aclcheck(rte->relid, GetUserId(), - ACL_SELECT) == ACLCHECK_OK) || - (pg_attribute_aclcheck(rte->relid, var->varattno, GetUserId(), - ACL_SELECT) == ACLCHECK_OK); + rte->securityQuals == NIL && + ((pg_class_aclcheck(rte->relid, userid, + ACL_SELECT) == ACLCHECK_OK) || + (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 996b4d9fd78c6f5928c933ec6b138ada3b4307a8..0ba2f473c3ea40f988e07919cfca5fc6e0c32285 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 --- regressuser1 will own a table and provide a view for it. +-- regressuser1 will own a table and provide views for it. SET SESSION AUTHORIZATION regressuser1; 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 regressuser2 can break security. SET SESSION AUTHORIZATION regressuser2; 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 relation 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 regressuser1 grants sufficient access to regressuser2. SET SESSION AUTHORIZATION regressuser1; GRANT SELECT (a, b) ON atest12 TO PUBLIC; SET SESSION AUTHORIZATION regressuser2; --- Now regressuser2 will also get a good row estimate. +-- regressuser2 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 27441a8b23c530bbf31b55a9b6de6cbe192e3e25..a4d27713f53a0b88b0e82db67f497502a9644908 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 --- regressuser1 will own a table and provide a view for it. +-- regressuser1 will own a table and provide views for it. SET SESSION AUTHORIZATION regressuser1; 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 regressuser2 can break security. SET SESSION AUTHORIZATION regressuser2; @@ -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 regressuser1 grants sufficient access to regressuser2. SET SESSION AUTHORIZATION regressuser1; GRANT SELECT (a, b) ON atest12 TO PUBLIC; SET SESSION AUTHORIZATION regressuser2; --- Now regressuser2 will also get a good row estimate. +-- regressuser2 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