(2010/09/02 13:30), KaiGai Kohei wrote:
> (2010/09/02 12:38), Robert Haas wrote:
>> 2010/9/1 KaiGai Kohei<kai...@ak.jp.nec.com>:
>>> (2010/09/02 11:57), Robert Haas wrote:
>>>> 2010/9/1 KaiGai Kohei<kai...@ak.jp.nec.com>:
>>>>> Right now, it stands on a strict assumption that considers operators
>>>>> implemented with built-in functions are safe; it does not have no
>>>>> possibility to leak supplied arguments anywhere.
>>>>>
>>>>> Please note that this patch does not case about a case when
>>>>> a function inside a view and a function outside a view are
>>>>> distributed into same level and the later function has lower
>>>>> cost value.
>>>>
>>>> Without making some attempt to address these two points, I don't see
>>>> the point of this patch.
>>>>
>>>> Also, I believe we decided previously do this deoptimization only in
>>>> case the user requests it with CREATE SECURITY VIEW.
>>>>
>>> Perhaps, I remember the previous discussion incorrectly.
>>>
>>> If we have a hint about whether the supplied view is intended to security
>>> purpose, or not, it seems to me it is a reliable method to prevent pulling
>>> up the subqueries come from security views.
>>> Is it too much deoptimization?
>>
>> Well, that'd prevent something like id = 3 from getting pushed down,
>> which seems a bit harsh.
>>

I've tried to implement a proof of the concept patch according to the
following logic.
(For the quick hack, it does not include statement support. It just
 considers view with name begun from "s" as security views.)

> Hmm. If so, we need to remember what FromExpr was come from subqueries
> of security views, and what were not. Then, we need to prevent leakable
> clause will be distributed to inside of them.
> In addition, we also need to care about the order of function calls in
> same level, because it is not implicitly solved.
> 
> At first, let's consider top-half of the matter.
> 
> When views are expanded into subqueries in query-rewriter, Query tree
> lost an information OID of the view, because RangeTblEntry does not have
> OID of the relation when it is RTE_SUBQUERY. So, we need to patch here
> to mark a flag whether the supplied view is security focused, or not.
> 
This patch added 'security_view' flag into RangeTblEntry.
It shall be set when the query rewriter expands security views.

> Then, pull_up_simple_subquery() pulls up a supplied subquery into normal
> join, if possible. In this case, FromExpr is chained into the upper level.
> Of course, FromExpr does not have a flag to show its origin, so we also
> need to copy the new flag in RangeTblEntry to FromExpr.
> 
This patch also added 'security_view' flag into FromExpr.
It shall be set when the pull_up_simple_subquery() pulled up
a RangeTblEntry with security_view = true.

> Then, when distribute_qual_to_rels() is called, the caller also provides
> a Bitmapset of relation-Ids which are contained under the FromExpr with
> the flag saying it came from the security views.
> Even if the supplied clause references a part of the Bitmapset, we need
> to prevent the clause being pushed down into the relations came from
> security views, except for ones we can make sure these are safe.
> 
Just before distribute_qual_to_rels(), deconstruct_recurse() is invoked.
It walks on the supplied join-tree to collect what relations are appeared
under the current FromExpr/JoinExpr.
The deconstruct_recurse() was modified to take two new arguments of
'bool below_sec_barriers' and 'Relids *sec_barriers'.
The first one means the current recursion is under the FromExpr with
security_view being true. At that time, it set appeared relations on
the sec_barriers, then returns.
In the result, the 'sec_barriers' shall become a bitmapset of relations
being under the FromExpr which is originated by security views.

Then, 'sec_barriers' shall be delivered to distribute_qual_to_rels().
If the supplied qualifier references a part of 'sec_barriers' and
contains possibly leakable functions, it appends whole of the sec_barriers
to the bitmapset of relations on which the clause is depending.
In the result, it shall not be pushed down into the security view.

Example)
testdb=# CREATE VIEW n_view AS SELECT * FROM t1 JOIN t2 ON t1.a = t2.x;
CREATE VIEW
testdb=# CREATE VIEW s_view AS SELECT * FROM t1 JOIN t2 ON t1.a = t2.x;
CREATE VIEW

