On 28 May 2015 at 08:51, Stephen Frost <sfr...@snowman.net> wrote:
> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
>> Actually I think a new boolean field is unnecessary, and probably the
>> policy_id field is too. Re-reading the code in rowsecurity.c, I think
>> it could use a bit of refactoring. Essentially what it needs to do
>> (for permissive policies at least) is just
>>
>> * fetch a list of internal policies
>> * fetch a list of external policies
>> * concatenate them
>> * if the resulting list is empty, add a default-deny qual and/or WCO
>>
>> By only building the default-deny qual/WCO at the end, rather than
>> half-way through and pretending that it's a fully-fledged policy, the
>> code ought to be simpler.
>
> I tend to agree.
>
>> I might get some time at the weekend, so I can take a look and see if
>> refactoring it this way works out in practice.
>
> That would certainly be fantastic and most appreciated.  Beyond that, we
> have the add-a-default-deny-policy logic in multiple places, as I
> recall, and I wonder if that's overkill and done out of paranoia rather
> than sound judgement.  I'm certainly not suggesting that we take
> unnecessary risks, and so perhaps we should keep it, but I do think it's
> something which should be reviewed and considered (I've been planning to
> do so for a while, in fact).
>

So I had a go at refactoring the code in rowsecurity.c to simplify the
default-deny policy handling, as described. In the end my changes were
quite extensive, so I'll understand if you don't have time to review
it, but I think that it's worth it for the improved code clarity,
simplicity and more detailed error messages for restrictive policies.
In any case, there are a couple of bug fixes in there that ought to be
considered.

The main changes are:

* pull_row_security_policies() is replaced by
get_policies_for_relation(), whose remit is to fetch both the internal
and external policies that apply to the relation. It returns the
permissive and restrictive policies as separate lists, each of which
contains any internal policies followed by any external policies
(except of course that internal restrictive policies aren't possible
yet). Unlike pull_row_security_policies() this does not try to build a
default-deny policy, and instead may return empty lists. All the
returned policies are filtered according to the current role, thus
fixing the bug that external policies weren't being filtered.

* process_policies() is replaced by build_security_quals() and
build_with_check_options(), whose remits are to build the lists of
security quals and WithCheckOption checks from the lists of permissive
and restrictive policies. These new functions now have sole
responsibility for handling the default-deny case if there are no
explicit policies to apply, which means that it is no longer necessary
to build RowSecurityPolicy objects representing the default-deny case
(hence no more InvalidOid policy_id).

* get_row_security_policies() is now greatly simplified, since it no
longer has to merge internal and external policies, or worry about
default-deny policies. The guts of the work is now done by the new
functions described above.

* The way that ON CONFLICT DO UPDATE is handled is also simplified ---
rather than recursively calling get_row_security_policies() and
turning the returned list of security quals into WCOs, it now simply
calls build_with_check_options() a couple more times to build the
additional kinds of WCOs needed for the auxiliary UPDATE.

* RelationBuildRowSecurity() no longer builds a default-deny policy,
and the resulting policy list is now allowed to be empty. This removes
the need for RowSecurityPolicy's policy_id field. Actually the old
code had 3 separate places with default-deny policy handling. That is
now all localised in the functions that build security quals and WCOs
from the policy lists.

* Finally, WCOs for restrictive policies now report the name of the
policy violated. Of course this means that the actual error might
depend on the order in which the policies are checked. I've tackled
that in the same way as was recently used for CHECK constraints, which
is to always check restrictive policies in a well-defined order (name
order) so that self-test output is predictable.

Overall, I think this reduces the code complexity (although I think
the total line count is about the same), and there is now a clearer
separation of concerns across the various functions. Also I think it
will make it easier to add support for internal restrictive policies
in the future.

While going through this, I spotted another issue --- in a DML query
with additional non-target relations, such as UPDATE t1 .. FROM t2 ..,
the old code was checking the UPDATE policies of both t1 and t2, but
really I think it ought to be checking the SELECT policies of t2 (in
the same way as this query requires SELECT table permissions on t2,
not UPDATE permissions). I've changed that and added new regression
tests to test that change.

Regards,
Dean
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
new file mode 100644
index 6e95ba2..3b3cf3d
*** a/src/backend/commands/policy.c
--- b/src/backend/commands/policy.c
*************** policy_role_list_to_array(List *roles)
*** 186,194 ****
  /*
   * Load row security policy from the catalog, and store it in
   * the relation's relcache entry.
-  *
-  * We will always set up some kind of policy here.  If no explicit policies
-  * are found then an implicit default-deny policy is created.
   */
  void
  RelationBuildRowSecurity(Relation relation)
--- 186,191 ----
*************** RelationBuildRowSecurity(Relation relati
*** 246,252 ****
  			char	   *with_check_value;
  			Expr	   *with_check_qual;
  			char	   *policy_name_value;
- 			Oid			policy_id;
  			bool		isnull;
  			RowSecurityPolicy *policy;
  
--- 243,248 ----
*************** RelationBuildRowSecurity(Relation relati
*** 298,311 ****
  			else
  				with_check_qual = NULL;
  
- 			policy_id = HeapTupleGetOid(tuple);
- 
  			/* Now copy everything into the cache context */
  			MemoryContextSwitchTo(rscxt);
  
  			policy = palloc0(sizeof(RowSecurityPolicy));
  			policy->policy_name = pstrdup(policy_name_value);
- 			policy->policy_id = policy_id;
  			policy->polcmd = cmd_value;
  			policy->roles = DatumGetArrayTypePCopy(roles_datum);
  			policy->qual = copyObject(qual_expr);
--- 294,304 ----
*************** RelationBuildRowSecurity(Relation relati
*** 326,365 ****
  
  		systable_endscan(sscan);
  		heap_close(catalog, AccessShareLock);
- 
- 		/*
- 		 * Check if no policies were added
- 		 *
- 		 * If no policies exist in pg_policy for this relation, then we need
- 		 * to create a single default-deny policy.  We use InvalidOid for the
- 		 * Oid to indicate that this is the default-deny policy (we may decide
- 		 * to ignore the default policy if an extension adds policies).
- 		 */
- 		if (rsdesc->policies == NIL)
- 		{
- 			RowSecurityPolicy *policy;
- 			Datum		role;
- 
- 			MemoryContextSwitchTo(rscxt);
- 
- 			role = ObjectIdGetDatum(ACL_ID_PUBLIC);
- 
- 			policy = palloc0(sizeof(RowSecurityPolicy));
- 			policy->policy_name = pstrdup("default-deny policy");
- 			policy->policy_id = InvalidOid;
- 			policy->polcmd = '*';
- 			policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
- 											'i');
- 			policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
- 										   sizeof(bool), BoolGetDatum(false),
- 											  false, true);
- 			policy->with_check_qual = copyObject(policy->qual);
- 			policy->hassublinks = false;
- 
- 			rsdesc->policies = lcons(policy, rsdesc->policies);
- 
- 			MemoryContextSwitchTo(oldcxt);
- 		}
  	}
  	PG_CATCH();
  	{
--- 319,324 ----
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index a1561ce..31c9430
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*************** ExecWithCheckOptions(WCOKind kind, Resul
*** 1815,1828 ****
  					break;
  				case WCO_RLS_INSERT_CHECK:
  				case WCO_RLS_UPDATE_CHECK:
! 					ereport(ERROR,
! 							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
  							 errmsg("new row violates row level security policy for \"%s\"",
  									wco->relname)));
  					break;
  				case WCO_RLS_CONFLICT_CHECK:
! 					ereport(ERROR,
! 							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
  							 errmsg("new row violates row level security policy (USING expression) for \"%s\"",
  									wco->relname)));
  					break;
