Thread: [GENERAL] intentional or oversight? pg_dump -c does not restore default priviliges on schema public

L.S.

I noticed the following and wondered whether this is intentional or an
oversight in pg_dump's '-c' option?

The clean option causes the public schema to be dropped and recreated, but
this is done with the default schema priviliges, which are not the same as the
ones assigned during create database:


*** USING PSQL

postgres=# create database publictest;

postgres=# \c publictest;

publictest=# \dn+
                          List of schemas
  Name  |  Owner   |  Access privileges   |      Description
--------+----------+----------------------+------------------------
 public | postgres | postgres=UC/postgres+| standard public schema
        |          | =UC/postgres         |
(1 row)



*** USING SHELL

host:~ # pg_dump -c -f /tmp/publictest -h localhost -Fp -U postgres publictest



*** USING PSQL

publictest=# \i /tmp/publictest

publictest=# \dn+
                        List of schemas
  Name  |  Owner   | Access privileges |      Description
--------+----------+-------------------+------------------------
 public | postgres |                   | standard public schema
(1 row)

publictest=# grant usage on schema public to public;
GRANT
publictest=# grant create on schema public to public;
GRANT

testje=# \dn+
                          List of schemas
  Name  |  Owner   |  Access privileges   |      Description
--------+----------+----------------------+------------------------
 public | postgres | postgres=UC/postgres+| standard public schema
        |          | =UC/postgres         |
(1 row)



--

Best,




Frank.

On 02/11/2017 12:42 PM, Frank van Vugt wrote:
> L.S.
>
> I noticed the following and wondered whether this is intentional or an
> oversight in pg_dump's '-c' option?

What version of Postgres?

Because when I do it on 9.5.5 I get:

test=# create database publictest;
                                                           
CREATE DATABASE
                                                           
test=# \c publictest;
                                                           
You are now connected to database "publictest" as user "postgres".
                                                           
publictest=# \dn+
                                                           
                          List of schemas
                                                           
  Name  |  Owner   |  Access privileges   |      Description
                                                           
--------+----------+----------------------+------------------------
                                                           
 public | postgres | postgres=UC/postgres+| standard public schema
                                                           
        |          | =UC/postgres         |
                                                           
(1 row)


aklaver@tito:~> pg_dump -c -f /tmp/publictest -h localhost -Fp -U postgres publictest

aklaver@tito:~> psql -d publictest -U postgres
                    
Null display is "NULL".
psql (9.5.5)
Type "help" for help.

publictest=# \i /tmp/publictest
SET
SET
SET
SET
SET
SET
SET
SET
DROP EXTENSION
DROP SCHEMA
CREATE SCHEMA
ALTER SCHEMA
COMMENT
CREATE EXTENSION
COMMENT
REVOKE
REVOKE
GRANT
GRANT

publictest=# \dn+
                          List of schemas
  Name  |  Owner   |  Access privileges   |      Description
--------+----------+----------------------+------------------------
 public | postgres | postgres=UC/postgres+| standard public schema
        |          | =UC/postgres         |
(1 row)

>
> The clean option causes the public schema to be dropped and recreated, but
> this is done with the default schema priviliges, which are not the same as the
> ones assigned during create database:
>
>
> *** USING PSQL
>
> postgres=# create database publictest;
>
> postgres=# \c publictest;
>
> publictest=# \dn+
>                           List of schemas
>   Name  |  Owner   |  Access privileges   |      Description
> --------+----------+----------------------+------------------------
>  public | postgres | postgres=UC/postgres+| standard public schema
>         |          | =UC/postgres         |
> (1 row)
>
>
>
> *** USING SHELL
>
> host:~ # pg_dump -c -f /tmp/publictest -h localhost -Fp -U postgres publictest
>
>
>
> *** USING PSQL
>
> publictest=# \i /tmp/publictest
>
> publictest=# \dn+
>                         List of schemas
>   Name  |  Owner   | Access privileges |      Description
> --------+----------+-------------------+------------------------
>  public | postgres |                   | standard public schema
> (1 row)
>
> publictest=# grant usage on schema public to public;
> GRANT
> publictest=# grant create on schema public to public;
> GRANT
>
> testje=# \dn+
>                           List of schemas
>   Name  |  Owner   |  Access privileges   |      Description
> --------+----------+----------------------+------------------------
>  public | postgres | postgres=UC/postgres+| standard public schema
>         |          | =UC/postgres         |
> (1 row)
>
>
>