testdb=# EXPLAIN SELECT * FROM n_view WHERE f_malicious(y);
                            QUERY PLAN
-------------------------------------------------------------------
 Hash Join  (cost=334.93..365.94 rows=410 width=72)
   Hash Cond: (t1.a = t2.x)
   ->  Seq Scan on t1  (cost=0.00..22.30 rows=1230 width=36)
   ->  Hash  (cost=329.80..329.80 rows=410 width=36)
         ->  Seq Scan on t2  (cost=0.00..329.80 rows=410 width=36)
               Filter: f_malicious(y)
(6 rows)

testdb=# EXPLAIN SELECT * FROM s_view WHERE f_malicious(y);
                            QUERY PLAN
-------------------------------------------------------------------
 Hash Join  (cost=37.68..384.39 rows=410 width=72)
   Hash Cond: (t1.a = t2.x)
   Join Filter: f_malicious(t2.y)
   ->  Seq Scan on t1  (cost=0.00..22.30 rows=1230 width=36)
   ->  Hash  (cost=22.30..22.30 rows=1230 width=36)
         ->  Seq Scan on t2  (cost=0.00..22.30 rows=1230 width=36)
(6 rows)

  ==> f_malicious() was moved to outside of the join loop


testdb=# EXPLAIN SELECT * FROM n_view WHERE f_malicious(y) AND a = 2;
                               QUERY PLAN
-------------------------------------------------------------------------
 Nested Loop  (cost=0.00..16.80 rows=1 width=72)
   ->  Index Scan using t1_pkey on t1  (cost=0.00..8.27 rows=1 width=36)
         Index Cond: (a = 2)
   ->  Index Scan using t2_pkey on t2  (cost=0.00..8.52 rows=1 width=36)
         Index Cond: (x = 2)
         Filter: f_malicious(y)
(6 rows)

testdb=# EXPLAIN SELECT * FROM s_view WHERE f_malicious(y) AND a = 2;
                               QUERY PLAN
-------------------------------------------------------------------------
 Nested Loop  (cost=0.00..16.80 rows=1 width=72)
   Join Filter: f_malicious(t2.y)
   ->  Index Scan using t1_pkey on t1  (cost=0.00..8.27 rows=1 width=36)
         Index Cond: (a = 2)
   ->  Index Scan using t2_pkey on t2  (cost=0.00..8.27 rows=1 width=36)
         Index Cond: (x = 2)
(6 rows)
  ==> Because 'a = 2' is not leakable, this clause was pushed down into
      the join loop. But f_malicious() is not in 's_view' example.


testdb=# CREATE VIEW n_view2 AS SELECT * FROM t1 WHERE f_policy(a);
CREATE VIEW
testdb=# CREATE VIEW s_view2 AS SELECT * FROM t1 WHERE f_policy(a);
CREATE VIEW
testdb=# EXPLAIN SELECT * FROM n_view2 WHERE f_malicious(b);
                      QUERY PLAN
-------------------------------------------------------
 Seq Scan on t1  (cost=0.00..329.80 rows=137 width=36)
   Filter: (f_malicious(b) AND f_policy(a))
(2 rows)

testdb=# EXPLAIN SELECT * FROM s_view2 WHERE f_malicious(b);
                      QUERY PLAN
-------------------------------------------------------
 Seq Scan on t1  (cost=0.00..329.80 rows=137 width=36)
   Filter: (f_malicious(b) AND f_policy(a))
(2 rows)

  ==> But it does not affect anything on the case when a leakable
      function from outside of the view is chained to same level
      with security policy function of the view.
      In this case, we need to mark functions the original nest
      level, then sort by the nest level rather than cost on the
      order_qual_clauses().

Thanks,
-- 
KaiGai Kohei <kai...@ak.jp.nec.com>
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 1671,1676 **** _copyFromExpr(FromExpr *from)
--- 1671,1677 ----
  
  	COPY_NODE_FIELD(fromlist);
  	COPY_NODE_FIELD(quals);
+ 	COPY_SCALAR_FIELD(security_view);
  
  	return newnode;
  }
