postgres.git / summary / log / commit / refs
commit 891afa84c27efb2d496540cbc0b5dd1faaec4e91
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon Nov 13 22:04:10 2023 +0000
First released in: 17.0, 12.18, 13.14, 14.11, 15.6, 16.2
Don't try to dump RLS policies or security labels for extension objects.
checkExtensionMembership() set the DUMP_COMPONENT_SECLABEL and
DUMP_COMPONENT_POLICY flags for extension member objects, even though
we lack any infrastructure for tracking extensions' initial settings
of these properties. This is not OK. The result was that a dump
would always include commands to set these properties for extension
objects that have them, with at least three negative consequences:
1. The restoring user might not have privilege to set these properties
on these objects.
2. The properties might be incorrect/irrelevant for the version of the
extension that's installed in the destination database.
3. The dump itself might fail, in the case of RLS properties attached
to extension tables that the dumping user lacks privilege to LOCK.
(That's because we must get at least AccessShareLock to ensure that
we don't fail while trying to decompile the RLS expressions.)
When and if somebody cares to invent initial-state infrastructure for
extensions' RLS policies and security labels, we could think about
finding another way around problem #3. But in the absence of such
infrastructure, this whole thing is just wrong and we shouldn't do it.
(Note: this applies only to ordinary dumps; binary-upgrade dumps
still dump and restore extension member objects separately, with
all properties.)
Tom Lane and Jacob Champion. Back-patch to all supported branches.
Discussion: https://postgr.es/m/00d46a48-3324-d9a0-49bf-e7f0f11d1038@timescale.com
src/bin/pg_dump/pg_dump.c | 22 ++++++++--------
src/test/modules/test_pg_dump/t/001_base.pl | 29 +++++++++++++++++++++-
.../modules/test_pg_dump/test_pg_dump--1.0.sql | 2 ++
3 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2bf4c668b20..29014602144 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1432,13 +1432,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.
*
@@ -1446,6 +1443,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;
@@ -1454,9 +1458,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.pl
index 501aff09201..1aee7f62672 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -169,6 +169,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",
@@ -235,7 +248,8 @@ my %full_runs = (
defaults => 1,
exclude_table => 1,
no_privs => 1,
- no_owner => 1,);
+ no_owner => 1,
+ privileged_internals => 1,);
my %tests = (
'ALTER EXTENSION test_pg_dump' => {
@@ -271,6 +285,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, },
+ },
+
'CREATE SEQUENCE regress_pg_dump_table_col1_seq' => {
regexp => qr/^
\QCREATE SEQUENCE public.regress_pg_dump_table_col1_seq\E
@@ -611,6 +635,7 @@ my %tests = (
data_only => 1,
extension_schema => 1,
pg_dumpall_globals => 1,
+ privileged_internals => 1,
section_data => 1,
section_pre_data => 1,
},
@@ -625,6 +650,7 @@ my %tests = (
data_only => 1,
extension_schema => 1,
pg_dumpall_globals => 1,
+ privileged_internals => 1,
section_data => 1,
section_pre_data => 1,
},
@@ -644,6 +670,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 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.sql
index 110f7eef66c..1c68e146d91 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;
[parent: b64926fbfda6]