pgsql-v9.2-fix-leaky-view-part-3.v3.patch

application/octet-stream

Filename: pgsql-v9.2-fix-leaky-view-part-3.v3.patch
Type: application/octet-stream
Part: 1
Message: Re: [v9.2] Fix Leaky View Problem

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
Series: patch v9
File+
src/backend/nodes/copyfuncs.c 1 0
src/backend/nodes/equalfuncs.c 1 0
src/backend/nodes/outfuncs.c 1 0
src/backend/nodes/readfuncs.c 1 0
src/backend/optimizer/plan/initsplan.c 144 7
src/backend/optimizer/prep/prepjointree.c 10 1
src/backend/optimizer/util/clauses.c 120 0
src/backend/utils/cache/lsyscache.c 19 0
src/include/nodes/primnodes.h 1 0
src/include/optimizer/clauses.h 1 0
src/include/utils/lsyscache.h 1 0
 src/backend/nodes/copyfuncs.c             |    1 +
 src/backend/nodes/equalfuncs.c            |    1 +
 src/backend/nodes/outfuncs.c              |    1 +
 src/backend/nodes/readfuncs.c             |    1 +
 src/backend/optimizer/plan/initsplan.c    |  151 +++++++++++++++++++++++++++--
 src/backend/optimizer/prep/prepjointree.c |   11 ++-
 src/backend/optimizer/util/clauses.c      |  120 +++++++++++++++++++++++
 src/backend/utils/cache/lsyscache.c       |   19 ++++
 src/include/nodes/primnodes.h             |    1 +
 src/include/optimizer/clauses.h           |    1 +
 src/include/utils/lsyscache.h             |    1 +
 11 files changed, 300 insertions(+), 8 deletions(-)

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 4e61789..7d69731 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1792,6 +1792,7 @@ _copyFromExpr(FromExpr *from)
 
 	COPY_NODE_FIELD(fromlist);
 	COPY_NODE_FIELD(quals);
+	COPY_SCALAR_FIELD(security_barrier);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index abef8a7..f2188f4 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -780,6 +780,7 @@ _equalFromExpr(FromExpr *a, FromExpr *b)
 {
 	COMPARE_NODE_FIELD(fromlist);
 	COMPARE_NODE_FIELD(quals);
+	COMPARE_SCALAR_FIELD(security_barrier);
 
 	return true;
 }
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 16a0239..0795d88 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1448,6 +1448,7 @@ _outFromExpr(StringInfo str, FromExpr *node)
 
 	WRITE_NODE_FIELD(fromlist);
 	WRITE_NODE_FIELD(quals);
+	WRITE_BOOL_FIELD(security_barrier);
 }
 
 /*****************************************************************************
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index c4f934d..9e2fad8 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1169,6 +1169,7 @@ _readFromExpr(void)
 
 	READ_NODE_FIELD(fromlist);
 	READ_NODE_FIELD(quals);
+	READ_BOOL_FIELD(security_barrier);
 
 	READ_DONE();
 }
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 5b170b3..0ed72aa 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -40,7 +40,9 @@ static SpecialJoinInfo *make_outerjoininfo(PlannerInfo *root,
 				   Relids left_rels, Relids right_rels,
 				   Relids inner_join_rels,
 				   JoinType jointype, List *clause);
-static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
+static void distribute_qual_to_rels(PlannerInfo *root,
+						Node *clause,
+						Node *jtnode,
 						bool is_deduced,
 						bool below_outer_join,
 						JoinType jointype,
@@ -52,7 +54,7 @@ static bool check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
 static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause);
 static void check_mergejoinable(RestrictInfo *restrictinfo);
 static void check_hashjoinable(RestrictInfo *restrictinfo);
-
+static bool check_security_barrier(Expr *restrictinfo, Node *jtnode);
 
 /*****************************************************************************
  *
@@ -345,7 +347,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 		{
 			Node	   *qual = (Node *) lfirst(l);
 
-			distribute_qual_to_rels(root, qual,
+			distribute_qual_to_rels(root, qual, jtnode,
 									false, below_outer_join, JOIN_INNER,
 									*qualscope, NULL, NULL);
 		}
@@ -468,7 +470,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 		{
 			Node	   *qual = (Node *) lfirst(l);
 
-			distribute_qual_to_rels(root, qual,
+			distribute_qual_to_rels(root, qual, jtnode,
 									false, below_outer_join, j->jointype,
 									*qualscope,
 									ojscope, nonnullable_rels);
@@ -769,6 +771,7 @@ make_outerjoininfo(PlannerInfo *root,
  *	  EquivalenceClasses.
  *
  * 'clause': the qual clause to be distributed
+ * 'jtnode' : either FromExpr or JoinExpr above 'clause' being chained
  * 'is_deduced': TRUE if the qual came from implied-equality deduction
  * 'below_outer_join': TRUE if the qual is from a JOIN/ON that is below the
  *		nullable side of a higher-level outer join
@@ -789,7 +792,9 @@ make_outerjoininfo(PlannerInfo *root,
  * all and only those special joins that are syntactically below this qual.
  */
 static void