--- 1815,1840 ----
  					break;
  				case WCO_RLS_INSERT_CHECK:
  				case WCO_RLS_UPDATE_CHECK:
! 					if (wco->polname != NULL)
! 						ereport(ERROR,
! 								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! 							 errmsg("new row violates row level security policy \"%s\" for \"%s\"",
! 									wco->polname, wco->relname)));
! 					else
! 						ereport(ERROR,
! 								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
  							 errmsg("new row violates row level security policy for \"%s\"",
  									wco->relname)));
  					break;
  				case WCO_RLS_CONFLICT_CHECK:
! 					if (wco->polname != NULL)
! 						ereport(ERROR,
! 								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! 							 errmsg("new row violates row level security policy \"%s\" (USING expression) for \"%s\"",
! 									wco->polname, wco->relname)));
! 					else
! 						ereport(ERROR,
! 								(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
  							 errmsg("new row violates row level security policy (USING expression) for \"%s\"",
  									wco->relname)));
  					break;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
new file mode 100644
index 4c363d3..54318b1
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyWithCheckOption(const WithCheckOpti
*** 2150,2155 ****
--- 2150,2156 ----
  
  	COPY_SCALAR_FIELD(kind);
  	COPY_STRING_FIELD(relname);
+ 	COPY_STRING_FIELD(polname);
  	COPY_NODE_FIELD(qual);
  	COPY_SCALAR_FIELD(cascaded);
  
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
new file mode 100644
index f19251e..35ce400
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalWithCheckOption(const WithCheckOpt
*** 2423,2428 ****
--- 2423,2429 ----
  {
  	COMPARE_SCALAR_FIELD(kind);
  	COMPARE_STRING_FIELD(relname);
+ 	COMPARE_STRING_FIELD(polname);
  	COMPARE_NODE_FIELD(qual);
  	COMPARE_SCALAR_FIELD(cascaded);
  
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
new file mode 100644
index 4775acf..243f27b
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
*************** _outWithCheckOption(StringInfo str, cons
*** 2397,2402 ****
--- 2397,2403 ----
  
  	WRITE_ENUM_FIELD(kind, WCOKind);
  	WRITE_STRING_FIELD(relname);
+ 	WRITE_STRING_FIELD(polname);
  	WRITE_NODE_FIELD(qual);
  	WRITE_BOOL_FIELD(cascaded);
  }
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
new file mode 100644
index f5a40fb..a6f09fe
*** a/src/backend/nodes/readfuncs.c
--- b/src/backend/nodes/readfuncs.c
*************** _readWithCheckOption(void)
*** 270,275 ****
--- 270,276 ----
  
  	READ_ENUM_FIELD(kind, WCOKind);
  	READ_STRING_FIELD(relname);
+ 	READ_STRING_FIELD(polname);
  	READ_NODE_FIELD(qual);
  	READ_BOOL_FIELD(cascaded);
  
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
new file mode 100644
index bbd6b77..66c286a
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
*************** fireRIRrules(Query *parsetree, List *act
*** 1775,1782 ****
  		/*
  		 * Fetch any new security quals that must be applied to this RTE.
  		 */
! 		get_row_security_policies(parsetree, parsetree->commandType, rte,
! 								  rt_index, &securityQuals, &withCheckOptions,
  								  &hasRowSecurity, &hasSubLinks);
  
  		if (securityQuals != NIL || withCheckOptions != NIL)
--- 1775,1782 ----
  		/*
  		 * 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)
*************** rewriteTargetView(Query *parsetree, Rela
*** 2981,2986 ****
--- 2981,2987 ----
  			wco = makeNode(WithCheckOption);
  			wco->kind = WCO_VIEW_CHECK;
  			wco->relname = pstrdup(RelationGetRelationName(view));
+ 			wco->polname = NULL;
  			wco->qual = NULL;
  			wco->cascaded = cascaded;
  
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
new file mode 100644
index aaf0061..9054652
*** a/src/backend/rewrite/rowsecurity.c
--- b/src/backend/rewrite/rowsecurity.c
***************
*** 13,19 ****
   * Any part of the system which is returning records back to the user, or
   * which is accepting records from the user to add to a table, needs to
   * consider the policies associated with the table (if any).  For normal
!  * queries, this is handled by calling prepend_row_security_policies() during
   * rewrite, which looks at each RTE and adds the expressions defined by the
   * policies to the securityQuals list for the RTE.  For queries which modify
   * the relation, any WITH CHECK policies are added to the list of
--- 13,19 ----
   * Any part of the system which is returning records back to the user, or
   * which is accepting records from the user to add to a table, needs to
   * consider the policies associated with the table (if any).  For normal
!  * queries, this is handled by calling get_policies_for_relation() during
   * rewrite, which looks at each RTE and adds the expressions defined by the
   * policies to the securityQuals list for the RTE.  For queries which modify
   * the relation, any WITH CHECK policies are added to the list of
***************
*** 56,68 ****
  #include "utils/syscache.h"
  #include "tcop/utility.h"
  
! 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,
! 				 BoolExprType boolop);
  static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
  
  /*
--- 56,84 ----
  #include "utils/syscache.h"
  #include "tcop/utility.h"
  
! static void get_policies_for_relation(Relation relation,
! 						  CmdType cmd, Oid user_id,
! 						  List **permissive_policies,
! 						  List **restrictive_policies);
! 
! static List *sort_policies_by_name(List *policies);
! 
! static int row_security_policy_cmp(const void *a, const void *b);
! 
! static void build_security_quals(int rt_index,
! 					 List *permissive_policies,
! 					 List *restrictive_policies,
! 					 List **securityQuals,
! 					 bool *hasSubLinks);
! 
! static void build_with_check_options(Relation rel,
! 						 int rt_index,
! 						 WCOKind kind,
! 						 List *permissive_policies,
! 						 List *restrictive_policies,
! 						 List **withCheckOptions,
! 						 bool *hasSubLinks);
! 
  static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
  
  /*
*************** static bool check_role_for_policy(ArrayT
*** 73,115 ****
   *
   * row_security_policy_hook_restrictive can be used to add policies which
   * are enforced, regardless of other policies (they are "AND"d).
-  *
-  * See below where the hook is called in prepend_row_security_policies for
-  * insight into how to use this hook.
   */
  row_security_policy_hook_type row_security_policy_hook_permissive = NULL;
  row_security_policy_hook_type row_security_policy_hook_restrictive = 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, CmdType commandType, RangeTblEntry *rte,
! 						  int rt_index, List **securityQuals,
! 						  List **withCheckOptions, bool *hasRowSecurity,
! 						  bool *hasSubLinks)
  {
- 	Expr	   *rowsec_expr = NULL;
- 	Expr	   *rowsec_with_check_expr = NULL;
- 	Expr	   *hook_expr_restrictive = NULL;
- 	Expr	   *hook_with_check_expr_restrictive = NULL;
- 	Expr	   *hook_expr_permissive = NULL;
- 	Expr	   *hook_with_check_expr_permissive = NULL;
- 
- 	List	   *rowsec_policies;
- 	List	   *hook_policies_restrictive = NIL;
- 	List	   *hook_policies_permissive = NIL;
- 
- 	Relation	rel;
  	Oid			user_id;
  	int			sec_context;
  	int			rls_status;
! 	bool		defaultDeny = false;
  
  	/* Defaults for the return values */
  	*securityQuals = NIL;
--- 89,118 ----
   *
   * row_security_policy_hook_restrictive can be used to add policies which
   * are enforced, regardless of other policies (they are "AND"d).
   */
  row_security_policy_hook_type row_security_policy_hook_permissive = NULL;
  row_security_policy_hook_type row_security_policy_hook_restrictive = NULL;
  
  /*
!  * Get any row security quals and WithCheckOption checks 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)
  {
  	Oid			user_id;
  	int			sec_context;
  	int			rls_status;
! 	Relation	rel;
! 	CmdType		commandType;
! 	List	   *permissive_policies;
! 	List	   *restrictive_policies;
  
  	/* Defaults for the return values */
  	*securityQuals = NIL;
*************** get_row_security_policies(Query *root, C
*** 157,615 ****
  		return;
  	}
  
- 	/* Grab the built-in policies which should be applied to this relation. */
- 	rel = heap_open(rte->relid, NoLock);
- 
- 	rowsec_policies = pull_row_security_policies(commandType, rel,
- 												 user_id);
- 
  	/*
! 	 * Check if this is only the default-deny policy.
! 	 *
! 	 * Normally, if the table has row security enabled but there are no
! 	 * policies, we use a default-deny policy and not allow anything. However,
! 	 * when an extension uses the hook to add their own policies, we don't
! 	 * want to include the default deny policy or there won't be any way for a
! 	 * user to use an extension exclusively for the policies to be used.
! 	 */
! 	if (((RowSecurityPolicy *) linitial(rowsec_policies))->policy_id
! 		== InvalidOid)
! 		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, OR_EXPR);
! 
! 	/*
! 	 * Also, allow extensions to add their own policies.
! 	 *
! 	 * extensions can add either permissive or restrictive policies.
! 	 *
! 	 * Note that, as with the internal policies, if multiple policies are
! 	 * returned then they will be combined into a single expression with all
! 	 * of them OR'd (for permissive) or AND'd (for restrictive) together.
! 	 *
! 	 * If only a USING policy is returned by the extension then it will be
! 	 * used for WITH CHECK as well, similar to how internal policies are
! 	 * handled.
  	 *
! 	 * The only caveat to this is that if there are NO internal policies
! 	 * defined, there ARE policies returned by the extension, and RLS is
! 	 * 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_restrictive)
! 	{
! 		hook_policies_restrictive = (*row_security_policy_hook_restrictive) (commandType, rel);
! 
! 		/* Build the expression from any policies returned. */
! 		if (hook_policies_restrictive != NIL)
! 			process_policies(root, hook_policies_restrictive, rt_index,
! 							 &hook_expr_restrictive,
! 							 &hook_with_check_expr_restrictive,
! 							 hasSubLinks,
! 							 AND_EXPR);
! 	}
  
! 	if (row_security_policy_hook_permissive)
! 	{
! 		hook_policies_permissive = (*row_security_policy_hook_permissive) (commandType, rel);
  
! 		/* Build the expression from any policies returned. */
! 		if (hook_policies_permissive != NIL)
! 			process_policies(root, hook_policies_permissive, rt_index,
! 							 &hook_expr_permissive,
! 							 &hook_with_check_expr_permissive, hasSubLinks,
! 							 OR_EXPR);
! 	}
  
  	/*
! 	 * If the only built-in policy is the default-deny one, and hook policies
! 	 * exist, then use the hook policies only and do not apply the
! 	 * default-deny policy.  Otherwise, we will apply both sets below.
  	 */
! 	if (defaultDeny &&
! 		(hook_policies_restrictive != NIL || hook_policies_permissive != NIL))
  	{
! 		rowsec_expr = NULL;
! 		rowsec_with_check_expr = NULL;
  	}
  
  	/*
! 	 * For INSERT or UPDATE, we need to add the WITH CHECK quals to Query's
! 	 * withCheckOptions to verify that any new records pass the WITH CHECK
! 	 * policy (this will be a copy of the USING policy, if no explicit WITH
! 	 * CHECK policy exists).
  	 */
  	if (commandType == CMD_INSERT || commandType == CMD_UPDATE)
  	{
! 		/*
! 		 * WITH CHECK OPTIONS wants a WCO node which wraps each Expr, so
! 		 * create them as necessary.
! 		 */
! 
! 		/*
! 		 * Handle any restrictive policies first.
! 		 *
! 		 * They can simply be added.
! 		 */
! 		if (hook_with_check_expr_restrictive)
! 		{
! 			WithCheckOption *wco;
  
! 			wco = (WithCheckOption *) makeNode(WithCheckOption);
! 			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
! 				WCO_RLS_UPDATE_CHECK;
! 			wco->relname = pstrdup(RelationGetRelationName(rel));
! 			wco->qual = (Node *) hook_with_check_expr_restrictive;
! 			wco->cascaded = false;
! 			*withCheckOptions = lappend(*withCheckOptions, wco);
! 		}
  
  		/*
! 		 * Handle built-in policies, if there are no permissive policies from
! 		 * the hook.
  		 */
! 		if (rowsec_with_check_expr && !hook_with_check_expr_permissive)
  		{
! 			WithCheckOption *wco;
  
! 			wco = (WithCheckOption *) makeNode(WithCheckOption);
! 			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
! 				WCO_RLS_UPDATE_CHECK;
! 			wco->relname = pstrdup(RelationGetRelationName(rel));
! 			wco->qual = (Node *) rowsec_with_check_expr;
! 			wco->cascaded = false;
! 			*withCheckOptions = lappend(*withCheckOptions, wco);
! 		}
! 		/* Handle the hook policies, if there are no built-in ones. */
! 		else if (!rowsec_with_check_expr && hook_with_check_expr_permissive)
! 		{
! 			WithCheckOption *wco;
  
! 			wco = (WithCheckOption *) makeNode(WithCheckOption);
! 			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
! 				WCO_RLS_UPDATE_CHECK;
! 			wco->relname = pstrdup(RelationGetRelationName(rel));
! 			wco->qual = (Node *) hook_with_check_expr_permissive;
! 			wco->cascaded = false;
! 			*withCheckOptions = lappend(*withCheckOptions, wco);
  		}
! 		/* Handle the case where there are both. */
! 		else if (rowsec_with_check_expr && hook_with_check_expr_permissive)
! 		{
! 			WithCheckOption *wco;
! 			List	   *combined_quals = NIL;
! 			Expr	   *combined_qual_eval;
  
! 			combined_quals = lcons(copyObject(rowsec_with_check_expr),
! 								   combined_quals);
  
! 			combined_quals = lcons(copyObject(hook_with_check_expr_permissive),
! 								   combined_quals);
  
! 			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
  
! 			wco = (WithCheckOption *) makeNode(WithCheckOption);
! 			wco->kind = commandType == CMD_INSERT ? WCO_RLS_INSERT_CHECK :
! 				WCO_RLS_UPDATE_CHECK;
! 			wco->relname = pstrdup(RelationGetRelationName(rel));
! 			wco->qual = (Node *) combined_qual_eval;
! 			wco->cascaded = false;
! 			*withCheckOptions = lappend(*withCheckOptions, wco);
! 		}
  
! 		/*
! 		 * ON CONFLICT DO UPDATE has an RTE that is subject to both INSERT and
! 		 * UPDATE RLS enforcement.  Those are enforced (as a special, distinct
! 		 * kind of WCO) on the target tuple.
! 		 *
! 		 * Make a second, recursive pass over the RTE for this, gathering
! 		 * UPDATE-applicable RLS checks/WCOs, and gathering and converting
! 		 * UPDATE-applicable security quals into WCO_RLS_CONFLICT_CHECK RLS
! 		 * checks/WCOs.  Finally, these distinct kinds of RLS checks/WCOs are
! 		 * concatenated with our own INSERT-applicable list.
! 		 */
! 		if (root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE &&
! 			commandType == CMD_INSERT)
! 		{
! 			List	   *conflictSecurityQuals = NIL;
! 			List	   *conflictWCOs = NIL;
! 			ListCell   *item;
! 			bool		conflictHasRowSecurity = false;
! 			bool		conflictHasSublinks = false;
  
! 			/* Assume that RTE is target resultRelation */
! 			get_row_security_policies(root, CMD_UPDATE, rte, rt_index,
! 									  &conflictSecurityQuals, &conflictWCOs,
! 									  &conflictHasRowSecurity,
! 									  &conflictHasSublinks);
  
! 			if (conflictHasRowSecurity)
! 				*hasRowSecurity = true;
! 			if (conflictHasSublinks)
! 				*hasSubLinks = true;
  
  			/*
! 			 * Append WITH CHECK OPTIONs/RLS checks, which should not conflict
! 			 * between this INSERT and the auxiliary UPDATE
  			 */
! 			*withCheckOptions = list_concat(*withCheckOptions,
! 											conflictWCOs);
  
! 			foreach(item, conflictSecurityQuals)
! 			{
! 				Expr	   *conflict_rowsec_expr = (Expr *) lfirst(item);
! 				WithCheckOption *wco;
  
! 				wco = (WithCheckOption *) makeNode(WithCheckOption);
  
! 				wco->kind = WCO_RLS_CONFLICT_CHECK;
! 				wco->relname = pstrdup(RelationGetRelationName(rel));
! 				wco->qual = (Node *) copyObject(conflict_rowsec_expr);
! 				wco->cascaded = false;
! 				*withCheckOptions = lappend(*withCheckOptions, wco);
! 			}
  		}
  	}
  
! 	/* For SELECT, UPDATE, and DELETE, set the security quals */
! 	if (commandType == CMD_SELECT
! 		|| commandType == CMD_UPDATE
! 		|| commandType == CMD_DELETE)
  	{
! 		/* restrictive policies can simply be added to the list first */
! 		if (hook_expr_restrictive)
! 			*securityQuals = lappend(*securityQuals, hook_expr_restrictive);
  
! 		/* If we only have internal permissive, then just add those */
! 		if (rowsec_expr && !hook_expr_permissive)
! 			*securityQuals = lappend(*securityQuals, rowsec_expr);
! 		/* .. and if we have only permissive policies from the hook */
! 		else if (!rowsec_expr && hook_expr_permissive)
! 			*securityQuals = lappend(*securityQuals, hook_expr_permissive);
! 		/* if we have both, we have to combine them with an OR */
! 		else if (rowsec_expr && hook_expr_permissive)
  		{
! 			List	   *combined_quals = NIL;
! 			Expr	   *combined_qual_eval;
  
! 			combined_quals = lcons(copyObject(rowsec_expr), combined_quals);
! 			combined_quals = lcons(copyObject(hook_expr_permissive),
! 								   combined_quals);
  
! 			combined_qual_eval = makeBoolExpr(OR_EXPR, combined_quals, -1);
  
! 			*securityQuals = lappend(*securityQuals, combined_qual_eval);
! 		}
  	}
  
! 	heap_close(rel, NoLock);
  
! 	/*
! 	 * Mark this query as having row security, so plancache can invalidate it
! 	 * when necessary (eg: role changes)
! 	 */
! 	*hasRowSecurity = true;
  
! 	return;
  }
  
  /*
!  * pull_row_security_policies
   *
!  * Returns the list of policies to be added for this relation, based on the
!  * type of command and the roles to which it applies, from the relation cache.
   *
   */
! static List *
! pull_row_security_policies(CmdType cmd, Relation relation, Oid user_id)
  {
! 	List	   *policies = NIL;
  	ListCell   *item;
  
  	/*
! 	 * Row security is enabled for the relation and the row security GUC is
! 	 * either 'on' or 'force' here, so find the policies to apply to the
! 	 * table. There must always be at least one policy defined (may be the
! 	 * simple 'default-deny' policy, if none are explicitly defined on the
! 	 * table).
  	 */
! 	foreach(item, relation->rd_rsdesc->policies)
  	{
  		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
  
! 		/* Always add ALL policies, if they exist. */
! 		if (policy->polcmd == '*' &&
! 			check_role_for_policy(policy->roles, user_id))
! 			policies = lcons(policy, policies);
! 
! 		/* Add relevant command-specific policies to the list. */
! 		switch (cmd)
  		{
! 			case CMD_SELECT:
! 				if (policy->polcmd == ACL_SELECT_CHR
! 					&& check_role_for_policy(policy->roles, user_id))
! 					policies = lcons(policy, policies);
! 				break;
! 			case CMD_INSERT:
! 				/* If INSERT then only need to add the WITH CHECK qual */
! 				if (policy->polcmd == ACL_INSERT_CHR
! 					&& check_role_for_policy(policy->roles, user_id))
! 					policies = lcons(policy, policies);
! 				break;
! 			case CMD_UPDATE:
! 				if (policy->polcmd == ACL_UPDATE_CHR
! 					&& check_role_for_policy(policy->roles, user_id))
! 					policies = lcons(policy, policies);
! 				break;
! 			case CMD_DELETE:
! 				if (policy->polcmd == ACL_DELETE_CHR
! 					&& check_role_for_policy(policy->roles, user_id))
! 					policies = lcons(policy, policies);
! 				break;
! 			default:
! 				elog(ERROR, "unrecognized policy command type %d", (int) cmd);
! 				break;
  		}
  	}
  
  	/*
! 	 * There should always be a policy applied.  If there are none found then
! 	 * create a simply defauly-deny policy (might be that policies exist but
! 	 * that none of them apply to the role which is querying the table).
  	 */
! 	if (policies == NIL)
  	{
! 		RowSecurityPolicy *policy = NULL;
! 		Datum		role;
  
! 		role = ObjectIdGetDatum(ACL_ID_PUBLIC);
  
! 		policy = palloc0(sizeof(RowSecurityPolicy));
! 		policy->policy_name = pstrdup("default-deny policy");
! 		policy->policy_id = InvalidOid;
! 		policy->polcmd = '*';
! 		policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true,
! 										'i');
! 		policy->qual = (Expr *) makeConst(BOOLOID, -1, InvalidOid,
! 										  sizeof(bool), BoolGetDatum(false),
! 										  false, true);
! 		policy->with_check_qual = copyObject(policy->qual);
! 		policy->hassublinks = false;
  
! 		policies = list_make1(policy);
  	}
  
! 	Assert(policies != NIL);
! 
! 	return policies;
  }
  
  /*
!  * process_policies
   *
!  * This will step through the policies which are passed in (which would come
!  * from either the built-in ones created on a table, or from policies provided
!  * by an extension through the hook provided), work out how to combine them,
!  * rewrite them as necessary, and produce an Expr for the normal security
!  * quals and an Expr for the with check quals.
   *
!  * 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,
! 				 BoolExprType boolop)
  {
  	ListCell   *item;
! 	List	   *quals = NIL;
! 	List	   *with_check_quals = NIL;
  
  	/*
! 	 * 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)
  	{
  		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
  
! 		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);
! 
! 		/*
! 		 * For each policy, if there is only a USING clause then copy/use it
! 		 * for the WITH CHECK policy also, if this RTE is the query's result
! 		 * relation.
! 		 */
! 		if (policy->qual != NULL && policy->with_check_qual == NULL &&
! 			rt_index == root->resultRelation)
! 			with_check_quals = lcons(copyObject(policy->qual),
! 									 with_check_quals);
  
  
! 		if (policy->hassublinks)
! 			*hassublinks = true;
  	}
  
  	/*
! 	 * If we end up without any normal quals (perhaps the only policy matched
! 	 * was for INSERT), then create a single all-false one.
  	 */
