Обсуждение: pg_restore -L reordering of the statements does not work

Aditya D <dsaditya91@gmail.com> writes:
> Problem Statement - The ACL command i.e. The grant statement gets called
> out at the end while performing pg_restore and the use case is to call it
> at the beginning. I believe even though the re-ordering is done the
> statements are executed using the "Internal Archive Id" while referring
> to the dump file.

Yeah, see the comments for RestorePass:

 * For historical reasons, ACL items are interspersed with everything else in
 * a dump file's TOC; typically they're right after the object they're for.
 * However, we need to restore data before ACLs, as otherwise a read-only
 * table (ie one where the owner has revoked her own INSERT privilege) causes
 * data restore failures.  On the other hand, matview REFRESH commands should
 * come out after ACLs, as otherwise non-superuser-owned matviews might not
 * be able to execute.  (If the permissions at the time of dumping would not
 * allow a REFRESH, too bad; we won't fix that for you.)  We also want event
 * triggers to be restored after ACLs, so that they can't mess those up.
 *
 * These considerations force us to make three passes over the TOC,
 * restoring the appropriate subset of items in each pass.  We assume that
 * the dependency sort resulted in an appropriate ordering of items within
 * each subset.

Use of an -L switch overrides the dependency sort, but not this
pass mechanism.

Whatever you're hoping to do by overriding that is most likely
just going to replace one kind of breakage by another.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Sat, Oct 28, 2023, 18:37 Aditya D <dsaditya91@gmail.com> wrote:
>> PS: Don't have super user privileges on the target.

> Why can you not create a role on the target that is a member of every other
> role and thus can create and reassign delegated permissions and ownership
> to any of them.  In short, a role that behaves as superuser within this
> limited context even if they are not a true superuser.

That's going to be essential in any case if the restoring user is
to be able to successfully execute the ALTER OWNER commands in the
script.  You can't give away ownership to a role you don't have
the privileges of.

            regards, tom lane

Hi,

On 2023-10-27 14:34:46 -0400, Tom Lane wrote:
> Use of an -L switch overrides the dependency sort, but not this
> pass mechanism.

Aditya pinged me on this issue internally. I think there's a real bug here,
and the use of -L was to work around that.

A schema like:

CREATE USER nosuper_1;
CREATE USER nosuper_2;

GRANT nosuper_2 TO nosuper_1;
GRANT nosuper_1 TO nosuper_3;

CREATE SCHEMA nosuper AUTHORIZATIOn nosuper_1;
SET ROLE nosuper_1;
GRANT ALL ON SCHEMA nosuper TO nosuper_2;

CREATE TABLE nosuper.tbl();
ALTER TABLE nosuper.tbl OWNER TO nosuper_2;


results in the following, abbreviated, dump on HEAD:
CREATE SCHEMA nosuper;
ALTER SCHEMA nosuper OWNER TO nosuper_1;
..
CREATE TABLE nosuper.tbl (
);
ALTER TABLE nosuper.tbl OWNER TO nosuper_2;
...
GRANT ALL ON SCHEMA nosuper TO nosuper_2;


Which is bad because the ALTER TABLE OWNER TO cannot be executed before
the GRANT ALL:
  ERROR:  42501: permission denied for schema nosuper
  LOCATION:  aclcheck_error, aclchk.c:2833

We don't allow the OWNER TO without the GRANT ... ON SCHEMA - which is
scheduled subsequently.


I think there are reasonable dependencies in the database - but pg_dump
doesn't seem to actually process shared dependencies, unless I am missing
something?

SELECT dbid, deptype, classid, classid::regclass, objid, objsubid, refclassid, refclassid::regclass, refobjid,
pg_describe_object(classid,objid, objsubid::int) objdesc, pg_describe_object(refclassid, refobjid, 0) refobjdesc FROM
pg_shdepend;
 

┌──────┬─────────┬─────────┬──────────────┬───────┬──────────┬────────────┬────────────┬──────────┬───────────────────┬────────────────┐
│ dbid │ deptype │ classid │   classid    │ objid │ objsubid │ refclassid │ refclassid │ refobjid │      objdesc      │
 refobjdesc   │
 

├──────┼─────────┼─────────┼──────────────┼───────┼──────────┼────────────┼────────────┼──────────┼───────────────────┼────────────────┤
│    5 │ o       │    2615 │ pg_namespace │ 42225 │        0 │       1260 │ pg_authid  │    42221 │ schema nosuper    │
rolenosuper_1 │
 
│    5 │ a       │    2615 │ pg_namespace │ 42225 │        0 │       1260 │ pg_authid  │    42222 │ schema nosuper    │
rolenosuper_2 │
 
│    5 │ o       │    1259 │ pg_class     │ 42226 │        0 │       1260 │ pg_authid  │    42222 │ table nosuper.tbl │
rolenosuper_2 │
 

└──────┴─────────┴─────────┴──────────────┴───────┴──────────┴────────────┴────────────┴──────────┴───────────────────┴────────────────┘

pg_depend does have the following dependency:

┌─────────┬─────────┬──────────┬───────┬──────────┬────────────┬──────────────┬──────────┬─────────────┬───────────────────┬────────────────┐
│ deptype │ classid │ classid  │ objid │ objsubid │ refclassid │  refclassid  │ refobjid │ refobjsubid │      objdesc
  │   refobjdesc   │
 

├─────────┼─────────┼──────────┼───────┼──────────┼────────────┼──────────────┼──────────┼─────────────┼───────────────────┼────────────────┤
│ n       │    1259 │ pg_class │ 42226 │        0 │       2615 │ pg_namespace │    42225 │           0 │ table
nosuper.tbl│ schema nosuper │
 

└─────────┴─────────┴──────────┴───────┴──────────┴────────────┴──────────────┴──────────┴─────────────┴───────────────────┴────────────────┘

Without knowing about the dependency between the schema and the grant, pg_dump
can't schedule them reasonably. The TOC shows the following:

; Selected TOC Entries:
;
4002; 0 0 ENCODING - ENCODING
4003; 0 0 STDSTRINGS - STDSTRINGS
4004; 0 0 SEARCHPATH - SEARCHPATH
4005; 1262 5 DATABASE - postgres andres
4006; 0 0 COMMENT - DATABASE postgres andres
;    depends on: 4005
5; 2615 42225 SCHEMA - nosuper nosuper_1
4007; 0 0 ACL - SCHEMA nosuper nosuper_1
;    depends on: 5
217; 1259 42226 TABLE nosuper tbl nosuper_2
;    depends on: 5
3999; 0 42226 TABLE DATA nosuper tbl nosuper_2
;    depends on: 217

Given these dependencies, there's indeed no reason to schedule the GRANT
before the ALTER TABLE.


I feel like I must be missing something - there must be other negative
consequences of not looking at pg_shdepend at all?


I attached a script to create a schema in the problematic state.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> A schema like:
> ...
> results in the following, abbreviated, dump on HEAD:
> ...
> Which is bad because the ALTER TABLE OWNER TO cannot be executed before
> the GRANT ALL:
>   ERROR:  42501: permission denied for schema nosuper
>   LOCATION:  aclcheck_error, aclchk.c:2833

Works fine for me.  I agree that it might not work if you're restoring
as non-superuser, but if you try that the ALTER OWNER commands are all
going to fail too.  Moreover, reordering the GRANTs is no solution,
because who promised that the schema owner granted you any permissions?

The bigger picture here is that pg_dump effectively relies on all
objects being treated throughout the restore as though the restoring
user is their owner --- either via --no-owner, or because the
restoring user is superuser, or perhaps because the restoring user is
a member of every object owner named in the dump.  Postponing
execution of GRANTs to the end should therefore be perfectly safe, and
indeed it's *necessary* if you want to successfully restore cases in
which an object owner has revoked some of their own privileges.

I experimented with making the restoring user be a member with inherit
of the nosuper_N roles, and indeed I still see the failure above,
which makes me wonder if the ACL check is being done correctly for
that specific case.  The INHERIT bit ought to let it work.

            regards, tom lane

Hi,

On 2023-11-14 15:42:22 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > A schema like:
> > ...
> > results in the following, abbreviated, dump on HEAD:
> > ...
> > Which is bad because the ALTER TABLE OWNER TO cannot be executed before
> > the GRANT ALL:
> >   ERROR:  42501: permission denied for schema nosuper
> >   LOCATION:  aclcheck_error, aclchk.c:2833
> 
> Works fine for me.  I agree that it might not work if you're restoring
> as non-superuser, but if you try that the ALTER OWNER commands are all
> going to fail too.

It's indeed dependent on restoring as a non-superuser. Notably even if
restoring as nosuper_1.


> Moreover, reordering the GRANTs is no solution, because who promised that
> the schema owner granted you any permissions?

I'm not quite following - the schema is created in the dump, so the grant is
part of it?


> The bigger picture here is that pg_dump effectively relies on all
> objects being treated throughout the restore as though the restoring
> user is their owner --- either via --no-owner, or because the
> restoring user is superuser, or perhaps because the restoring user is
> a member of every object owner named in the dump.

In my repro I was restoring with nosuper_1, which is granted membership to
nosuper_2.


> Postponing execution of GRANTs to the end should therefore be perfectly
> safe, and indeed it's *necessary* if you want to successfully restore cases
> in which an object owner has revoked some of their own privileges.
> 
> I experimented with making the restoring user be a member with inherit
> of the nosuper_N roles, and indeed I still see the failure above,
> which makes me wonder if the ACL check is being done correctly for
> that specific case.  The INHERIT bit ought to let it work.

The check is for nosuper_2 to have permission on the schema and the check
happens before the grant on the schema. For inherit to help, nosuper_2 would
have to be granted membership to the presumably more privileged user doing the
restore.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2023-11-14 15:42:22 -0500, Tom Lane wrote:
>> Moreover, reordering the GRANTs is no solution, because who promised that
>> the schema owner granted you any permissions?

> I'm not quite following - the schema is created in the dump, so the grant is
> part of it?

Yeah, but the GRANT will restore whatever permissions existed in the
source database.  If the restoring user isn't super, those permissions
don't necessarily grant him access.

>> I experimented with making the restoring user be a member with inherit
>> of the nosuper_N roles, and indeed I still see the failure above,
>> which makes me wonder if the ACL check is being done correctly for
>> that specific case.  The INHERIT bit ought to let it work.

> The check is for nosuper_2 to have permission on the schema

... no, it should be for the user executing the ALTER to have permission.

> ... and the check
> happens before the grant on the schema. For inherit to help, nosuper_2 would
> have to be granted membership to the presumably more privileged user doing the
> restore.

No, surely the other way?  Restoring user must be member of nosuper_2,
else the ALTER OWNER won't work either.

            regards, tom lane

Hi,

On 2023-11-14 17:40:02 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2023-11-14 15:42:22 -0500, Tom Lane wrote:
> >> Moreover, reordering the GRANTs is no solution, because who promised that
> >> the schema owner granted you any permissions?
>
> > I'm not quite following - the schema is created in the dump, so the grant is
> > part of it?
>
> Yeah, but the GRANT will restore whatever permissions existed in the
> source database.  If the restoring user isn't super, those permissions
> don't necessarily grant him access.

Sure - that could obviously fail. But I just don't think it's the failure at
hand, given that the problem occurs even with the restorer being a member of
all the roles involved in the dump.

If you reorder the dump so that "GRANT ALL ON SCHEMA nosuper TO nosuper_2"
happens earlier, before "ALTER TABLE nosuper.tbl OWNER TO nosuper_2", the
restore succeeds.


> >> I experimented with making the restoring user be a member with inherit
> >> of the nosuper_N roles, and indeed I still see the failure above,
> >> which makes me wonder if the ACL check is being done correctly for
> >> that specific case.  The INHERIT bit ought to let it work.
>
> > The check is for nosuper_2 to have permission on the schema
>
> ... no, it should be for the user executing the ALTER to have permission.

That check succeed - what fails is a check on the new owner of the table. See
tablecmds.c ATExecChangeOwner:

                /* New owner must have CREATE privilege on namespace */
                aclresult = object_aclcheck(NamespaceRelationId, namespaceOid, newOwnerId,
                                            ACL_CREATE);
                if (aclresult != ACLCHECK_OK)
                    aclcheck_error(aclresult, OBJECT_SCHEMA,
                                   get_namespace_name(namespaceOid));
            }

If shared dependencies were taken into account and thus the
  "GRANT ALL ON SCHEMA nosuper TO nosuper_2"
were happening before
  "ALTER TABLE nosuper.tbl OWNER TO nosuper_2"
, it'd succeed.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2023-11-14 17:40:02 -0500, Tom Lane wrote:
>> ... no, it should be for the user executing the ALTER to have permission.

> That check succeed - what fails is a check on the new owner of the table. See
> tablecmds.c ATExecChangeOwner:

>                 /* New owner must have CREATE privilege on namespace */
>                 aclresult = object_aclcheck(NamespaceRelationId, namespaceOid, newOwnerId,
>                                             ACL_CREATE);

Oh!  I was just thinking about the initial object-lookup check,
I'd forgotten about the one in the ALTER OWNER code itself.

Hm.  The intent of that check is to ensure that the ALTER doesn't
produce a situation that the object-recipient user couldn't have
created by himself.  But I wonder if that's too narrow-minded, and
we should craft a new rule that allows things dependent on only the
calling user's permissions.  Maybe allow if either the calling user or
the recipient has CREATE on the schema?  Or allow if calling user has
ownership on the schema (implying that he could temporarily GRANT the
necessary rights and then undo it)?  Either of those would legitimize
what pg_dump wants to do.

This is getting way off-topic for pgsql-admin, btw.

            regards, tom lane