opensourcemirror
/
postgresql
mirror of git://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Add security checks to the multivariate MCV estimation code. 

The multivariate MCV estimation code may run user-defined operators on
the values in the MCV list, which means that those operators may
potentially leak the values from the MCV list. Guard against leaking
data to unprivileged users by checking that the user has SELECT
privileges on the table or all of the columns referred to by the
statistics.

Additionally, if there are any securityQuals on the RTE (either due to
RLS policies on the table, or accessing the table via a security
barrier view), not all rows may be visible to the current user, even
if they have table or column privileges. Thus we further insist that
the operator be leakproof in this case.

Dean Rasheed, reviewed by Tomas Vondra.

Discussion: https://postgr.es/m/CAEZATCUhT9rt7Ui=Vdx4N==VV5XOK5dsXfnGgVOz_JhAicB=ZA@mail.gmail.com
This commit is contained in:
Dean Rasheed 2019-06-23 18:50:08 +01:00
parent 89ff7c08ee
commit d7f8d26d9f
3 changed files with 186 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/backend/statistics/extended_stats.c
View File

@ -24,6 +24,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_statistic_ext_data.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
#include "optimizer/clauses.h"
#include "optimizer/optimizer.h"
@ -760,7 +761,8 @@ choose_best_statistics(List *stats, Bitmapset *attnums, char requiredkind)
* 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))
@ -791,6 +793,7 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
/* (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;
@ -833,9 +836,24 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
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 */
@ -866,7 +884,8 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
* 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;
}
@ -886,7 +905,8 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
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;
@ -909,9 +929,12 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
* 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;
@ -924,8 +947,43 @@ statext_is_compatible_clause(Node *clause, Index relid, Bitmapset **attnums)
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;
}
/*
@ -1027,7 +1085,7 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli
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);

60
src/test/regress/expected/stats_ext.out
View File

@ -696,3 +696,63 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND
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;

60
src/test/regress/sql/stats_ext.sql
View File

@ -446,3 +446,63 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND
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;