On 14 January 2015 at 08:43, Dean Rasheed <dean.a.rash...@gmail.com> wrote: > On 12 January 2015 at 14:24, Stephen Frost <sfr...@snowman.net> wrote: >> Interesting, thanks for the work! I had been suspicious that there was >> an issue with the recursion handling. >> > > So continuing to review the RLS code, I spotted the following in > prepend_row_security_policies(): > > /* > * We may end up getting called multiple times for the same RTE, so check > * to make sure we aren't doing double-work. > */ > if (rte->securityQuals != NIL) > return false; > > which looked suspicious. I assume that's just a hang-up from an > earlier attempt to prevent infinite recursion in RLS expansion, but > actually it's wrong because in the case of an UPDATE to a security > barrier view on top of a table with RLS enabled, the view's > securityQuals will get added to the RTE first, and that shouldn't > prevent the underlying table's RLS securityQuals from being added. > > AFAICT, it should be safe to just remove the above code. I can't see > how prepend_row_security_policies() could end up getting called more > than once for the same RTE. >
Turns out it wasn't as simple as that. prepend_row_security_policies() really could get called multiple times for the same RTE, because the call to query_tree_walker() at the end of fireRIRrules() would descend into the just-added quals again. The simplest fix seems to be to process RLS in a separate loop at the end, so that it can have it's own infinite recursion detection, which is different from that needed for pre-existing security quals and with check options from security barrier views. This approach simplifies things a bit, and ensures that we only try to expand RLS once for each RTE. > Also, I'm thinking that it would be better to refactor things a bit > and have prepend_row_security_policies() just return the new > securityQuals and withCheckOptions to add. Then fireRIRrules() would > only have to recurse into the new quals being added, not the > already-processed quals. > Turns out that refactoring actually became necessary in order to fix this bug, but I think it makes things cleaner and more efficient. Here's an updated patch with a new test for this bug. I've been developing the fixes for these RLS issues as one big patch, but I suppose it would be easy to split up, if that's preferred. Regards, Dean
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c new file mode 100644 index 9cbbcfb..34ddf41 *** a/src/backend/optimizer/plan/planner.c --- b/src/backend/optimizer/plan/planner.c *************** inheritance_planner(PlannerInfo *root) *** 790,795 **** --- 790,796 ---- { Query *parse = root->parse; int parentRTindex = parse->resultRelation; + Bitmapset *resultRTindexes = NULL; List *final_rtable = NIL; int save_rel_array_size = 0; RelOptInfo **save_rel_array = NULL; *************** inheritance_planner(PlannerInfo *root) *** 814,820 **** --- 815,835 ---- * (1) would result in a rangetable of length O(N^2) for N targets, with * at least O(N^3) work expended here; and (2) would greatly complicate * management of the rowMarks list. + * + * Note that any RTEs with security barrier quals will be turned into + * subqueries during planning, and so we must create copies of them too, + * except where they are target relations, which will each only be used + * in a single plan. */ + resultRTindexes = bms_add_member(resultRTindexes, parentRTindex); + foreach(lc, root->append_rel_list) + { + AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc); + if (appinfo->parent_relid == parentRTindex) + resultRTindexes = bms_add_member(resultRTindexes, + appinfo->child_relid); + } + foreach(lc, root->append_rel_list) { AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc); *************** inheritance_planner(PlannerInfo *root) *** 885,905 **** { RangeTblEntry *rte = (RangeTblEntry *) lfirst(lr); ! if (rte->rtekind == RTE_SUBQUERY) { Index newrti; /* * The RTE can't contain any references to its own RT ! * index, so we can save a few cycles by applying ! * ChangeVarNodes before we append the RTE to the ! * rangetable. */ newrti = list_length(subroot.parse->rtable) + 1; ChangeVarNodes((Node *) subroot.parse, rti, newrti, 0); ChangeVarNodes((Node *) subroot.rowMarks, rti, newrti, 0); ChangeVarNodes((Node *) subroot.append_rel_list, rti, newrti, 0); rte = copyObject(rte); subroot.parse->rtable = lappend(subroot.parse->rtable, rte); } --- 900,928 ---- { RangeTblEntry *rte = (RangeTblEntry *) lfirst(lr); ! /* ! * Copy subquery RTEs and RTEs with security barrier quals ! * that will be turned into subqueries, except those that are ! * target relations. ! */ ! if (rte->rtekind == RTE_SUBQUERY || ! (rte->securityQuals != NIL && ! !bms_is_member(rti, resultRTindexes))) { Index newrti; /* * The RTE can't contain any references to its own RT ! * index, except in the security barrier quals, so we can ! * save a few cycles by applying ChangeVarNodes before we ! * append the RTE to the rangetable. */ newrti = list_length(subroot.parse->rtable) + 1; ChangeVarNodes((Node *) subroot.parse, rti, newrti, 0); ChangeVarNodes((Node *) subroot.rowMarks, rti, newrti, 0); ChangeVarNodes((Node *) subroot.append_rel_list, rti, newrti, 0); rte = copyObject(rte); + ChangeVarNodes((Node *) rte->securityQuals, rti, newrti, 0); subroot.parse->rtable = lappend(subroot.parse->rtable, rte); } *************** preprocess_rowmarks(PlannerInfo *root) *** 2254,2261 **** newrc = makeNode(PlanRowMark); newrc->rti = newrc->prti = i; newrc->rowmarkId = ++(root->glob->lastRowMarkId); ! /* real tables support REFERENCE, anything else needs COPY */ if (rte->rtekind == RTE_RELATION && rte->relkind != RELKIND_FOREIGN_TABLE) newrc->markType = ROW_MARK_REFERENCE; else --- 2277,2289 ---- newrc = makeNode(PlanRowMark); newrc->rti = newrc->prti = i; newrc->rowmarkId = ++(root->glob->lastRowMarkId); ! /* ! * Real tables support REFERENCE, anything else needs COPY. Since ! * RTEs with security barrier quals will become subqueries, they also ! * need COPY. ! */ if (rte->rtekind == RTE_RELATION && + rte->securityQuals == NIL && rte->relkind != RELKIND_FOREIGN_TABLE) newrc->markType = ROW_MARK_REFERENCE; else diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c new file mode 100644 index b8e6e7a..149191e *** a/src/backend/rewrite/rewriteHandler.c --- b/src/backend/rewrite/rewriteHandler.c *************** fireRIRrules(Query *parsetree, List *act *** 1714,1764 **** 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); } --- 1714,1719 ---- *************** fireRIRrules(Query *parsetree, List *act *** 1780,1785 **** --- 1735,1822 ---- 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; } diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c new file mode 100644 index 35790a9..bb47429 *** a/src/backend/rewrite/rowsecurity.c --- b/src/backend/rewrite/rowsecurity.c *************** *** 57,63 **** static List *pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id); ! static void process_policies(List *policies, int rt_index, Expr **final_qual, Expr **final_with_check_qual, bool *hassublinks); --- 57,63 ---- static List *pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id); ! static void process_policies(Query* root, List *policies, int rt_index, Expr **final_qual, Expr **final_with_check_qual, bool *hassublinks); *************** static bool check_role_for_policy(ArrayT *** 72,87 **** row_security_policy_hook_type row_security_policy_hook = NULL; /* ! * Check the given RTE to see whether it's already had row security quals ! * expanded and, if not, prepend any row security rules from built-in or ! * plug-in sources to the securityQuals. The security quals are rewritten (for ! * view expansion, etc) before being added to the RTE. * ! * Returns true if any quals were added. Note that quals may have been found ! * but not added if user rights make the user exempt from row security. */ ! bool ! prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) { Expr *rowsec_expr = NULL; Expr *rowsec_with_check_expr = NULL; --- 72,88 ---- row_security_policy_hook_type row_security_policy_hook = NULL; /* ! * Get any row security quals and check quals that should be applied to the ! * specified RTE. * ! * In addition hasRowSecurity is set to true if row level security is enabled ! * (even if this RTE doesn't have any row security quals), and hasSubLinks is ! * set to true if any of the quals returned contain sublinks. */ ! void ! get_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index, ! List **securityQuals, List **withCheckOptions, ! bool *hasRowSecurity, bool *hasSubLinks) { Expr *rowsec_expr = NULL; Expr *rowsec_with_check_expr = NULL; *************** prepend_row_security_policies(Query* roo *** 95,102 **** Oid user_id; int sec_context; int rls_status; ! bool defaultDeny = true; ! bool hassublinks = false; /* This is just to get the security context */ GetUserIdAndSecContext(&user_id, &sec_context); --- 96,108 ---- Oid user_id; int sec_context; int rls_status; ! bool defaultDeny = false; ! ! /* Defaults for the return values */ ! *securityQuals = NIL; ! *withCheckOptions = NIL; ! *hasRowSecurity = false; ! *hasSubLinks = false; /* This is just to get the security context */ GetUserIdAndSecContext(&user_id, &sec_context); *************** prepend_row_security_policies(Query* roo *** 112,125 **** if (rte->relid < FirstNormalObjectId || rte->relkind != RELKIND_RELATION || (sec_context & SECURITY_ROW_LEVEL_DISABLED)) ! return false; /* Determine the state of RLS for this, pass checkAsUser explicitly */ rls_status = check_enable_rls(rte->relid, rte->checkAsUser); /* If there is no RLS on this table at all, nothing to do */ if (rls_status == RLS_NONE) ! return false; /* * RLS_NONE_ENV means we are not doing any RLS now, but that may change --- 118,131 ---- if (rte->relid < FirstNormalObjectId || rte->relkind != RELKIND_RELATION || (sec_context & SECURITY_ROW_LEVEL_DISABLED)) ! return; /* Determine the state of RLS for this, pass checkAsUser explicitly */ rls_status = check_enable_rls(rte->relid, rte->checkAsUser); /* If there is no RLS on this table at all, nothing to do */ if (rls_status == RLS_NONE) ! return; /* * RLS_NONE_ENV means we are not doing any RLS now, but that may change *************** prepend_row_security_policies(Query* roo *** 133,150 **** * be replanned if the environment changes (GUCs, role), but we * are not adding anything here. */ ! root->hasRowSecurity = true; ! return false; } - /* - * We may end up getting called multiple times for the same RTE, so check - * to make sure we aren't doing double-work. - */ - if (rte->securityQuals != NIL) - return false; - /* Grab the built-in policies which should be applied to this relation. */ rel = heap_open(rte->relid, NoLock); --- 139,149 ---- * be replanned if the environment changes (GUCs, role), but we * are not adding anything here. */ ! *hasRowSecurity = true; ! return; } /* Grab the built-in policies which should be applied to this relation. */ rel = heap_open(rte->relid, NoLock); *************** prepend_row_security_policies(Query* roo *** 166,173 **** defaultDeny = true; /* Now that we have our policies, build the expressions from them. */ ! process_policies(rowsec_policies, rt_index, &rowsec_expr, ! &rowsec_with_check_expr, &hassublinks); /* * Also, allow extensions to add their own policies. --- 165,172 ---- defaultDeny = true; /* Now that we have our policies, build the expressions from them. */ ! process_policies(root, rowsec_policies, rt_index, &rowsec_expr, ! &rowsec_with_check_expr, hasSubLinks); /* * Also, allow extensions to add their own policies. *************** prepend_row_security_policies(Query* roo *** 190,203 **** * enabled on the table, then we will ignore the internally-generated * default-deny policy and use only the policies returned by the * extension. */ if (row_security_policy_hook) { hook_policies = (*row_security_policy_hook)(root->commandType, rel); /* Build the expression from any policies returned. */ ! process_policies(hook_policies, rt_index, &hook_expr, ! &hook_with_check_expr, &hassublinks); } /* --- 189,205 ---- * enabled on the table, then we will ignore the internally-generated * default-deny policy and use only the policies returned by the * extension. + * + * Any extension policies are applied after any internally-generated + * policies, so that extensions cannot bypass built-in RLS. */ if (row_security_policy_hook) { hook_policies = (*row_security_policy_hook)(root->commandType, rel); /* Build the expression from any policies returned. */ ! process_policies(root, hook_policies, rt_index, &hook_expr, ! &hook_with_check_expr, hasSubLinks); } /* *************** prepend_row_security_policies(Query* roo *** 229,235 **** wco->viewname = RelationGetRelationName(rel); wco->qual = (Node *) rowsec_with_check_expr; wco->cascaded = false; ! root->withCheckOptions = lcons(wco, root->withCheckOptions); } /* --- 231,237 ---- wco->viewname = RelationGetRelationName(rel); wco->qual = (Node *) rowsec_with_check_expr; wco->cascaded = false; ! *withCheckOptions = lappend(*withCheckOptions, wco); } /* *************** prepend_row_security_policies(Query* roo *** 243,249 **** wco->viewname = RelationGetRelationName(rel); wco->qual = (Node *) hook_with_check_expr; wco->cascaded = false; ! root->withCheckOptions = lcons(wco, root->withCheckOptions); } } --- 245,251 ---- wco->viewname = RelationGetRelationName(rel); wco->qual = (Node *) hook_with_check_expr; wco->cascaded = false; ! *withCheckOptions = lappend(*withCheckOptions, wco); } } *************** prepend_row_security_policies(Query* roo *** 253,283 **** || root->commandType == CMD_DELETE) { if (rowsec_expr) ! rte->securityQuals = lcons(rowsec_expr, rte->securityQuals); if (hook_expr) ! rte->securityQuals = lcons(hook_expr, ! rte->securityQuals); } heap_close(rel, NoLock); /* ! * Mark this query as having row security, so plancache can invalidate ! * it when necessary (eg: role changes) ! */ ! root->hasRowSecurity = true; ! ! /* ! * If we have sublinks added because of the policies being added to the ! * query, then set hasSubLinks on the Query to force subLinks to be ! * properly expanded. */ ! if (hassublinks) ! root->hasSubLinks = hassublinks; ! ! /* If we got this far, we must have added quals */ ! return true; } /* --- 255,273 ---- || root->commandType == CMD_DELETE) { if (rowsec_expr) ! *securityQuals = lappend(*securityQuals, rowsec_expr); if (hook_expr) ! *securityQuals = lappend(*securityQuals, hook_expr); } heap_close(rel, NoLock); /* ! * The query has row security, so plancache must invalidate it when ! * necessary (eg: role changes) */ ! *hasRowSecurity = true; } /* *************** pull_row_security_policies(CmdType cmd, *** 383,389 **** * qual_eval, with_check_eval, and hassublinks are output variables */ static void ! process_policies(List *policies, int rt_index, Expr **qual_eval, Expr **with_check_eval, bool *hassublinks) { ListCell *item; --- 373,379 ---- * qual_eval, with_check_eval, and hassublinks are output variables */ static void ! process_policies(Query* root, List *policies, int rt_index, Expr **qual_eval, Expr **with_check_eval, bool *hassublinks) { ListCell *item; *************** process_policies(List *policies, int rt_ *** 392,398 **** /* * Extract the USING and WITH CHECK quals from each of the policies ! * and add them to our lists. */ foreach(item, policies) { --- 382,389 ---- /* * Extract the USING and WITH CHECK quals from each of the policies ! * and add them to our lists. We only want WITH CHECK quals if this ! * RTE is the query's result relation. */ foreach(item, policies) { *************** process_policies(List *policies, int rt_ *** 401,407 **** if (policy->qual != NULL) quals = lcons(copyObject(policy->qual), quals); ! if (policy->with_check_qual != NULL) with_check_quals = lcons(copyObject(policy->with_check_qual), with_check_quals); --- 392,399 ---- if (policy->qual != NULL) quals = lcons(copyObject(policy->qual), quals); ! if (policy->with_check_qual != NULL && ! rt_index == root->resultRelation) with_check_quals = lcons(copyObject(policy->with_check_qual), with_check_quals); *************** process_policies(List *policies, int rt_ *** 421,427 **** * If we end up with only USING quals, then use those as * WITH CHECK quals also. */ ! if (with_check_quals == NIL) with_check_quals = copyObject(quals); /* --- 413,419 ---- * If we end up with only USING quals, then use those as * WITH CHECK quals also. */ ! if (with_check_quals == NIL && rt_index == root->resultRelation) with_check_quals = copyObject(quals); /* *************** process_policies(List *policies, int rt_ *** 450,457 **** */ if (list_length(with_check_quals) > 1) *with_check_eval = makeBoolExpr(OR_EXPR, with_check_quals, -1); ! else *with_check_eval = (Expr*) linitial(with_check_quals); return; } --- 442,451 ---- */ if (list_length(with_check_quals) > 1) *with_check_eval = makeBoolExpr(OR_EXPR, with_check_quals, -1); ! else if (with_check_quals != NIL) *with_check_eval = (Expr*) linitial(with_check_quals); + else + *with_check_eval = NULL; return; } diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h new file mode 100644 index aa1b45b..43f528d *** a/src/include/rewrite/rowsecurity.h --- b/src/include/rewrite/rowsecurity.h *************** typedef List *(*row_security_policy_hook *** 73,80 **** extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook; ! extern bool prepend_row_security_policies(Query* root, RangeTblEntry* rte, ! int rt_index); extern int check_enable_rls(Oid relid, Oid checkAsUser); --- 73,81 ---- extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook; ! extern void get_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index, ! List **securityQuals, List **withCheckOptions, ! bool *hasRowSecurity, bool *hasSubLinks); extern int check_enable_rls(Oid relid, Oid checkAsUser); diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out new file mode 100644 index 430da55..81f4095 *** a/src/test/regress/expected/rowsecurity.out --- b/src/test/regress/expected/rowsecurity.out *************** NOTICE: f_leak => yyyyyy *** 1111,1116 **** --- 1111,1185 ---- 302 | 2 | yyyyyy | (2,yyyyyy) (5 rows) + -- update with from clause self join + EXPLAIN (COSTS OFF) UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2 + WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b + AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2; + QUERY PLAN + --------------------------------------------------------------- + Update on t1 t1_1_3 + -> Nested Loop + Join Filter: (t1_1.b = t1_2.b) + -> Subquery Scan on t1_1 + Filter: f_leak(t1_1.b) + -> Seq Scan on t1 t1_1_4 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Subquery Scan on t1_2 + Filter: f_leak(t1_2.b) + -> Append + -> Seq Scan on t1 t1_2_3 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Seq Scan on t2 t1_2_4 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Seq Scan on t3 t1_2_5 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Nested Loop + Join Filter: (t1_1_1.b = t1_2_1.b) + -> Subquery Scan on t1_1_1 + Filter: f_leak(t1_1_1.b) + -> Seq Scan on t2 t1_1_5 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Subquery Scan on t1_2_1 + Filter: f_leak(t1_2_1.b) + -> Append + -> Seq Scan on t1 t1_2_6 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Seq Scan on t2 t1_2_7 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Seq Scan on t3 t1_2_8 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Nested Loop + Join Filter: (t1_1_2.b = t1_2_2.b) + -> Subquery Scan on t1_1_2 + Filter: f_leak(t1_1_2.b) + -> Seq Scan on t3 t1_1_6 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Subquery Scan on t1_2_2 + Filter: f_leak(t1_2_2.b) + -> Append + -> Seq Scan on t1 t1_2_9 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Seq Scan on t2 t1_2_10 + Filter: ((a = 4) AND ((a % 2) = 0)) + -> Seq Scan on t3 t1_2_11 + Filter: ((a = 4) AND ((a % 2) = 0)) + (46 rows) + + UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2 + WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b + AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2; + NOTICE: f_leak => dddddd_updt + NOTICE: f_leak => dddddd_updt + NOTICE: f_leak => defdef + NOTICE: f_leak => defdef + NOTICE: f_leak => dddddd_updt + NOTICE: f_leak => defdef + a | b | a | b | t1_1 | t1_2 + ---+-------------+---+-------------+-----------------+----------------- + 4 | dddddd_updt | 4 | dddddd_updt | (4,dddddd_updt) | (4,dddddd_updt) + 4 | defdef | 4 | defdef | (4,defdef) | (4,defdef) + (2 rows) + RESET SESSION AUTHORIZATION; SET row_security TO OFF; SELECT * FROM t1; *************** NOTICE: f_leak => yyyyyy *** 1180,1185 **** --- 1249,1349 ---- (3 rows) -- + -- S.b. view on top of Row-level security + -- + SET SESSION AUTHORIZATION rls_regress_user0; + CREATE TABLE b1 (a int, b text); + INSERT INTO b1 (SELECT x, md5(x::text) FROM generate_series(-10,10) x); + CREATE POLICY p1 ON b1 USING (a % 2 = 0); + ALTER TABLE b1 ENABLE ROW LEVEL SECURITY; + GRANT ALL ON b1 TO rls_regress_user1; + SET SESSION AUTHORIZATION rls_regress_user1; + CREATE VIEW bv1 WITH (security_barrier) AS SELECT * FROM b1 WHERE a > 0 WITH CHECK OPTION; + GRANT ALL ON bv1 TO rls_regress_user2; + SET SESSION AUTHORIZATION rls_regress_user2; + EXPLAIN (COSTS OFF) SELECT * FROM bv1 WHERE f_leak(b); + QUERY PLAN + --------------------------------------------- + Subquery Scan on bv1 + Filter: f_leak(bv1.b) + -> Seq Scan on b1 + Filter: ((a > 0) AND ((a % 2) = 0)) + (4 rows) + + SELECT * FROM bv1 WHERE f_leak(b); + NOTICE: f_leak => c81e728d9d4c2f636f067f89cc14862c + NOTICE: f_leak => a87ff679a2f3e71d9181a67b7542122c + NOTICE: f_leak => 1679091c5a880faf6fb5e6087eb1b2dc + NOTICE: f_leak => c9f0f895fb98ab9159f51fd0297e236d + NOTICE: f_leak => d3d9446802a44259755d38e6d163e820 + a | b + ----+---------------------------------- + 2 | c81e728d9d4c2f636f067f89cc14862c + 4 | a87ff679a2f3e71d9181a67b7542122c + 6 | 1679091c5a880faf6fb5e6087eb1b2dc + 8 | c9f0f895fb98ab9159f51fd0297e236d + 10 | d3d9446802a44259755d38e6d163e820 + (5 rows) + + INSERT INTO bv1 VALUES (-1, 'xxx'); -- should fail view WCO + ERROR: new row violates WITH CHECK OPTION for "b1" + DETAIL: Failing row contains (-1, xxx). + INSERT INTO bv1 VALUES (11, 'xxx'); -- should fail RLS check + ERROR: new row violates WITH CHECK OPTION for "b1" + DETAIL: Failing row contains (11, xxx). + INSERT INTO bv1 VALUES (12, 'xxx'); -- ok + EXPLAIN (COSTS OFF) UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b); + QUERY PLAN + --------------------------------------------------------------- + Update on b1 b1_1 + -> Subquery Scan on b1 + Filter: f_leak(b1.b) + -> Seq Scan on b1 b1_2 + Filter: ((a > 0) AND (a = 4) AND ((a % 2) = 0)) + (5 rows) + + UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b); + NOTICE: f_leak => a87ff679a2f3e71d9181a67b7542122c + EXPLAIN (COSTS OFF) DELETE FROM bv1 WHERE a = 6 AND f_leak(b); + QUERY PLAN + --------------------------------------------------------------- + Delete on b1 b1_1 + -> Subquery Scan on b1 + Filter: f_leak(b1.b) + -> Seq Scan on b1 b1_2 + Filter: ((a > 0) AND (a = 6) AND ((a % 2) = 0)) + (5 rows) + + DELETE FROM bv1 WHERE a = 6 AND f_leak(b); + NOTICE: f_leak => 1679091c5a880faf6fb5e6087eb1b2dc + SET SESSION AUTHORIZATION rls_regress_user0; + SELECT * FROM b1; + a | b + -----+---------------------------------- + -10 | 1b0fd9efa5279c4203b7c70233f86dbf + -9 | 252e691406782824eec43d7eadc3d256 + -8 | a8d2ec85eaf98407310b72eb73dda247 + -7 | 74687a12d3915d3c4d83f1af7b3683d5 + -6 | 596a3d04481816330f07e4f97510c28f + -5 | 47c1b025fa18ea96c33fbb6718688c0f + -4 | 0267aaf632e87a63288a08331f22c7c3 + -3 | b3149ecea4628efd23d2f86e5a723472 + -2 | 5d7b9adcbe1c629ec722529dd12e5129 + -1 | 6bb61e3b7bce0931da574d19d1d82c88 + 0 | cfcd208495d565ef66e7dff9f98764da + 1 | c4ca4238a0b923820dcc509a6f75849b + 2 | c81e728d9d4c2f636f067f89cc14862c + 3 | eccbc87e4b5ce2fe28308fd9f2a7baf3 + 5 | e4da3b7fbbce2345d7772b0674a318d5 + 7 | 8f14e45fceea167a5a36dedd4bea2543 + 8 | c9f0f895fb98ab9159f51fd0297e236d + 9 | 45c48cce2e2d7fbdea1afc51c7c6ad26 + 10 | d3d9446802a44259755d38e6d163e820 + 12 | xxx + 4 | yyy + (21 rows) + + -- -- ROLE/GROUP -- SET SESSION AUTHORIZATION rls_regress_user0; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql new file mode 100644 index ee28a2c..49b3201 *** a/src/test/regress/sql/rowsecurity.sql --- b/src/test/regress/sql/rowsecurity.sql *************** UPDATE only t1 SET b = b WHERE f_leak(b) *** 423,428 **** --- 423,437 ---- UPDATE t1 SET b = b WHERE f_leak(b) RETURNING *; UPDATE t1 SET b = b WHERE f_leak(b) RETURNING oid, *, t1; + -- update with from clause self join + EXPLAIN (COSTS OFF) UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2 + WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b + AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2; + + UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2 + WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b + AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2; + RESET SESSION AUTHORIZATION; SET row_security TO OFF; SELECT * FROM t1; *************** DELETE FROM only t1 WHERE f_leak(b) RETU *** 436,441 **** --- 445,483 ---- DELETE FROM t1 WHERE f_leak(b) RETURNING oid, *, t1; -- + -- S.b. view on top of Row-level security + -- + SET SESSION AUTHORIZATION rls_regress_user0; + CREATE TABLE b1 (a int, b text); + INSERT INTO b1 (SELECT x, md5(x::text) FROM generate_series(-10,10) x); + + CREATE POLICY p1 ON b1 USING (a % 2 = 0); + ALTER TABLE b1 ENABLE ROW LEVEL SECURITY; + GRANT ALL ON b1 TO rls_regress_user1; + + SET SESSION AUTHORIZATION rls_regress_user1; + CREATE VIEW bv1 WITH (security_barrier) AS SELECT * FROM b1 WHERE a > 0 WITH CHECK OPTION; + GRANT ALL ON bv1 TO rls_regress_user2; + + SET SESSION AUTHORIZATION rls_regress_user2; + + EXPLAIN (COSTS OFF) SELECT * FROM bv1 WHERE f_leak(b); + SELECT * FROM bv1 WHERE f_leak(b); + + INSERT INTO bv1 VALUES (-1, 'xxx'); -- should fail view WCO + INSERT INTO bv1 VALUES (11, 'xxx'); -- should fail RLS check + INSERT INTO bv1 VALUES (12, 'xxx'); -- ok + + EXPLAIN (COSTS OFF) UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b); + UPDATE bv1 SET b = 'yyy' WHERE a = 4 AND f_leak(b); + + EXPLAIN (COSTS OFF) DELETE FROM bv1 WHERE a = 6 AND f_leak(b); + DELETE FROM bv1 WHERE a = 6 AND f_leak(b); + + SET SESSION AUTHORIZATION rls_regress_user0; + SELECT * FROM b1; + + -- -- ROLE/GROUP -- SET SESSION AUTHORIZATION rls_regress_user0;
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers