Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0bf22e0

Browse files
committed
RLS fixes, new hooks, and new test module
In prepend_row_security_policies(), defaultDeny was always true, so ifthere were any hook policies, the RLS policies on the table would justget discarded. Fixed to start off with defaultDeny as false and thenproperly set later if we detect that only the default deny policy existsfor the internal policies.The infinite recursion detection in fireRIRrules() didn't properlymanage the activeRIRs list in the case of WCOs, so it would incorrectlyreport infinite recusion if the same relation with RLS appeared morethan once in the rtable, for example "UPDATE t ... FROM t ...".Further, the RLS expansion code in fireRIRrules() was handling RLS inthe main loop through the rtable, which lead to RTEs being visited twiceif they contained sublink subqueries, whichprepend_row_security_policies() attempted to handle by exiting early ifthe RTE already had securityQuals. That doesn't work, however, sinceif the query involved a security barrier view on top of a table withRLS, the RTE would already have securityQuals (from the view) by thetime fireRIRrules() was invoked, and so the table's RLS policies wouldbe ignored. This is fixed in fireRIRrules() by handling RLS in aseparate loop at the end, after dealing with any other sublinksubqueries, thus ensuring that each RTE is only visited once for RLSexpansion.The inheritance planner code didn't correctly handle non-targetrelations with RLS, which would get turned into subqueries duringplanning. Thus an update of the form "UPDATE t1 ... FROM t2 ..." wheret1 has inheritance and t2 has RLS quals would fail. Fix by making sureto copy in and update the securityQuals when they exist for non-targetrelations.process_policies() was adding WCOs to non-target relations, which isunnecessary, and could lead to a lot of wasted time in the rewriter andthe planner. Fix by only adding WCO policies when working on the resultrelation. Also in process_policies, we should be copying the USINGpolicies to the WITH CHECK policies on a per-policy basis, fix by movingthe copying up into the per-policy loop.Lastly, as noted by Dean, we were simply adding policies returned by thehook provided to the list of quals being AND'd, meaning that they wouldactually restrict records returned and there was no option to haveinternal policies and hook-based policies work together permissively (asall internal policies currently work). Instead, explicitly add supportfor both permissive and restrictive policies by having a hook for eachand combining the results appropriately. To ensure this is all donecorrectly, add a new test module (test_rls_hooks) to test the variouscombinations of internal, permissive, and restrictive hook policies.Largely from Dean Rasheed (thanks!):CAEZATCVmFUfUOwwhnBTcgi6AquyjQ0-1fyKd0T3xBWJvn+xsFA@mail.gmail.comAuthor: Dean Rasheed, though I added the new hooks and test module.
1 parent4ccc5bd commit0bf22e0

File tree

16 files changed

+1271
-147
lines changed

16 files changed

+1271
-147
lines changed