! 	if (quals == NIL)
! 		quals = lcons(makeConst(BOOLOID, -1, InvalidOid, sizeof(bool),
! 								BoolGetDatum(false), false, true), quals);
  
! 	/*
! 	 * Row security quals always have the target table as varno 1, as no joins
! 	 * are permitted in row security expressions. We must walk the expression,
! 	 * updating any references to varno 1 to the varno the table has in the
! 	 * outer query.
! 	 *
! 	 * We rewrite the expression in-place.
! 	 *
! 	 * We must have some quals at this point; the default-deny policy, if
! 	 * nothing else.  Note that we might not have any WITH CHECK quals- that's
! 	 * fine, as this might not be the resultRelation.
! 	 */
! 	Assert(quals != NIL);
  
! 	ChangeVarNodes((Node *) quals, 1, rt_index, 0);
  
! 	if (with_check_quals != NIL)
! 		ChangeVarNodes((Node *) with_check_quals, 1, rt_index, 0);
  
! 	/*
! 	 * If more than one security qual is returned, then they need to be
! 	 * combined together.
! 	 */
! 	if (list_length(quals) > 1)
! 		*qual_eval = makeBoolExpr(boolop, quals, -1);
! 	else
! 		*qual_eval = (Expr *) linitial(quals);
  
  	/*
! 	 * Similarly, if more than one WITH CHECK qual is returned, then they need
! 	 * to be combined together.
! 	 *
! 	 * with_check_quals is allowed to be NIL here since this might not be the
! 	 * resultRelation (see above).
  	 */
