Re: public schema default ACL - Mailing list 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