-distribute_qual_to_rels(PlannerInfo *root, Node *clause,
+distribute_qual_to_rels(PlannerInfo *root,
+						Node *clause,
+						Node *jtnode,
 						bool is_deduced,
 						bool below_outer_join,
 						JoinType jointype,
@@ -803,6 +808,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 	bool		pseudoconstant = false;
 	bool		maybe_equivalence;
 	bool		maybe_outer_join;
+	bool		barrier_touched;
 	Relids		nullable_relids;
 	RestrictInfo *restrictinfo;
 
@@ -1018,6 +1024,10 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 									 pseudoconstant,
 									 relids,
 									 nullable_relids);
+	/*
+	 * Check variable references across security barrier
+	 */
+	barrier_touched = check_security_barrier((Expr *)restrictinfo, jtnode);
 
 	/*
 	 * If it's a join clause (either naturally, or because delayed by
@@ -1083,7 +1093,8 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 	{
 		if (maybe_equivalence)
 		{
-			if (process_equivalence(root, restrictinfo, below_outer_join))
+			if (!barrier_touched &&
+				process_equivalence(root, restrictinfo, below_outer_join))
 				return;
 			/* EC rejected it, so set left_ec/right_ec the hard way ... */
 			initialize_mergeclause_eclasses(root, restrictinfo);
@@ -1419,7 +1430,7 @@ process_implied_equality(PlannerInfo *root,
 	/*
 	 * Push the new clause into all the appropriate restrictinfo lists.
 	 */
-	distribute_qual_to_rels(root, (Node *) clause,
+	distribute_qual_to_rels(root, (Node *) clause, NULL,
 							true, below_outer_join, JOIN_INNER,
 							qualscope, NULL, NULL);
 }
@@ -1475,6 +1486,132 @@ build_implied_join_equality(Oid opno,
 	return restrictinfo;
 }
 
+/*
+ * walk_security_barrier
+ *
+ * is a helper routine of check_security_barrier
+ */
+static Relids
+walk_security_barrier(Node *jtnode, bool top_level,
+					  Relids *relids, bool *touched)
+{
+	if (IsA(jtnode, RangeTblRef))
+	{
+		int		varno = ((RangeTblRef *) jtnode)->rtindex;
+
+		return bms_make_singleton(varno);
+	}
+	else if (IsA(jtnode, FromExpr))
+	{
+		FromExpr   *f = (FromExpr *)jtnode;
+		ListCell   *l;
+		Relids		temp;
+		Relids		results = NULL;
+
+		foreach(l, f->fromlist)
+		{
+			temp = walk_security_barrier(lfirst(l), false, relids, touched);
+
+			results = bms_join(results, temp);
+		}
+
+		/*
+		 * If this walker routine goes across security barrier view,
+		 * we check whether the supplied relids references relations
+		 * across security barrier. If it referenced them, we expand
+		 * relids into whole of the security barrier to prevent
+		 * unexpected push-down of the qualifier.
+		 */
+		if (f->security_barrier && !top_level)
+		{
+			if (bms_overlap(*relids, results))
+			{
+				*relids = bms_union(*relids, results);
+				*touched = true;
+			}
+		}
+		return results;
+	}
+	else if (IsA(jtnode, JoinExpr))
+	{
+		JoinExpr   *j = (JoinExpr *)jtnode;
+		Relids		relids_r;
+		Relids		relids_l;
+
+		relids_r = walk_security_barrier(j->rarg, false, relids, touched);
+		relids_l = walk_security_barrier(j->larg, false, relids, touched);
+
+		return bms_join(relids_r, relids_l);
+	}
+	elog(ERROR, "unexpected node type: %d", (int) nodeTag(jtnode));
+	return NULL;	/* for compiler quiet */
+}
+
+/*
+ * check_security_barrier
+ *
+ * It checks whether the supplied RestrictInfo tried to reference
+ * relations from outside of security barrier; excepr for the clause
+ * that does not contain any leakable functions.
+ * If it touches any relations inside of security barriers from outside
+ * of them, Relids of RestrictInfo shall be expanded and returns true.
+ * It enables to prevent unexpected pushing-down of qualifiers that
+ * potentially cause security breakage.
+ */
+static bool
+check_security_barrier(Expr *restrictinfo, Node *jtnode)
+{
+	bool		result = false;
+
+	if (!jtnode)
+		return false;
+
+	if (IsA(restrictinfo, RestrictInfo))
+	{
+		RestrictInfo   *rinfo = (RestrictInfo *)restrictinfo;
+
+		if (!contain_leakable_functions((Node *)rinfo->clause))
+			return false;
+
+		if (rinfo->orclause &&
+			check_security_barrier((Expr *)rinfo->orclause, jtnode))
+			result = true;
+
+		if (rinfo->left_relids)
+			walk_security_barrier(jtnode, true,
+								  &rinfo->left_relids, &result);
+		if (rinfo->right_relids)
+			walk_security_barrier(jtnode, true,
+								  &rinfo->right_relids, &result);
+		if (rinfo->clause_relids)
+			walk_security_barrier(jtnode, true,
+								  &rinfo->clause_relids, &result);
+		if (rinfo->required_relids)
+			walk_security_barrier(jtnode, true,
+								  &rinfo->required_relids, &result);
+	}
+	else if (IsA(restrictinfo, BoolExpr))
+	{
+		BoolExpr   *b = (BoolExpr *)restrictinfo;
+
+		if (check_security_barrier((Expr *)b->args, jtnode))
+			result = true;
+	}
+	else if (IsA(restrictinfo, List))
+	{
+		ListCell   *l;
+
+		foreach (l, (List *)restrictinfo)
+		{
+			if (check_security_barrier(lfirst(l), jtnode))
+				result = true;
+		}
+	}
+	else
+		elog(ERROR, "unexpected node tag (%d)", (int)nodeTag(restrictinfo));
+
+	return result;
+}
 
 /*****************************************************************************
  *
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 4bd9956..1336812 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -671,6 +671,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 	int			rtoffset;
 	pullup_replace_vars_context rvcontext;
 	ListCell   *lc;
+	bool		security_barrier = false;
 
 	/*
 	 * Need a modifiable copy of the subquery to hack on.  Even if we didn't
@@ -724,7 +725,8 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 
 		class_form = (Form_pg_class) GETSTRUCT(tup);
 		Assert(class_form->relkind == RELKIND_VIEW);
-		if (class_form->relissecbarrier)
+		security_barrier = class_form->relissecbarrier;
+		if (security_barrier)
 			subquery_depth += 1;
 
 		ReleaseSysCache(tup);
@@ -757,6 +759,13 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 						   NULL, NULL, subquery_depth);
 
 	/*
+	 * The security_barrier flag shall be referenced later, to determine
+	 * whether the sub-query originally come from a view definition being
+	 * performed as security barrier.
+	 */
+	subquery->jointree->security_barrier = security_barrier;
+
+	/*
 	 * Now we must recheck whether the subquery is still simple enough to pull
 	 * up.	If not, abandon processing it.
 	 *
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 6df99f6..af226ea 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -92,6 +92,7 @@ static bool contain_subplans_walker(Node *node, void *context);
 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,6 +1130,125 @@ contain_nonstrict_functions_walker(Node *node, void *context)
 								  context);
 }
 
+/*****************************************************************************
+ *        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 restrains 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))
+	{
+		FuncExpr *expr = (FuncExpr *) node;
+
+		if (!get_func_leakproof(expr->funcid))
+			return true;
+	}
+	else if (IsA(node, OpExpr))
+	{
+		OpExpr *expr = (OpExpr *) node;
+
+		set_opfuncid(expr);
+		if (!get_func_leakproof(expr->opfuncid))
+			return true;
+	}
+	else if (IsA(node, DistinctExpr))
+	{
+		DistinctExpr *expr = (DistinctExpr *) node;
+
+		set_opfuncid((OpExpr *) expr);
+		if (!get_func_leakproof(expr->opfuncid))
+			return true;
+	}
+	else if (IsA(node, ScalarArrayOpExpr))
+	{
+		ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
+
+		set_sa_opfuncid(expr);
+		if (!get_func_leakproof(expr->opfuncid))
+			return true;
+	}
+	else if (IsA(node, CoerceViaIO))
+	{
+		CoerceViaIO *expr = (CoerceViaIO *) node;
+		Oid		funcid;
+		Oid		ioparam;
+		bool	varlena;
+
+		getTypeInputInfo(exprType((Node *)expr->arg), &funcid, &ioparam);
+		if (!get_func_leakproof(funcid))
+			return true;
+
+		getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
+		if (!get_func_leakproof(funcid))
+			return true;
+	}
+	else if (IsA(node, ArrayCoerceExpr))
+	{
+		ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
+		Oid			funcid;
+		Oid			ioparam;
+		bool		varlena;
+
+		getTypeInputInfo(exprType((Node *)expr->arg), &funcid, &ioparam);
+		if (!get_func_leakproof(funcid))
+			return true;
+		getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
+		if (!get_func_leakproof(funcid))
+			return true;
+	}
+	else if (IsA(node, NullIfExpr))
+	{
+		NullIfExpr *expr = (NullIfExpr *) node;
+
+		set_opfuncid((OpExpr *) expr);  /* rely on struct equivalence */
+		if (!get_func_leakproof(expr->opfuncid))
+			return true;
+	}
+	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_leakproof(funcid))
+				return true;
+		}
+	}
+	return expression_tree_walker(node, contain_leakable_functions_walker,
+								  context);
+}
 
 /*
  * find_nonnullable_rels
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 326f1ee..6180dc9 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -1534,6 +1534,25 @@ func_volatile(Oid funcid)
 }
 
 /*
+ * get_func_leakproof
+ *		Given procedure id, return the function's leakproof field.
+ */
+bool
+get_func_leakproof(Oid funcid)
+{
+	HeapTuple	tp;
+	bool		result;
+
+	tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid));
+	if (!HeapTupleIsValid(tp))
+		elog(ERROR, "cache lookup failed for function %u", funcid);
+
+	result = ((Form_pg_proc) GETSTRUCT(tp))->proleakproof;
+	ReleaseSysCache(tp);
+	return result;
+}
+
+/*
  * get_func_cost
  *		Given procedure id, return the function's procost field.
  */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index b626386..274495f 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1265,6 +1265,7 @@ typedef struct FromExpr
 	NodeTag		type;
 	List	   *fromlist;		/* List of join subtrees */
 	Node	   *quals;			/* qualifiers on join, if any */
+	bool		security_barrier;	/* Come from security-barrier view? */
 } FromExpr;
 
 #endif   /* PRIMNODES_H */
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
index 4cef7fa..7dc3657 100644
--- a/src/include/optimizer/clauses.h
+++ b/src/include/optimizer/clauses.h
@@ -61,6 +61,7 @@ extern bool contain_subplans(Node *clause);
 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);
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 2159515..1100a2d 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -91,6 +91,7 @@ extern Oid	get_func_signature(Oid funcid, Oid **argtypes, int *nargs);
 extern bool get_func_retset(Oid funcid);
 extern bool func_strict(Oid funcid);
 extern char func_volatile(Oid funcid);
+extern bool get_func_leakproof(Oid funcid);
 extern float4 get_func_cost(Oid funcid);
 extern float4 get_func_rows(Oid funcid);
 extern Oid	get_relname_relid(const char *relname, Oid relnamespace);