database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-15 19:21:59 +03:00
Code Activity

RLS fixes, new hooks, and new test module

Browse Source 
In prepend_row_security_policies(), defaultDeny was always true, so if
there were any hook policies, the RLS policies on the table would just
get discarded.  Fixed to start off with defaultDeny as false and then
properly set later if we detect that only the default deny policy exists
for the internal policies.

The infinite recursion detection in fireRIRrules() didn't properly
manage the activeRIRs list in the case of WCOs, so it would incorrectly
report infinite recusion if the same relation with RLS appeared more
than once in the rtable, for example "UPDATE t ... FROM t ...".

Further, the RLS expansion code in fireRIRrules() was handling RLS in
the main loop through the rtable, which lead to RTEs being visited twice
if they contained sublink subqueries, which
prepend_row_security_policies() attempted to handle by exiting early if
the RTE already had securityQuals.  That doesn't work, however, since
if the query involved a security barrier view on top of a table with
RLS, the RTE would already have securityQuals (from the view) by the
time fireRIRrules() was invoked, and so the table's RLS policies would
be ignored.  This is fixed in fireRIRrules() by handling RLS in a
separate loop at the end, after dealing with any other sublink
subqueries, thus ensuring that each RTE is only visited once for RLS
expansion.

The inheritance planner code didn't correctly handle non-target
relations with RLS, which would get turned into subqueries during
planning. Thus an update of the form "UPDATE t1 ... FROM t2 ..." where
t1 has inheritance and t2 has RLS quals would fail.  Fix by making sure
to copy in and update the securityQuals when they exist for non-target
relations.

process_policies() was adding WCOs to non-target relations, which is
unnecessary, and could lead to a lot of wasted time in the rewriter and
the planner. Fix by only adding WCO policies when working on the result
relation.  Also in process_policies, we should be copying the USING
policies to the WITH CHECK policies on a per-policy basis, fix by moving
the copying up into the per-policy loop.

Lastly, as noted by Dean, we were simply adding policies returned by the
hook provided to the list of quals being AND'd, meaning that they would
actually restrict records returned and there was no option to have
internal policies and hook-based policies work together permissively (as
all internal policies currently work).  Instead, explicitly add support
for both permissive and restrictive policies by having a hook for each
and combining the results appropriately.  To ensure this is all done
correctly, add a new test module (test_rls_hooks) to test the various
combinations of internal, permissive, and restrictive hook policies.

Largely from Dean Rasheed (thanks!):

CAEZATCVmFUfUOwwhnBTcgi6AquyjQ0-1fyKd0T3xBWJvn+xsFA@mail.gmail.com

Author: Dean Rasheed, though I added the new hooks and test module.
This commit is contained in:
Stephen Frost
2015-04-22 12:01:06 -04:00
parent 4ccc5bd28e
commit 0bf22e0c8b
16 changed files with 1273 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files

127
src/backend/rewrite/rewriteHandler.c
View File

@ -1714,51 +1714,6 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
activeRIRs = list_delete_first(activeRIRs);
}
}
/*
* If the RTE has row security quals, apply them and recurse into the
* securityQuals.
*/
if (prepend_row_security_policies(parsetree, rte, rt_index))
{
/*
* We applied security quals, check for infinite recursion and
* then expand any nested queries.
*/
if (list_member_oid(activeRIRs, RelationGetRelid(rel)))
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("infinite recursion detected in policy for relation \"%s\"",
RelationGetRelationName(rel))));
/*
* Make sure we check for recursion in either securityQuals or
* WITH CHECK quals.
*/
if (rte->securityQuals != NIL)
{
activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs);
expression_tree_walker( (Node*) rte->securityQuals,
fireRIRonSubLink, (void*)activeRIRs );
activeRIRs = list_delete_first(activeRIRs);
}
if (parsetree->withCheckOptions != NIL)
{
WithCheckOption *wco;
List *quals = NIL;
wco = (WithCheckOption *) makeNode(WithCheckOption);
quals = lcons(wco->qual, quals);
activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs);
expression_tree_walker( (Node*) quals, fireRIRonSubLink,
(void*)activeRIRs);
}
}
heap_close(rel, NoLock);
}
@ -1780,6 +1735,88 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
query_tree_walker(parsetree, fireRIRonSubLink, (void *) activeRIRs,
QTW_IGNORE_RC_SUBQUERIES);
/*
* Apply any row level security policies. We do this last because it
* requires special recursion detection if the new quals have sublink
* subqueries, and if we did it in the loop above query_tree_walker
* would then recurse into those quals a second time.
*/
rt_index = 0;
foreach(lc, parsetree->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
Relation rel;
List *securityQuals;
List *withCheckOptions;
bool hasRowSecurity;
bool hasSubLinks;
++rt_index;
/* Only normal relations can have RLS policies */
if (rte->rtekind != RTE_RELATION ||
rte->relkind != RELKIND_RELATION)
continue;
rel = heap_open(rte->relid, NoLock);
/*
* Fetch any new security quals that must be applied to this RTE.
*/
get_row_security_policies(parsetree, rte, rt_index,
&securityQuals, &withCheckOptions,
&hasRowSecurity, &hasSubLinks);
if (securityQuals != NIL || withCheckOptions != NIL)
{
if (hasSubLinks)
{
/*
* Recursively process the new quals, checking for infinite
* recursion.
*/
if (list_member_oid(activeRIRs, RelationGetRelid(rel)))
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("infinite recursion detected in policy for relation \"%s\"",
RelationGetRelationName(rel))));
activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs);
expression_tree_walker( (Node*) securityQuals,
fireRIRonSubLink, (void*)activeRIRs );
expression_tree_walker( (Node*) withCheckOptions,
fireRIRonSubLink, (void*)activeRIRs );
activeRIRs = list_delete_first(activeRIRs);
}
/*
* Add the new security quals to the start of the RTE's list so
* that they get applied before any existing security quals (which
* might have come from a user-written security barrier view, and
* might contain malicious code).
*/
rte->securityQuals = list_concat(securityQuals,
rte->securityQuals);
parsetree->withCheckOptions = list_concat(withCheckOptions,
parsetree->withCheckOptions);
}
/*
* Make sure the query is marked correctly if row level security
* applies, or if the new quals had sublinks.
*/
if (hasRowSecurity)
parsetree->hasRowSecurity = true;
if (hasSubLinks)
parsetree->hasSubLinks = true;
heap_close(rel, NoLock);
}
return parsetree;
}