! 	if (list_length(with_check_quals) > 1)
! 		*with_check_eval = makeBoolExpr(boolop, with_check_quals, -1);
! 	else if (with_check_quals != NIL)
! 		*with_check_eval = (Expr *) linitial(with_check_quals);
! 	else
! 		*with_check_eval = NULL;
  
! 	return;
  }
  
  /*
--- 160,641 ----
  		return;
  	}
  
  	/*
! 	 * RLS is enabled for this relation.
  	 *
! 	 * Get the security policies that should be applied, based on the command
! 	 * type.  Note that if this isn't the target relation, we actually want
! 	 * the relation's SELECT policies, regardless of the query command type,
! 	 * for example in UPDATE t1 ... FROM t2 we need to apply t1's UPDATE
! 	 * policies and t2's SELECT policies.
  	 */
! 	rel = heap_open(rte->relid, NoLock);
  
! 	commandType = rt_index == root->resultRelation ?
! 				  root->commandType : CMD_SELECT;
  
! 	get_policies_for_relation(rel, commandType, user_id,
! 							  &permissive_policies, &restrictive_policies);
  
  	/*
! 	 * For SELECT, UPDATE and DELETE, build security quals to enforce these
! 	 * policies.  These security quals control access to existing table rows.
! 	 * Restrictive policies are "AND"d together, and permissive policies are
! 	 * "OR"d together.
! 	 *
! 	 * If there are no policy clauses controlling access to the table, this
! 	 * will add a single always-false clause (a default-deny policy).
  	 */
