Thread: BUG #2085: pg_dump incompletely dumps ACLs

The following bug has been logged online:

Bug reference:      2085
Logged by:          David Begley
Email address:      d.begley@uws.edu.au
PostgreSQL version: 8.1.0
Operating system:   Windows XP
Description:        pg_dump incompletely dumps ACLs
Details:

Running the following command:

pg_dump -v -C -f \windows\temp\dump.sql dbname

produced a plain text dump of the named database containing various
ACL-related statements (GRANT/REVOKE), but unfortunately some information
from the original running database was lost.

This means when the dump is imported to another PostgreSQL instance, the
resultant ACLs do not match those of the original instance.

The ACLs in question are those represented as "{}" (ie., nothing between the
braces);  such an ACL on a database object or language object (there may be
others affected but these are the only two I have so far confirmed) is not
properly recorded by pg_dump, but the same ACL on a function is recorded
(and recreated on the destination PostgreSQL instance) perfectly fine.

The dump generated contains no GRANT/REVOKE statements for the database
itself at all, whilst the language object includes only a REVOKE from PUBLIC
(a REVOKE from the owner is missing).  For functions, a REVOKE from both
PUBLIC and the owner are correctly included by pg_dump.

The dump was generated/recreated by a user with "superuser" access to the
database.

This may sound like a puzzling ACL, but regardless the dump should include
all information from the source database so it can be reproduced correctly
when restored.

Cheers..

"David Begley" <d.begley@uws.edu.au> writes:
> This means when the dump is imported to another PostgreSQL instance, the
> resultant ACLs do not match those of the original instance.

Please provide a concrete test case, not handwaving.

            regards, tom lane

Earlier today, Tom Lane wrote:

> "David Begley" <d.begley@uws.edu.au> writes:
> > This means when the dump is imported to another PostgreSQL instance, the
> > resultant ACLs do not match those of the original instance.
>
> Please provide a concrete test case, not handwaving.

C:\Program Files\PostgreSQL\8.1\bin>createdb concrete "Test Case"
CREATE DATABASE
COMMENT

C:\Program Files\PostgreSQL\8.1\bin>psql -q concrete
concrete=# REVOKE ALL ON DATABASE concrete FROM PUBLIC;
concrete=# REVOKE ALL ON DATABASE concrete FROM david;
concrete=# SELECT datacl FROM pg_database WHERE datname = 'concrete';
 datacl
--------
 {}
(1 row)

concrete=# \q

C:\Program Files\PostgreSQL\8.1\bin>pg_dump -C -f \windows\temp\dump.sql concrete

C:\Program Files\PostgreSQL\8.1\bin>dropdb -q concrete

C:\Program Files\PostgreSQL\8.1\bin>psql -f \windows\temp\dump.sql postgres
SET
SET
SET
CREATE DATABASE
ALTER DATABASE
You are now connected to database "concrete".
SET
SET
SET
COMMENT
COMMENT
CREATE LANGUAGE
REVOKE
REVOKE
GRANT
GRANT

C:\Program Files\PostgreSQL\8.1\bin>psql -q concrete
concrete=# SELECT datacl FROM pg_database WHERE datname = 'concrete';
 datacl
--------

(1 row)

concrete=# -- datacl in restored database does not match origin database
concrete=# \q

David J N Begley <d.begley@uws.edu.au> writes:
> Earlier today, Tom Lane wrote:
>> Please provide a concrete test case, not handwaving.

> C:\Program Files\PostgreSQL\8.1\bin>createdb concrete "Test Case"
> CREATE DATABASE
> COMMENT

> C:\Program Files\PostgreSQL\8.1\bin>psql -q concrete
> concrete=# REVOKE ALL ON DATABASE concrete FROM PUBLIC;
> concrete=# REVOKE ALL ON DATABASE concrete FROM david;
> concrete=# SELECT datacl FROM pg_database WHERE datname = 'concrete';
>  datacl
> --------
>  {}
> (1 row)

> concrete=# \q

> C:\Program Files\PostgreSQL\8.1\bin>pg_dump -C -f \windows\temp\dump.sql concrete

Database ACLs are dumped by pg_dumpall, not pg_dump.  I agree this is a
bit inconsistent considering that pg_dump has a -C option, but the -C
option has always been pretty poorly thought out :-(.

Given that -C overlaps pg_dumpall functionality anyway, maybe it should
dump GRANT/REVOKE commands for the database too?  Any thoughts pro or
con out there?

            regards, tom lane

Tom Lane wrote:

> Given that -C overlaps pg_dumpall functionality anyway, maybe it should
> dump GRANT/REVOKE commands for the database too?  Any thoughts pro or
> con out there?

I agree.  Why have only half a funcionality if we can have the whole
thing?  Maybe we can take that part of of pg_dumpall if at all possible,
and make it use pg_dump's.

Does pg_dump -C include the database comment already?  If not, maybe
it's worth to add it as well.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Fri, 2 Dec 2005, Alvaro Herrera wrote:

> Tom Lane wrote:
>
> > Given that -C overlaps pg_dumpall functionality anyway, maybe it should
> > dump GRANT/REVOKE commands for the database too?  Any thoughts pro or
> > con out there?

See below - I hadn't tried it previously, but having now tried pg_dumpall I've
found it also has an ACL-loss bug.

> I agree.  Why have only half a funcionality if we can have the whole
> thing?  Maybe we can take that part of of pg_dumpall if at all possible,
> and make it use pg_dump's.

Speaking as just a dumb end-user, I get the impression that pg_dump is used to
extract a single database, not just part of a database;  sayeth the manual:

  "pg_dump is a utility for backing up a PostgreSQL database. [...] Script
  dumps are plain-text files containing the SQL commands required to
  reconstruct the database to the state it was in at the time it was
  saved."

Unless I'm missing something, that means pg_dump should include (when told to
do so, as in this case) _all_ commands required to restore the database "to
the state it was in at the time it was saved".  What's happening at the moment
is that some GRANT/REVOKE commands are included but others are missing,
meaning any transfer of a database is potentially losing (at least some) ACLs
along the way.