‎src/backend/optimizer/plan/planner.c

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,7 @@ inheritance_planner(PlannerInfo *root)
790790
{
791791
Query*parse=root->parse;
792792
intparentRTindex=parse->resultRelation;
793+
Bitmapset*resultRTindexes=NULL;
793794
intnominalRelation=-1;
794795
List*final_rtable=NIL;
795796
intsave_rel_array_size=0;
@@ -815,7 +816,21 @@ inheritance_planner(PlannerInfo *root)
815816
* (1) would result in a rangetable of length O(N^2) for N targets, with
816817
* at least O(N^3) work expended here; and (2) would greatly complicate
817818
* management of the rowMarks list.
819+
*
820+
* Note that any RTEs with security barrier quals will be turned into
821+
* subqueries during planning, and so we must create copies of them too,
822+
* except where they are target relations, which will each only be used
823+
* in a single plan.
818824
*/
825+
resultRTindexes=bms_add_member(resultRTindexes,parentRTindex);
826+
foreach(lc,root->append_rel_list)
827+
{
828+
AppendRelInfo*appinfo= (AppendRelInfo*)lfirst(lc);
829+
if (appinfo->parent_relid==parentRTindex)
830+
resultRTindexes=bms_add_member(resultRTindexes,
831+
appinfo->child_relid);
832+
}
833+
819834
foreach(lc,root->append_rel_list)
820835
{
821836
AppendRelInfo*appinfo= (AppendRelInfo*)lfirst(lc);
@@ -886,21 +901,29 @@ inheritance_planner(PlannerInfo *root)
886901
{
887902
RangeTblEntry*rte= (RangeTblEntry*)lfirst(lr);
888903

889-
if (rte->rtekind==RTE_SUBQUERY)
904+
/*
905+
* Copy subquery RTEs and RTEs with security barrier quals
906+
* that will be turned into subqueries, except those that are
907+
* target relations.
908+
*/
909+
if (rte->rtekind==RTE_SUBQUERY||
910+
(rte->securityQuals!=NIL&&
911+
!bms_is_member(rti,resultRTindexes)))
890912
{
891913
Indexnewrti;
892914

893915
/*
894916
* The RTE can't contain any references to its own RT
895-
* index,so we can save a few cycles by applying
896-
*ChangeVarNodes before we append the RTE to the
897-
* rangetable.
917+
* index,except in the security barrier quals, so we can
918+
*save a few cycles by applying ChangeVarNodes before we
919+
*append the RTE to therangetable.
898920
*/
899921
newrti=list_length(subroot.parse->rtable)+1;
900922
ChangeVarNodes((Node*)subroot.parse,rti,newrti,0);
901923
ChangeVarNodes((Node*)subroot.rowMarks,rti,newrti,0);
902924
ChangeVarNodes((Node*)subroot.append_rel_list,rti,newrti,0);
903925
rte=copyObject(rte);
926+
ChangeVarNodes((Node*)rte->securityQuals,rti,newrti,0);
904927
subroot.parse->rtable=lappend(subroot.parse->rtable,
905928
rte);
906929
}
@@ -2283,7 +2306,19 @@ select_rowmark_type(RangeTblEntry *rte, LockClauseStrength strength)
22832306
switch (strength)
22842307
{
22852308
caseLCS_NONE:
2286-
/* don't need tuple lock, only ability to re-fetch the row */
2309+
/*
2310+
* We don't need a tuple lock, only the ability to re-fetch
2311+
* the row. Regular tables support ROW_MARK_REFERENCE, but if
2312+
* this RTE has security barrier quals, it will be turned into
2313+
* a subquery during planning, so use ROW_MARK_COPY.
2314+
*
2315+
* This is only necessary for LCS_NONE, since real tuple locks
2316+
* on an RTE with security barrier quals are supported by
2317+
* pushing the lock down into the subquery --- see
2318+
* expand_security_qual.
2319+
*/
2320+
if (rte->securityQuals!=NIL)
2321+
returnROW_MARK_COPY;
22872322
returnROW_MARK_REFERENCE;
22882323
break;
22892324
caseLCS_FORKEYSHARE:

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 82 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,51 +1714,6 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
17141714
activeRIRs=list_delete_first(activeRIRs);
17151715
}
17161716
}
1717-
/*
1718-
* If the RTE has row security quals, apply them and recurse into the
1719-
* securityQuals.
1720-
*/
1721-
if (prepend_row_security_policies(parsetree,rte,rt_index))
1722-
{
1723-
/*
1724-
* We applied security quals, check for infinite recursion and
1725-
* then expand any nested queries.
1726-
*/
1727-
if (list_member_oid(activeRIRs,RelationGetRelid(rel)))
1728-
ereport(ERROR,
1729-
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
1730-
errmsg("infinite recursion detected in policy for relation \"%s\"",
1731-
RelationGetRelationName(rel))));
1732-
1733-
/*
1734-
* Make sure we check for recursion in either securityQuals or
1735-
* WITH CHECK quals.
1736-
*/
1737-
if (rte->securityQuals!=NIL)
1738-
{
1739-
activeRIRs=lcons_oid(RelationGetRelid(rel),activeRIRs);
1740-
1741-
expression_tree_walker( (Node*)rte->securityQuals,
1742-
fireRIRonSubLink, (void*)activeRIRs );
1743-
1744-
activeRIRs=list_delete_first(activeRIRs);
1745-
}
1746-
1747-
if (parsetree->withCheckOptions!=NIL)
1748-
{
1749-
WithCheckOption*wco;
1750-
List*quals=NIL;
1751-
1752-
wco= (WithCheckOption*)makeNode(WithCheckOption);
1753-
quals=lcons(wco->qual,quals);
1754-
1755-
activeRIRs=lcons_oid(RelationGetRelid(rel),activeRIRs);
1756-
1757-
expression_tree_walker( (Node*)quals,fireRIRonSubLink,
1758-
(void*)activeRIRs);
1759-
}
1760-
1761-
}
17621717

17631718
heap_close(rel,NoLock);
17641719
}
@@ -1780,6 +1735,88 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
17801735
query_tree_walker(parsetree,fireRIRonSubLink, (void*)activeRIRs,
17811736
QTW_IGNORE_RC_SUBQUERIES);
17821737

1738+
/*
1739+
* Apply any row level security policies. We do this last because it
1740+
* requires special recursion detection if the new quals have sublink
1741+
* subqueries, and if we did it in the loop above query_tree_walker
1742+
* would then recurse into those quals a second time.
1743+
*/
1744+
rt_index=0;
1745+
foreach(lc,parsetree->rtable)
1746+
{
1747+
RangeTblEntry*rte= (RangeTblEntry*)lfirst(lc);
1748+
Relationrel;
1749+
List*securityQuals;
1750+
List*withCheckOptions;
1751+
boolhasRowSecurity;
1752+
boolhasSubLinks;
1753+
1754+
++rt_index;
1755+
1756+
/* Only normal relations can have RLS policies */
1757+
if (rte->rtekind!=RTE_RELATION||
1758+
rte->relkind!=RELKIND_RELATION)
1759+
continue;
1760+
1761+
rel=heap_open(rte->relid,NoLock);
1762+
1763+
/*
1764+
* Fetch any new security quals that must be applied to this RTE.
1765+
*/
1766+
get_row_security_policies(parsetree,rte,rt_index,
1767+
&securityQuals,&withCheckOptions,
1768+
&hasRowSecurity,&hasSubLinks);
1769+
1770+
if (securityQuals!=NIL||withCheckOptions!=NIL)
1771+
{
1772+
if (hasSubLinks)
1773+
{
1774+
/*
1775+
* Recursively process the new quals, checking for infinite
1776+
* recursion.
1777+
*/
1778+
if (list_member_oid(activeRIRs,RelationGetRelid(rel)))
1779+
ereport(ERROR,
1780+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
1781+
errmsg("infinite recursion detected in policy for relation \"%s\"",
1782+
RelationGetRelationName(rel))));
1783+
1784+
activeRIRs=lcons_oid(RelationGetRelid(rel),activeRIRs);
1785+
1786+
expression_tree_walker( (Node*)securityQuals,
1787+
fireRIRonSubLink, (void*)activeRIRs );
1788+
1789+
expression_tree_walker( (Node*)withCheckOptions,
1790+
fireRIRonSubLink, (void*)activeRIRs );
1791+
1792+
activeRIRs=list_delete_first(activeRIRs);
1793+
}
1794+
1795+
/*
1796+
* Add the new security quals to the start of the RTE's list so
1797+
* that they get applied before any existing security quals (which
1798+
* might have come from a user-written security barrier view, and
1799+
* might contain malicious code).
1800+
*/
1801+
rte->securityQuals=list_concat(securityQuals,
1802+
rte->securityQuals);
1803+
1804+
parsetree->withCheckOptions=list_concat(withCheckOptions,
1805+
parsetree->withCheckOptions);
1806+
}
1807+
1808+
/*
1809+
* Make sure the query is marked correctly if row level security
1810+
* applies, or if the new quals had sublinks.
1811+
*/
1812+
if (hasRowSecurity)
1813+
parsetree->hasRowSecurity= true;
1814+
if (hasSubLinks)
1815+
parsetree->hasSubLinks= true;
1816+
1817+
heap_close(rel,NoLock);
1818+
}
1819+
17831820
returnparsetree;
17841821
}
17851822

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp