Horton

Thread

Commits

  1. Doc: word-smith the discussion of secure schema usage patterns.

  2. Fix the public schema's permissions in a separate test script.

  3. Revoke PUBLIC CREATE from public schema, now owned by pg_database_owner.

  1. pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Noah Misch <noah@leadboat.com> — 2021-09-10T06:39:18Z

    Revoke PUBLIC CREATE from public schema, now owned by pg_database_owner.

This switches the default ACL to what the documentation has recommended
since CVE-2018-1058.  Upgrades will carry forward any old ownership and
ACL.  Sites that declined the 2018 recommendation should take a fresh
look.  Recipes for commissioning a new database cluster from scratch may
need to create a schema, grant more privileges, etc.  Out-of-tree test
suites may require such updates.

Reviewed by Peter Eisentraut.

Discussion: https://postgr.es/m/20201031163518.GB4039133@rfd.leadboat.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/b073c3ccd06e4cb845e121387a43faa8c68a7b62

Modified Files
--------------
contrib/postgres_fdw/expected/postgres_fdw.out |  2 +-
contrib/postgres_fdw/sql/postgres_fdw.sql      |  2 +-
doc/src/sgml/ddl.sgml                          | 56 ++++++++++++++------------
doc/src/sgml/user-manag.sgml                   | 19 ++++-----
src/bin/initdb/initdb.c                        |  3 +-
src/bin/pg_dump/pg_dump.c                      | 28 ++++++++-----
src/bin/pg_dump/t/002_pg_dump.pl               | 19 ++++-----
src/include/catalog/catversion.h               |  2 +-
src/include/catalog/pg_namespace.dat           |  2 +-
src/pl/plperl/expected/plperl_setup.out        |  4 ++
src/pl/plperl/sql/plperl_setup.sql             |  4 ++
src/test/regress/input/tablespace.source       |  5 ++-
src/test/regress/output/tablespace.source      |  4 +-
13 files changed, 86 insertions(+), 64 deletions(-)

  2. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-17T17:52:39Z

    Noah Misch <noah@leadboat.com> writes:
> Revoke PUBLIC CREATE from public schema, now owned by pg_database_owner.

I've just stumbled across a testing problem created by this commit:
if you try to skip the tablespace test, the rest of the run falls
over, because this bit doesn't get executed:

-- Rest of this suite can use the public schema freely.
GRANT ALL ON SCHEMA public TO public;

Skipping the tablespace test is something I've been accustomed to do
when testing replication with the standby on the same machine as the
primary, because otherwise you've got to fool with keeping the
standby from overwriting the primary's tablespaces.  This hack made
that a lot more painful.

I'm inclined to think the cleanest fix is to move this step into a
new script, say "test_setup.sql", that is scheduled by itself just
after tablespace.sql.  It's sort of annoying to fire up a psql+backend
for just one command, but perhaps there's other stuff that could be
put there too.

Another possibility is to add that GRANT to the list of stuff that
pg_regress.c does by default.  If there's actually reason for
tablespace.sql to run without that, it could revoke and re-grant
the public permissions.  This way would have the advantage of
being less likely to break other test suites.

			regards, tom lane

  3. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Noah Misch <noah@leadboat.com> — 2021-12-17T18:25:18Z

    On Fri, Dec 17, 2021 at 12:52:39PM -0500, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > Revoke PUBLIC CREATE from public schema, now owned by pg_database_owner.
> 
> I've just stumbled across a testing problem created by this commit:
> if you try to skip the tablespace test, the rest of the run falls
> over, because this bit doesn't get executed:
> 
> -- Rest of this suite can use the public schema freely.
> GRANT ALL ON SCHEMA public TO public;
> 
> Skipping the tablespace test is something I've been accustomed to do
> when testing replication with the standby on the same machine as the
> primary, because otherwise you've got to fool with keeping the
> standby from overwriting the primary's tablespaces.  This hack made
> that a lot more painful.
> 
> I'm inclined to think the cleanest fix is to move this step into a
> new script, say "test_setup.sql", that is scheduled by itself just
> after tablespace.sql.

I like that solution for your use case.

> It's sort of annoying to fire up a psql+backend
> for just one command, but perhaps there's other stuff that could be
> put there too.