! 	if (commandType == CMD_SELECT ||
! 		commandType == CMD_UPDATE ||
! 		commandType == CMD_DELETE)
  	{
! 		build_security_quals(rt_index,
! 							 permissive_policies,
! 							 restrictive_policies,
! 							 securityQuals,
! 							 hasSubLinks);
  	}
  
  	/*
! 	 * For INSERT and UPDATE add withCheckOptions to verify that any new
! 	 * records added are consistent with the security policies.  This will use
! 	 * each policy's WITH CHECK clause, or its USING clause if no explicit
! 	 * WITH CHECK clause is defined.
  	 */
  	if (commandType == CMD_INSERT || commandType == CMD_UPDATE)
  	{
! 		/* This should be the target relation */
! 		Assert(rt_index == root->resultRelation);
  
! 		build_with_check_options(rel, rt_index,
! 								 commandType == CMD_INSERT ?
! 								 WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
! 								 permissive_policies,
! 								 restrictive_policies,
! 								 withCheckOptions,
! 								 hasSubLinks);
  
  		/*
! 		 * For INSERT ... ON CONFLICT DO UPDATE we need additional policy
! 		 * checks for the UPDATE which may be applied to the same RTE.
  		 */
! 		if (commandType == CMD_INSERT &&
! 			root->onConflict && root->onConflict->action == ONCONFLICT_UPDATE)
  		{
! 			List	   *conflict_permissive_policies;
! 			List	   *conflict_restrictive_policies;
  
! 			/* Get the policies that apply to the auxiliary UPDATE */
! 			get_policies_for_relation(rel, CMD_UPDATE, user_id,
! 									  &conflict_permissive_policies,
! 									  &conflict_restrictive_policies);
  
! 			/*
! 			 * Enforce the USING clauses of the UPDATE policies using WCOs
! 			 * rather than security quals.  This ensures that an error is
! 			 * raised if the conflicting row cannot be updated due to RLS,
! 			 * rather than it being silently skipped.
! 			 */
! 			build_with_check_options(rel, rt_index,
! 									 WCO_RLS_CONFLICT_CHECK,
! 									 conflict_permissive_policies,
! 									 conflict_restrictive_policies,
! 									 withCheckOptions,
! 									 hasSubLinks);
! 
! 			/* Enforce the WITH CHECK clauses of the UPDATE policies */
! 			build_with_check_options(rel, rt_index,
! 									 WCO_RLS_UPDATE_CHECK,
! 									 conflict_permissive_policies,
! 									 conflict_restrictive_policies,
! 									 withCheckOptions,
! 									 hasSubLinks);
  		}
! 	}
  
! 	heap_close(rel, NoLock);
  
! 	/*
! 	 * Mark this query as having row security, so plancache can invalidate it
! 	 * when necessary (eg: role changes)
! 	 */
! 	*hasRowSecurity = true;
  
! 	return;
! }
  
! /*
!  * get_policies_for_relation
!  *
!  * Returns lists of permissive and restrictive policies to be applied to the
!  * specified relation, based on the command type and role.
!  *
!  * This includes any policies added by extensions.
!  */
! static void
! get_policies_for_relation(Relation relation,
! 						  CmdType cmd, Oid user_id,
! 						  List **permissive_policies,
! 						  List **restrictive_policies)
! {
! 	ListCell   *item;
  
! 	*permissive_policies = NIL;
! 	*restrictive_policies = NIL;
  
! 	/*
! 	 * First find all internal policies for the relation.  CREATE POLICY does
! 	 * not currently support defining restrictive policies, so for now all
! 	 * internal policies are permissive.
! 	 */
! 	foreach(item, relation->rd_rsdesc->policies)
! 	{
! 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
! 		bool		cmd_matches = false;
  
! 		/* Check whether the policy applies to the specified command type */
! 		if (policy->polcmd == '*')
! 			cmd_matches = true;
! 		else
! 		{
! 			switch (cmd)
! 			{
! 				case CMD_SELECT:
! 					if (policy->polcmd == ACL_SELECT_CHR)
! 						cmd_matches = true;
! 					break;
! 				case CMD_INSERT:
! 					if (policy->polcmd == ACL_INSERT_CHR)
! 						cmd_matches = true;
! 					break;
! 				case CMD_UPDATE:
! 					if (policy->polcmd == ACL_UPDATE_CHR)
! 						cmd_matches = true;
! 					break;
! 				case CMD_DELETE:
! 					if (policy->polcmd == ACL_DELETE_CHR)
! 						cmd_matches = true;
! 					break;
! 				default:
! 					elog(ERROR, "unrecognized policy command type %d", (int) cmd);
! 					break;
! 			}
! 		}
  
+ 		if (cmd_matches)
+ 		{
  			/*
! 			 * Add this policy to the list of permissive policies if it
! 			 * applies to the specified role
  			 */
! 			if (check_role_for_policy(policy->roles, user_id))
! 				*permissive_policies = lappend(*permissive_policies, policy);
! 		}
! 	}
  
! 	/*
! 	 * Then add any permissive and restrictive policies defined by extensions.
! 	 * These are simply appended to the lists of internal policies, if they
! 	 * apply to the specified role.
! 	 */
! 	if (row_security_policy_hook_restrictive)
! 	{
! 		List	   *hook_policies = (*row_security_policy_hook_restrictive) (cmd, relation);
  
! 		/*
! 		 * We sort restrictive policies by name so that any WCOs they generate
! 		 * are checked in a well-defined order.
! 		 */
! 		hook_policies = sort_policies_by_name(hook_policies);
  
! 		foreach(item, hook_policies)
! 		{
! 			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
! 
! 			if (check_role_for_policy(policy->roles, user_id))
! 				*restrictive_policies = lappend(*restrictive_policies, policy);
  		}
  	}
  
! 	if (row_security_policy_hook_permissive)
  	{
! 		List	   *hook_policies = (*row_security_policy_hook_permissive) (cmd, relation);
  
! 		foreach(item, hook_policies)
  		{
! 			RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
  
! 			if (check_role_for_policy(policy->roles, user_id))
! 				*permissive_policies = lappend(*permissive_policies, policy);
! 		}
! 	}
! }
  
