Re: Extension security improvement: Add support for extensions with an owned schema

Hello Jelte, everyone,

v11 applies cleanly to master (09a579abaca) and CI is green, so I went
looking at the one part that CI cannot reach. In June 2024 you wrote:

> I also think I changed the pg_upgrade to do the correct thing, but I'm
> not sure how to test this (even manually). Because part of it would
> only be relevant once we support upgrading from PG18. So for now the
> upgrade_code I haven't actually run.

That is still true, and unfortunately that code does not work. It is
easy to run now: pg_upgrade invokes the NEW pg_dump against the OLD
server, so all it takes is a real PG18 cluster and the patched binary.

Everything below was measured on master 09a579abaca with clean builds
(--enable-cassert), against a PG18.6 cluster built from REL_18_STABLE,
with unpatched master as the control. Scripts attached.

1. pg_dump against any older server fails
------------------------------------------

getExtensions() asks:

if (fout->remoteVersion >= 180000)
appendPQExpBufferStr(query, ", x.extownedschema ");

The patch was written when 18 was the development target. master is
20devel now, so the column would appear in 20, not in 18. Against an
18 or 19 server the query asks for a column that does not exist:

pg_dump (master), PG18 server: ok
pg_dump (v11), PG18 server: ERROR: column x.extownedschema
does not exist
pg_dumpall (v11), PG18 server: same error
pg_upgrade PG18 -> v11: Failure, exiting

This is not only pg_upgrade. getExtensions() runs for every pg_dump,
so dumping an older server with new binaries - the normal way to move
between major versions - fails too.

The one-line change is attached as nocfbot-fix-remoteVersion.diff.txt
(180000 -> 200000). With it, on the same clusters:

pg_dump (fixed), PG18 server: ok, same output as master except
for the random \restrict token
pg_upgrade PG18 -> fixed: rc=0

It might be worth a comment there, since this number has to be bumped
again every time the patch misses a release.

2. Every pg_dump of a database with an owned-schema extension warns
--------------------------------------------------------------------

pg_dump: warning: could not resolve dependency loop among these items:
pg_dump: detail: EXTENSION loopdemo_owned (ID 2 OID 16386)
pg_dump: detail: SCHEMA loopdemo_owned (ID 8 OID 16385)

Control: the same trivial extension installed twice, where the only
difference is owned_schema in the control file.

owned_schema = true -> warning on every dump (plain and
--binary-upgrade)
owned_schema = false -> no warning

The dump still restores correctly, because pg_dump breaks the loop on
its own, but it is a loop it knows nothing about, and users of the
feature would see this on every single dump.

The loop is real: the extension is in the schema, and the schema is a
member of the extension. pg_dump_sort.c already repairs the loops it
knows (type/function, view/rule), so the second attached diff adds
this case: drop the extension's dependency on the schema, since CREATE
EXTENSION is what creates the schema. That is the direction that can
go, and it leaves the schema-is-a-member dependency intact.

With that diff the warning is gone, and make check, src/test/modules,
src/bin/pg_dump and src/bin/pg_upgrade all still pass.

3. test_pg_dump cannot be run on its own any more
--------------------------------------------------

make -C src/test/modules/test_pg_dump check

master: PASS (1044 tests)
v11: FAIL - extension "test_ext_owned_schema" is not available

The new case in test_pg_dump/t/001_base.pl installs an extension that
lives in src/test/modules/test_extensions, and nothing makes that
module available to this one. Running the whole src/test/modules
directory passes, which is presumably why CI never noticed.

4. What I checked that does work
---------------------------------

Same-version pg_upgrade of a cluster with an owned-schema extension
installed keeps the catalog right (extownedschema still true, schema
still the extension's own), the --binary-upgrade dump emits the
expected binary_upgrade_create_empty_extension(..., true, ...) and no
separate CREATE SCHEMA, and a plain dump/restore round trip works.
make check, src/test/modules and src/bin/pg_dump pass with v11 as is.

I have not reviewed the security model itself; the thread has already
covered that ground at length and I did not want to reopen it. These
are only the things I could measure.

Regards,
Manu