Yes.  The src/test/regress suite would be in a better place if one could run
most test files via a schedule containing only two files, the setup file and
the file of interest.  Adding things like the "CREATE TABLE tenk1" to the
setup file would help that.

  4. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-17T18:41:00Z

    Noah Misch <noah@leadboat.com> writes:
> On Fri, Dec 17, 2021 at 12:52:39PM -0500, Tom Lane wrote:
>> It's sort of annoying to fire up a psql+backend
>> for just one command, but perhaps there's other stuff that could be
>> put there too.

> Yes.  The src/test/regress suite would be in a better place if one could run
> most test files via a schedule containing only two files, the setup file and
> the file of interest.  Adding things like the "CREATE TABLE tenk1" to the
> setup file would help that.

If we're thinking of a generalized setup file, putting it after the
tablespace test feels pretty weird.  What was your motivation for
doing this at the end of tablespace.source rather than the start?
It doesn't look like that test in itself had any interesting
dependencies on public not being writable.

			regards, tom lane

  5. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Noah Misch <noah@leadboat.com> — 2021-12-17T19:47:20Z

    On Fri, Dec 17, 2021 at 01:41:00PM -0500, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Fri, Dec 17, 2021 at 12:52:39PM -0500, Tom Lane wrote:
> >> It's sort of annoying to fire up a psql+backend
> >> for just one command, but perhaps there's other stuff that could be
> >> put there too.
> 
> > Yes.  The src/test/regress suite would be in a better place if one could run
> > most test files via a schedule containing only two files, the setup file and
> > the file of interest.  Adding things like the "CREATE TABLE tenk1" to the
> > setup file would help that.
> 
> If we're thinking of a generalized setup file, putting it after the
> tablespace test feels pretty weird.  What was your motivation for
> doing this at the end of tablespace.source rather than the start?

I did it that way so a bit of the "make check" suite would exercise the
standard user experience.  That's a minor concern, so putting the setup file
before the tablespace file is fine.  Various contrib and TAP suites will still
test the standard user experience.

> It doesn't look like that test in itself had any interesting
> dependencies on public not being writable.

Right.

  6. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-17T19:57:08Z

    Noah Misch <noah@leadboat.com> writes:
> On Fri, Dec 17, 2021 at 01:41:00PM -0500, Tom Lane wrote:
>> If we're thinking of a generalized setup file, putting it after the
>> tablespace test feels pretty weird.  What was your motivation for
>> doing this at the end of tablespace.source rather than the start?

> I did it that way so a bit of the "make check" suite would exercise the
> standard user experience.  That's a minor concern, so putting the setup file
> before the tablespace file is fine.  Various contrib and TAP suites will still
> test the standard user experience.

Check.  I'll make it so in a little bit.

			regards, tom lane

  7. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Robert Haas <robertmhaas@gmail.com> — 2022-11-29T19:22:59Z

    On Fri, Sep 10, 2021 at 2:39 AM Noah Misch <noah@leadboat.com> wrote:
> Revoke PUBLIC CREATE from public schema, now owned by pg_database_owner.
>
> This switches the default ACL to what the documentation has recommended
> since CVE-2018-1058.  Upgrades will carry forward any old ownership and
> ACL.  Sites that declined the 2018 recommendation should take a fresh
> look.  Recipes for commissioning a new database cluster from scratch may
> need to create a schema, grant more privileges, etc.  Out-of-tree test
> suites may require such updates.

I was looking at the changes that this commit made to ddl.sgml today
and I feel that it's not quite ideal. Under "Constrain ordinary users
to user-private schemas" it first says "To implement this, first issue
<literal>REVOKE CREATE ON SCHEMA public FROM PUBLIC</literal>" and
then later says, oh but wait, you actually don't need to do that
unless you're upgrading. That seems a bit backwards to me: I think we
should talk about the current state of play first, and then add the
notes about upgrading afterwards.

Here's a proposed patch to do that.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  8. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Justin Pryzby <pryzby@telsasoft.com> — 2022-11-29T19:31:59Z

    On Tue, Nov 29, 2022 at 02:22:59PM -0500, Robert Haas wrote:
> Here's a proposed patch to do that.

If I'm not wrong, you message includes a diffstat but without the patch
itself.

  9. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Robert Haas <robertmhaas@gmail.com> — 2022-11-29T19:34:01Z

    On Tue, Nov 29, 2022 at 2:32 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> On Tue, Nov 29, 2022 at 02:22:59PM -0500, Robert Haas wrote:
