Hi all
I've hit an interesting wrinkle and am interested in opinions. By
integrating updatable security barrier view support with row-security,
it has become a lot harder to detect and prevent infinite recursion
(including mutual recursion) in row-security policy expansion.
The code is attached, but it's not an independent patch so it's way
easier to grab it from git:
g...@github.com:ringerc/postgres.git
branch rls-9.4-upd-sb-views (subject to rebase); or
tag rls-9.4-upd-sb-views-v1
(Only contains half the old row-security patch; I'll rebase the docs,
tests, etc on top of this once it's working properly).
I've integrated the updatable security barrier view support into RLS by
injecting securityQuals in subquery_planner() just before
preprocess_rowmarks . (I'm still thinking about some inheritance related
aspects to that, but that's for later).
The problem is that this causes infinite recursion - the securityQuals
get expanded into a subquery over the original RTE that had the
row-security policy on it. Then subquery_planner is re-entered when
processing the subquery, a row-security qual is found on the target RTE,
and ... *boom*.
Fixing this is not as simple as suppressing expansion of row-security
policy when processing a security barrier subquery created by a
row-security policy, as it is desirable to respect the row-security
policy of *other* tables that get referenced in the expanded
row-security qual.
If we just record the relid of the relation a subquery was expanded from
and avoid expanding that inside the generated subquery we prevent simple
linear recursion, but not mutual recursion between two or more rels with
row-security policy.
KaiGai's row-security patch handles this because it does its own
recursive expansion, so (like the rewriter) it can keep a breadcrumb
trail and detect when it is repeating a path. That's not so practical
when row-security code tags RTEs with policy, then updatable s.b. views
goes and expands them.
So. Options.
1.Come up with a reasonable way to track recursive row-security
expansion, detect infinite recursion, and bail out. Likely to involve
a new global structure with planner/optimizer lifetime that gets
checked and maintained by apply_row_security_rangetbl.
2.Solve the linear recursion case by storing a relid that should not get
expanded for security quals when processing a subquery. Say "don't do
that, expect stack exceeded crashes if you do" for the mutual
recursion case. Seems user hostile, incomplete, and likely to break
people's systems when they really don't expect it.
3.Disregard row-security policy on referenced tables when expanding
row-security qualifiers. There's precendent here in foreign keys,
which ignore row-security policy, but I don't think this is at all
appealing.
4.Magic?
Unless someone has a suggestion for #4, I'll be going with #1. I'd
appreciate tips on doing this in a sane and efficient manner if anyone
has them. I'll be reading over the rewriter's infinite loop protection
and that of KaiGai's RLS patch for ideas in the mean time, and will give
it a good go.
BTW, I feel like we should be letting the rewriter do this job; it's
good at dealing with recursion problems already. That won't work so long
as security barrier qual expansion happens during planning, not rewrite,
though - and we've already explored the fun problems with doing upd.
s.b. qual expansion in rewrite.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 4f8ed71e4cf18db7a4285da9b9c9f8f7f1030e32 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Fri, 24 Jan 2014 16:18:40 +0800
Subject: [PATCH] RLS: First attempt to integrate with updatable s.b. views
Enters infinite recursion because the rls-protected table gets expanded
and the new RTE gets checked for row-security quals, gets securityQuals added,
and ... infinite recursion!
---
src/backend/optimizer/plan/planner.c | 7 ++
src/backend/optimizer/prep/prepunion.c | 6 +
src/backend/optimizer/util/Makefile | 3 +-
src/backend/optimizer/util/rowsecurity.c | 185 +++++++++++++++++++++++++++++++
src/include/optimizer/rowsecurity.h | 27 +++++
5 files changed, 227 insertions(+), 1 deletion(-)
create mode 100644 src/backend/optimizer/util/rowsecurity.c
create mode 100644 src/include/optimizer/rowsecurity.h
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 80940ea..8cdd580 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -388,6 +388,13 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
}
/*
+ * Check RTEs for row-security policies and set securityQuals on the
+ * RTE if a policy is found. This must happen before inherited table
+ * expansion so that the quals get copied to the child rels.
+ */
+ apply_row_security_policy(root);
+
+ /*
* Preprocess RowMark information. We need to do this after subquery
* pullup (so that all non-inherited RTEs are present) and before
* inheritance expansion (so that the info is available for
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index c7e0199..20a634d 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1349,6 +1349,12 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
childRTindex = list_length(parse->rtable);
/*
+ * Check for row-security quals on the newly added child
+ * relation and update securityQuals as appropriate.
+ */
+ apply_row_security_rte(root, childRTindex);
+
+ /*
* Build an AppendRelInfo for this parent and child.
*/
appinfo = makeNode(AppendRelInfo);
diff --git a/src/backend/optimizer/util/Makefile b/src/backend/optimizer/util/Makefile
index c54d0a6..d42a285 100644
--- a/src/backend/optimizer/util/Makefile
+++ b/src/backend/optimizer/util/Makefile
@@ -13,6 +13,7 @@ top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
OBJS = clauses.o joininfo.o orclauses.o pathnode.o placeholder.o \
- plancat.o predtest.o relnode.o restrictinfo.o tlist.o var.o
+ plancat.o predtest.o relnode.o restrictinfo.o tlist.o var.o \
+ rowsecurity.o
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/optimizer/util/rowsecurity.c b/src/backend/optimizer/util/rowsecurity.c
new file mode 100644
index 0000000..f002891
--- /dev/null
+++ b/src/backend/optimizer/util/rowsecurity.c
@@ -0,0 +1,185 @@
+/*
+ * rewrite/rowsecurity.c
+ * Routines to support row-security feature
+ *
+ * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ */
+#include "postgres.h"
+
+#include "access/heapam.h"
+#include "access/htup_details.h"
+#include "access/sysattr.h"
+#include "catalog/pg_class.h"
+#include "catalog/pg_inherits_fn.h"
+#include "catalog/pg_rowsecurity.h"
+#include "catalog/pg_type.h"
+#include "miscadmin.h"
+#include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.h"
+#include "nodes/plannodes.h"
+#include "optimizer/clauses.h"
+#include "optimizer/prep.h"
+#include "optimizer/rowsecurity.h"
+#include "parser/parsetree.h"
+#include "rewrite/rewriteHandler.h"
+#include "utils/lsyscache.h"
+#include "utils/rel.h"
+#include "utils/syscache.h"
+#include "tcop/utility.h"
+
+/* hook to allow extensions to apply their own security policy */
+row_security_policy_hook_type row_security_policy_hook = NULL;
+
+/*
+ * pull_row_security_policy
+ *
+ * Fetches the configured row-security policy of both built-in catalogs and any
+ * extensions. If any policy is found a list of qualifier expressions is
+ * returned, where each is treated as a securityQual.
+ */
+static List *
+pull_row_security_policy(CmdType cmd, Relation relation)
+{
+ List *quals = NIL;
+ Expr *qual = NULL;
+
+ /*
+ * Pull the row-security policy configured with built-in features,
+ * if unprivileged users. Please note that superuser can bypass it.
+ */
+ if (relation->rsdesc && !superuser())
+ {
+ RowSecurityDesc *rsdesc = relation->rsdesc;
+ qual = copyObject(rsdesc->rsall.qual);
+ quals = lcons(qual, quals);
+ }
+
+ /*
+ * Also, ask extensions whether they want to apply their own
+ * row-security policy. If both built-in and extension has
+ * their own policy they're applied as nested qualifiers.
+ */
+ if (row_security_policy_hook)
+ {
+ List *temp;
+
+ temp = (*row_security_policy_hook)(cmd, relation);
+ if (temp != NIL)
+ lcons(temp, quals);
+ }
+ return quals;
+}
+
+/*
+ * apply_row_security_rte
+ *
+ * Apply row-security policy to a RangeTblEntry if policy for the
+ * RTE's Relation exists.
+ *
+ * In addition to the initial RTE scan in subquery planner, this also
+ * gets called directly from expand_inherited_rtentry to ensure
+ * that row-security quals applied to children get added.
+ */
+bool
+apply_row_security_rte(PlannerInfo *root, RangeTblEntry* rte)
+{
+ Relation rel;
+ List *rowsecquals;
+ bool result = false;
+
+ Assert(rte->rtekind == RTE_RELATION);
+ rel = heap_open(rte->relid, NoLock);
+ rowsecquals = pull_row_security_policy(root->parse->commandType, rel);
+ if (rowsecquals)
+ {
+ rte->securityQuals = lcons(rowsecquals, rte->securityQuals);
+ result = true;
+ }
+ heap_close(rel, NoLock);
+ return result;
+}
+
+/*
+ * Scan the rangetable of the Query, looking for relations with
+ * row-security policies. Where a policy is found, add its securityQuals
+ * to the RTE then recurse into the quals
+ */
+static bool
+apply_row_security_rangetbl(PlannerInfo *root)
+{
+ ListCell *lc;
+ RangeTblEntry * rte;
+ bool policy_found = false;
+
+ foreach (lc, root->parse->rtable)
+ {
+ rte = (RangeTblEntry*) lfirst(lc);
+ /* Only act on normal tables that are not builtin system catalogs */
+ if (rte->rtekind == RTE_RELATION && rte->relid >= FirstNormalObjectId)
+ policy_found &= apply_row_security_rte(root, rte);
+ }
+ return policy_found;
+}
+
+/*
+ * apply_row_security_policy
+ *
+ * Entrypoint to apply configured row-security policy of the relation.
+ *
+ * In the case when the supplied query references relations with row-security
+ * policy, the relation's RangeTblEntry has the row-security qualifiers
+ * appended to the RTE's securityQual list. This will cause the optimizer to
+ * replace the relation with a security_barrier subquery over the original
+ * relation
+ *
+ * That happens after inheritance expansion has occurred, so any qual on a
+ * parent relation automatically applies to the child as the RTE is copied.
+ * RLS quals are checked for during inheritance expansion and any child
+ * table-specific quals are applied then, producing a second level of
+ * security_barrier subquery.
+ */
+void
+apply_row_security_policy(PlannerInfo *root)
+{
+ Query *parse = root->parse;
+ Oid curr_userid;
+ int curr_seccxt;
+
+ /*
+ * Mode checks. In case when SECURITY_ROW_LEVEL_DISABLED is set,
+ * no row-level security policy should be applied regardless
+ * whether it is built-in or extension.
+ */
+ GetUserIdAndSecContext(&curr_userid, &curr_seccxt);
+ if (curr_seccxt & SECURITY_ROW_LEVEL_DISABLED)
+ return;
+
+ if (apply_row_security_rangetbl(root))
+ {
+ /*
+ * We added at least one securityQual, making the plan
+ * potentially dependent on whether the current user
+ * is row-security exempt.
+ */
+
+ PlannerGlobal *glob = root->glob;
+ PlanInvalItem *pi_item;
+
+ if (!OidIsValid(glob->planUserId))
+ {
+ /* Plan invalidation on session user-id */
+ glob->planUserId = GetUserId();
+
+ /* Plan invalidation on catalog updates of pg_authid */
+ pi_item = makeNode(PlanInvalItem);
+ pi_item->cacheId = AUTHOID;
+ pi_item->hashValue =
+ GetSysCacheHashValue1(AUTHOID,
+ ObjectIdGetDatum(glob->planUserId));
+ glob->invalItems = lappend(glob->invalItems, pi_item);
+ }
+ else
+ Assert(glob->planUserId == GetUserId());
+ }
+}
diff --git a/src/include/optimizer/rowsecurity.h b/src/include/optimizer/rowsecurity.h
new file mode 100644
index 0000000..a996479
--- /dev/null
+++ b/src/include/optimizer/rowsecurity.h
@@ -0,0 +1,27 @@
+/* -------------------------------------------------------------------------
+ *
+ * rowsecurity.h
+ * prototypes for optimizer/rowsecurity.c
+ *
+ * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * -------------------------------------------------------------------------
+ */
+#ifndef ROWSECURITY_H
+#define ROWSECURITY_H
+
+#include "nodes/execnodes.h"
+#include "nodes/parsenodes.h"
+#include "nodes/relation.h"
+#include "utils/rel.h"
+
+typedef List *(*row_security_policy_hook_type)(CmdType cmdtype,
+ Relation relation);
+extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook;
+
+extern bool apply_row_security_rte(PlannerInfo *root, RangeTblEntry* rte);
+
+extern void apply_row_security_policy(PlannerInfo *root);
+
+#endif /* ROWSECURITY_H */
--
1.8.3.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
