Thread: pgsql: Revoke PUBLIC CREATE from public schema, now owned by pg_databas
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(-)
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
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.
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
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.
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