> > Here's a proposed patch to do that.
>
> If I'm not wrong, you message includes a diffstat but without the patch
> itself.

D'oh.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  10. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Noah Misch <noah@leadboat.com> — 2022-11-30T07:07:01Z

    On Tue, Nov 29, 2022 at 02:22:59PM -0500, Robert Haas wrote:
> On Fri, Sep 10, 2021 at 2:39 AM Noah Misch <noah@leadboat.com> wrote:
> > Revoke PUBLIC CREATE from public schema, now owned by pg_database_owner.
> >
> > This switches the default ACL to what the documentation has recommended
> > since CVE-2018-1058.  Upgrades will carry forward any old ownership and
> > ACL.  Sites that declined the 2018 recommendation should take a fresh
> > look.  Recipes for commissioning a new database cluster from scratch may
> > need to create a schema, grant more privileges, etc.  Out-of-tree test
> > suites may require such updates.
> 
> I was looking at the changes that this commit made to ddl.sgml today
> and I feel that it's not quite ideal. Under "Constrain ordinary users
> to user-private schemas" it first says "To implement this, first issue
> <literal>REVOKE CREATE ON SCHEMA public FROM PUBLIC</literal>" and
> then later says, oh but wait, you actually don't need to do that
> unless you're upgrading. That seems a bit backwards to me: I think we
> should talk about the current state of play first, and then add the
> notes about upgrading afterwards.

In general, the documentation should prefer simpler decision trees.
Especially so where the wrong choice causes no error, yet leaves a security
vulnerability.  The unconditional REVOKE has no drawbacks; it's harmless where
it's a no-op.  That was the rationale behind the current text.  Upgrades
aren't the only issue; another DBA may have changed the ACL since initdb.

  11. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Robert Haas <robertmhaas@gmail.com> — 2022-11-30T13:39:23Z

    On Wed, Nov 30, 2022 at 2:07 AM Noah Misch <noah@leadboat.com> wrote:
> In general, the documentation should prefer simpler decision trees.

True, but I found the current text confusing, which is also something
to consider.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  12. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Noah Misch <noah@leadboat.com> — 2022-11-30T15:01:36Z

    On Wed, Nov 30, 2022 at 08:39:23AM -0500, Robert Haas wrote:
> On Wed, Nov 30, 2022 at 2:07 AM Noah Misch <noah@leadboat.com> wrote:
> > In general, the documentation should prefer simpler decision trees.
> 
> True, but I found the current text confusing, which is also something
> to consider.

Could remove the paragraph about v14.  Could have that paragraph say
explicitly that the REVOKE is a no-op.  Would either of those be an
improvement?

  13. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Robert Haas <robertmhaas@gmail.com> — 2022-11-30T15:44:12Z

    On Wed, Nov 30, 2022 at 10:01 AM Noah Misch <noah@leadboat.com> wrote:
> On Wed, Nov 30, 2022 at 08:39:23AM -0500, Robert Haas wrote:
> > On Wed, Nov 30, 2022 at 2:07 AM Noah Misch <noah@leadboat.com> wrote:
> > > In general, the documentation should prefer simpler decision trees.
> >
> > True, but I found the current text confusing, which is also something
> > to consider.
>
> Could remove the paragraph about v14.  Could have that paragraph say
> explicitly that the REVOKE is a no-op.  Would either of those be an
> improvement?

Well, I thought what I proposed was a nice improvement, but I guess if
you don't like it I'm not inclined to spend a lot of time discussing
other possibilities. If we get some opinions from more people that may
make it clearer which direction to go; if I'm the only one that
doesn't like the way it is now, it's probably not that important.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  14. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-30T22:35:01Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Nov 30, 2022 at 10:01 AM Noah Misch <noah@leadboat.com> wrote:
>> Could remove the paragraph about v14.  Could have that paragraph say
>> explicitly that the REVOKE is a no-op.  Would either of those be an
>> improvement?

> Well, I thought what I proposed was a nice improvement, but I guess if
> you don't like it I'm not inclined to spend a lot of time discussing
> other possibilities. If we get some opinions from more people that may
> make it clearer which direction to go; if I'm the only one that
> doesn't like the way it is now, it's probably not that important.

Hey, I'll step up to the plate ;-)

I agree that it's confusing to tell people to do a REVOKE that might do
nothing.  A parenthetical note explaining that might help, but the text
is pretty dense already, so really I'd rather have that info in a
separate para.

Also, I'd like to structure things so that the first para covers what
you need to know in a clean v15+ installation, and details that only
apply in upgrade scenarios are in the second para.  The upgrade scenario
is going to be interesting to fewer and fewer people over time, so let's
not clutter the lede with it.

So maybe about like this?

    Constrain ordinary users to user-private schemas.  To implement
    this pattern, for every user needing to create non-temporary
    objects, create a schema with the same name as that user.  (Recall
    that the default search path starts with $user, which resolves to
    the user name. Therefore, if each user has a separate schema, they
    access their own schemas by default.)  Also ensure that no other
    schemas have public CREATE privileges.  This pattern is a secure
    schema usage pattern unless an untrusted user is the database
    owner or holds the CREATEROLE privilege, in which case no secure
    schema usage pattern exists.

    In PostgreSQL 15 and later, the default configuration supports
    this usage pattern.  In prior versions, or when using a database
    that has been upgraded from a prior version, you will need to
    remove the public CREATE privilege from the public schema (issue
    REVOKE CREATE ON SCHEMA public FROM PUBLIC).  Then consider
    auditing the public schema for objects named like objects in
    schema pg_catalog.

This is close to what Robert wrote, but not exactly the same,
so probably it will make neither of you happy ;-)

BTW, is "create a schema with the same name" sufficient detail?
You have to either make it owned by that user, or explicitly
grant CREATE permission on it.  I'm not sure if that detail
belongs here, but it feels like maybe it does.

			regards, tom lane

  15. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Isaac Morland <isaac.morland@gmail.com> — 2022-11-30T22:57:20Z

    On Wed, 30 Nov 2022 at 17:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:

BTW, is "create a schema with the same name" sufficient detail?
> You have to either make it owned by that user, or explicitly
> grant CREATE permission on it.  I'm not sure if that detail
> belongs here, but it feels like maybe it does.


It might be worth mentioning AUTHORIZATION. The easiest way to create an
appropriately named schema for a user is "CREATE SCHEMA AUTHORIZATION
username".

  16. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    David G. Johnston <david.g.johnston@gmail.com> — 2022-11-30T22:58:36Z

    On Wed, Nov 30, 2022 at 3:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

>
> BTW, is "create a schema with the same name" sufficient detail?
> You have to either make it owned by that user, or explicitly
> grant CREATE permission on it.  I'm not sure if that detail
> belongs here, but it feels like maybe it does.
>
>
I'd mention the ownership variant and suggest using the AUTHORIZATION
clause, with an explicit example.

CREATE SCHEMA role_name AUTHORIZATION role_name;

David J.

  17. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Robert Haas <robertmhaas@gmail.com> — 2022-12-01T04:32:40Z

    On Wed, Nov 30, 2022 at 5:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Also, I'd like to structure things so that the first para covers what
> you need to know in a clean v15+ installation, and details that only
> apply in upgrade scenarios are in the second para.  The upgrade scenario
> is going to be interesting to fewer and fewer people over time, so let's
> not clutter the lede with it.

Right, that was my main feeling about this.

> So maybe about like this?
>
>     Constrain ordinary users to user-private schemas.  To implement
>     this pattern, for every user needing to create non-temporary
>     objects, create a schema with the same name as that user.  (Recall
>     that the default search path starts with $user, which resolves to
>     the user name. Therefore, if each user has a separate schema, they
>     access their own schemas by default.)  Also ensure that no other
>     schemas have public CREATE privileges.  This pattern is a secure
>     schema usage pattern unless an untrusted user is the database
>     owner or holds the CREATEROLE privilege, in which case no secure
>     schema usage pattern exists.
>
>     In PostgreSQL 15 and later, the default configuration supports
>     this usage pattern.  In prior versions, or when using a database
>     that has been upgraded from a prior version, you will need to
>     remove the public CREATE privilege from the public schema (issue
>     REVOKE CREATE ON SCHEMA public FROM PUBLIC).  Then consider
>     auditing the public schema for objects named like objects in
>     schema pg_catalog.
>
> This is close to what Robert wrote, but not exactly the same,
> so probably it will make neither of you happy ;-)

