pg_dump issue with renamed system schemas

Started by Bossart, Nathanalmost 6 years ago2 messages
#1Bossart, Nathan
bossartn@amazon.com
1 attachment(s)

Hi hackers,

I think I've found a small bug in pg_dump that could cause some schema
privileges to be missed. In short, if you've renamed a schema that
has an entry in pg_init_privs, pg_dump will skip dumping the initial
ACL for the schema. This results in missing privileges on restore.

I've attached a small patch with a test case to handle this. This
patch fixes the problem by adjusting the LEFT JOIN on pg_init_privs to
only match for schemas that match the default system names. I've only
included 'public' and 'pg_catalog' for now, since AFAICT those are the
only two system schemas with corresponding pg_init_privs entries for
which pg_dump dumps ACLs. Also, I haven't attempted to handle the
case where an extension schema with a pg_init_privs entry has been
renamed. Perhaps a sturdier approach would be to adjust the way
pg_init_privs is maintained, but that might be too invasive.

Even with this patch, I think there are still some interesting corner
cases involving the 'public' schema (e.g. recreating it, changing its
ownership). I don't know if it's worth trying to address all these
corner cases with special system schemas, but the first one I
mentioned seemed simple enough to fix.

Nathan

Attachments:

v1-0001-Do-not-consider-pg_init_privs-entries-when-dumpin.patchapplication/octet-stream; name=v1-0001-Do-not-consider-pg_init_privs-entries-when-dumpin.patchDownload
From ff29917ff0802ef51d42ff8d4ae461b2aa2fcfff Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Fri, 10 Apr 2020 19:54:29 +0000
Subject: [PATCH v1 1/1] Do not consider pg_init_privs entries when dumping
 renamed system schemas.

---
 src/bin/pg_dump/pg_dump.c        |  2 ++
 src/bin/pg_dump/t/002_pg_dump.pl | 20 +++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c579227b19..3070fe17e1 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4715,6 +4715,8 @@ getNamespaces(Archive *fout, int *numNamespaces)
 						  "FROM pg_namespace n "
 						  "LEFT JOIN pg_init_privs pip "
 						  "ON (n.oid = pip.objoid "
+						  "AND (pip.objoid IN (SELECT oid FROM pg_namespace WHERE nspname IN ('public', 'pg_catalog')) "
+						  "OR pip.privtype != 'i') "
 						  "AND pip.classoid = 'pg_namespace'::regclass "
 						  "AND pip.objsubid = 0",
 						  username_subquery,
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 1b90cbd9b5..3bf6f5d57a 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -268,6 +268,15 @@ my %pgdump_runs = (
 			'postgres',
 		],
 	},
+	renamed_default_schema => {
+		database => 'renamed_default_schema',
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/renamed_default_schema.sql",
+			'renamed_default_schema',
+		],
+	},
 	role => {
 		dump_cmd => [
 			'pg_dump',
@@ -3037,6 +3046,14 @@ my %tests = (
 		},
 	},
 
+	'GRANT ALL ON renamed_public_schema TO public' => {
+		database => 'renamed_default_schema',
+		create_order => 100,
+		create_sql => 'ALTER SCHEMA public RENAME TO renamed_public_schema;',
+		regexp => qr/^GRANT ALL ON SCHEMA renamed_public_schema TO PUBLIC;/m,
+		like => { renamed_default_schema => 1, },
+	},
+
 	'GRANT INSERT(col1) ON TABLE test_second_table' => {
 		create_order => 8,
 		create_sql =>
@@ -3322,8 +3339,9 @@ if ($collation_check_stderr !~ /ERROR: /)
 	$collation_support = 1;
 }
 
-# Create a second database for certain tests to work against
+# Create extra databases for certain tests to work against
 $node->psql('postgres', 'create database regress_pg_dump_test;');
+$node->psql('postgres', 'create database renamed_default_schema;');
 
 # Start with number of command_fails_like()*2 tests below (each
 # command_fails_like is actually 2 tests)
-- 
2.16.6

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bossart, Nathan (#1)
Re: pg_dump issue with renamed system schemas

"Bossart, Nathan" <bossartn@amazon.com> writes:

I think I've found a small bug in pg_dump that could cause some schema
privileges to be missed. In short, if you've renamed a schema that
has an entry in pg_init_privs, pg_dump will skip dumping the initial
ACL for the schema. This results in missing privileges on restore.

This seems like a special case of the issue discussed in

/messages/by-id/f85991ad-bbd4-ad57-fde4-e12f0661dbf0@postgrespro.ru

AFAICT we didn't think we'd found a satisfactory solution yet.

regards, tom lane