! /*
!  * sort_policies_by_name
!  *
!  * This is only used for restrictive policies, ensuring that any
!  * WithCheckOptions they generate are applied in a well-defined order.
!  * This is not necessary for permissive policies, since they are all "OR"d
!  * together into a single WithCheckOption check.
!  */
! static List *
! sort_policies_by_name(List *policies)
! {
! 	int			npol = list_length(policies);
! 	RowSecurityPolicy *pols;
! 	ListCell   *item;
! 	int			ii = 0;
  
! 	if (npol <= 1)
! 		return policies;
! 
! 	pols = (RowSecurityPolicy *) palloc(sizeof(RowSecurityPolicy) * npol);
! 
! 	foreach(item, policies)
! 	{
! 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
! 		pols[ii++] = *policy;
  	}
  
! 	qsort(pols, npol, sizeof(RowSecurityPolicy), row_security_policy_cmp);
  
! 	policies = NIL;
! 	for (ii = 0; ii < npol; ii++)
! 		policies = lappend(policies, &pols[ii]);
  
! 	return policies;
  }
  
  /*
!  * qsort comparator to sort RowSecurityPolicy entries by name
!  */
! static int
! row_security_policy_cmp(const void *a, const void *b)
! {
! 	const RowSecurityPolicy *pa = (const RowSecurityPolicy *) a;
! 	const RowSecurityPolicy *pb = (const RowSecurityPolicy *) b;
! 
! 	/* Guard against NULL policy names from extensions */
! 	if (pa->policy_name == NULL)
! 		return pb->policy_name == NULL ? 0 : 1;
! 	if (pb->policy_name == NULL)
! 		return -1;
! 
! 	return strcmp(pa->policy_name, pb->policy_name);
! }
! 
! /*
!  * build_security_quals
   *
!  * Build security quals to enforce the specified RLS policies, restricting
!  * access to existing data in a table.  If there are no policies controlling
!  * 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.
   */
! static void
! build_security_quals(int rt_index,
! 					 List *permissive_policies,
! 					 List *restrictive_policies,
! 					 List **securityQuals,
! 					 bool *hasSubLinks)
  {
! 	bool		quals_found = false;
  	ListCell   *item;
+ 	List	   *permissive_quals;
+ 	Expr	   *rowsec_expr;
  
  	/*
! 	 * First add security quals based on the USING clauses from the
! 	 * restrictive policies.  Since these need to be "AND"d together, we can
! 	 * just add them one at a time.
  	 */
! 	foreach(item, restrictive_policies)
  	{
  		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+ 		Expr	   *qual;
  
! 		if (policy->qual != NULL)
  		{
! 			qual = copyObject(policy->qual);
! 			ChangeVarNodes((Node *) qual, 1, rt_index, 0);
! 
! 			*securityQuals = lappend(*securityQuals, qual);
! 			*hasSubLinks |= policy->hassublinks;
! 			quals_found = true;
  		}
  	}
  
  	/*
! 	 * Then add a single security qual "OR"ing together the USING clauses from
! 	 * all the permissive policies.
  	 */
! 	permissive_quals = NIL;
! 
! 	foreach(item, permissive_policies)
  	{
! 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
  
! 		if (policy->qual != NULL)
! 		{
! 			permissive_quals = lappend(permissive_quals,
! 									   copyObject(policy->qual));
! 			*hasSubLinks |= policy->hassublinks;
! 		}
! 	}
  
! 	if (permissive_quals != NIL)
! 	{
! 		if (list_length(permissive_quals) == 1)
! 			rowsec_expr = (Expr *) linitial(permissive_quals);
! 		else
! 			rowsec_expr = makeBoolExpr(OR_EXPR, permissive_quals, -1);
  
! 		ChangeVarNodes((Node *) rowsec_expr, 1, rt_index, 0);
! 		*securityQuals = lappend(*securityQuals, rowsec_expr);
! 		quals_found = true;
  	}
  
! 	/*
! 	 * Finally, if there were no policy clauses to control access to the
! 	 * table, add a single always-false clause (a default-deny policy).
! 	 */
! 	if (!quals_found)
! 	{
! 		*securityQuals = lappend(*securityQuals,
! 								 makeConst(BOOLOID, -1, InvalidOid,
! 										   sizeof(bool), BoolGetDatum(false),
! 										   false, true));
! 	}
  }
  
  /*
!  * build_with_check_options
   *
!  * Build WithCheckOptions of the specified kind to check that new records
!  * added by an INSERT or UPDATE are consistent with the specified RLS
!  * policies.  Normally new data must satisfy the WITH CHECK clauses from the
!  * policies.  If a policy has no explicit WITH CHECK clause, its USING clause
!  * is used instead.  In the special case of an UPDATE arising from an
!  * INSERT ... ON CONFLICT DO UPDATE, existing records are first checked using
!  * a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
!  * clauses from RLS policies.
   *
!  * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
!  * any of the check clauses added contain sublink subqueries.
   */
  static void