--
Adrian Klaver
adrian.klaver@aklaver.com

Hi Adrian,

Op zaterdag 11 februari 2017 13:02:29 schreef Adrian Klaver:
> What version of Postgres?

Ah, sorry, missed copying that in:

postgres=# select version();
                                   version
------------------------------------------------------------------------------
 PostgreSQL 9.6.2 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.2, 64-bit
(1 row)



--

Best,




Frank.

On 02/11/2017 01:14 PM, Frank van Vugt wrote:
> Hi Adrian,
>
> Op zaterdag 11 februari 2017 13:02:29 schreef Adrian Klaver:
>> What version of Postgres?
>
> Ah, sorry, missed copying that in:
>
> postgres=# select version();
>                                    version
> ------------------------------------------------------------------------------
>  PostgreSQL 9.6.2 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.2, 64-bit
> (1 row)
>
>

I see the same thing now. A diff reveals:

aklaver@tito:~> diff /tmp/publictest95  /tmp/publictest96
5,6c5,6
< -- Dumped from database version 9.5.5
< -- Dumped by pg_dump version 9.5.5
---
 > -- Dumped from database version 9.6.1
 > -- Dumped by pg_dump version 9.6.1
9a10
 > SET idle_in_transaction_session_timeout = 0;
47,56d47
<
<
< --
< -- Name: public; Type: ACL; Schema: -; Owner: postgres
< --
<
< REVOKE ALL ON SCHEMA public FROM PUBLIC;
< REVOKE ALL ON SCHEMA public FROM postgres;
< GRANT ALL ON SCHEMA public TO postgres;
< GRANT ALL ON SCHEMA public TO PUBLIC;

That seems to cause a problem:

aklaver@tito:~> psql -d publictest -U guest
Null display is "NULL".
psql (9.5.5)
Type "help" for help.

publictest=> create table public.public_test(id int);
CREATE TABLE


aklaver@tito:~> /usr/local/pgsql96/bin/psql -d publictest -U guest -p 5442
Null display is "NULL".
psql (9.6.1)
Type "help" for help.

publictest=> create table public.public_test(id int);
ERROR:  permission denied for schema public
LINE 1: create table public.public_test(id int);




--
Adrian Klaver
adrian.klaver@aklaver.com

Hi Adrian,

Op zaterdag 11 februari 2017 13:31:17 schreef Adrian Klaver:
> I see the same thing now.

Glad you do ;)

> That seems to cause a problem

Yeah, I originally ran into this when I noticed that on a restored db a
regular user lost access to tables created by him in the public schema.

Granting 'usage' solves it, but I expect this isn't suppose to happen.



--

Best,




Frank.

Greetings,

* Frank van Vugt (ftm.van.vugt@foxi.nl) wrote:
> I noticed the following and wondered whether this is intentional or an
> oversight in pg_dump's '-c' option?
>
> The clean option causes the public schema to be dropped and recreated, but
> this is done with the default schema priviliges, which are not the same as the
> ones assigned during create database:

Interesting.  The reason this happens is that the privileges for the
public schema aren't dumped when they are the same as what you would get
from a default install in 9.6+, but using -c will end up dropping and
recreating it, which, as you note, will end up having different
privileges than the default install because they'll be the regular
default privilegs of "nothing" for schemas.

This is happening just for the public schema due to how it's handled in
a special way in pg_dump_archive.c:_printTocEntry().  This only impacts
ACLs because those are the only things which are different for the
public schema vs. it's initdb settings (there's no SECURITY LABEL, for
example, on the initdb'd public schema).

Due to how the public schema is (and always has been) handled in this
special way, this is a bug which needs to be fixed by having the default
ACLs for the public schema included in the dump output if -c is being
used.

