Fix row level security checks during view maintenance
The view maintenance is performed under the view owner privilege. If a modified table has a RLS policy, the policy must be applied to relation for the pre-update-state table and the delta table that contained inserted or deleted tuples. Previously, the security quals were set to each ENR in a subquery that represents such relation. However, the security check on the delta table was not properly handled, and this caused that rows that must not be accessed from the view owner could appear in the view contents when the view was refreshed incrementally during a query containing multiple types of commands, like a modifying CTE that contains INSERT and UPDATE, or a MERGE command. This patch fixes it by setting RLS policy to a subquery that presents the pre-update-state table and the delta able instead of to each RLS. Also, this change makes the code more simple and easy to maintain. CVE-2023-22847
This commit is contained in:
Yugo Nagata
parent
0587e78651
commit
99a176ab97
3 changed files with 93 additions and 107 deletions
|
|
@ -932,7 +932,7 @@ SELECT * FROM mv_cte ORDER BY i,j,k;
|
|||
(8 rows)
|
||||
|
||||
ROLLBACK;
|
||||
-- nested CTE
|
||||
-- nested CTE
|
||||
BEGIN;
|
||||
SELECT create_immv('mv_ivm_nested_cte', 'WITH v AS ( WITH a AS (SELECT * FROM mv_base_a) SELECT i, a.j, b.k FROM mv_base_b b INNER JOIN a USING(i)) SELECT * FROM v');
|
||||
NOTICE: could not create an index on immv "mv_ivm_nested_cte" automatically
|
||||
|
|
@ -1291,13 +1291,14 @@ SELECT create_immv('mv_ivm_only_values1', 'values(1)');
|
|||
ERROR: VALUES is not supported on incrementally maintainable materialized view
|
||||
SELECT create_immv('mv_ivm_only_values2', 'SELECT * FROM (values(1)) AS tmp');
|
||||
ERROR: VALUES is not supported on incrementally maintainable materialized view
|
||||
-- base table which has row level security
|
||||
-- views containing base tables with Row Level Security
|
||||
DROP USER IF EXISTS ivm_admin;
|
||||
NOTICE: role "ivm_admin" does not exist, skipping
|
||||
DROP USER IF EXISTS ivm_user;
|
||||
NOTICE: role "ivm_user" does not exist, skipping
|
||||
CREATE USER ivm_admin;
|
||||
CREATE USER ivm_user;
|
||||
--- create a table with RLS
|
||||
SET SESSION AUTHORIZATION ivm_admin;
|
||||
CREATE TABLE rls_tbl(id int, data text, owner name);
|
||||
INSERT INTO rls_tbl VALUES
|
||||
|
|
@ -1308,12 +1309,15 @@ INSERT INTO num_tbl VALUES
|
|||
(1,'one'),
|
||||
(2,'two'),
|
||||
(3,'three'),
|
||||
(4,'four');
|
||||
(4,'four'),
|
||||
(5,'five'),
|
||||
(6,'six');
|
||||
--- Users can access only their own rows
|
||||
CREATE POLICY rls_tbl_policy ON rls_tbl FOR SELECT TO PUBLIC USING(owner = current_user);
|
||||
CREATE POLICY rls_tbl_policy2 ON rls_tbl FOR INSERT TO PUBLIC WITH CHECK(current_user LIKE 'ivm_%');
|
||||
ALTER TABLE rls_tbl ENABLE ROW LEVEL SECURITY;
|
||||
GRANT ALL on rls_tbl TO PUBLIC;
|
||||
GRANT ALL on num_tbl TO PUBLIC;
|
||||
--- create a view owned by ivm_user
|
||||
SET SESSION AUTHORIZATION ivm_user;
|
||||
SELECT create_immv('ivm_rls', 'SELECT * FROM rls_tbl');
|
||||
NOTICE: could not create an index on immv "ivm_rls" automatically
|
||||
|
|
@ -1330,6 +1334,8 @@ SELECT id, data, owner FROM ivm_rls ORDER BY 1,2,3;
|
|||
1 | foo | ivm_user
|
||||
(1 row)
|
||||
|
||||
RESET SESSION AUTHORIZATION;
|
||||
--- inserts rows owned by different users
|
||||
INSERT INTO rls_tbl VALUES
|
||||
(3,'baz','ivm_user'),
|
||||
(4,'qux','postgres');
|
||||
|
|
@ -1340,13 +1346,32 @@ SELECT id, data, owner FROM ivm_rls ORDER BY 1,2,3;
|
|||
3 | baz | ivm_user
|
||||
(2 rows)
|
||||
|
||||
--- combination of diffent kinds of commands
|
||||
WITH
|
||||
i AS (INSERT INTO rls_tbl VALUES(5,'quux','postgres'), (6,'corge','ivm_user')),
|
||||
u AS (UPDATE rls_tbl SET owner = 'postgres' WHERE id = 1),
|
||||
u2 AS (UPDATE rls_tbl SET owner = 'ivm_user' WHERE id = 2)
|
||||
SELECT;
|
||||
--
|
||||
(1 row)
|
||||
|
||||
SELECT id, data, owner FROM ivm_rls ORDER BY 1,2,3;
|
||||
id | data | owner
|
||||
----+-------+----------
|
||||
2 | bar | ivm_user
|
||||
3 | baz | ivm_user
|
||||
6 | corge | ivm_user
|
||||
(3 rows)
|
||||
|
||||
---
|
||||
SET SESSION AUTHORIZATION ivm_user;
|
||||
SELECT create_immv('ivm_rls2', 'SELECT * FROM rls_tbl JOIN num_tbl USING(id)');
|
||||
NOTICE: could not create an index on immv "ivm_rls2" automatically
|
||||
DETAIL: This target list does not have all the primary key columns, or this view does not contain GROUP BY or DISTINCT clause.
|
||||
HINT: Create an index on the immv for efficient incremental maintenance.
|
||||
create_immv
|
||||
-------------
|
||||
2
|
||||
3
|
||||
(1 row)
|
||||
|
||||
RESET SESSION AUTHORIZATION;
|
||||
|
|
@ -1360,9 +1385,10 @@ SELECT;
|
|||
SELECT * FROM ivm_rls2 ORDER BY 1,2,3;
|
||||
id | data | owner | num
|
||||
----+-------+----------+---------
|
||||
1 | foo | ivm_user | one
|
||||
2 | bar | ivm_user | two
|
||||
3 | baz_2 | ivm_user | three_2
|
||||
(2 rows)
|
||||
6 | corge | ivm_user | six
|
||||
(3 rows)
|
||||
|
||||
DROP TABLE rls_tbl CASCADE;
|
||||
NOTICE: drop cascades to 2 other objects
|
||||
|
|
|
|||
129
matview.c
129
matview.c
|
|
@ -208,8 +208,6 @@ static void mv_HashPreparedPlan(MV_QueryKey *key, SPIPlanPtr plan);
|
|||
static void mv_BuildQueryKey(MV_QueryKey *key, Oid matview_id, int32 query_type);
|
||||
static void clean_up_IVM_hash_entry(MV_TriggerHashEntry *entry, bool is_abort);
|
||||
|
||||
static List *get_securityQuals(Oid relId, int rt_index, Query *query);
|
||||
|
||||
/* SQL callable functions */
|
||||
PG_FUNCTION_INFO_V1(IVM_immediate_before);
|
||||
PG_FUNCTION_INFO_V1(IVM_immediate_maintenance);
|
||||
|
|
@ -1159,7 +1157,31 @@ rewrite_query_for_preupdate_state(Query *query, List *tables,
|
|||
*/
|
||||
if (r->relid == table->table_id)
|
||||
{
|
||||
lfirst(lc) = get_prestate_rte(r, table, pstate->p_queryEnv, matviewid);
|
||||
List *securityQuals;
|
||||
List *withCheckOptions;
|
||||
bool hasRowSecurity;
|
||||
bool hasSubLinks;
|
||||
|
||||
RangeTblEntry *rte_pre = get_prestate_rte(r, table, pstate->p_queryEnv, matviewid);
|
||||
|
||||
/*
|
||||
* Set a row security poslicies of the modified table to the subquery RTE which
|
||||
* represents the pre-update state of the table.
|
||||
*/
|
||||
get_row_security_policies(query, table->original_rte, i,
|
||||
&securityQuals, &withCheckOptions,
|
||||
&hasRowSecurity, &hasSubLinks);
|
||||
if (hasRowSecurity)
|
||||
{
|
||||
query->hasRowSecurity = true;
|
||||
rte_pre->security_barrier = true;
|
||||
}
|
||||
if (hasSubLinks)
|
||||
query->hasSubLinks = true;
|
||||
|
||||
rte_pre->securityQuals = securityQuals;
|
||||
lfirst(lc) = rte_pre;
|
||||
|
||||
table->rte_paths = lappend(table->rte_paths, lappend_int(list_copy(rte_path), i));
|
||||
break;
|
||||
}
|
||||
|
|
@ -1212,8 +1234,6 @@ register_delta_ENRs(ParseState *pstate, Query *query, List *tables)
|
|||
|
||||
nsitem = addRangeTableEntryForENR(pstate, makeRangeVar(NULL, enr->md.name, -1), true);
|
||||
rte = nsitem->p_rte;
|
||||
/* if base table has RLS, set security condition to enr */
|
||||
rte->securityQuals = get_securityQuals(table->table_id, list_length(query->rtable) + 1, query);
|
||||
|
||||
query->rtable = lappend(query->rtable, rte);
|
||||
table->old_rtes = lappend(table->old_rtes, rte);
|
||||
|
|
@ -1239,8 +1259,6 @@ register_delta_ENRs(ParseState *pstate, Query *query, List *tables)
|
|||
|
||||
nsitem = addRangeTableEntryForENR(pstate, makeRangeVar(NULL, enr->md.name, -1), true);
|
||||
rte = nsitem->p_rte;
|
||||
/* if base table has RLS, set security condition to enr*/
|
||||
rte->securityQuals = get_securityQuals(table->table_id, list_length(query->rtable) + 1, query);
|
||||
|
||||
query->rtable = lappend(query->rtable, rte);
|
||||
table->new_rtes = lappend(table->new_rtes, rte);
|
||||
|
|
@ -1348,27 +1366,6 @@ get_prestate_rte(RangeTblEntry *rte, MV_TriggerTable *table,
|
|||
#endif
|
||||
sub = transformStmt(pstate, raw->stmt);
|
||||
|
||||
/* If this query has setOperations, RTEs in rtables has a subquery which contains ENR */
|
||||
if (sub->setOperations != NULL)
|
||||
{
|
||||
ListCell *lc;
|
||||
|
||||
/* add securityQuals for tuplestores */
|
||||
foreach (lc, sub->rtable)
|
||||
{
|
||||
RangeTblEntry *rte;
|
||||
RangeTblEntry *sub_rte;
|
||||
|
||||
rte = (RangeTblEntry *)lfirst(lc);
|
||||
Assert(rte->subquery != NULL);
|
||||
|
||||
sub_rte = (RangeTblEntry *)linitial(rte->subquery->rtable);
|
||||
if (sub_rte->rtekind == RTE_NAMEDTUPLESTORE)
|
||||
/* rt_index is always 1, bacause subquery has enr_rte only */
|
||||
sub_rte->securityQuals = get_securityQuals(sub_rte->relid, 1, sub);
|
||||
}
|
||||
}
|
||||
|
||||
/* save the original RTE */
|
||||
table->original_rte = copyObject(rte);
|
||||
|
||||
|
|
@ -1413,8 +1410,8 @@ make_delta_enr_name(const char *prefix, Oid relid, int count)
|
|||
/*
|
||||
* union_ENRs
|
||||
*
|
||||
* Make a single table delta by unionning all transition tables of the modified table
|
||||
* whose RTE is specified by
|
||||
* Replace RTE of the modified table with a single table delta that combine its
|
||||
* all transition tables.
|
||||
*/
|
||||
static RangeTblEntry*
|
||||
union_ENRs(RangeTblEntry *rte, Oid relid, List *enr_rtes, const char *prefix,
|
||||
|
|
@ -1425,7 +1422,9 @@ union_ENRs(RangeTblEntry *rte, Oid relid, List *enr_rtes, const char *prefix,
|
|||
RawStmt *raw;
|
||||
Query *sub;
|
||||
int i;
|
||||
RangeTblEntry *enr_rte;
|
||||
|
||||
/* the previous RTE must be a subquery which represents "pre-state" table */
|
||||
Assert(rte->rtekind == RTE_SUBQUERY);
|
||||
|
||||
/* Create a ParseState for rewriting the view definition query */
|
||||
pstate = make_parsestate(NULL);
|
||||
|
|
@ -1451,26 +1450,13 @@ union_ENRs(RangeTblEntry *rte, Oid relid, List *enr_rtes, const char *prefix,
|
|||
#endif
|
||||
sub = transformStmt(pstate, raw->stmt);
|
||||
|
||||
rte->rtekind = RTE_SUBQUERY;
|
||||
/*
|
||||
* Update the subquery so that it represent the combined transition
|
||||
* table. Note that we leave the security_barrier and securityQuals
|
||||
* fields so that the subquery relation can be protected by the RLS
|
||||
* policy as same as the modified table.
|
||||
*/
|
||||
rte->subquery = sub;
|
||||
rte->security_barrier = false;
|
||||
/* Clear fields that should not be set in a subquery RTE */
|
||||
rte->relid = InvalidOid;
|
||||
rte->relkind = 0;
|
||||
rte->rellockmode = 0;
|
||||
rte->tablesample = NULL;
|
||||
rte->inh = false; /* must not be set for a subquery */
|
||||
|
||||
rte->requiredPerms = 0; /* no permission check on subquery itself */
|
||||
rte->checkAsUser = InvalidOid;
|
||||
rte->selectedCols = NULL;
|
||||
rte->insertedCols = NULL;
|
||||
rte->updatedCols = NULL;
|
||||
rte->extraUpdatedCols = NULL;
|
||||
/* if base table has RLS, set security condition to enr*/
|
||||
enr_rte = (RangeTblEntry *)linitial(sub->rtable);
|
||||
/* rt_index is always 1, bacause subquery has enr_rte only */
|
||||
enr_rte->securityQuals = get_securityQuals(relid, 1, sub);
|
||||
|
||||
return rte;
|
||||
}
|
||||
|
|
@ -2960,46 +2946,3 @@ isIvmName(const char *s)
|
|||
return (strncmp(s, "__ivm_", 6) == 0);
|
||||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* get_securityQuals
|
||||
*
|
||||
* Get row security policy on a relation.
|
||||
* This is used by IVM for copying RLS from base table to enr.
|
||||
*/
|
||||
static List *
|
||||
get_securityQuals(Oid relId, int rt_index, Query *query)
|
||||
{
|
||||
ParseState *pstate;
|
||||
Relation rel;
|
||||
ParseNamespaceItem *nsitem;
|
||||
RangeTblEntry *rte;
|
||||
List *securityQuals;
|
||||
List *withCheckOptions;
|
||||
bool hasRowSecurity;
|
||||
bool hasSubLinks;
|
||||
|
||||
securityQuals = NIL;
|
||||
pstate = make_parsestate(NULL);
|
||||
|
||||
rel = table_open(relId, NoLock);
|
||||
nsitem = addRangeTableEntryForRelation(pstate, rel, AccessShareLock, NULL, false, false);
|
||||
rte = nsitem->p_rte;
|
||||
|
||||
get_row_security_policies(query, rte, rt_index,
|
||||
&securityQuals, &withCheckOptions,
|
||||
&hasRowSecurity, &hasSubLinks);
|
||||
|
||||
/*
|
||||
* Make sure the query is marked correctly if row level security
|
||||
* applies, or if the new quals had sublinks.
|
||||
*/
|
||||
if (hasRowSecurity)
|
||||
query->hasRowSecurity = true;
|
||||
if (hasSubLinks)
|
||||
query->hasSubLinks = true;
|
||||
|
||||
table_close(rel, NoLock);
|
||||
|
||||
return securityQuals;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -318,7 +318,7 @@ SELECT;
|
|||
SELECT * FROM mv_cte ORDER BY i,j,k;
|
||||
ROLLBACK;
|
||||
|
||||
-- nested CTE
|
||||
-- nested CTE
|
||||
BEGIN;
|
||||
SELECT create_immv('mv_ivm_nested_cte', 'WITH v AS ( WITH a AS (SELECT * FROM mv_base_a) SELECT i, a.j, b.k FROM mv_base_b b INNER JOIN a USING(i)) SELECT * FROM v');
|
||||
WITH
|
||||
|
|
@ -522,13 +522,14 @@ SELECT create_immv('mv_ivm_only_values1', 'values(1)');
|
|||
SELECT create_immv('mv_ivm_only_values2', 'SELECT * FROM (values(1)) AS tmp');
|
||||
|
||||
|
||||
-- base table which has row level security
|
||||
-- views containing base tables with Row Level Security
|
||||
DROP USER IF EXISTS ivm_admin;
|
||||
DROP USER IF EXISTS ivm_user;
|
||||
CREATE USER ivm_admin;
|
||||
CREATE USER ivm_user;
|
||||
SET SESSION AUTHORIZATION ivm_admin;
|
||||
|
||||
--- create a table with RLS
|
||||
SET SESSION AUTHORIZATION ivm_admin;
|
||||
CREATE TABLE rls_tbl(id int, data text, owner name);
|
||||
INSERT INTO rls_tbl VALUES
|
||||
(1,'foo','ivm_user'),
|
||||
|
|
@ -538,23 +539,39 @@ INSERT INTO num_tbl VALUES
|
|||
(1,'one'),
|
||||
(2,'two'),
|
||||
(3,'three'),
|
||||
(4,'four');
|
||||
(4,'four'),
|
||||
(5,'five'),
|
||||
(6,'six');
|
||||
|
||||
--- Users can access only their own rows
|
||||
CREATE POLICY rls_tbl_policy ON rls_tbl FOR SELECT TO PUBLIC USING(owner = current_user);
|
||||
CREATE POLICY rls_tbl_policy2 ON rls_tbl FOR INSERT TO PUBLIC WITH CHECK(current_user LIKE 'ivm_%');
|
||||
ALTER TABLE rls_tbl ENABLE ROW LEVEL SECURITY;
|
||||
GRANT ALL on rls_tbl TO PUBLIC;
|
||||
GRANT ALL on num_tbl TO PUBLIC;
|
||||
|
||||
--- create a view owned by ivm_user
|
||||
SET SESSION AUTHORIZATION ivm_user;
|
||||
|
||||
SELECT create_immv('ivm_rls', 'SELECT * FROM rls_tbl');
|
||||
SELECT id, data, owner FROM ivm_rls ORDER BY 1,2,3;
|
||||
RESET SESSION AUTHORIZATION;
|
||||
|
||||
--- inserts rows owned by different users
|
||||
INSERT INTO rls_tbl VALUES
|
||||
(3,'baz','ivm_user'),
|
||||
(4,'qux','postgres');
|
||||
SELECT id, data, owner FROM ivm_rls ORDER BY 1,2,3;
|
||||
SELECT create_immv('ivm_rls2', 'SELECT * FROM rls_tbl JOIN num_tbl USING(id)');
|
||||
|
||||
--- combination of diffent kinds of commands
|
||||
WITH
|
||||
i AS (INSERT INTO rls_tbl VALUES(5,'quux','postgres'), (6,'corge','ivm_user')),
|
||||
u AS (UPDATE rls_tbl SET owner = 'postgres' WHERE id = 1),
|
||||
u2 AS (UPDATE rls_tbl SET owner = 'ivm_user' WHERE id = 2)
|
||||
SELECT;
|
||||
SELECT id, data, owner FROM ivm_rls ORDER BY 1,2,3;
|
||||
|
||||
---
|
||||
SET SESSION AUTHORIZATION ivm_user;
|
||||
SELECT create_immv('ivm_rls2', 'SELECT * FROM rls_tbl JOIN num_tbl USING(id)');
|
||||
RESET SESSION AUTHORIZATION;
|
||||
|
||||
WITH
|
||||
|
|
|
|||
Loading…
Reference in a new issue