diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c new file mode 100644 index ac0ae52..14a7041 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -23,6 +23,7 @@ #include "catalog/indexing.h" #include "catalog/pg_collation.h" #include "catalog/pg_statistic_ext.h" +#include "miscadmin.h" #include "nodes/nodeFuncs.h" #include "optimizer/clauses.h" #include "optimizer/optimizer.h" @@ -753,7 +754,8 @@ choose_best_statistics(List *stats, Bitm * attribute numbers from all compatible clauses (recursively). */ static bool -statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **attnums) +statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, + Index relid, Bitmapset **attnums) { /* Look inside any binary-compatible relabeling (as in examine_variable) */ if (IsA(clause, RelabelType)) @@ -784,6 +786,7 @@ statext_is_compatible_clause_internal(No /* (Var op Const) or (Const op Var) */ if (is_opclause(clause)) { + RangeTblEntry *rte = root->simple_rte_array[relid]; OpExpr *expr = (OpExpr *) clause; Var *var; bool varonleft = true; @@ -826,9 +829,24 @@ statext_is_compatible_clause_internal(No return false; } + /* + * If there are any securityQuals on the RTE from security barrier + * views or RLS policies, then the user may not have access to all the + * table's data, and we must check that the operator is leak-proof. + * + * If the operator is leaky, then we must ignore this clause for the + * purposes of estimating with MCV lists, otherwise the operator might + * reveal values from the MCV list that the user doesn't have + * permission to see. + */ + if (rte->securityQuals != NIL && + !get_func_leakproof(get_opcode(expr->opno))) + return false; + var = (varonleft) ? linitial(expr->args) : lsecond(expr->args); - return statext_is_compatible_clause_internal((Node *) var, relid, attnums); + return statext_is_compatible_clause_internal(root, (Node *) var, + relid, attnums); } /* AND/OR/NOT clause */ @@ -859,7 +877,8 @@ statext_is_compatible_clause_internal(No * Had we found incompatible clause in the arguments, treat the * whole clause as incompatible. */ - if (!statext_is_compatible_clause_internal((Node *) lfirst(lc), + if (!statext_is_compatible_clause_internal(root, + (Node *) lfirst(lc), relid, attnums)) return false; } @@ -879,7 +898,8 @@ statext_is_compatible_clause_internal(No if (!IsA(nt->arg, Var)) return false; - return statext_is_compatible_clause_internal((Node *) (nt->arg), relid, attnums); + return statext_is_compatible_clause_internal(root, (Node *) (nt->arg), + relid, attnums); } return false; @@ -902,9 +922,12 @@ statext_is_compatible_clause_internal(No * complex cases, for example (Var op Var). */ static bool -statext_is_compatible_clause(Node *clause, Index relid, Bitmapset **attnums) +statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid, + Bitmapset **attnums) { + RangeTblEntry *rte = root->simple_rte_array[relid]; RestrictInfo *rinfo = (RestrictInfo *) clause; + Oid userid; if (!IsA(rinfo, RestrictInfo)) return false; @@ -917,8 +940,43 @@ statext_is_compatible_clause(Node *claus if (bms_membership(rinfo->clause_relids) != BMS_SINGLETON) return false; - return statext_is_compatible_clause_internal((Node *) rinfo->clause, - relid, attnums); + /* Check the clause and determine what attributes it references. */ + if (!statext_is_compatible_clause_internal(root, (Node *) rinfo->clause, + relid, attnums)) + return false; + + /* + * Check that the user has permission to read all these attributes. Use + * checkAsUser if it's set, in case we're accessing the table via a view. + */ + userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); + + if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK) + { + /* Don't have table privilege, must check individual columns */ + if (bms_is_member(InvalidAttrNumber, *attnums)) + { + /* Have a whole-row reference, must have access to all columns */ + if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT, + ACLMASK_ALL) != ACLCHECK_OK) + return false; + } + else + { + /* Check the columns referenced by the clause */ + int attnum = -1; + + while ((attnum = bms_next_member(*attnums, attnum)) >= 0) + { + if (pg_attribute_aclcheck(rte->relid, attnum, userid, + ACL_SELECT) != ACLCHECK_OK) + return false; + } + } + } + + /* If we reach here, the clause is OK */ + return true; } /* @@ -1020,7 +1078,7 @@ statext_mcv_clauselist_selectivity(Plann Bitmapset *attnums = NULL; if (!bms_is_member(listidx, *estimatedclauses) && - statext_is_compatible_clause(clause, rel->relid, &attnums)) + statext_is_compatible_clause(root, clause, rel->relid, &attnums)) { list_attnums[listidx] = attnums; clauses_attnums = bms_add_members(clauses_attnums, attnums); diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out new file mode 100644 index 6dfca7a..aea5ada --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -684,3 +684,63 @@ SELECT * FROM check_estimated_rows('SELE 1 | 0 (1 row) +-- Permission tests. Users should not be able to see specific data values in +-- the extended statistics, if they lack permission to see those values in +-- the underlying table. +-- +-- Currently this is only relevant for MCV stats. +CREATE TABLE priv_test_tbl ( + a int, + b int +); +INSERT INTO priv_test_tbl + SELECT mod(i,5), mod(i,10) FROM generate_series(1,100) s(i); +CREATE STATISTICS priv_test_stats (mcv) ON a, b + FROM priv_test_tbl; +ANALYZE priv_test_tbl; +-- User with no access +CREATE USER regress_stats_user1; +SET SESSION AUTHORIZATION regress_stats_user1; +SELECT * FROM priv_test_tbl; -- Permission denied +ERROR: permission denied for table priv_test_tbl +-- Attempt to gain access using a leaky operator +CREATE FUNCTION op_leak(int, int) RETURNS bool + AS 'BEGIN RAISE NOTICE ''op_leak => %, %'', $1, $2; RETURN $1 < $2; END' + LANGUAGE plpgsql; +CREATE OPERATOR <<< (procedure = op_leak, leftarg = int, rightarg = int, + restrict = scalarltsel); +SELECT * FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Permission denied +ERROR: permission denied for table priv_test_tbl +DELETE FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Permission denied +ERROR: permission denied for table priv_test_tbl +-- Grant access via a security barrier view, but hide all data +RESET SESSION AUTHORIZATION; +CREATE VIEW priv_test_view WITH (security_barrier=true) + AS SELECT * FROM priv_test_tbl WHERE false; +GRANT SELECT, DELETE ON priv_test_view TO regress_stats_user1; +-- Should now have access via the view, but see nothing and leak nothing +SET SESSION AUTHORIZATION regress_stats_user1; +SELECT * FROM priv_test_view WHERE a <<< 0 AND b <<< 0; -- Should not leak + a | b +---+--- +(0 rows) + +DELETE FROM priv_test_view WHERE a <<< 0 AND b <<< 0; -- Should not leak +-- Grant table access, but hide all data with RLS +RESET SESSION AUTHORIZATION; +ALTER TABLE priv_test_tbl ENABLE ROW LEVEL SECURITY; +GRANT SELECT, DELETE ON priv_test_tbl TO regress_stats_user1; +-- Should now have direct table access, but see nothing and leak nothing +SET SESSION AUTHORIZATION regress_stats_user1; +SELECT * FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Should not leak + a | b +---+--- +(0 rows) + +DELETE FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Should not leak +-- Tidy up +DROP OPERATOR <<< (int, int); +DROP FUNCTION op_leak(int, int); +RESET SESSION AUTHORIZATION; +DROP VIEW priv_test_view; +DROP TABLE priv_test_tbl; diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql new file mode 100644 index c6a5776..2ac1d27 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -434,3 +434,63 @@ SELECT * FROM check_estimated_rows('SELE SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND NOT b AND c'); SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND b AND NOT c'); + +-- Permission tests. Users should not be able to see specific data values in +-- the extended statistics, if they lack permission to see those values in +-- the underlying table. +-- +-- Currently this is only relevant for MCV stats. +CREATE TABLE priv_test_tbl ( + a int, + b int +); + +INSERT INTO priv_test_tbl + SELECT mod(i,5), mod(i,10) FROM generate_series(1,100) s(i); + +CREATE STATISTICS priv_test_stats (mcv) ON a, b + FROM priv_test_tbl; + +ANALYZE priv_test_tbl; + +-- User with no access +CREATE USER regress_stats_user1; +SET SESSION AUTHORIZATION regress_stats_user1; +SELECT * FROM priv_test_tbl; -- Permission denied + +-- Attempt to gain access using a leaky operator +CREATE FUNCTION op_leak(int, int) RETURNS bool + AS 'BEGIN RAISE NOTICE ''op_leak => %, %'', $1, $2; RETURN $1 < $2; END' + LANGUAGE plpgsql; +CREATE OPERATOR <<< (procedure = op_leak, leftarg = int, rightarg = int, + restrict = scalarltsel); +SELECT * FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Permission denied +DELETE FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Permission denied + +-- Grant access via a security barrier view, but hide all data +RESET SESSION AUTHORIZATION; +CREATE VIEW priv_test_view WITH (security_barrier=true) + AS SELECT * FROM priv_test_tbl WHERE false; +GRANT SELECT, DELETE ON priv_test_view TO regress_stats_user1; + +-- Should now have access via the view, but see nothing and leak nothing +SET SESSION AUTHORIZATION regress_stats_user1; +SELECT * FROM priv_test_view WHERE a <<< 0 AND b <<< 0; -- Should not leak +DELETE FROM priv_test_view WHERE a <<< 0 AND b <<< 0; -- Should not leak + +-- Grant table access, but hide all data with RLS +RESET SESSION AUTHORIZATION; +ALTER TABLE priv_test_tbl ENABLE ROW LEVEL SECURITY; +GRANT SELECT, DELETE ON priv_test_tbl TO regress_stats_user1; + +-- Should now have direct table access, but see nothing and leak nothing +SET SESSION AUTHORIZATION regress_stats_user1; +SELECT * FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Should not leak +DELETE FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Should not leak + +-- Tidy up +DROP OPERATOR <<< (int, int); +DROP FUNCTION op_leak(int, int); +RESET SESSION AUTHORIZATION; +DROP VIEW priv_test_view; +DROP TABLE priv_test_tbl;