Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qsrLX-009o3i-SL for pgsql-hackers@arkaria.postgresql.org; Tue, 17 Oct 2023 21:11:24 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1qsrLV-00ERMg-Sa for pgsql-hackers@arkaria.postgresql.org; Tue, 17 Oct 2023 21:11:22 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qsrLV-00ERME-Ib for pgsql-hackers@lists.postgresql.org; Tue, 17 Oct 2023 21:11:22 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qsrLS-001FBj-6Z for pgsql-hackers@lists.postgresql.org; Tue, 17 Oct 2023 21:11:21 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 39HLBGB22984518; Tue, 17 Oct 2023 17:11:16 -0400 From: Tom Lane To: Jacob Champion cc: Stephen Frost , Andrew Dunstan , Akshat Jaimini , PostgreSQL Hackers Subject: Re: pg_dump needs SELECT privileges on irrelevant extension table In-reply-to: <2905669.1697573572@sss.pgh.pa.us> References: <00d46a48-3324-d9a0-49bf-e7f0f11d1038@timescale.com> <954419.1679334206@sss.pgh.pa.us> <2905669.1697573572@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Tue, 17 Oct 2023 16:12:52 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <2984401.1697577038.0@sss.pgh.pa.us> Date: Tue, 17 Oct 2023 17:11:16 -0400 Message-ID: <2984517.1697577076@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <2984401.1697577038.1@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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="v4-0001-dont-try-to-dump-extension-RLS-policies.patch"; charset="us-ascii" Content-ID: <2984401.1697577038.2@sss.pgh.pa.us> Content-Description: v4-0001-dont-try-to-dump-extension-RLS-policies.patch Content-Transfer-Encoding: quoted-printable 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, Arc= hive *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 w= e - * extract the information about the object. We don't provide support f= or - * 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, Arch= ive *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 n= ot + * 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 =3D ext->dobj.dump; @@ -1702,9 +1706,7 @@ checkExtensionMembership(DumpableObject *dobj, Archi= ve *fout) if (fout->remoteVersion < 90600) dobj->dump =3D DUMP_COMPONENT_NONE; else - dobj->dump =3D ext->dobj.dump_contains & (DUMP_COMPONENT_ACL | - DUMP_COMPONENT_SECLABEL | - DUMP_COMPONENT_POLICY); + dobj->dump =3D 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/module= s/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 =3D ( 'postgres', ], }, + + # regress_dump_login_role shouldn't need SELECT rights on internal + # (undumped) extension tables + privileged_internals =3D> { + dump_cmd =3D> [ + 'pg_dump', '--no-sync', "--file=3D$tempdir/privileged_internals.sql", + # these two tables are irrelevant to the test case + '--exclude-table=3Dregress_pg_dump_schema.external_tab', + '--exclude-table=3Dregress_pg_dump_schema.extdependtab', + '--username=3Dregress_dump_login_role', 'postgres', + ], + }, + schema_only =3D> { dump_cmd =3D> [ 'pg_dump', '--no-sync', "--file=3D$tempdir/schema_only.sql", @@ -284,6 +297,7 @@ my %full_runs =3D ( exclude_table =3D> 1, no_privs =3D> 1, no_owner =3D> 1, + privileged_internals =3D> 1, with_extension =3D> 1, without_extension =3D> 1); = @@ -321,6 +335,16 @@ my %tests =3D ( like =3D> { pg_dumpall_globals =3D> 1, }, }, = + 'CREATE ROLE regress_dump_login_role' =3D> { + create_order =3D> 1, + create_sql =3D> 'CREATE ROLE regress_dump_login_role LOGIN;', + regexp =3D> qr/^ + \QCREATE ROLE regress_dump_login_role;\E + \n\QALTER ROLE regress_dump_login_role WITH \E.*\Q LOGIN \E.*; + \n/xm, + like =3D> { pg_dumpall_globals =3D> 1, }, + }, + 'GRANT ALTER SYSTEM ON PARAMETER full_page_writes TO regress_dump_test_r= ole' =3D> { create_order =3D> 2, @@ -704,6 +728,7 @@ my %tests =3D ( data_only =3D> 1, extension_schema =3D> 1, pg_dumpall_globals =3D> 1, + privileged_internals =3D> 1, section_data =3D> 1, section_pre_data =3D> 1, # Excludes this schema as extension is not listed. @@ -720,6 +745,7 @@ my %tests =3D ( data_only =3D> 1, extension_schema =3D> 1, pg_dumpall_globals =3D> 1, + privileged_internals =3D> 1, section_data =3D> 1, section_pre_data =3D> 1, # Excludes this schema as extension is not listed. @@ -743,6 +769,7 @@ my %tests =3D ( # Excludes the extension and keeps the schema's data. without_extension_internal_schema =3D> 1, }, + unlike =3D> { privileged_internals =3D> 1 }, },); = ######################################### diff --git a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql b/src/tes= t/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; = ------- =_aaaaaaaaaa0--