***************
*** 1824,1829 **** _copyRangeTblEntry(RangeTblEntry *from)
--- 1825,1831 ----
  	COPY_SCALAR_FIELD(rtekind);
  	COPY_SCALAR_FIELD(relid);
  	COPY_NODE_FIELD(subquery);
+ 	COPY_SCALAR_FIELD(security_view);
  	COPY_SCALAR_FIELD(jointype);
  	COPY_NODE_FIELD(joinaliasvars);
  	COPY_NODE_FIELD(funcexpr);
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
***************
*** 730,735 **** _equalFromExpr(FromExpr *a, FromExpr *b)
--- 730,736 ----
  {
  	COMPARE_NODE_FIELD(fromlist);
  	COMPARE_NODE_FIELD(quals);
+ 	COMPARE_SCALAR_FIELD(security_view);
  
  	return true;
  }
***************
*** 2170,2175 **** _equalRangeTblEntry(RangeTblEntry *a, RangeTblEntry *b)
--- 2171,2177 ----
  	COMPARE_SCALAR_FIELD(rtekind);
  	COMPARE_SCALAR_FIELD(relid);
  	COMPARE_NODE_FIELD(subquery);
+ 	COMPARE_SCALAR_FIELD(security_view);
  	COMPARE_SCALAR_FIELD(jointype);
  	COMPARE_NODE_FIELD(joinaliasvars);
  	COMPARE_NODE_FIELD(funcexpr);
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 1341,1346 **** _outFromExpr(StringInfo str, FromExpr *node)
--- 1341,1347 ----
  
  	WRITE_NODE_FIELD(fromlist);
  	WRITE_NODE_FIELD(quals);
+ 	WRITE_BOOL_FIELD(security_view);
  }
  
  /*****************************************************************************
***************
*** 2120,2125 **** _outRangeTblEntry(StringInfo str, RangeTblEntry *node)
--- 2121,2127 ----
  			break;
  		case RTE_SUBQUERY:
  			WRITE_NODE_FIELD(subquery);
+ 			WRITE_BOOL_FIELD(security_view);
  			break;
  		case RTE_JOIN:
  			WRITE_ENUM_FIELD(jointype, JoinType);
*** a/src/backend/nodes/readfuncs.c
--- b/src/backend/nodes/readfuncs.c
***************
*** 1110,1115 **** _readFromExpr(void)
--- 1110,1116 ----
  
  	READ_NODE_FIELD(fromlist);
  	READ_NODE_FIELD(quals);
+ 	READ_BOOL_FIELD(security_view);
  
  	READ_DONE();
  }
***************
*** 1140,1145 **** _readRangeTblEntry(void)
--- 1141,1147 ----
  			break;
  		case RTE_SUBQUERY:
  			READ_NODE_FIELD(subquery);
+ 			READ_BOOL_FIELD(security_view);
  			break;
  		case RTE_JOIN:
  			READ_ENUM_FIELD(jointype, JoinType);
*** a/src/backend/optimizer/plan/initsplan.c
--- b/src/backend/optimizer/plan/initsplan.c
***************
*** 40,46 **** int			join_collapse_limit;
  
  static List *deconstruct_recurse(PlannerInfo *root, Node *jtnode,
  					bool below_outer_join,
! 					Relids *qualscope, Relids *inner_join_rels);
  static SpecialJoinInfo *make_outerjoininfo(PlannerInfo *root,
  				   Relids left_rels, Relids right_rels,
  				   Relids inner_join_rels,
--- 40,47 ----
  
  static List *deconstruct_recurse(PlannerInfo *root, Node *jtnode,
  					bool below_outer_join,
! 					Relids *qualscope, Relids *inner_join_rels,
! 					bool below_sec_barriers, Relids *sec_barriers);
  static SpecialJoinInfo *make_outerjoininfo(PlannerInfo *root,
  				   Relids left_rels, Relids right_rels,
  				   Relids inner_join_rels,
***************
*** 51,57 **** static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
  						JoinType jointype,
  						Relids qualscope,
  						Relids ojscope,
! 						Relids outerjoin_nonnullable);
  static bool check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
  					  Relids *nullable_relids_p, bool is_pushed_down);
  static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause);
--- 52,59 ----
  						JoinType jointype,
  						Relids qualscope,
  						Relids ojscope,
! 						Relids outerjoin_nonnullable,
! 						Relids sec_barriers);
  static bool check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
  					  Relids *nullable_relids_p, bool is_pushed_down);
  static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause);
***************
*** 231,243 **** deconstruct_jointree(PlannerInfo *root)
  {
  	Relids		qualscope;
  	Relids		inner_join_rels;
  
  	/* Start recursion at top of jointree */