I'm not seeing a very simple answer for this, unfortunately.  I'm
thinking we're going to need to pull the public schema's permissions
differently if we're in clean mode (by comparing to the default schema
privileges) vs. when we're not (in which case we should be comparing to
*public*'s initdb-time privileges, as we do now).  One option would be
to handle that by hacking up buildACLQueries() to take a flag which
basically means "we are dropping the public schema, do not consider its
pg_init_privs settings" but that strikes me as awful grotty.  Another
option would be to change getNamespaces() to run a special query
(perhaps as a UNION-ALL combination with the existing query) that is
just to get the info for the 'public' schema (and exclude the 'public'
schema from the first half of the query, of course).

Thanks for the report!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> I'm not seeing a very simple answer for this, unfortunately.

I'm inclined to argue that it was a mistake to include any non-pinned
objects in pg_init_privs.  The reason initdb leaves some objects unpinned
is exactly because they can be dropped and recreated, and that means
that their "initial" privileges are not static system properties.

We might need to fix pg_dump too, but I think these entries in
pg_init_privs should simply not be there.

            regards, tom lane

On 02/11/2017 02:06 PM, Stephen Frost wrote:
> Greetings,
>
> * Frank van Vugt (ftm.van.vugt@foxi.nl) wrote:
>> I noticed the following and wondered whether this is intentional or an
>> oversight in pg_dump's '-c' option?
>>
>> The clean option causes the public schema to be dropped and recreated, but
>> this is done with the default schema priviliges, which are not the same as the
>> ones assigned during create database:
>
> Interesting.  The reason this happens is that the privileges for the
> public schema aren't dumped when they are the same as what you would get
> from a default install in 9.6+, but using -c will end up dropping and
> recreating it, which, as you note, will end up having different
> privileges than the default install because they'll be the regular
> default privilegs of "nothing" for schemas.
>
> This is happening just for the public schema due to how it's handled in
> a special way in pg_dump_archive.c:_printTocEntry().  This only impacts
> ACLs because those are the only things which are different for the
> public schema vs. it's initdb settings (there's no SECURITY LABEL, for
> example, on the initdb'd public schema).
>
> Due to how the public schema is (and always has been) handled in this
> special way, this is a bug which needs to be fixed by having the default
> ACLs for the public schema included in the dump output if -c is being
> used.

I am following this up to the point of not understanding what exactly
changed between 9.5 and 9.6. Namely 9.5 does include the default ACL's
in the dump output and 9.6 does not.

>
> I'm not seeing a very simple answer for this, unfortunately.  I'm
> thinking we're going to need to pull the public schema's permissions
> differently if we're in clean mode (by comparing to the default schema
> privileges) vs. when we're not (in which case we should be comparing to
> *public*'s initdb-time privileges, as we do now).  One option would be
> to handle that by hacking up buildACLQueries() to take a flag which
> basically means "we are dropping the public schema, do not consider its
> pg_init_privs settings" but that strikes me as awful grotty.  Another
> option would be to change getNamespaces() to run a special query
> (perhaps as a UNION-ALL combination with the existing query) that is
> just to get the info for the 'public' schema (and exclude the 'public'
> schema from the first half of the query, of course).
>
> Thanks for the report!
>
> Stephen
>


--
Adrian Klaver
adrian.klaver@aklaver.com

Hi Tom/Stephen/Adrian,

Op zaterdag 11 februari 2017 15:28:55 schreef Tom Lane:
> I'm inclined to argue that it was a mistake to include any non-pinned
> objects in pg_init_privs.
<cut>
> We might need to fix pg_dump too, but I think these entries in
> pg_init_privs should simply not be there.

Thanks for picking this up, I'll probably see this subject pop up on hackers
and/or committers at some point ;)

Allow me to emphasize that this issue basically means that for v9.6 after
restoring a dump created with the '-c' option one ends up in a situation that
might be quite confusing for users that didn't have to pay much attention yet
to handling priviliges... i.e. trying even a plain select on table_a in the
public schema as a non-system user returns something like:
    ERROR:  relation "table_a" does not exist



--

Best,




Frank.

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > I'm not seeing a very simple answer for this, unfortunately.
>
> I'm inclined to argue that it was a mistake to include any non-pinned
> objects in pg_init_privs.  The reason initdb leaves some objects unpinned
> is exactly because they can be dropped and recreated, and that means
> that their "initial" privileges are not static system properties.
>
> We might need to fix pg_dump too, but I think these entries in
> pg_init_privs should simply not be there.

Wouldn't that mean we then need to drop and recreate all of those
non-pinned objects, with their ACLs, on a -c?

Or are you thinking we would just dump out whatever ACLs they have that
are different from default object creation, meaning that we'd have ACLs
for nearly all the views in pg_catalog and information_schema included
in regular pg_dump output?  Just dumping out the ACLs wouldn't
necessairly get us back to the same state as the database which was
dumped out though as the existing objects might already have some GRANTs
performed on them.

pg_init_privs also handles the initial privileges for extension objects,
though I'm guessing you aren't including those.  The initdb-time objects
are certainly similar though, in some ways, to extensions, the main
difference being that you can't really drop/recreate all of the initdb
time objects.

It seems to me that the oddball here is the public schema, and
specifically, the public schema when the user has been using the
from-initdb public schema (which has initdb-time privileges which are
recorded into pg_init_privs).  Had the user dropped and recreated the
public schema, the initdb-time privs for the original public schema in
pg_init_privs would have been removed, and the new public schema
wouldn't have had any entries in pg_init_privs.  The reason it's an
oddball is that it's the only from-initdb time object which is dropped
and recreated as part of pg_dump -c.  If we dropped all non-pinned
objects and recreated them when running pg_dump -c then perhaps what
you're suggesting would work, but we don't and I don't think that's
really what you were thinking we would do.

While it's a bit grotty, I tend to think the best approach is to
continue to assume that the only from-initdb-time object that the user
actually wants us to drop/recreate on a pg_dump -c is the public schema,
which means we need to handle things a bit differently when working in
-c mode for the public schema when pulling ACLs.

Thanks!

Stephen

Adrian,

* Adrian Klaver (adrian.klaver@aklaver.com) wrote:
> I am following this up to the point of not understanding what
> exactly changed between 9.5 and 9.6. Namely 9.5 does include the
> default ACL's in the dump output and 9.6 does not.

Quite a bit in pg_dump changed, but the relevant bit here is that we now
try to include in the pg_dump output any ACLs which have been changed
from their initdb-time settings for initdb-time objects.  What that
means is that if you don't change the privileges for the public schema
from what they're set to at initdb-time, then we don't dump out any ACL
commands for the public schema.  That ends up being incorrect in '-c'
mode because we drop the public schema in that mode and recreate it, in
which case we need to re-implement the ACLs which existed for the public
schema at initdb-time.

Thanks!

Stephen

Frank,

* Frank van Vugt (ftm.van.vugt@foxi.nl) wrote:
> Op zaterdag 11 februari 2017 15:28:55 schreef Tom Lane:
> > I'm inclined to argue that it was a mistake to include any non-pinned
> > objects in pg_init_privs.
> <cut>
> > We might need to fix pg_dump too, but I think these entries in
> > pg_init_privs should simply not be there.
>
> Thanks for picking this up, I'll probably see this subject pop up on hackers
> and/or committers at some point ;)

We should be able to get it addressed shortly.

> Allow me to emphasize that this issue basically means that for v9.6 after
> restoring a dump created with the '-c' option one ends up in a situation that
> might be quite confusing for users that didn't have to pay much attention yet
> to handling priviliges... i.e. trying even a plain select on table_a in the
> public schema as a non-system user returns something like:
>     ERROR:  relation "table_a" does not exist

Yes, it's unfortunate that many users aren't really familiar with
schema-level privileges.

For your specific case, if you drop/recreate the public schema in the
system that you're dumping the data from, and then set the ACLs to what
you want, they should be dumped out, even with a pg_dump -c.  It's only
when you're using -c with the initdb-time public schema, and initdb-time
ACLs, that the issue arises.

Thanks!

Stephen

On 02/13/2017 06:04 AM, Stephen Frost wrote:
> Adrian,
>
> * Adrian Klaver (adrian.klaver@aklaver.com) wrote:
>> I am following this up to the point of not understanding what
>> exactly changed between 9.5 and 9.6. Namely 9.5 does include the
>> default ACL's in the dump output and 9.6 does not.
>
> Quite a bit in pg_dump changed, but the relevant bit here is that we now
> try to include in the pg_dump output any ACLs which have been changed
> from their initdb-time settings for initdb-time objects.  What that
> means is that if you don't change the privileges for the public schema
> from what they're set to at initdb-time, then we don't dump out any ACL
> commands for the public schema.  That ends up being incorrect in '-c'
> mode because we drop the public schema in that mode and recreate it, in
> which case we need to re-implement the ACLs which existed for the public
> schema at initdb-time.

Thanks for the explanation in this post and your previous one. If I am
following pg_init_privs is the initial state of objects ACLs and if that
changes then those entries are removed. So would not the general case
be, on recreating an object use the ACLs in pg_init_privs if they exist
otherwise use the ACLs as they exist wherever they go to on change away
from pg_init_privs? I gather that is what you are proposing as a special
case for the public schema. Just wondering why it should not be the
general case?

>
> Thanks!
>
> Stephen
>


--
Adrian Klaver
adrian.klaver@aklaver.com

Greetings,

* Adrian Klaver (adrian.klaver@aklaver.com) wrote:
> On 02/13/2017 06:04 AM, Stephen Frost wrote:
> >* Adrian Klaver (adrian.klaver@aklaver.com) wrote:
> >>I am following this up to the point of not understanding what
> >>exactly changed between 9.5 and 9.6. Namely 9.5 does include the
> >>default ACL's in the dump output and 9.6 does not.
> >
> >Quite a bit in pg_dump changed, but the relevant bit here is that we now
> >try to include in the pg_dump output any ACLs which have been changed
> >from their initdb-time settings for initdb-time objects.  What that
> >means is that if you don't change the privileges for the public schema
> >from what they're set to at initdb-time, then we don't dump out any ACL
> >commands for the public schema.  That ends up being incorrect in '-c'
> >mode because we drop the public schema in that mode and recreate it, in
> >which case we need to re-implement the ACLs which existed for the public
> >schema at initdb-time.
>
> Thanks for the explanation in this post and your previous one. If I
> am following pg_init_privs is the initial state of objects ACLs and
> if that changes then those entries are removed.

No, if the object is *dropped* then the entry is removed from
pg_init_privs.  Otherwise, the entries in pg_init_privs aren't changed.

> So would not the
> general case be, on recreating an object use the ACLs in
> pg_init_privs if they exist otherwise use the ACLs as they exist
> wherever they go to on change away from pg_init_privs?

