From: Jeff Davis <pgsql@j-davis.com>
To: Noah Misch <noah@leadboat.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Nathan Bossart <nathandbossart@gmail.com>
Cc: pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
Date: Fri, 12 Jul 2024 14:50:52 -0700
Message-ID: <d97b888293e7588164b602850969a4b7d9550cc5.camel@j-davis.com> (raw)
In-Reply-To: <20240711125207.98.nmisch@google.com>
References: <20240214190226.GA373350@nathanxps13>
<4d497d82e4516037dc1082d5a62d199ea00d62e7.camel@j-davis.com>
<20240223213055.GA1805646@nathanxps13>
<196d1c86d43a147e777bb74159774f255dbe73e3.camel@j-davis.com>
<20240228165523.GA2435539@nathanxps13>
<31ae1bf73e8a1dae3ee7ef2dd43a9bbda1e1ab86.camel@j-davis.com>
<fe5d572b02d07d8e9940f9b518d900708e0b0166.camel@j-davis.com>
<20240630222344.db.nmisch@google.com>
<ZozWqg0XCkiPCnop@paquier.xyz>
<23fac3b2a722373a52b92a12ee0f500821450f97.camel@j-davis.com>
<20240711125207.98.nmisch@google.com>
On Thu, 2024-07-11 at 05:52 -0700, Noah Misch wrote:
> > I could try to refactor it into two statements and execute them
> > separately, or I could try to rewrite the statement to use a fully-
> > qualified destination table before execution. Thoughts?
>
> Those sound fine. Also fine: just adding a comment on why creation
> namespace
> considerations led to not doing it there.
Attached. 0002 separates the CREATE MATERIALIZED VIEW ... WITH DATA
into (effectively):
CREATE MATERIALIZED VIEW ... WITH NO DATA;
REFRESH MATERIALIZED VIEW ...;
Using refresh also achieves the stated goal more directly: to (mostly)
ensure that a subsequent REFRESH will succeed.
Note: the creation itself no longer executes in a security-restricted
context, but I don't think that's a problem. The only reason it's using
the security restricted context is so the following REFRESH will
succeed, right?
Regards,
Jeff Davis
Attachments:download all (2 files, .tar.gz)v2-0001-Add-missing-RestrictSearchPath-calls.patch (text/x-patch, 1.5K) download
From cf1722bf0716897f42ce89282f361af4be56b60b Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Tue, 9 Jul 2024 11:12:46 -0700
Subject: [PATCH v2 1/2] Add missing RestrictSearchPath() calls.
Reported-by: Noah Misch
Backpatch-through: 17
Discussion: https://postgr.es/m/20240630222344.db.nmisch@google.com
---
src/backend/commands/indexcmds.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex 2caab88aa58..c5a56c75f69 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -1230,6 +1230,7 @@ DefineIndex(Oid tableId,
*/
AtEOXact_GUC(false, root_save_nestlevel);
root_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
/* Add any requested comment */
if (stmt->idxcomment != NULL)
@@ -2027,6 +2028,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
{
SetUserIdAndSecContext(save_userid, save_sec_context);
*ddl_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
}
}
@@ -2074,6 +2076,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
{
SetUserIdAndSecContext(save_userid, save_sec_context);
*ddl_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
}
/*
@@ -2104,6 +2107,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
{
SetUserIdAndSecContext(save_userid, save_sec_context);
*ddl_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
}
/*
--
2.34.1
v2-0002-For-materialized-views-use-REFRESH-to-load-data-d.patch (text/x-patch, 3.8K) download
From 00323c08d07f1fb53e29e89e44b9393d442c15b1 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Fri, 12 Jul 2024 14:23:17 -0700
Subject: [PATCH v2 2/2] For materialized views, use REFRESH to load data
during creation.
Previously, CREATE MATERIALIZED VIEW ... WITH DATA populated the MV
during creation. Instead, use REFRESH, which locks down
security-restricted operations and restricts the
search_path. Otherwise, it's possible to create MVs that cannot be
refreshed.
Reported-by: Noah Misch
Backpatch-through: 17
Discussion: https://postgr.es/m/20240630222344.db.nmisch@google.com
---
src/backend/commands/createas.c | 35 +++++++++++++------------
src/test/regress/expected/namespace.out | 4 +--
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.cindex 62050f4dc59..166ade5faa7 100644--- a/src/backend/commands/createas.c+++ b/src/backend/commands/createas.c@@ -225,10 +225,8 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
Query *query = castNode(Query, stmt->query);
IntoClause *into = stmt->into;
bool is_matview = (into->viewQuery != NULL);
+ bool do_refresh = false;
DestReceiver *dest;
- Oid save_userid = InvalidOid;- int save_sec_context = 0;- int save_nestlevel = 0;
ObjectAddress address;
List *rewritten;
PlannedStmt *plan;
@@ -263,18 +261,13 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
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.+ * For materialized views, always skip data during table creation, and use+ * REFRESH instead.
*/
if (is_matview)
{
- GetUserIdAndSecContext(&save_userid, &save_sec_context);- SetUserIdAndSecContext(save_userid,- save_sec_context | SECURITY_RESTRICTED_OPERATION);- save_nestlevel = NewGUCNestLevel();+ do_refresh = !into->skipData;+ into->skipData = true;
}
if (into->skipData)
@@ -346,13 +339,21 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
PopActiveSnapshot();
}
- if (is_matview)+ /*+ * For materialized views, use REFRESH, which locks down+ * security-restricted operations and restricts the search_path.+ * Otherwise, one could create a materialized view not possible to+ * refresh.+ */+ if (do_refresh)
{
- /* Roll back any GUC changes */- AtEOXact_GUC(false, save_nestlevel);+ RefreshMatViewStmt *refresh = makeNode(RefreshMatViewStmt);- /* Restore userid and security context */- SetUserIdAndSecContext(save_userid, save_sec_context);+ refresh->relation = into->rel;+ ExecRefreshMatView(refresh, pstate->p_sourcetext, NULL, qc);++ if (qc)+ qc->commandTag = CMDTAG_SELECT;
}
return address;
diff --git a/src/test/regress/expected/namespace.out b/src/test/regress/expected/namespace.outindex 7d36e9cc0c4..dbbda72d395 100644--- a/src/test/regress/expected/namespace.out+++ b/src/test/regress/expected/namespace.out@@ -129,8 +129,8 @@ $$;
CREATE TABLE test_maint(i INT);
INSERT INTO test_maint VALUES (1), (2);
CREATE MATERIALIZED VIEW test_maint_mv AS SELECT fn(i) FROM test_maint;
-NOTICE: current search_path: test_maint_search_path-NOTICE: current search_path: test_maint_search_path+NOTICE: current search_path: pg_catalog, pg_temp+NOTICE: current search_path: pg_catalog, pg_temp
-- the following commands should see search_path as pg_catalog, pg_temp
CREATE INDEX test_maint_idx ON test_maint_search_path.test_maint (fn(i));
NOTICE: current search_path: pg_catalog, pg_temp
--
2.34.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pgsql@j-davis.com, noah@leadboat.com, michael@paquier.xyz, nathandbossart@gmail.com, mail@joeconway.com, robertmhaas@gmail.com
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
In-Reply-To: <d97b888293e7588164b602850969a4b7d9550cc5.camel@j-davis.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox