Re: public schema default ACL

From: Noah Misch <noah(at)leadboat(dot)com>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: public schema default ACL
Date: 2021-02-13 12:56:29
Message-ID: 20210213125629.GA1297650@rfd.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I'm attaching the patch for $SUBJECT, which applies atop the four patches from
the two other threads below. For convenience of testing, I've included a
rollup patch, equivalent to applying all five patches.

On Sat, Oct 31, 2020 at 09:35:18AM -0700, Noah Misch wrote:
> More details on the semantics I'll use:
>
> 1. initdb will change like this:
> @@ -1721 +1721 @@ setup_privileges(FILE *cmdfd)
> - "GRANT CREATE, USAGE ON SCHEMA public TO PUBLIC;\n\n",
> + "GRANT USAGE ON SCHEMA public TO PUBLIC;\n\n",
> + "ALTER SCHEMA public OWNER TO DATABASE_OWNER;\n\n",

(I ended up assigning the ownership via pg_namespace.dat, not here.)

> 2. If schema public does not exist, pg_dump will emit nothing about it. This
> is what happens today. (I suspect it would be better for pg_dump to emit
> DROP SCHEMA public RESTRICT, but that is drifting offtopic for $SUBJECT.)
> Otherwise, when dumping from v13 or earlier, pg_dump will always emit
> REVOKE and/or GRANT statements to reproduce the old ACL.

More precisely, it diffs the source database ownership and ACL to the v14
defaults, then emits ALTER, GRANT, and/or REVOKE as needed. That yields no
GRANT or REVOKE if the source database is an early adopter of the new default.

> When dumping from
> v14 or later, pg_dump will use pg_init_privs to compute GRANT and REVOKE
> statements, as it does today.

(It doesn't actually use pg_init_privs, but the effect is similar.)

> (This may interfere with cross-version
> pg_upgrade testing. I haven't looked at how best to fix that. Perhaps add
> more fix_sql in test.sh.)

src/bin/pg_upgrade/test.sh doesn't need changes. Upgrades from 9.6 (the first
version having pg_init_privs) or later get no new diffs. Upgrades from v8.4
or v9.5 to v14 have a relevant diff before or after this change. In master:

-REVOKE ALL ON SCHEMA public FROM PUBLIC;
-REVOKE ALL ON SCHEMA public FROM nm;
-GRANT ALL ON SCHEMA public TO nm;
-GRANT ALL ON SCHEMA public TO PUBLIC;

After $SUBJECT:

-REVOKE ALL ON SCHEMA public FROM PUBLIC;
-REVOKE ALL ON SCHEMA public FROM nm;
-GRANT ALL ON SCHEMA public TO nm;
+REVOKE USAGE ON SCHEMA public FROM PUBLIC;
GRANT ALL ON SCHEMA public TO PUBLIC;

> 3. pg_upgrade from v13 to later versions will transfer template1's ACL for
> schema public, even if that ACL was unchanged since v13 initdb. (This is
> purely a consequence of the pg_dump behavior decision.) template0 will
> keep the new default.
>
> 4. OWNER TO DATABASE_OWNER will likely be available for schemas only, though I
> might propose it for all object classes if class-specific complexity proves
> negligible.

Class-specific complexity was negligible, so I made it available for all
objects. The syntax is "OWNER TO pg_database_owner", because it's a special
predefined role. That patch has its own thread:
https://postgr.es/m/20201228043148.GA1053024@rfd.leadboat.com

> 5. ALTER DATABASE OWNER TO changes access control decisions involving
> nspowner==DATABASE_OWNER. Speed of nspacl checks is more important than
> reacting swiftly to ALTER DATABASE OWNER TO. Sessions running concurrently
> will be eventually-consistent with respect to the ALTER DATABASE.
> (Existing access control decisions, too, allow this sort of anomaly.)
>
> 6. pg_dump hasn't been reproducing ALTER SCHEMA public OWNER TO. That's a
> mild defect today, but it wouldn't be mild anymore. We'll need pg_dump of
> v13 databases to emit "ALTER SCHEMA public OWNER TO postgres" and for a v14
> => v15 upgrade to propagate that. This project can stand by itself; would
> anyone else like to own it?

That patch has its own thread:
https://postgr.es/m/20201229134924.GA1431748@rfd.leadboat.com

Changing this ACL caused 13 of 202 tests to fail in "make check". I first
intended to modify tests as needed for that suite to keep the default ACL.
For complicated cases, my strategy was to make a test create a schema and
change search_path. However, that created large expected output diffs
(e.g. ~120 lines in updatable_views.out), mostly in EXPLAIN and \d output
bearing the schema name. I didn't want that kind of obstacle to future
back-patched test updates, so I did make the first test install the old ACL.
All other in-tree suites do test the new default.

Thanks,
nm

Attachment Content-Type Size
public-default-acl-v1.patch_no_cfbot text/plain 18.2 KB
public-default-acl-v1-rollup.patch text/plain 59.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2021-02-13 14:44:30 Re: [HACKERS] Custom compression methods
Previous Message John Naylor 2021-02-13 12:36:10 Re: Preventing free space from being reused