I haven't looked at how it's different from what I wrote exactly, but
it seems fine to me.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  18. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Noah Misch <noah@leadboat.com> — 2022-12-01T08:25:33Z

    On Wed, Nov 30, 2022 at 05:35:01PM -0500, Tom Lane wrote:
> Also, I'd like to structure things so that the first para covers what
> you need to know in a clean v15+ installation, and details that only
> apply in upgrade scenarios are in the second para.  The upgrade scenario
> is going to be interesting to fewer and fewer people over time, so let's
> not clutter the lede with it.
> 
> So maybe about like this?
> 
>     Constrain ordinary users to user-private schemas.  To implement
>     this pattern, for every user needing to create non-temporary
>     objects, create a schema with the same name as that user.  (Recall
>     that the default search path starts with $user, which resolves to
>     the user name. Therefore, if each user has a separate schema, they
>     access their own schemas by default.)  Also ensure that no other
>     schemas have public CREATE privileges.  This pattern is a secure
>     schema usage pattern unless an untrusted user is the database
>     owner or holds the CREATEROLE privilege, in which case no secure
>     schema usage pattern exists.

This is free from the problem found in ddl-create-public-reorg-really.patch.
However, the word "other" doesn't belong there.  (The per-user schemas should
not have public CREATE privilege.)  I would also move that same sentence up
front, like this:

    Constrain ordinary users to user-private schemas.  To implement this
    pattern, first ensure that no schemas have public CREATE privileges.
    Then, for every user needing to create non-temporary objects, create a
    schema with the same name as that user.  (Recall that the default search
    path starts with $user, which resolves to the user name. Therefore, if
    each user has a separate schema, they access their own schemas by
    default.)  This pattern is a secure schema usage pattern unless an
    untrusted user is the database owner or holds the CREATEROLE privilege, in
    which case no secure schema usage pattern exists.

With that, I think you have improved on the status quo.  Thanks.

>     In PostgreSQL 15 and later, the default configuration supports
>     this usage pattern.  In prior versions, or when using a database
>     that has been upgraded from a prior version, you will need to
>     remove the public CREATE privilege from the public schema (issue
>     REVOKE CREATE ON SCHEMA public FROM PUBLIC).  Then consider
>     auditing the public schema for objects named like objects in
>     schema pg_catalog.

> BTW, is "create a schema with the same name" sufficient detail?
> You have to either make it owned by that user, or explicitly
> grant CREATE permission on it.  I'm not sure if that detail
> belongs here, but it feels like maybe it does.

Maybe.  Failing to GRANT that will yield a clear error when the user starts
work, so it's not critical to explain here.

  19. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-12-01T11:16:39Z

    On 2022-Dec-01, Noah Misch wrote:

> This is free from the problem found in ddl-create-public-reorg-really.patch.
> However, the word "other" doesn't belong there.  (The per-user schemas should
> not have public CREATE privilege.)  I would also move that same sentence up
> front, like this:
> 
>     Constrain ordinary users to user-private schemas.  To implement this
>     pattern, first ensure that no schemas have public CREATE privileges.
>     Then, for every user needing to create non-temporary objects, create a
>     schema with the same name as that user.  (Recall that the default search
>     path starts with $user, which resolves to the user name. Therefore, if
>     each user has a separate schema, they access their own schemas by
>     default.)  This pattern is a secure schema usage pattern unless an
>     untrusted user is the database owner or holds the CREATEROLE privilege, in
>     which case no secure schema usage pattern exists.

+1 LGTM

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

  20. Re: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-12-01T14:24:43Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2022-Dec-01, Noah Misch wrote:
>> This is free from the problem found in ddl-create-public-reorg-really.patch.
>> However, the word "other" doesn't belong there.  (The per-user schemas should
>> not have public CREATE privilege.)  I would also move that same sentence up
>> front, like this:
>> 
>> Constrain ordinary users to user-private schemas.  To implement this
>> pattern, first ensure that no schemas have public CREATE privileges.
>> Then, for every user needing to create non-temporary objects, create a
>> schema with the same name as that user.  (Recall that the default search
>> path starts with $user, which resolves to the user name. Therefore, if
>> each user has a separate schema, they access their own schemas by
>> default.)  This pattern is a secure schema usage pattern unless an
>> untrusted user is the database owner or holds the CREATEROLE privilege, in
>> which case no secure schema usage pattern exists.

> +1 LGTM

Sounds good.  I'll make it so in a bit.

			regards, tom lane