Use checkAsUser for selectivity estimator checks, if it's set.
authorDean Rasheed <dean.a.rasheed@gmail.com>
Mon, 6 May 2019 10:54:32 +0000 (11:54 +0100)
committerDean Rasheed <dean.a.rasheed@gmail.com>
Mon, 6 May 2019 10:54:32 +0000 (11:54 +0100)
In examine_variable() and examine_simple_variable(), when checking the
user's table and column privileges to determine whether to grant
access to the pg_statistic data, use checkAsUser for the privilege
checks, if it's set. This will be the case if we're accessing the
table via a view, to indicate that we should perform privilege checks
as the view owner rather than the current user.

This change makes this planner check consistent with the check in the
executor, so the planner will be able to make use of statistics if the
table is accessible via the view. This fixes a performance regression
introduced by commit e2d4ef8de8, which affects queries against
non-security barrier views in the case where the user doesn't have
privileges on the underlying table, but the view owner does.

Note that it continues to provide the same safeguards controlling
access to pg_statistic for direct table access (in which case
checkAsUser won't be set) and for security barrier views, because of
the nearby checks on rte->security_barrier and rte->securityQuals.

Back-patch to all supported branches because e2d4ef8de8 was.

Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.

src/backend/utils/adt/selfuncs.c
src/test/regress/expected/privileges.out
src/test/regress/sql/privileges.sql

index 514612857ad67be34a1140e2a82c253ef4f8cad7..65fabb7f3b42aba872e4941357a3417ec94fb9a0 100644 (file)
@@ -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
index 256890ab72a5f439d519e37008b6a40016b68eb1..0ddbd8e89fd55809ab4a0ab254a50a50f7ac04f2 100644 (file)
@@ -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                    
 -------------------------------------------------
index d2cfcb61c775f52c3919bb42501be5a53eebdb7a..f15d1f377370bdcb637fb9bb07cd076ff74ef6f2 100644 (file)
@@ -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