From e1b420bf11640cb84c45f5f345c06edf499863f1 Mon Sep 17 00:00:00 2001 From: Andreas Lind Date: Thu, 25 Sep 2025 21:42:09 +0200 Subject: [PATCH v2 2/5] Support BYPASSLEAKPROOF in rowsecurity rewriting --- src/backend/commands/policy.c | 3 + src/backend/rewrite/rewriteHandler.c | 42 ++++++++++-- src/backend/rewrite/rowsecurity.c | 95 ++++++++++++++++++++++------ src/backend/utils/cache/relcache.c | 4 ++ src/include/rewrite/rowsecurity.h | 2 + 5 files changed, 121 insertions(+), 25 deletions(-) diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index a5b2a9dfc87..ce74b13b5e4 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -253,6 +253,9 @@ RelationBuildRowSecurity(Relation relation) /* Get policy, permissive or restrictive */ policy->permissive = policy_form->polpermissive; + /* Get policy bypassleakproof flag */ + policy->bypassleakproof = policy_form->polbypassleakproof; + /* Get policy name */ policy->policy_name = MemoryContextStrdup(rscxt, NameStr(policy_form->polname)); diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index adc9e7600e1..7cf93ec08e5 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -2199,6 +2199,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs) Relation rel; List *securityQuals; List *withCheckOptions; + List *normalQuals; bool hasRowSecurity; bool hasSubLinks; @@ -2217,9 +2218,10 @@ fireRIRrules(Query *parsetree, List *activeRIRs) */ get_row_security_policies(parsetree, rte, rt_index, &securityQuals, &withCheckOptions, + &normalQuals, &hasRowSecurity, &hasSubLinks); - if (securityQuals != NIL || withCheckOptions != NIL) + if (securityQuals != NIL || withCheckOptions != NIL || normalQuals != NIL) { if (hasSubLinks) { @@ -2239,17 +2241,19 @@ fireRIRrules(Query *parsetree, List *activeRIRs) activeRIRs = lappend_oid(activeRIRs, RelationGetRelid(rel)); /* - * get_row_security_policies just passed back securityQuals - * and/or withCheckOptions, and there were SubLinks, make sure - * we lock any relations which are referenced. + * get_row_security_policies just passed back securityQuals, + * withCheckOptions, and/or normalQuals, and there were + * SubLinks, make sure we lock any relations which are + * referenced. * * These locks would normally be acquired by the parser, but - * securityQuals and withCheckOptions are added post-parsing. + * these quals are added post-parsing. */ context.for_execute = true; (void) acquireLocksOnSubLinks((Node *) securityQuals, &context); (void) acquireLocksOnSubLinks((Node *) withCheckOptions, &context); + (void) acquireLocksOnSubLinks((Node *) normalQuals, &context); /* * Now that we have the locks on anything added by @@ -2264,6 +2268,9 @@ fireRIRrules(Query *parsetree, List *activeRIRs) expression_tree_walker((Node *) withCheckOptions, fireRIRonSubLink, &fire_context); + expression_tree_walker((Node *) normalQuals, + fireRIRonSubLink, &fire_context); + /* * We can ignore the value of fire_context.hasRowSecurity * since we only reach this code in cases where hasRowSecurity @@ -2285,6 +2292,31 @@ fireRIRrules(Query *parsetree, List *activeRIRs) parsetree->withCheckOptions = list_concat(withCheckOptions, parsetree->withCheckOptions); + + /* + * Add normalQuals from BYPASSLEAKPROOF policies to the query's + * WHERE clause. These are treated as regular quals rather than + * security barriers. + */ + if (normalQuals != NIL) + { + Node *whereClause; + Node *newQual; + + /* Build a combined qual from all normalQuals */ + if (list_length(normalQuals) == 1) + newQual = (Node *) linitial(normalQuals); + else + newQual = (Node *) make_ands_explicit(normalQuals); + + /* Add to existing WHERE clause */ + whereClause = parsetree->jointree->quals; + if (whereClause) + parsetree->jointree->quals = (Node *) make_and_qual((Expr *) whereClause, + (Expr *) newQual); + else + parsetree->jointree->quals = newQual; + } } /* diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index 4dad384d04d..6788e0fb077 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -61,6 +61,7 @@ static void add_security_quals(int rt_index, List *permissive_policies, List *restrictive_policies, List **securityQuals, + List **normalQuals, bool *hasSubLinks); static void add_with_check_options(Relation rel, @@ -93,10 +94,14 @@ row_security_policy_hook_type row_security_policy_hook_restrictive = NULL; * 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. + * + * normalQuals receives quals from policies marked BYPASSLEAKPROOF, which + * should be treated as regular WHERE clauses rather than security barriers. */ void get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, List **securityQuals, List **withCheckOptions, + List **normalQuals, bool *hasRowSecurity, bool *hasSubLinks) { Oid user_id; @@ -110,6 +115,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, /* Defaults for the return values */ *securityQuals = NIL; *withCheckOptions = NIL; + *normalQuals = NIL; *hasRowSecurity = false; *hasSubLinks = false; @@ -205,6 +211,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, update_permissive_policies, update_restrictive_policies, securityQuals, + normalQuals, hasSubLinks); } @@ -225,6 +232,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, permissive_policies, restrictive_policies, securityQuals, + normalQuals, hasSubLinks); /* @@ -252,6 +260,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, select_permissive_policies, select_restrictive_policies, securityQuals, + normalQuals, hasSubLinks); } @@ -693,23 +702,30 @@ row_security_policy_cmp(const ListCell *a, const ListCell *b) * access to the table, then all access is prohibited --- i.e., an implicit * default-deny policy is used. * - * New security quals are added to securityQuals, and hasSubLinks is set to - * true if any of the quals added contain sublink subqueries. + * New security quals are added to securityQuals or normalQuals based on + * the bypassleakproof flag of policies. Quals from BYPASSLEAKPROOF policies + * go to normalQuals to be treated as regular WHERE clauses. + * hasSubLinks is set to true if any of the quals added contain sublink subqueries. */ static void add_security_quals(int rt_index, List *permissive_policies, List *restrictive_policies, List **securityQuals, + List **normalQuals, bool *hasSubLinks) { ListCell *item; List *permissive_quals = NIL; + List *bypass_permissive_quals = NIL; + bool all_permissive_bypass = true; + bool has_permissive_policies = false; Expr *rowsec_expr; /* - * First collect up the permissive quals. If we do not find any - * permissive policies then no rows are visible (this is handled below). + * First collect up the permissive quals, separating BYPASSLEAKPROOF + * policies from regular ones. If we do not find any permissive policies + * then no rows are visible (this is handled below). */ foreach(item, permissive_policies) { @@ -717,8 +733,18 @@ add_security_quals(int rt_index, if (policy->qual != NULL) { - permissive_quals = lappend(permissive_quals, - copyObject(policy->qual)); + has_permissive_policies = true; + if (policy->bypassleakproof) + { + bypass_permissive_quals = lappend(bypass_permissive_quals, + copyObject(policy->qual)); + } + else + { + permissive_quals = lappend(permissive_quals, + copyObject(policy->qual)); + all_permissive_bypass = false; + } *hasSubLinks |= policy->hassublinks; } } @@ -729,13 +755,11 @@ add_security_quals(int rt_index, * If we do not, then we simply return a single 'false' qual which results * in no rows being visible. */ - if (permissive_quals != NIL) + if (has_permissive_policies) { /* - * We now know that permissive policies exist, so we can now add - * security quals based on the USING clauses from the restrictive - * policies. Since these need to be combined together using AND, we - * can just add them one at a time. + * Handle restrictive policies - each goes to the appropriate list + * based on its bypassleakproof flag. */ foreach(item, restrictive_policies) { @@ -747,25 +771,55 @@ add_security_quals(int rt_index, qual = copyObject(policy->qual); ChangeVarNodes((Node *) qual, 1, rt_index, 0); - *securityQuals = list_append_unique(*securityQuals, qual); + if (policy->bypassleakproof) + *normalQuals = lappend(*normalQuals, qual); + else + + /* + * list_append_unique deduplicates based on pointer + * equality, which is harmless here + */ + *securityQuals = list_append_unique(*securityQuals, qual); *hasSubLinks |= policy->hassublinks; } } /* - * Then add a single security qual combining together the USING - * clauses from all the permissive policies using OR. + * Handle the combined permissive qual. If all permissive policies are + * BYPASSLEAKPROOF, add the combined qual to normalQuals. Otherwise, + * combine all permissive quals (both bypass and regular) and add to + * securityQuals. */ - if (list_length(permissive_quals) == 1) - rowsec_expr = (Expr *) linitial(permissive_quals); + if (all_permissive_bypass && bypass_permissive_quals != NIL) + { + /* All permissive policies bypass - add to normal quals */ + if (list_length(bypass_permissive_quals) == 1) + rowsec_expr = (Expr *) linitial(bypass_permissive_quals); + else + rowsec_expr = makeBoolExpr(OR_EXPR, bypass_permissive_quals, -1); + + ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0); + *normalQuals = lappend(*normalQuals, rowsec_expr); + } else - rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals, -1); + { + /* + * Some non-bypass policies exist - combine all and add to + * security quals + */ + List *all_permissive_quals = list_concat(permissive_quals, bypass_permissive_quals); - ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0); - *securityQuals = list_append_unique(*securityQuals, rowsec_expr); + if (list_length(all_permissive_quals) == 1) + rowsec_expr = (Expr *) linitial(all_permissive_quals); + else + rowsec_expr = makeBoolExpr(OR_EXPR, all_permissive_quals, -1); + + ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0); + *securityQuals = list_append_unique(*securityQuals, rowsec_expr); + } } else - + { /* * A permissive policy must exist for rows to be visible at all. * Therefore, if there were no permissive policies found, return a @@ -775,6 +829,7 @@ add_security_quals(int rt_index, makeConst(BOOLOID, -1, InvalidOid, sizeof(bool), BoolGetDatum(false), false, true)); + } } /* diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 2b798b823ea..3d469de7a43 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -982,6 +982,10 @@ equalPolicy(RowSecurityPolicy *policy1, RowSecurityPolicy *policy2) if (policy1->polcmd != policy2->polcmd) return false; + if (policy1->permissive != policy2->permissive) + return false; + if (policy1->bypassleakproof != policy2->bypassleakproof) + return false; if (policy1->hassublinks != policy2->hassublinks) return false; if (strcmp(policy1->policy_name, policy2->policy_name) != 0) diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h index 8f81c40a289..8b12228cf85 100644 --- a/src/include/rewrite/rowsecurity.h +++ b/src/include/rewrite/rowsecurity.h @@ -23,6 +23,7 @@ typedef struct RowSecurityPolicy char polcmd; /* Type of command policy is for */ ArrayType *roles; /* Array of roles policy is for */ bool permissive; /* restrictive or permissive policy */ + bool bypassleakproof; Expr *qual; /* Expression to filter rows */ Expr *with_check_qual; /* Expression to limit rows allowed */ bool hassublinks; /* If either expression has sublinks */ @@ -44,6 +45,7 @@ extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restri extern void get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index, List **securityQuals, List **withCheckOptions, + List **normalQuals, bool *hasRowSecurity, bool *hasSubLinks); #endif /* ROWSECURITY_H */ -- 2.50.1 (Apple Git-155)