pg_init_privs doesn't track the object's name, so this isn't actually
possible.  Even if we did track the name of the object, I don't think
we'd actually want to set the privileges to what they were set to at
initdb time.  If you drop the public schema and then recreate it, are
you really expecting it to get the initdb-time privileges it had..?
How would you reconsile that with default privileges (which we don't
have for schemas right now, but it's been proposed...).

This case is about a pg_dump, which is a very different case in that we
want to recreate the state of the system as it existed at the time of
the dump.

> I gather that
> is what you are proposing as a special case for the public schema.
> Just wondering why it should not be the general case?

Not quite..  This is about what pg_dump does when a -c is used.
Unfortunately, it's *already* doing something special with the public
schema (if it wasn't, then this wouldn't really be an issue..).  This is
just about making it do the right thing in that already-existing
special-case.

Thanks!

Stephen

On 02/13/2017 07:52 AM, Stephen Frost wrote:
> Greetings,
>
> * Adrian Klaver (adrian.klaver@aklaver.com) wrote:
>> On 02/13/2017 06:04 AM, Stephen Frost wrote:
>>> * Adrian Klaver (adrian.klaver@aklaver.com) wrote:
>>>> I am following this up to the point of not understanding what
>>>> exactly changed between 9.5 and 9.6. Namely 9.5 does include the
>>>> default ACL's in the dump output and 9.6 does not.
>>>
>>> Quite a bit in pg_dump changed, but the relevant bit here is that we now
>>> try to include in the pg_dump output any ACLs which have been changed
>> >from their initdb-time settings for initdb-time objects.  What that
>>> means is that if you don't change the privileges for the public schema
>> >from what they're set to at initdb-time, then we don't dump out any ACL
>>> commands for the public schema.  That ends up being incorrect in '-c'
>>> mode because we drop the public schema in that mode and recreate it, in
>>> which case we need to re-implement the ACLs which existed for the public
>>> schema at initdb-time.
>>
>> Thanks for the explanation in this post and your previous one. If I
>> am following pg_init_privs is the initial state of objects ACLs and
>> if that changes then those entries are removed.
>
> No, if the object is *dropped* then the entry is removed from
> pg_init_privs.  Otherwise, the entries in pg_init_privs aren't changed.
>
>> So would not the
>> general case be, on recreating an object use the ACLs in
>> pg_init_privs if they exist otherwise use the ACLs as they exist
>> wherever they go to on change away from pg_init_privs?
>
> pg_init_privs doesn't track the object's name, so this isn't actually
> possible.  Even if we did track the name of the object, I don't think
> we'd actually want to set the privileges to what they were set to at
> initdb time.  If you drop the public schema and then recreate it, are
> you really expecting it to get the initdb-time privileges it had..?
> How would you reconsile that with default privileges (which we don't
> have for schemas right now, but it's been proposed...).
>
> This case is about a pg_dump, which is a very different case in that we
> want to recreate the state of the system as it existed at the time of
> the dump.
>
>> I gather that
>> is what you are proposing as a special case for the public schema.
>> Just wondering why it should not be the general case?
>
> Not quite..  This is about what pg_dump does when a -c is used.
> Unfortunately, it's *already* doing something special with the public
> schema (if it wasn't, then this wouldn't really be an issue..).  This is
> just about making it do the right thing in that already-existing
> special-case.

Alright I see now, thanks.

>
> Thanks!
>
> Stephen
>


--
Adrian Klaver
adrian.klaver@aklaver.com

Hi Stephen,

Op maandag 13 februari 2017 09:10:42 schreef Stephen Frost:
> We should be able to get it addressed shortly.

Great, 'as always', I'd like to add!

Thanks for the great work, people. This cannot be stated too often...

> For your specific case

Thanks for the additional info, interesting.

> Yes, it's unfortunate that many users aren't really familiar with
> schema-level privileges.

Well, I didn't run into this issue with any of my db's that 'nicely' use
tables in various schema's, it was actually the one 'older' db with everything
in the public schema that brought it up, so maybe keeping one of those around
isn't too bad an idea ;)




--

Best,




Frank.

Frank,

* Frank van Vugt (ftm.van.vugt@foxi.nl) wrote:
> Well, I didn't run into this issue with any of my db's that 'nicely' use
> tables in various schema's, it was actually the one 'older' db with everything
> in the public schema that brought it up, so maybe keeping one of those around
> isn't too bad an idea ;)

Yeah, I'll be including this in a regression test also, to make sure we
don't end up breaking this special case again in the future.

Thanks!

Stephen

Greetings,

* Stephen Frost (sfrost@snowman.net) wrote:
> * Frank van Vugt (ftm.van.vugt@foxi.nl) wrote:
> > Well, I didn't run into this issue with any of my db's that 'nicely' use
> > tables in various schema's, it was actually the one 'older' db with everything
> > in the public schema that brought it up, so maybe keeping one of those around
> > isn't too bad an idea ;)
>
> Yeah, I'll be including this in a regression test also, to make sure we
> don't end up breaking this special case again in the future.

I've now pushed a fix for this, would be great to know if it solves your
issue.

Thanks!

Stephen