From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jacob Champion <champion.p@gmail.com>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: Akshat Jaimini <destrex271@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_dump needs SELECT privileges on irrelevant extension table
Date: Tue, 17 Oct 2023 17:11:16 -0400
Message-ID: <2984517.1697577076@sss.pgh.pa.us> (raw)
In-Reply-To: <2905669.1697573572@sss.pgh.pa.us>
References: <00d46a48-3324-d9a0-49bf-e7f0f11d1038@timescale.com>
<954419.1679334206@sss.pgh.pa.us>
<eed220a4-c7f3-2164-02f2-b42783694221@timescale.com>
<a185f66f-1edc-426e-41a4-091dad230c75@dunslane.net>
<CAAWbhmjsm+bJrwC1eTGpGk+6MQFG_1vA2g1+Fqa0xE78v1Xncg@mail.gmail.com>
<CAAWbhmi+fu6Aiucjf76Ss2GPEijrz6VLriby1LaG=p9zNTOxZg@mail.gmail.com>
<2905669.1697573572@sss.pgh.pa.us>
I wrote:
> Why are we marking extension member objects as being subject to SECLABEL
> or POLICY dumping? As the comment notes, that isn't really sensible
> unless what we are dumping is a delta from the extension's initial
> assignments. But we have no infrastructure for that, and none seems
> likely to appear in the near future.
Here's a quick patch that does it that way. The test changes
are identical to Jacob's v3-0001.
regards, tom lane
Attachments:v4-0001-dont-try-to-dump-extension-RLS-policies.patch (text/x-diff, 5.1K) downloaddiff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.cindex 83aeef2751..fd6d8bb5dd 100644--- a/src/bin/pg_dump/pg_dump.c+++ b/src/bin/pg_dump/pg_dump.c@@ -1680,13 +1680,10 @@ checkExtensionMembership(DumpableObject *dobj, Archive *fout)
addObjectDependency(dobj, ext->dobj.dumpId);
/*
- * In 9.6 and above, mark the member object to have any non-initial ACL,- * policies, and security labels dumped.- *- * Note that any initial ACLs (see pg_init_privs) will be removed when we- * extract the information about the object. We don't provide support for- * initial policies and security labels and it seems unlikely for those to- * ever exist, but we may have to revisit this later.+ * In 9.6 and above, mark the member object to have any non-initial ACLs+ * dumped. (Any initial ACLs will be removed later, using data from+ * pg_init_privs, so that we'll dump only the delta from the extension's+ * initial setup.)
*
* Prior to 9.6, we do not include any extension member components.
*
@@ -1694,6 +1691,13 @@ checkExtensionMembership(DumpableObject *dobj, Archive *fout)
* individually, since the idea is to exactly reproduce the database
* contents rather than replace the extension contents with something
* different.
+ *+ * Note: it might be interesting someday to implement storage and delta+ * dumping of extension members' RLS policies and/or security labels.+ * However there is a pitfall for RLS policies: trying to dump them+ * requires getting a lock on their tables, and the calling user might not+ * have privileges for that. We need no lock to examine a table's ACLs,+ * so the current feature doesn't have a problem of that sort.
*/
if (fout->dopt->binary_upgrade)
dobj->dump = ext->dobj.dump;
@@ -1702,9 +1706,7 @@ checkExtensionMembership(DumpableObject *dobj, Archive *fout)
if (fout->remoteVersion < 90600)
dobj->dump = DUMP_COMPONENT_NONE;
else
- dobj->dump = ext->dobj.dump_contains & (DUMP_COMPONENT_ACL |- DUMP_COMPONENT_SECLABEL |- DUMP_COMPONENT_POLICY);+ dobj->dump = ext->dobj.dump_contains & (DUMP_COMPONENT_ACL);
}
return true;
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.plindex d00c3544e9..68a767d2f5 100644--- a/src/test/modules/test_pg_dump/t/001_base.pl+++ b/src/test/modules/test_pg_dump/t/001_base.pl@@ -175,6 +175,19 @@ my %pgdump_runs = (
'postgres',
],
},
++ # regress_dump_login_role shouldn't need SELECT rights on internal+ # (undumped) extension tables+ privileged_internals => {+ dump_cmd => [+ 'pg_dump', '--no-sync', "--file=$tempdir/privileged_internals.sql",+ # these two tables are irrelevant to the test case+ '--exclude-table=regress_pg_dump_schema.external_tab',+ '--exclude-table=regress_pg_dump_schema.extdependtab',+ '--username=regress_dump_login_role', 'postgres',+ ],+ },+
schema_only => {
dump_cmd => [
'pg_dump', '--no-sync', "--file=$tempdir/schema_only.sql",
@@ -284,6 +297,7 @@ my %full_runs = (
exclude_table => 1,
no_privs => 1,
no_owner => 1,
+ privileged_internals => 1,
with_extension => 1,
without_extension => 1);
@@ -321,6 +335,16 @@ my %tests = (
like => { pg_dumpall_globals => 1, },
},
+ 'CREATE ROLE regress_dump_login_role' => {+ create_order => 1,+ create_sql => 'CREATE ROLE regress_dump_login_role LOGIN;',+ regexp => qr/^+ \QCREATE ROLE regress_dump_login_role;\E+ \n\QALTER ROLE regress_dump_login_role WITH \E.*\Q LOGIN \E.*;+ \n/xm,+ like => { pg_dumpall_globals => 1, },+ },+
'GRANT ALTER SYSTEM ON PARAMETER full_page_writes TO regress_dump_test_role'
=> {
create_order => 2,
@@ -704,6 +728,7 @@ my %tests = (
data_only => 1,
extension_schema => 1,
pg_dumpall_globals => 1,
+ privileged_internals => 1,
section_data => 1,
section_pre_data => 1,
# Excludes this schema as extension is not listed.
@@ -720,6 +745,7 @@ my %tests = (
data_only => 1,
extension_schema => 1,
pg_dumpall_globals => 1,
+ privileged_internals => 1,
section_data => 1,
section_pre_data => 1,
# Excludes this schema as extension is not listed.
@@ -743,6 +769,7 @@ my %tests = (
# Excludes the extension and keeps the schema's data.
without_extension_internal_schema => 1,
},
+ unlike => { privileged_internals => 1 },
},);
#########################################
diff --git a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sqlindex 110f7eef66..1c68e146d9 100644--- a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql+++ b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql@@ -12,11 +12,13 @@ CREATE SEQUENCE regress_pg_dump_seq;
CREATE SEQUENCE regress_seq_dumpable;
SELECT pg_catalog.pg_extension_config_dump('regress_seq_dumpable', '');
+GRANT SELECT ON SEQUENCE regress_seq_dumpable TO public;
CREATE TABLE regress_table_dumpable (
col1 int check (col1 > 0)
);
SELECT pg_catalog.pg_extension_config_dump('regress_table_dumpable', '');
+GRANT SELECT ON regress_table_dumpable TO public;
CREATE SCHEMA regress_pg_dump_schema;
=
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-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, champion.p@gmail.com, sfrost@snowman.net, andrew@dunslane.net, destrex271@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_dump needs SELECT privileges on irrelevant extension table
In-Reply-To: <2984517.1697577076@sss.pgh.pa.us>
* 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