Have REFRESH MATERIALIZED VIEW run as the MV owner

Started by Noah Mischover 12 years ago3 messages
#1Noah Misch
noah@leadboat.com
1 attachment(s)

REFRESH MATERIALIZED VIEW should temporarily switch the current user ID to the
MV owner. REINDEX and VACUUM do so to let privileged users safely maintain
objects owned by others, and REFRESH MATERIALIZED VIEW belongs in that class
of commands. The MV query then runs as a "security-restricted operation",
which forbids a few commands. Most, e.g. UNLISTEN, are unlikely to arise in
practice. The most interesting restriction is probably CREATE TEMP TABLE.
Consider a function that creates and later drops a temporary table that it
uses for intermediate storage during a complicated calculation. That function
will no longer work in a MV query. As a workaround, modify the function to
use a permanent table as its work area.

See attached patch. The similar behavior of REINDEX et al. is undocumented.
Users are a bit more likely to notice limitations in the context of MVs, so I
added a brief documentation mention. Seeing that this narrows the range of
valid MV queries, I bring it up now so MVs can debut with the restrictions
already in place.

Thanks,
nm

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

Attachments:

matview-secure-refresh-v1.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml
index 0ed764b..b742e17 100644
--- a/doc/src/sgml/ref/create_materialized_view.sgml
+++ b/doc/src/sgml/ref/create_materialized_view.sgml
@@ -105,7 +105,9 @@ CREATE MATERIALIZED VIEW <replaceable>table_name</replaceable>
     <listitem>
      <para>
       A <xref linkend="sql-select">, <link linkend="sql-table">TABLE</link>,
-      or <xref linkend="sql-values"> command.
+      or <xref linkend="sql-values"> command.  This query will run within a
+      security-restricted operation; in particular, calls to functions that
+      themselves create temporary tables will fail.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 2bfe5fb..a3509d8 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -33,6 +33,7 @@
 #include "commands/prepare.h"
 #include "commands/tablecmds.h"
 #include "commands/view.h"
+#include "miscadmin.h"
 #include "parser/parse_clause.h"
 #include "rewrite/rewriteHandler.h"
 #include "storage/smgr.h"
@@ -69,7 +70,11 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
 {
 	Query	   *query = (Query *) stmt->query;
 	IntoClause *into = stmt->into;
+	bool		is_matview = (into->viewQuery != NULL);
 	DestReceiver *dest;
+	Oid			save_userid = InvalidOid;
+	int			save_sec_context = 0;
+	int			save_nestlevel = 0;
 	List	   *rewritten;
 	PlannedStmt *plan;
 	QueryDesc  *queryDesc;
@@ -90,6 +95,7 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
 	{
 		ExecuteStmt *estmt = (ExecuteStmt *) query->utilityStmt;
 
+		Assert(!is_matview);	/* excluded by syntax */
 		ExecuteQuery(estmt, into, queryString, params, dest, completionTag);
 
 		return;
@@ -97,6 +103,21 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
 	Assert(query->commandType == CMD_SELECT);
 
 	/*
+	 * For materialized views, lock down security-restricted operations and
+	 * arrange to make GUC variable changes local to this command.	This is
+	 * not necessary for security, but this keeps the behavior similar to
+	 * REFRESH MATERIALIZED VIEW.  Otherwise, one could create a materialized
+	 * view not possible to refresh.
+	 */
+	if (is_matview)
+	{
+		GetUserIdAndSecContext(&save_userid, &save_sec_context);
+		SetUserIdAndSecContext(save_userid,
+						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
+		save_nestlevel = NewGUCNestLevel();
+	}
+
+	/*
 	 * Parse analysis was done already, but we still have to run the rule
 	 * rewriter.  We do not do AcquireRewriteLocks: we assume the query either
 	 * came straight from the parser, or suitable locks were acquired by
@@ -160,6 +181,15 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
 	FreeQueryDesc(queryDesc);
 
 	PopActiveSnapshot();
+
+	if (is_matview)
+	{
+		/* Roll back any GUC changes */
+		AtEOXact_GUC(false, save_nestlevel);
+
+		/* Restore userid and security context */
+		SetUserIdAndSecContext(save_userid, save_sec_context);
+	}
 }
 
 /*
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 2ffdca3..1c383ba 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -122,6 +122,9 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 	RewriteRule *rule;
 	List	   *actions;
 	Query	   *dataQuery;
+	Oid			save_userid;
+	int			save_sec_context;
+	int			save_nestlevel;
 	Oid			tableSpace;
 	Oid			OIDNewHeap;
 	DestReceiver *dest;
@@ -192,6 +195,16 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 	CheckTableNotInUse(matviewRel, "REFRESH MATERIALIZED VIEW");
 
 	/*
+	 * Switch to the owner's userid, so that any functions are run as that
+	 * user.  Also lock down security-restricted operations and arrange to
+	 * make GUC variable changes local to this command.
+	 */
+	GetUserIdAndSecContext(&save_userid, &save_sec_context);
+	SetUserIdAndSecContext(matviewRel->rd_rel->relowner,
+						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
+	save_nestlevel = NewGUCNestLevel();
+
+	/*
 	 * Tentatively mark the matview as populated or not (this will roll back
 	 * if we fail later).
 	 */
@@ -217,6 +230,12 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 					 RecentXmin, ReadNextMultiXactId());
 
 	RelationCacheInvalidateEntry(matviewOid);
+
+	/* Roll back any GUC changes */
+	AtEOXact_GUC(false, save_nestlevel);
+
+	/* Restore userid and security context */
+	SetUserIdAndSecContext(save_userid, save_sec_context);
 }
 
 /*
#2Hitoshi Harada
umi.tanuki@gmail.com
In reply to: Noah Misch (#1)
Re: Have REFRESH MATERIALIZED VIEW run as the MV owner

On Fri, Jul 5, 2013 at 9:45 AM, Noah Misch <noah@leadboat.com> wrote:

REFRESH MATERIALIZED VIEW should temporarily switch the current user ID to the
MV owner. REINDEX and VACUUM do so to let privileged users safely maintain
objects owned by others, and REFRESH MATERIALIZED VIEW belongs in that class
of commands.

I was trying to understand why this is safe for a while. REINDEX and
VACUUM make sense to me because they never contain side-effect as far
as I know, but MV can contain some volatile functions which could have
some unintended operation that shouldn't be invoked by no one but the
owner. For example, if the function creates a permanent table per
call and doesn't clean it up, but later some other maintenance
operation is supposed to clean it up, and the owner schedules REFRESH
and maintenance once a day. A non-owner user now can refresh it so
many times until the disk gets full. Or is that operation supposed to
be restricted by the security context you are adding?

--
Hitoshi Harada

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Noah Misch
noah@leadboat.com
In reply to: Hitoshi Harada (#2)
Re: Have REFRESH MATERIALIZED VIEW run as the MV owner

On Fri, Jul 05, 2013 at 11:18:50PM -0700, Hitoshi Harada wrote:

On Fri, Jul 5, 2013 at 9:45 AM, Noah Misch <noah@leadboat.com> wrote:

REFRESH MATERIALIZED VIEW should temporarily switch the current user ID to the
MV owner. REINDEX and VACUUM do so to let privileged users safely maintain
objects owned by others, and REFRESH MATERIALIZED VIEW belongs in that class
of commands.

I was trying to understand why this is safe for a while. REINDEX and
VACUUM make sense to me because they never contain side-effect as far
as I know, but MV can contain some volatile functions which could have
some unintended operation that shouldn't be invoked by no one but the
owner. For example, if the function creates a permanent table per
call and doesn't clean it up, but later some other maintenance
operation is supposed to clean it up, and the owner schedules REFRESH
and maintenance once a day. A non-owner user now can refresh it so
many times until the disk gets full.

I'm not proposing to expand the set of people *permitted* to refresh the MV.
That's still limited to the owning role (including other roles acquiring that
role by membership) and superusers. My goal is to make it safe for a
superuser to refresh any MV, much like we've made it safe for a superuser to
REINDEX any index.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers