Re: pg_dump needs SELECT privileges on irrelevant extension table - Mailing list pgsql-hackers

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

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 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.pl
index 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.sql
index 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;