! 	Assert(root->parse->jointree != NULL &&
! 		   IsA(root->parse->jointree, FromExpr));
  
! 	return deconstruct_recurse(root, (Node *) root->parse->jointree, false,
! 							   &qualscope, &inner_join_rels);
  }
  
  /*
--- 233,247 ----
  {
  	Relids		qualscope;
  	Relids		inner_join_rels;
+ 	Relids		sec_barriers;
+ 	FromExpr   *f = (FromExpr *)root->parse->jointree;
  
  	/* Start recursion at top of jointree */
! 	Assert(root->parse->jointree != NULL && IsA(f, FromExpr));
  
! 	return deconstruct_recurse(root, (Node *) f, false,
! 							   &qualscope, &inner_join_rels,
! 							   f->security_view, &sec_barriers);
  }
  
  /*
***************
*** 261,267 **** deconstruct_jointree(PlannerInfo *root)
   */
  static List *
  deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
! 					Relids *qualscope, Relids *inner_join_rels)
  {
  	List	   *joinlist;
  
--- 265,272 ----
   */
  static List *
  deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
! 					Relids *qualscope, Relids *inner_join_rels,
! 					bool below_sec_barriers, Relids *sec_barriers)
  {
  	List	   *joinlist;
  
***************
*** 280,285 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
--- 285,293 ----
  		/* A single baserel does not create an inner join */
  		*inner_join_rels = NULL;
  		joinlist = list_make1(jtnode);
+ 		/* Is it in security barrier? */
+ 		*sec_barriers = (below_sec_barriers ?
+ 						 bms_make_singleton(varno) : NULL);
  	}
  	else if (IsA(jtnode, FromExpr))
  	{
***************
*** 295,300 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
--- 303,309 ----
  		 */
  		*qualscope = NULL;
  		*inner_join_rels = NULL;
+ 		*sec_barriers = NULL;
  		joinlist = NIL;
  		remaining = list_length(f->fromlist);
  		foreach(l, f->fromlist)
***************
*** 302,313 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
  			Relids		sub_qualscope;
  			List	   *sub_joinlist;
  			int			sub_members;
  
  			sub_joinlist = deconstruct_recurse(root, lfirst(l),
  											   below_outer_join,
  											   &sub_qualscope,
! 											   inner_join_rels);
  			*qualscope = bms_add_members(*qualscope, sub_qualscope);
  			sub_members = list_length(sub_joinlist);
  			remaining--;
  			if (sub_members <= 1 ||
--- 311,327 ----
  			Relids		sub_qualscope;
  			List	   *sub_joinlist;
  			int			sub_members;
+ 			Relids		sub_barriers;
  
  			sub_joinlist = deconstruct_recurse(root, lfirst(l),
  											   below_outer_join,
  											   &sub_qualscope,
! 											   inner_join_rels,
! 											   below_sec_barriers ?
! 											   true : f->security_view,
! 											   &sub_barriers);
  			*qualscope = bms_add_members(*qualscope, sub_qualscope);
+ 			*sec_barriers = bms_add_members(*sec_barriers, sub_barriers);
  			sub_members = list_length(sub_joinlist);
  			remaining--;
  			if (sub_members <= 1 ||
***************
*** 336,342 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
  
  			distribute_qual_to_rels(root, qual,
  									false, below_outer_join, JOIN_INNER,
! 									*qualscope, NULL, NULL);
  		}
  	}
  	else if (IsA(jtnode, JoinExpr))
--- 350,356 ----
  
  			distribute_qual_to_rels(root, qual,
  									false, below_outer_join, JOIN_INNER,
! 									*qualscope, NULL, NULL, *sec_barriers);
  		}
  	}
  	else if (IsA(jtnode, JoinExpr))