! build_with_check_options(Relation rel,
! 						 int rt_index,
! 						 WCOKind kind,
! 						 List *permissive_policies,
! 						 List *restrictive_policies,
! 						 List **withCheckOptions,
! 						 bool *hasSubLinks)
  {
+ 	bool		quals_found = false;
  	ListCell   *item;
! 	List	   *permissive_quals;
! 
! #define QUAL_FOR_WCO(policy) \
! 	( kind != WCO_RLS_CONFLICT_CHECK &&	\
! 	  (policy)->with_check_qual != NULL ? \
! 	  (policy)->with_check_qual : (policy)->qual )
  
  	/*
! 	 * First add WithCheckOptions for each of the restrictive policy clauses
! 	 * (which need to be "AND"d together).  We use a separate WithCheckOption
! 	 * for each restrictive policy to allow the policy name to be included in
! 	 * error reports if the policy is violated.
  	 */
! 	foreach(item, restrictive_policies)
  	{
  		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
+ 		Expr	   *qual = QUAL_FOR_WCO(policy);
+ 		WithCheckOption *wco;
  
! 		if (qual != NULL)
! 		{
! 			qual = copyObject(qual);
! 			ChangeVarNodes((Node *) qual, 1, rt_index, 0);
  
+ 			wco = (WithCheckOption *) makeNode(WithCheckOption);
+ 			wco->kind = kind;
+ 			wco->relname = pstrdup(RelationGetRelationName(rel));
+ 			wco->polname = pstrdup(policy->policy_name);
+ 			wco->qual = (Node *) qual;
+ 			wco->cascaded = false;
  
! 			*withCheckOptions = lappend(*withCheckOptions, wco);
! 			*hasSubLinks |= policy->hassublinks;
! 			quals_found = true;
! 		}
  	}
  
  	/*
! 	 * Then add a single WithCheckOption for all the permissive policy clauses
! 	 * "OR"d together.  This check has no policy name, since if the check
! 	 * fails it means that no policy granted permission to perform the update,
! 	 * rather than any particular policy being violated.
  	 */
! 	permissive_quals = NIL;
  
! 	foreach(item, permissive_policies)
! 	{
! 		RowSecurityPolicy *policy = (RowSecurityPolicy *) lfirst(item);
! 		Expr	   *qual = QUAL_FOR_WCO(policy);
  
! 		if (qual != NULL)
! 		{
! 			permissive_quals = lappend(permissive_quals, copyObject(qual));
! 			*hasSubLinks |= policy->hassublinks;
! 		}
! 	}
  
! 	if (permissive_quals != NIL)
! 	{
! 		WithCheckOption *wco;
  
! 		wco = (WithCheckOption *) makeNode(WithCheckOption);
! 		wco->kind = kind;
! 		wco->relname = pstrdup(RelationGetRelationName(rel));
! 		wco->polname = NULL;
! 		wco->cascaded = false;
! 
! 		if (list_length(permissive_quals) == 1)
! 			wco->qual = (Node *) linitial(permissive_quals);
! 		else
! 			wco->qual = (Node *) makeBoolExpr(OR_EXPR, permissive_quals, -1);
! 
! 		ChangeVarNodes(wco->qual, 1, rt_index, 0);
! 
! 		*withCheckOptions = lappend(*withCheckOptions, wco);
! 		quals_found = true;
! 	}
  
  	/*
! 	 * Finally, if there were no policy clauses to check new data, add a
! 	 * single always-false check (a default-deny policy).
  	 */
! 	if (!quals_found)
! 	{
! 		WithCheckOption *wco;
  
! 		wco = (WithCheckOption *) makeNode(WithCheckOption);
! 		wco->kind = kind;
! 		wco->relname = pstrdup(RelationGetRelationName(rel));
! 		wco->polname = NULL;
! 		wco->qual = (Node *) makeConst(BOOLOID, -1, InvalidOid,
! 									   sizeof(bool), BoolGetDatum(false),
! 									   false, true);
! 		wco->cascaded = false;
! 
! 		*withCheckOptions = lappend(*withCheckOptions, wco);
! 	}
  }
  
  /*
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
new file mode 100644
index f60f3cb..553783c
*** a/src/backend/utils/cache/relcache.c
--- b/src/backend/utils/cache/relcache.c
*************** equalPolicy(RowSecurityPolicy *policy1,
*** 867,874 ****
  		if (policy2 == NULL)
  			return false;
  
- 		if (policy1->policy_id != policy2->policy_id)
- 			return false;
  		if (policy1->polcmd != policy2->polcmd)
  			return false;
  		if (policy1->hassublinks != policy2->hassublinks)
--- 867,872 ----
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
new file mode 100644
index 868905b..c608711
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct WithCheckOption
*** 931,936 ****
--- 931,937 ----
  	NodeTag		type;
  	WCOKind		kind;			/* kind of WCO */
  	char	   *relname;		/* name of relation that specified the WCO */
+ 	char	   *polname;		/* name of RLS policy being checked */
  	Node	   *qual;			/* constraint qual to check */
  	bool		cascaded;		/* true for a cascaded WCO on a view */
  } WithCheckOption;
diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h
new file mode 100644
index 523c56e..4af244d
*** a/src/include/rewrite/rowsecurity.h
--- b/src/include/rewrite/rowsecurity.h
***************
*** 19,25 ****
  
  typedef struct RowSecurityPolicy
  {
- 	Oid			policy_id;		/* OID of the policy */
  	char	   *policy_name;	/* Name of the policy */
  	char		polcmd;			/* Type of command policy is for */
  	ArrayType  *roles;			/* Array of roles policy is for */
--- 19,24 ----
*************** extern PGDLLIMPORT row_security_policy_h
*** 41,47 ****
  
  extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restrictive;
  
! extern void get_row_security_policies(Query *root, CmdType commandType,
  						  RangeTblEntry *rte, int rt_index,
  						  List **securityQuals, List **withCheckOptions,
  						  bool *hasRowSecurity, bool *hasSubLinks);
--- 40,46 ----
  
  extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restrictive;
  
! extern void get_row_security_policies(Query *root,
  						  RangeTblEntry *rte, int rt_index,
  						  List **securityQuals, List **withCheckOptions,
  						  bool *hasRowSecurity, bool *hasSubLinks);
diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
new file mode 100644
index 3a7a4c3..e7bb30e
*** a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
--- b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
*************** SELECT * FROM rls_test_restrictive;
*** 78,84 ****
  INSERT INTO rls_test_restrictive VALUES ('r1','s1',10);
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r4','s4',10);
! ERROR:  new row violates row level security policy for "rls_test_restrictive"
  SET ROLE s1;
  -- With only the hook's policies, both
  -- permissive hook's policy is current_user = username
--- 78,84 ----
  INSERT INTO rls_test_restrictive VALUES ('r1','s1',10);
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r4','s4',10);
! ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
  SET ROLE s1;
  -- With only the hook's policies, both
  -- permissive hook's policy is current_user = username
*************** INSERT INTO rls_test_both VALUES ('r4','
*** 104,110 ****
  ERROR:  new row violates row level security policy for "rls_test_both"
  -- failure
  INSERT INTO rls_test_both VALUES ('r4','s4',10);
! ERROR:  new row violates row level security policy for "rls_test_both"
  RESET ROLE;
  -- Create "internal" policies, to check that the policies from
  -- the hooks are combined correctly.
--- 104,110 ----
  ERROR:  new row violates row level security policy for "rls_test_both"
  -- failure
  INSERT INTO rls_test_both VALUES ('r4','s4',10);
! ERROR:  new row violates row level security policy "extension policy" for "rls_test_both"
  RESET ROLE;
  -- Create "internal" policies, to check that the policies from
  -- the hooks are combined correctly.
*************** EXPLAIN (costs off) SELECT * FROM rls_te
*** 117,123 ****
                            QUERY PLAN                           
  ---------------------------------------------------------------
   Seq Scan on rls_test_permissive
!    Filter: (("current_user"() = username) OR ((data % 2) = 0))
  (2 rows)
  
  SELECT * FROM rls_test_permissive;
--- 117,123 ----
                            QUERY PLAN                           
  ---------------------------------------------------------------
   Seq Scan on rls_test_permissive
!    Filter: (((data % 2) = 0) OR ("current_user"() = username))
  (2 rows)
  
  SELECT * FROM rls_test_permissive;
*************** SELECT * FROM rls_test_restrictive;
*** 156,175 ****
  INSERT INTO rls_test_restrictive VALUES ('r1','s1',8);
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r3','s3',10);
! ERROR:  new row violates row level security policy for "rls_test_restrictive"
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r1','s1',7);
  ERROR:  new row violates row level security policy for "rls_test_restrictive"
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r4','s4',7);
! ERROR:  new row violates row level security policy for "rls_test_restrictive"
  -- With both internal and hook policies, both permissive
  -- and restrictive hook policies
  EXPLAIN (costs off) SELECT * FROM rls_test_both;
                                          QUERY PLAN                                         
  -------------------------------------------------------------------------------------------
   Subquery Scan on rls_test_both
!    Filter: (("current_user"() = rls_test_both.username) OR ((rls_test_both.data % 2) = 0))
     ->  Seq Scan on rls_test_both rls_test_both_1
           Filter: ("current_user"() = supervisor)
  (4 rows)
--- 156,175 ----
  INSERT INTO rls_test_restrictive VALUES ('r1','s1',8);
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r3','s3',10);
! ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r1','s1',7);
  ERROR:  new row violates row level security policy for "rls_test_restrictive"
  -- failure
  INSERT INTO rls_test_restrictive VALUES ('r4','s4',7);
! ERROR:  new row violates row level security policy "extension policy" for "rls_test_restrictive"
  -- With both internal and hook policies, both permissive
  -- and restrictive hook policies
  EXPLAIN (costs off) SELECT * FROM rls_test_both;
                                          QUERY PLAN                                         
  -------------------------------------------------------------------------------------------
   Subquery Scan on rls_test_both
!    Filter: (((rls_test_both.data % 2) = 0) OR ("current_user"() = rls_test_both.username))
     ->  Seq Scan on rls_test_both rls_test_both_1
           Filter: ("current_user"() = supervisor)
  (4 rows)
*************** SELECT * FROM rls_test_both;
*** 183,195 ****
  INSERT INTO rls_test_both VALUES ('r1','s1',8);
  -- failure
  INSERT INTO rls_test_both VALUES ('r3','s3',10);
! ERROR:  new row violates row level security policy for "rls_test_both"
  -- failure
  INSERT INTO rls_test_both VALUES ('r1','s1',7);
  ERROR:  new row violates row level security policy for "rls_test_both"
  -- failure
  INSERT INTO rls_test_both VALUES ('r4','s4',7);
! ERROR:  new row violates row level security policy for "rls_test_both"
  RESET ROLE;
  DROP TABLE rls_test_restrictive;
  DROP TABLE rls_test_permissive;
--- 183,195 ----
  INSERT INTO rls_test_both VALUES ('r1','s1',8);
  -- failure
  INSERT INTO rls_test_both VALUES ('r3','s3',10);
! ERROR:  new row violates row level security policy "extension policy" for "rls_test_both"
  -- failure
  INSERT INTO rls_test_both VALUES ('r1','s1',7);
  ERROR:  new row violates row level security policy for "rls_test_both"
  -- failure
  INSERT INTO rls_test_both VALUES ('r4','s4',7);
! ERROR:  new row violates row level security policy "extension policy" for "rls_test_both"
  RESET ROLE;
  DROP TABLE rls_test_restrictive;
  DROP TABLE rls_test_permissive;
diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c
new file mode 100644
index 61b62d5..e955dc6
*** a/src/test/modules/test_rls_hooks/test_rls_hooks.c
--- b/src/test/modules/test_rls_hooks/test_rls_hooks.c
*************** test_rls_hooks_permissive(CmdType cmdtyp
*** 87,93 ****
  	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
  
  	policy->policy_name = pstrdup("extension policy");
- 	policy->policy_id = InvalidOid;
  	policy->polcmd = '*';
  	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
  
--- 87,92 ----
*************** test_rls_hooks_restrictive(CmdType cmdty
*** 146,152 ****
  	role = ObjectIdGetDatum(ACL_ID_PUBLIC);
  
  	policy->policy_name = pstrdup("extension policy");
- 	policy->policy_id = InvalidOid;
  	policy->polcmd = '*';
  	policy->roles = construct_array(&role, 1, OIDOID, sizeof(Oid), true, 'i');
  
--- 145,150 ----
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
new file mode 100644
index 0ae5557..e4fc79a
*** a/src/test/regress/expected/rowsecurity.out
--- b/src/test/regress/expected/rowsecurity.out
*************** NOTICE:  f_leak => yyyyyy
*** 1420,1425 ****
--- 1420,1505 ----
  (3 rows)
  
  --
+ -- Non-target relations are only subject to SELECT policies
+ --
+ SET SESSION AUTHORIZATION rls_regress_user0;
+ CREATE TABLE r1 (a int);
+ CREATE TABLE r2 (a int);
+ INSERT INTO r1 VALUES (10), (20);
+ INSERT INTO r2 VALUES (10), (20);
+ GRANT ALL ON r1, r2 TO rls_regress_user1;
+ CREATE POLICY p1 ON r1 USING (true);
+ ALTER TABLE r1 ENABLE ROW LEVEL SECURITY;
+ CREATE POLICY p1 ON r2 FOR SELECT USING (true);
+ CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false);
+ CREATE POLICY p3 ON r2 FOR UPDATE USING (false);
+ CREATE POLICY p4 ON r2 FOR DELETE USING (false);
+ ALTER TABLE r2 ENABLE ROW LEVEL SECURITY;
+ SET SESSION AUTHORIZATION rls_regress_user1;
+ SELECT * FROM r1;
+  a  
+ ----
+  10
+  20
+ (2 rows)
+ 
+ SELECT * FROM r2;
+  a  
+ ----
+  10
+  20
+ (2 rows)
+ 
+ -- r2 is read-only
+ INSERT INTO r2 VALUES (2); -- Not allowed
+ ERROR:  new row violates row level security policy for "r2"
+ UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing
+  a 
+ ---
+ (0 rows)
+ 
+ DELETE FROM r2 RETURNING *; -- Deletes nothing
+  a 
+ ---
+ (0 rows)
+ 
+ -- r2 can be used as a non-target relation in DML
+ INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK
+  a  
+ ----
+  11
+  21
+ (2 rows)
+ 
+ UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK
+  a  | a  
+ ----+----
+  12 | 10
+  22 | 20
+ (2 rows)
+ 
+ DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK
+  a  | a  
+ ----+----
+  12 | 10
+  22 | 20
+ (2 rows)
+ 
+ SELECT * FROM r1;
+  a  
+ ----
+  11
+  21
+ (2 rows)
+ 
+ SELECT * FROM r2;
+  a  
+ ----
+  10
+  20
+ (2 rows)
+ 
+ --
  -- S.b. view on top of Row-level security
  --
  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 fdadf99..f3b2c40
*** a/src/test/regress/sql/rowsecurity.sql
--- b/src/test/regress/sql/rowsecurity.sql
*************** DELETE FROM only t1 WHERE f_leak(b) RETU
*** 484,489 ****
--- 484,525 ----
  DELETE FROM t1 WHERE f_leak(b) RETURNING oid, *, t1;
  
  --
+ -- Non-target relations are only subject to SELECT policies
+ --
+ SET SESSION AUTHORIZATION rls_regress_user0;
+ CREATE TABLE r1 (a int);
+ CREATE TABLE r2 (a int);
+ INSERT INTO r1 VALUES (10), (20);
+ INSERT INTO r2 VALUES (10), (20);
+ 
+ GRANT ALL ON r1, r2 TO rls_regress_user1;
+ 
+ CREATE POLICY p1 ON r1 USING (true);
+ ALTER TABLE r1 ENABLE ROW LEVEL SECURITY;
+ 
+ CREATE POLICY p1 ON r2 FOR SELECT USING (true);
+ CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false);
+ CREATE POLICY p3 ON r2 FOR UPDATE USING (false);
+ CREATE POLICY p4 ON r2 FOR DELETE USING (false);
+ ALTER TABLE r2 ENABLE ROW LEVEL SECURITY;
+ 
+ SET SESSION AUTHORIZATION rls_regress_user1;
+ SELECT * FROM r1;
+ SELECT * FROM r2;
+ 
+ -- r2 is read-only
+ INSERT INTO r2 VALUES (2); -- Not allowed
+ UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing
+ DELETE FROM r2 RETURNING *; -- Deletes nothing
+ 
+ -- r2 can be used as a non-target relation in DML
+ INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK
+ UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK
+ DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK
+ SELECT * FROM r1;
+ SELECT * FROM r2;
+ 
+ --
  -- S.b. view on top of Row-level security
  --
  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

Reply via email to