I have verified that _no_ GRANT/REVOKE commands are dumped for the database,
and only some GRANT/REVOKE commands are dumped for "language" objects (see
below);  near as I can tell, all GRANT/REVOKE commands are dumped for
"function" and "table" objects, but it's early in the morning so I may be
missing something.

When I say that "only some" commands are dumped for "language" objects,
remember my original bug report - I tripped over this whilst verifying whether
or not "{}" ACLs (ie., not even owner or PUBLIC) are correctly
dumped/restored.  For functions, I find this in the pg_dump output:

REVOKE ALL ON FUNCTION function_check() FROM PUBLIC;
REVOKE ALL ON FUNCTION function_check() FROM postgres;

In other words, dump/restore appears to do its job.  For the database, there
are no GRANT/REVOKE statements at all.  For "language" objects, pg_dump
provides:

REVOKE ALL ON LANGUAGE plpgsql FROM PUBLIC;

Here, a second statement revoking rights from the owner (in order to achieve
the "{}" ACL in the original database) is missing (yet it revoked rights
from PUBLIC - why only half an ACL?).

> Does pg_dump -C include the database comment already?  If not, maybe
> it's worth to add it as well.

A very basic test for me shows the database comment is already included.

Bearing in mind Tom's statement regarding pg_dump versus pg_dumpall, I ran the
latter to check its output (though in reality it's not the entire cluster I'm
trying to dump here):

- pg_dumpall includes the two missing REVOKE statements on the database
  that pg_dump is mising;

- both pg_dump and pg_dumpall include the database comment;  and,

- both pg_dump and pg_dumpall are missing the second REVOKE statement (for
  the owner) on the "language" object (ie., bug in both).

I don't know what other objects may be affected (either in pg_dump or
pg_dumpall).

Cheers..

David J N Begley <d.begley@uws.edu.au> writes:
> I have verified that _no_ GRANT/REVOKE commands are dumped for the database,
> and only some GRANT/REVOKE commands are dumped for "language" objects (see
> below);

The latter is not really a bug.  Languages don't currently have owners
(ie there is no owner column in pg_language).  For ACL-munging purposes
we act as though the bootstrap superuser owns the language, that is,
that userid is shown as the grantor of privileges.  But having a
superuser revoke his own privileges is a no-op, because he's a superuser
and the privileges aren't going to be enforced against him anyway.  So
the fact that pg_dump doesn't process that part of the ACL isn't very
meaningful.

Sooner or later we may get around to assigning explicit owners to
languages, but it's not a high-priority problem --- AFAICS the lack
of ownership doesn't create any problems worse than these sorts of
corner-case confusions.  It'll always be true that superuserdom is
needed to create a PL, and distinguishing one superuser from another
is not a particularly useful activity in the context of permission
checks ...

I fooled around with having pg_dump explicitly treat the language as
being owned by the bootstrap superuser, and think I may apply the patch
now even though it doesn't really matter, because it does clean up the
output a little bit --- instead of

--
-- Name: pltcl; Type: ACL; Schema: -; Owner:
--

REVOKE ALL ON LANGUAGE pltcl FROM PUBLIC;
SET SESSION AUTHORIZATION postgres;
GRANT ALL ON LANGUAGE pltcl TO postgres;
RESET SESSION AUTHORIZATION;
SET SESSION AUTHORIZATION postgres;
GRANT ALL ON LANGUAGE pltcl TO tgl;
RESET SESSION AUTHORIZATION;

I get

--
-- Name: pltcl; Type: ACL; Schema: -; Owner: postgres
--

REVOKE ALL ON LANGUAGE pltcl FROM PUBLIC;
REVOKE ALL ON LANGUAGE pltcl FROM postgres;
GRANT ALL ON LANGUAGE pltcl TO postgres;
GRANT ALL ON LANGUAGE pltcl TO tgl;

for a pg_language ACL of "{postgres=U/postgres,tgl=U/postgres}".
Avoiding the SET SESSION AUTHORIZATIONs seems like a good idea.

            regards, tom lane

On Sat, 3 Dec 2005, Tom Lane wrote:

> The latter is not really a bug.  Languages don't currently have owners
> (ie there is no owner column in pg_language).  For ACL-munging purposes

The true behaviour of the ACL may be somewhat unusual - understood (after all,
I admitted at the outset that the use of the "{}" ACLs could seem strange).

> and the privileges aren't going to be enforced against him anyway.  So
> the fact that pg_dump doesn't process that part of the ACL isn't very
> meaningful.

True, though that could have unintended side effects in future as certain
details (ACLs or whatever) are lost/modified as a database is dumped/restored
between different PostgreSQL versions (something that was a no-op before
suddenly creates problems a newer version due to feature changes/additions).

Thanks for taking the time to investigate this - my whole concern was just
that the doco-set user expectations be met by the dump tools in reality (ie.,
if a whole database dump is requested, then everything is restored in the
destination cluster).

Thanks again..