***************
*** 346,351 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
--- 360,367 ----
  					rightids,
  					left_inners,
  					right_inners,
+ 					left_barriers,
+ 					right_barriers,
  					nonnullable_rels,
  					ojscope;
  		List	   *leftjoinlist,
***************
*** 370,381 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
  			case JOIN_INNER:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   below_outer_join,
! 												   &leftids, &left_inners);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													below_outer_join,
! 													&rightids, &right_inners);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = *qualscope;
  				/* Inner join adds no restrictions for quals */
  				nonnullable_rels = NULL;
  				break;
--- 386,402 ----
  			case JOIN_INNER:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   below_outer_join,
! 												   &leftids, &left_inners,
! 												   below_sec_barriers,
! 												   &left_barriers);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													below_outer_join,
! 													&rightids, &right_inners,
! 													below_sec_barriers,
! 													&right_barriers);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = *qualscope;
+ 				*sec_barriers = bms_union(left_barriers, right_barriers);
  				/* Inner join adds no restrictions for quals */
  				nonnullable_rels = NULL;
  				break;
***************
*** 383,417 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
  			case JOIN_ANTI:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   below_outer_join,
! 												   &leftids, &left_inners);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													true,
! 													&rightids, &right_inners);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = bms_union(left_inners, right_inners);
  				nonnullable_rels = leftids;
  				break;
  			case JOIN_SEMI:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   below_outer_join,
! 												   &leftids, &left_inners);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													below_outer_join,
! 													&rightids, &right_inners);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = bms_union(left_inners, right_inners);
  				/* Semi join adds no restrictions for quals */
  				nonnullable_rels = NULL;
  				break;
  			case JOIN_FULL:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   true,
! 												   &leftids, &left_inners);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													true,
! 													&rightids, &right_inners);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = bms_union(left_inners, right_inners);
  				/* each side is both outer and inner */
  				nonnullable_rels = *qualscope;
  				break;
--- 404,453 ----
  			case JOIN_ANTI:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   below_outer_join,
! 												   &leftids, &left_inners,
! 												   below_sec_barriers,
! 												   &left_barriers);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													true,
! 													&rightids, &right_inners,
! 													below_sec_barriers,
! 													&right_barriers);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = bms_union(left_inners, right_inners);
+ 				*sec_barriers = bms_union(left_barriers, right_barriers);
  				nonnullable_rels = leftids;
  				break;
  			case JOIN_SEMI:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   below_outer_join,
! 												   &leftids, &left_inners,
! 												   below_sec_barriers,
! 												   &left_barriers);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													below_outer_join,
! 													&rightids, &right_inners,
! 													below_sec_barriers,
! 													&right_barriers);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = bms_union(left_inners, right_inners);
+ 				*sec_barriers = bms_union(left_barriers, right_barriers);
  				/* Semi join adds no restrictions for quals */
  				nonnullable_rels = NULL;
  				break;
  			case JOIN_FULL:
  				leftjoinlist = deconstruct_recurse(root, j->larg,
  												   true,
! 												   &leftids, &left_inners,
! 												   below_sec_barriers,
! 												   &left_barriers);
  				rightjoinlist = deconstruct_recurse(root, j->rarg,
  													true,
! 													&rightids, &right_inners,
! 													below_sec_barriers,
! 													&right_barriers);
  				*qualscope = bms_union(leftids, rightids);
  				*inner_join_rels = bms_union(left_inners, right_inners);
+ 				*sec_barriers = bms_union(left_barriers, right_barriers);
  				/* each side is both outer and inner */
  				nonnullable_rels = *qualscope;
  				break;
***************
*** 460,466 **** deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
  			distribute_qual_to_rels(root, qual,
  									false, below_outer_join, j->jointype,
  									*qualscope,
! 									ojscope, nonnullable_rels);
  		}
  
  		/* Now we can add the SpecialJoinInfo to join_info_list */
--- 496,503 ----
  			distribute_qual_to_rels(root, qual,
  									false, below_outer_join, j->jointype,
  									*qualscope,
! 									ojscope, nonnullable_rels,
! 									*sec_barriers);
  		}
  
  		/* Now we can add the SpecialJoinInfo to join_info_list */
***************
*** 754,760 **** distribute_qual_to_rels(PlannerInfo *root, Node *clause,
  						JoinType jointype,
  						Relids qualscope,
  						Relids ojscope,
! 						Relids outerjoin_nonnullable)
  {
  	Relids		relids;
  	bool		is_pushed_down;
--- 791,798 ----
  						JoinType jointype,
  						Relids qualscope,
  						Relids ojscope,
! 						Relids outerjoin_nonnullable,
! 						Relids sec_barriers)
  {
  	Relids		relids;
  	bool		is_pushed_down;
***************
*** 762,767 **** distribute_qual_to_rels(PlannerInfo *root, Node *clause,
--- 800,806 ----
  	bool		pseudoconstant = false;
  	bool		maybe_equivalence;
  	bool		maybe_outer_join;
+ 	bool		maybe_leakable_clause = false;
  	Relids		nullable_relids;
  	RestrictInfo *restrictinfo;
  
***************
*** 778,783 **** distribute_qual_to_rels(PlannerInfo *root, Node *clause,
--- 817,824 ----
  		elog(ERROR, "JOIN qualification cannot refer to other relations");
  	if (ojscope && !bms_is_subset(relids, ojscope))
  		elog(ERROR, "JOIN qualification cannot refer to other relations");
+ 	if (sec_barriers && !bms_is_subset(sec_barriers, qualscope))
+ 		elog(ERROR, "Bug? incorrect security barriers");
  
  	/*
  	 * If the clause is variable-free, our normal heuristic for pushing it
***************
*** 834,839 **** distribute_qual_to_rels(PlannerInfo *root, Node *clause,
--- 875,894 ----
  		}
  	}
  
+ 	/*
+ 	 * If the clause contains a leakable function, it may be used to
+ 	 * bypass row-level security using views. In this case, we don't
+ 	 * push down the clause not to evaluate the leakable clause prior
+ 	 * to the row-level policy functions.
+ 	 */
+ 	if (!bms_is_empty(sec_barriers) &&
+ 		contain_leakable_functions(clause) &&
+ 		bms_overlap(relids, sec_barriers))
+ 	{
+ 		maybe_leakable_clause = true;
+ 		relids = bms_add_members(relids, sec_barriers);
+ 	}
+ 
  	/*----------
  	 * Check to see if clause application must be delayed by outer-join
  	 * considerations.
***************
*** 1030,1036 **** distribute_qual_to_rels(PlannerInfo *root, Node *clause,
  	 * If none of the above hold, pass it off to
  	 * distribute_restrictinfo_to_rels().
  	 */
! 	if (restrictinfo->mergeopfamilies)
  	{
  		if (maybe_equivalence)
  		{
--- 1085,1091 ----
  	 * If none of the above hold, pass it off to
  	 * distribute_restrictinfo_to_rels().
  	 */
! 	if (!maybe_leakable_clause && restrictinfo->mergeopfamilies)
  	{
  		if (maybe_equivalence)
  		{
***************
*** 1359,1365 **** process_implied_equality(PlannerInfo *root,
  	 */
  	distribute_qual_to_rels(root, (Node *) clause,
  							true, below_outer_join, JOIN_INNER,
! 							qualscope, NULL, NULL);
  }
  
  /*
--- 1414,1420 ----
  	 */
  	distribute_qual_to_rels(root, (Node *) clause,
  							true, below_outer_join, JOIN_INNER,
! 							qualscope, NULL, NULL, NULL);
  }
  
  /*
*** a/src/backend/optimizer/prep/prepjointree.c
--- b/src/backend/optimizer/prep/prepjointree.c
***************
*** 845,850 **** pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
--- 845,856 ----
  	 */
  
  	/*
+ 	 * If the subquery is originated from security view, we mark it here
+ 	 * to prevent over optimization later.
+ 	 */
+ 	((FromExpr *) subquery->jointree)->security_view = rte->security_view;
+ 
+ 	/*
  	 * Return the adjusted subquery jointree to replace the RangeTblRef entry
  	 * in parent's jointree.
  	 */
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
***************
*** 86,91 **** static bool contain_subplans_walker(Node *node, void *context);
--- 86,92 ----
  static bool contain_mutable_functions_walker(Node *node, void *context);
  static bool contain_volatile_functions_walker(Node *node, void *context);
  static bool contain_nonstrict_functions_walker(Node *node, void *context);
+ static bool contain_leakable_functions_walker(Node *node, void *context);
  static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
  static List *find_nonnullable_vars_walker(Node *node, bool top_level);
  static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
***************
*** 1129,1134 **** contain_nonstrict_functions_walker(Node *node, void *context)
--- 1130,1249 ----
  }
  
  
+ 
+ /*****************************************************************************
+  *		Check clauses for leakable functions
+  *****************************************************************************/
+ 
+ /*
+  * contain_leakable_functions
+  *	  Recursively search for leakable functions within a clause.
+  *
+  * Returns true if any function call with side-effect is found.
+  * ie, some type-input/output handler will raise an error when given
+  *     argument does not have a valid format.
+  *
+  * When people uses views for row-level security purpose, given qualifiers
+  * come from outside of the view should not be pushed down into the views,
+  * if they have side-effect, because contents of tuples to be filtered out
+  * may be leaked via side-effectable functions within the qualifiers.
+  * 
+  * The idea here is that the planner restrain a part of optimization when
+  * the qualifiers contains leakable functions.
+  * This routine checks whether the given clause contains leakable functions,
+  * or not. If we return false, then the clause is clean.
+  */
+ bool
+ contain_leakable_functions(Node *clause)
+ {
+ 	return contain_leakable_functions_walker(clause, NULL);
+ }
+ 
+ static bool
+ contain_leakable_functions_walker(Node *node, void *context)
+ {
+ 	if (node == NULL)
+ 		return false;
+ 
+ 	if (IsA(node, FuncExpr))
+ 	{
+ 		/*
+ 		 * currently, we have no way to distinguish a safe function and
+ 		 * a leakable one, so all the function call shall be considered
+ 		 * as leakable one.
+ 		 */
+ 		return true;
+ 	}
+ 	else if (IsA(node, OpExpr))
+ 	{
+ 		OpExpr	   *expr = (OpExpr *) node;
+ 
+ 		/*
+ 		 * we assume built-in functions to implement operators are not
+ 		 * leakable, so don't need to prevent optimization.
+ 		 */
+ 		set_opfuncid(expr);
+ 		if (get_func_lang(expr->opfuncid) != INTERNALlanguageId)
+ 			return true;
+ 		/* else fall through to check args */
+ 	}
+ 	else if (IsA(node, DistinctExpr))
+ 	{
+ 		DistinctExpr *expr = (DistinctExpr *) node;
+ 
+ 		set_opfuncid((OpExpr *) expr);
+ 		if (get_func_lang(expr->opfuncid) != INTERNALlanguageId)
+ 			return true;
+ 		/* else fall through to check args */
+ 	}
+ 	else if (IsA(node, ScalarArrayOpExpr))
+ 	{
+ 		ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
+ 
+ 		set_sa_opfuncid(expr);
+ 		if (get_func_lang(expr->opfuncid) != INTERNALlanguageId)
+ 			return true;
+ 		/* else fall through to check args */
+ 	}
+ 	else if (IsA(node, CoerceViaIO) ||
+ 			 IsA(node, ArrayCoerceExpr))
+ 	{
+ 		/*
+ 		 * we assume type-in/out handlers are leakable, even if built-in
+ 		 * functions. 
+ 		 * ie, int4in() raises an error message with given argument,
+ 		 *     if it does not have valid format for numeric value.
+ 		 */
+ 		return true;
+ 	}
+ 	else if (IsA(node, NullIfExpr))
+ 	{
+ 		NullIfExpr *expr = (NullIfExpr *) node;
+ 
+ 		set_opfuncid((OpExpr *) expr);	/* rely on struct equivalence */
+ 		if (get_func_lang(expr->opfuncid) != INTERNALlanguageId)
+ 			return true;
+ 		/* else fall through to check args */
+ 	}
+ 	else if (IsA(node, RowCompareExpr))
+ 	{
+ 		/* RowCompare probably can't have volatile ops, but check anyway */
+ 		RowCompareExpr *rcexpr = (RowCompareExpr *) node;
+ 		ListCell   *opid;
+ 
+ 		foreach(opid, rcexpr->opnos)
+ 		{
+ 			Oid		funcId = get_opcode(lfirst_oid(opid));
+ 
+ 			if (get_func_lang(funcId) != INTERNALlanguageId)
+ 				return true;
+ 		}
+ 		/* else fall through to check args */
+ 	}
+ 	return expression_tree_walker(node, contain_leakable_functions_walker,
+ 								  context);
+ }
+ 
  /*
   * find_nonnullable_rels
   *		Determine which base rels are forced nonnullable by given clause.
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
***************
*** 1189,1194 **** ApplyRetrieveRule(Query *parsetree,
--- 1189,1202 ----
  	rte->inh = false;			/* must not be set for a subquery */
  
  	/*
+ 	 * XXX - Right now, we just assume views that its name started
+ 	 *       from "s" are security view, because of quick hack.
+ 	 */
+ 	if (RelationGetForm(relation)->relkind == RELKIND_VIEW &&
+ 		strncmp(RelationGetRelationName(relation), "s", 1) == 0)
+ 		rte->security_view = true;
+ 
+ 	/*
  	 * We move the view's permission check data down to its rangetable. The
  	 * checks will actually be done against the OLD entry therein.
  	 */
*** a/src/backend/utils/cache/lsyscache.c
--- b/src/backend/utils/cache/lsyscache.c
***************
*** 1275,1280 **** get_func_namespace(Oid funcid)
--- 1275,1299 ----
  }
  
  /*
+  * get_func_lang
+  *		Given procedure id, return the function's language
+  */
+ Oid
+ get_func_lang(Oid funcid)
+ {
+ 	HeapTuple	tp;
+ 	Oid			result;
+ 
+ 	tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid));
+ 	if (!HeapTupleIsValid(tp))
+ 		elog(ERROR, "cache lookup failed for function %u", funcid);
+ 
+ 	result = ((Form_pg_proc) GETSTRUCT(tp))->prolang;
+ 	ReleaseSysCache(tp);
+ 	return result;
+ }
+ 
+ /*
   * get_func_rettype
   *		Given procedure id, return the function's result type.
   */
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 684,689 **** typedef struct RangeTblEntry
--- 684,691 ----
  	 */
  	Query	   *subquery;		/* the sub-query */
  
+ 	bool		security_view;	/* Is the sub-query come from security view? */
+ 
  	/*
  	 * Fields valid for a join RTE (else NULL/zero):
  	 *
*** a/src/include/nodes/primnodes.h
--- b/src/include/nodes/primnodes.h
***************
*** 1208,1213 **** typedef struct FromExpr
--- 1208,1214 ----
  	NodeTag		type;
  	List	   *fromlist;		/* List of join subtrees */
  	Node	   *quals;			/* qualifiers on join, if any */
+ 	bool		security_view;	/* It came from security views */
  } FromExpr;
  
  #endif   /* PRIMNODES_H */
*** a/src/include/optimizer/clauses.h
--- b/src/include/optimizer/clauses.h
***************
*** 67,72 **** extern bool contain_subplans(Node *clause);
--- 67,73 ----
  extern bool contain_mutable_functions(Node *clause);
  extern bool contain_volatile_functions(Node *clause);
  extern bool contain_nonstrict_functions(Node *clause);
+ extern bool contain_leakable_functions(Node *clause);
  extern Relids find_nonnullable_rels(Node *clause);
  extern List *find_nonnullable_vars(Node *clause);
  extern List *find_forced_null_vars(Node *clause);
*** a/src/include/utils/lsyscache.h
--- b/src/include/utils/lsyscache.h
***************
*** 77,82 **** extern RegProcedure get_oprrest(Oid opno);
--- 77,83 ----
  extern RegProcedure get_oprjoin(Oid opno);
  extern char *get_func_name(Oid funcid);
  extern Oid	get_func_namespace(Oid funcid);
+ extern Oid	get_func_lang(Oid funcid);
  extern Oid	get_func_rettype(Oid funcid);
  extern int	get_func_nargs(Oid funcid);
  extern Oid	get_func_signature(Oid funcid, Oid **argtypes, int *nargs);
-- 
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