Обсуждение: Extension security improvement: Add support for extensions with an owned schema

Writing the sql migration scripts that are run by CREATE EXTENSION and
ALTER EXTENSION UPDATE are security minefields for extension authors.
One big reason for this is that search_path is set to the schema of the
extension while running these scripts, and thus if a user with lower
privileges can create functions or operators in that schema they can do
all kinds of search_path confusion attacks if not every function and
operator that is used in the script is schema qualified. While doing
such schema qualification is possible, it relies on the author to never
make a mistake in any of the sql files. And sadly humans have a tendency
to make mistakes.

This patch adds a new "owned_schema" option to the extension control
file that can be set to true to indicate that this extension wants to
own the schema in which it is installed. What that means is that the
schema should not exist before creating the extension, and will be
created during extension creation. This thus gives the extension author
an easy way to use a safe search_path, while still allowing all objects
to be grouped together in a schema. The implementation also has the
pleasant side effect that the schema will be automatically dropped when
the extension is dropped.

One way in which certain extensions currently hack around the
non-existence of this feature is by using the approach that pg_cron
uses: Setting the schema to pg_catalog and running "CREATE SCHEMA
pg_cron" from within the extension script. While this works, it's
obviously a hack, and a big downside of it is that it doesn't allow
users to choose the schema name used by the extension.

PS. I have never added fields to pg_catalag tables before, so there's
a clear TODO in the pg_upgrade code related to that. If anyone has
some pointers for me to look at to address that one that would be
helpful, if not I'll probably figure it out myself. All other code is
in pretty finished state, although I'm considering if
AlterExtensionNamespace should maybe be split a bit somehow, because
owned_schema skips most of the code in that function.

On Sun, Jun 2, 2024, 02:08 Jelte Fennema-Nio <me@jeltef.nl> wrote:
> This patch adds a new "owned_schema" option to the extension control
> file that can be set to true to indicate that this extension wants to
> own the schema in which it is installed.

Huge +1

Many managed PostgreSQL services block superuser access, but provide a
way for users to trigger a create/alter extension as superuser. There
have been various extensions whose SQL scripts can be tricked into
calling a function that was pre-created in the extension schema. This
is usually done by finding an unqualified call to a pg_catalog
function/operator, and overloading it with one whose arguments types
are a closer match for the provided values, which then takes
precedence regardless of search_path order. The custom function can
then do something like "alter user foo superuser".

The sequence of steps assumes the user already has some kind of admin
role and is gaining superuser access to their own database server.
However, the superuser implicitly has shell access, so it gives
attackers an additional set of tools to poke around in the managed
service. For instance, they can change the way the machine responds to
control plane requests, which can sometimes trigger further
escalations. In addition, many applications use the relatively
privileged default user, which means SQL injection issues can also
escalate into superuser access and beyond.

There are some static analysis tools like
https://github.com/timescale/pgspot that address this issue, though it
seems like a totally unnecessary hole. Using schema = pg_catalog,
relocatable = false, and doing an explicit create schema (without "if
not exists") plugs the hole by effectively disabling extension
schemas. For extensions I'm involved in, I consider this to be a hard
requirement.

I think Jelte's solution is preferable going forward, because it
preserves the flexibility that extension schemas were meant to
provide, and makes the potential hazards of reusing a schema more
explicit.

cheers,
Marco

On Sat, 2024-06-01 at 17:08 -0700, Jelte Fennema-Nio wrote:
> This patch adds a new "owned_schema" option to the extension control
> file that can be set to true to indicate that this extension wants to
> own the schema in which it is installed. What that means is that the
> schema should not exist before creating the extension, and will be
> created during extension creation. This thus gives the extension
> author
> an easy way to use a safe search_path, while still allowing all
> objects
> to be grouped together in a schema. The implementation also has the
> pleasant side effect that the schema will be automatically dropped
> when
> the extension is dropped.

Is this orthogonal to relocatability?

When you say "an easy way to use a safe search_path": the CREATE
EXTENSION command already sets the search_path, so your patch just
ensures that it's empty (and therefore safe) first, right?

Should we go further and try to prevent creating objects in an
extension-owned schema with normal SQL?

Approximately how many extensions should be adjusted to use
owned_schema=true? What are the reasons an extension would not want to
own the schema in which the objects are created? I assume some would
still create objects in pg_catalog, but ideally we'd come up with a
better solution to that as well.

This protects the extension script, but I'm left wondering if we could
do something here to make it easier to protect extension functions
called from outside the extension script, also. It would be nice if we
could implicitly tack on a "SET search_path TO @extschema@, pg_catalog,
pg_temp" to each function in the extension. I'm not proposing that, but
perhaps a related idea might work. Probably outside the scope of your
proposal.

Regards,
    Jeff Davis

On Wed, 5 Jun 2024 at 19:53, Jeff Davis <pgsql@j-davis.com> wrote:
> Is this orthogonal to relocatability?

It's fairly orthogonal, but it has some impact on relocatability: You
can only relocate to a schema name that does not exist yet (while
currently you can only relocate to a schema that already exists). This
is because, for owned_schema=true, relocation is not actually changing
the schema of the extension objects, it only renames the existing
schema to the new name.

> When you say "an easy way to use a safe search_path": the CREATE
> EXTENSION command already sets the search_path, so your patch just
> ensures that it's empty (and therefore safe) first, right?

Correct: **safe** is the key word in that sentence. Without
owned_schema, you get an **unsafe** search_path by default unless you
go out of your way to set "schema=pg_catalog" in the control file.

> Should we go further and try to prevent creating objects in an
> extension-owned schema with normal SQL?

That would be nice for sure, but security wise it doesn't matter
afaict. Only the creator of the extension would be able to add stuff
in the extension-owned schema anyway, so there's no privilege
escalation concern there.

> Approximately how many extensions should be adjusted to use
> owned_schema=true?

Adjusting existing extensions would be hard at the moment, because the
current patch does not introduce a migration path. But basically I
think for most new extension installs (even of existing extensions) it
would be fine if owned_schema=true would be the default. I didn't
propose (yet) to make it the default though, to avoid discussing the
tradeoff of security vs breaking installation for an unknown amount of
existing extensions.

I think having a generic migration path would be hard, due to the many
ways in which extensions can now be installed. But I think we might be
able to add one fairly easily for relocatable extensions: e.g. "ALTER
EXTESION SET SCHEMA new_schema OWNED_SCHEMA", which would essentially
do CREATE SCHEMA new_schema + move all objects from old_schema to
new_schema. And even for non-relocatable one you could do something
like:

CREATE SCHEMA temp_schema_{random_id};
-- move all objects from ext_schema to temp_schema_{random_id};
DROP SCHEMA ext_schema; -- if this fails, ext_schema was not empty
ALTER SCHEMA temp_schema_{random_id} RENAME TO ext_schema;

> What are the reasons an extension would not want to
> own the schema in which the objects are created? I assume some would
> still create objects in pg_catalog, but ideally we'd come up with a
> better solution to that as well.

Some extensions depend on putting stuff into the public schema. But
yeah it would be best if they didn't.

> This protects the extension script, but I'm left wondering if we could
> do something here to make it easier to protect extension functions
> called from outside the extension script, also. It would be nice if we
> could implicitly tack on a "SET search_path TO @extschema@, pg_catalog,
> pg_temp" to each function in the extension. I'm not proposing that, but
> perhaps a related idea might work. Probably outside the scope of your
> proposal.

Yeah, this proposal definitely doesn't solve all security problems
with extensions. And indeed what you're proposing would solve another
major issue, another option would be to default to the "safe"
search_path that you proposed a while back. But yeah I agree that it's
outside of the scope of this proposal. I feel like if we try to solve
every security problem at once, probably nothing gets solved instead.
That's why I tried to keep this proposal very targeted, i.e. have this
be step 1 of an N step plan to make extensions more secure by default.

Attached is an updated version of this patch that fixes a few issues
that CI reported (autoconf, compiler warnings and broken docs).

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.

On Sat, Jun 1, 2024 at 8:08 PM Jelte Fennema-Nio <me@jeltef.nl> wrote:
> Writing the sql migration scripts that are run by CREATE EXTENSION and
> ALTER EXTENSION UPDATE are security minefields for extension authors.
> One big reason for this is that search_path is set to the schema of the
> extension while running these scripts, and thus if a user with lower
> privileges can create functions or operators in that schema they can do
> all kinds of search_path confusion attacks if not every function and
> operator that is used in the script is schema qualified. While doing
> such schema qualification is possible, it relies on the author to never
> make a mistake in any of the sql files. And sadly humans have a tendency
> to make mistakes.

I agree that this is a problem. I also think that the patch might be a
reasonable solution (but I haven't reviewed it).

But I wonder if there might also be another possible approach: could
we, somehow, prevent object references in extension scripts from
resolving to anything other than the system catalogs and the contents
of that extension? Perhaps with a control file setting to specify a
list of trusted extensions which we're also allowed to reference?

I have a feeling that this might be pretty annoying to implement, and
if that is true, then never mind. But if it isn't that annoying to
implement, it would make a lot of unsafe extensions safe by default,
without the extension author needing to take any action. Which could
be pretty cool. It would also make it possible for extensions to
safely share a schema, if desired.

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

On Wed, 19 Jun 2024 at 17:28, Robert Haas <robertmhaas@gmail.com> wrote:
> But I wonder if there might also be another possible approach: could
> we, somehow, prevent object references in extension scripts from
> resolving to anything other than the system catalogs and the contents
> of that extension?

This indeed does sound like the behaviour that pretty much every
existing extension wants to have. One small addition/clarification
that I would make to your definition: fully qualified references to
other objects should still be allowed.

I do think, even if we have this, there would be other good reasons to
use "owned schemas" for extension authors. At least the following two:
1. To have a safe search_path that can be used in SET search_path on a
function (see also [1]).
2. To make it easy for extension authors to avoid conflicts with other
extensions/UDFs.

> Perhaps with a control file setting to specify a
> list of trusted extensions which we're also allowed to reference?

I think we could simply use the already existing "requires" field from
the control file. i.e. you're allowed to reference only your own
extension

> I have a feeling that this might be pretty annoying to implement, and
> if that is true, then never mind.

Based on a quick look it's not trivial, but also not super bad.
Basically it seems like in src/backend/catalog/namespace.c, every time
we loop over activeSearchPath and CurrentExtensionObject is set, then
we should skip any item that's not stored in pg_catalog, unless
there's a DEPENDENCY_EXTENSION pg_depend entry for the item (and that
pg_depend entry references the extension or the requires list).

There's quite a few loops over activeSearchPath in namespace.c, but
they all seem pretty similar. So while a bunch of code would need to
be changed, the changes could probably be well encapsulated in a
function.

[1]:
https://www.postgresql.org/message-id/flat/00d8f046156e355ec0eb49585408bafc8012e4a5.camel%40j-davis.com#3ad66667a8073d5ef50cfe44e305c38d

Jelte Fennema-Nio <me@jeltef.nl> writes:
> On Wed, 19 Jun 2024 at 17:28, Robert Haas <robertmhaas@gmail.com> wrote:
>> I have a feeling that this might be pretty annoying to implement, and
>> if that is true, then never mind.

> Based on a quick look it's not trivial, but also not super bad.
> Basically it seems like in src/backend/catalog/namespace.c, every time
> we loop over activeSearchPath and CurrentExtensionObject is set, then
> we should skip any item that's not stored in pg_catalog, unless
> there's a DEPENDENCY_EXTENSION pg_depend entry for the item (and that
> pg_depend entry references the extension or the requires list).

We could change the lookup rules that apply during execution of
an extension script, but we already restrict search_path at that
time so I'm not sure how much further this'd move the goalposts.

The *real* problem IMO is that if you create a PL function or
(old-style) SQL function within an extension, execution of that
function is not similarly protected.

            regards, tom lane

On Wed, 19 Jun 2024 at 19:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Jelte Fennema-Nio <me@jeltef.nl> writes:
> > On Wed, 19 Jun 2024 at 17:28, Robert Haas <robertmhaas@gmail.com> wrote:
> >> I have a feeling that this might be pretty annoying to implement, and
> >> if that is true, then never mind.
>
> > Based on a quick look it's not trivial, but also not super bad.
> > Basically it seems like in src/backend/catalog/namespace.c, every time
> > we loop over activeSearchPath and CurrentExtensionObject is set, then
> > we should skip any item that's not stored in pg_catalog, unless
> > there's a DEPENDENCY_EXTENSION pg_depend entry for the item (and that
> > pg_depend entry references the extension or the requires list).
>
> We could change the lookup rules that apply during execution of
> an extension script, but we already restrict search_path at that
> time so I'm not sure how much further this'd move the goalposts.

The point I tried to make in my first email is that this restricted
search_path you mention, is not very helpful at preventing privilege
escalations. Since it's often possible for a non-superuser to create
functions in one of the schemas in this search_path, e.g. by having
the non-superuser first create the schema & create some functions in
it, and then asking the DBA/control plane to create the extension in
that schema.

My patch tries to address that problem by creating the schema of the
extension during extension creation, and failing if it already exists.
Thus implicitly ensuring that a non-superuser cannot mess with the
schema.

The proposal from Robert tries to instead address by changing the
lookup rules during execution of an extension script to be more strict
than they would be outside of it (i.e. even if a function/operator
matches search_path it might still not be picked).

> The *real* problem IMO is that if you create a PL function or
> (old-style) SQL function within an extension, execution of that
> function is not similarly protected.

That's definitely a big problem too, and that's the problem that [1]
tries to fix. But first the lookup in extension scripts would need to
be made secure, because it doesn't seem very useful (security wise) to
use the same lookup mechanism in functions as we do in extension
scripts, if the lookup in extension scripts is not secure in the first
place. I think the method of making the lookup secure in my patch
would transfer over well, because it adds a way for a safe search_path
to exist, so all that's needed is for the PL function to use that
search_path. Robbert's proposal would be more difficult I think. When
executing a PL function from an extension we'd need to use the same
changed lookup rules that we'd use during the extension script of that
extension. I think that should be possible, but it's definitely more
involved.

[1]:
https://www.postgresql.org/message-id/flat/CAE9k0P%253DFcZ%253Dao3ZpEq29BveF%252B%253D27KBcRT2HFowJxoNCv02dHLA%2540mail.gmail.com

On Wed, 19 Jun 2024 at 17:22, David G. Johnston
<david.g.johnston@gmail.com> wrote:
>
> On Wed, Jun 19, 2024 at 8:19 AM Jelte Fennema-Nio <me@jeltef.nl> wrote:
>
>>
>> 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.
>
>
> Does it apply against v16?  If so, branch off there, apply it, then upgrade from the v16 branch to master.


I realized it's possible to do an "upgrade" with pg_upgrade from v17
to v17. So I was able to test both the pre and post PG18 upgrade logic
manually by changing the version in this line:

if (fout->remoteVersion >= 180000)

As expected the new pg_upgrade code was severely broken. Attached is a
new patch where the pg_upgrade code now actually works.

On Wed, Jun 19, 2024 at 1:50 PM Jelte Fennema-Nio <me@jeltef.nl> wrote:
> I do think, even if we have this, there would be other good reasons to
> use "owned schemas" for extension authors. At least the following two:
> 1. To have a safe search_path that can be used in SET search_path on a
> function (see also [1]).
> 2. To make it easy for extension authors to avoid conflicts with other
> extensions/UDFs.

(1) is a very good point. (2) I don't know about one way or the other.

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

On Jun 19, 2024, at 11:28, Robert Haas <robertmhaas@gmail.com> wrote:

> But I wonder if there might also be another possible approach: could
> we, somehow, prevent object references in extension scripts from
> resolving to anything other than the system catalogs and the contents
> of that extension? Perhaps with a control file setting to specify a
> list of trusted extensions which we're also allowed to reference?

It would also have to allow access to other extensions it depends upon.

D

On Jun 19, 2024, at 13:50, Jelte Fennema-Nio <me@jeltef.nl> wrote:

> This indeed does sound like the behaviour that pretty much every
> existing extension wants to have. One small addition/clarification
> that I would make to your definition: fully qualified references to
> other objects should still be allowed.

Would be tricky for referring to objects from other extensions  with no defined schema, or are relatable.

> 1. To have a safe search_path that can be used in SET search_path on a
> function (see also [1]).
> 2. To make it easy for extension authors to avoid conflicts with other
> extensions/UDFs.

These would indeed be nice improvements IMO.

Best,

David

Hi,

I've spent a bit of time looking at this patch. It seems there's a clear
consensus that having "owned schemas" for extensions would be good for
security. To me it also seems as a convenient way to organize stuff. It
was possible to create extensions in a separate schema before, ofc, but
that's up to the DBA. With this the extension author to enforce that.

One thing that's not quite clear to me is what's the correct way for
existing extensions to switch to an "owned schema". Let's say you have
an extension. How do you transition to this? Can you just add it to the
control file and then some magic happens?

A couple minor comments:


1) doc/src/sgml/extend.sgml

  An extension is <firstterm>owned_schema</firstterm> if it requires a
  new dedicated schema for its objects. Such a requirement can make
  security concerns related to <literal>search_path</literal> injection
  much easier to reason about. The default is <literal>false</literal>,
  i.e., the extension can be installed into an existing schema.

Doesn't "extension is owned_schema" sound a bit weird? I'd probably say
"extension may own a schema" or something like that.

Also, "requires a new dedicated schema" is a bit ambiguous. It's not
clear if it means the schema is expected to exist, or if it creates the
schema itself.

And perhaps it should clarify what "much easier to reason about" means.
That's pretty vague, and as a random extension author I wouldn't know
about the risks to consider. Maybe there's a section about this stuff
that we could reference?


2) doc/src/sgml/ref/create_extension.sgml

  relocated.  The named schema must already exist if the extension's
  control file does not specify <literal>owned_schema</literal>.

Seems a bit unclear, I'd say having "owned_schema = false" in the
control file still qualifies as "specifies owned_schema". So might be
better to say it needs to be set to true?

Also, perhaps "dedicated_schema" would be better than "owned_schema"? I
mean, the point is not that it's "owned" by the extension, but that
there's nothing else in it. But that's nitpicking.


3) src/backend/commands/extension.c

I'm not sure why AlterExtensionNamespace moves the privilege check. Why
should it not check the privilege for owned schema too?


4) src/bin/pg_dump/pg_dump.c

checkExtensionMembership has typo "owned_schem".

Shouldn't binary_upgrade_extension_member still set ext=NULL in the for
loop, the way the original code does that?

The long if conditions might need some indentation, I guess. pgindent
leaves them like this, but 100 columns seems a bit too much. I'd do a
line break after each condition, I guess.




regards

-- 
Tomas Vondra

(a) The patch affects DROP EXTENSION in that it drops the schema as well, if it's owned by the extension. This needs to be mentioned in the documentation. In addition, an extra confirmation (e.g., "This will drop schema nnnn as well, do you wish to continue?") when dropping the extension might be desired, as the extension schema could contain user data (e.g., pg_cron keeps the jobs and their execution details).

On Mon, 28 Jul 2025 at 00:03, Sadeq Dousti <msdousti@gmail.com> wrote:
> (a) The patch affects DROP EXTENSION in that it drops the schema as well, if it's owned by the extension. This needs
tobe mentioned in the documentation. In addition, an extra confirmation (e.g., "This will drop schema nnnn as well, do
youwish to continue?") when dropping the extension might be desired, as the extension schema could contain user data
(e.g.,pg_cron keeps the jobs and their execution details). 

This is already covered by docs for DROP EXTENSION:
"Dropping an extension causes its member objects <snip> to be dropped as well.

An extra confirmation or requiring of CASCADE seems unnecessary. The
schema itself will not contain user objects (that's entirely the point
of this change). It indeed might contain tables from the extension,
that contain user data, but that's no difference from extension
creating tables in other schemas. The only thing that will
additionally get removed for owned_schema extensions is the empty
schema that contained all of the other objects, that would normally be
removed.

> (b) From the patch description:
> > Writing the sql migration scripts that are run by CREATE EXTENSION
> > and ALTER EXTENSION UPDATE are security minefields for extension authors.
>
> While "ALTER EXTENSION UPDATE" is mentioned as a minefield, the patch does not fix it (only ALTER EXTENSION ... SET
SCHEMAis affected AFAICS). A possible remedy could be that, before the update, the extension makes sure no (sensitive?)
object(e.g., UDF/Operator) created by a non-superuser exists in its schema. 

The whole security issue comes from the fact that the schema that an
extension gets installed in might be owned by a low-privileged user.
By having the schema be created by (and thus be owned by) the
extension creator, this whole problem goes away.

> (c) Does it make sense to add the "owned_schema" option to the CREATE EXTENSION command? Something like:
>
> CREATE EXTENSION xyz
> WITH owned_schema=true

I don't think that's a good idea. Since the migration script behaviour
can change significantly, I don't think it's safe to allow people to
specify it in CREATE EXTENSION. Especially because then people would
likely also be able to set it to false.

> An alternative could be to have it by default true (security by default), and if the DBA doesn't want it for whatever
reason,they have to explicitly set it to false during CREATE EXTENSION. 

I think changing the default would probably be good. Extension authors
can then explicitly opt-in to the less secure option if they require
that. I didn't want to do that for the initial change, as this seems
probably more contentious than adding a new option.

> (d) As David (Wheeler) mentioned in the thread, an extension control file can have the "requires" field, in which an
extensionX installation depends on other extensions Y & Z to be installed. I was thinking if X calls a function from Y
duringinstallation, and Y does not have owned_schema, the search_path confusion attack can be transitively applied. It
couldmake sense that X refuses to install, unless both Y & Z (= all required extensions) are marked as
owned_schema=true.Although in favor of backwards compatibility, this can be overridable by an option in CREATE
EXTENSION,such as "WITH transitive_owned_schema=false". 

I think it's fine to depend on extensions without owned_schema as an
owned_schema extension. It's not as if it's impossible to write safe
extension scripts now, it's just quite hard. An extension author can
choose to use owned_schema, to make their own life easier when writing
those scripts, but they can still depend on a perfectly safe extension
that did not use owned_schema but correctly hardened its extension
migration scripts.

On Wed Jul 23, 2025 at 7:12 PM CEST, Artem Gavrilov wrote:
> Hello Jelte,
>
> 1) I noticed that pg_dump changes weren't covered with tests.
>
> 2) I assume these error messages may be confusing, especially first one:

Attached is an updated version that addresses these issues.

On Tue, Jul 29, 2025 at 5:35 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> On Wed Jul 23, 2025 at 7:12 PM CEST, Artem Gavrilov wrote:
> > 1) I noticed that pg_dump changes weren't covered with tests.
> >
> > 2) I assume these error messages may be confusing, especially first one:
>
> Attached is an updated version that addresses these issues.

I generally like the direction that this is going but there are places
where the new stuff looks too much like it was bolted onto whatever
was there already. It's important to go back and edit things so that
they look natural, as if the new feature had been present all along.

-        relocated.  The named schema must already exist.
+        relocated.  The named schema must already exist, unless
+        <literal>owned_schema</literal> is set to <literal>true</literal> in
+        the control file, then the schema must not exist.

This reads awkwardly, at least to me. The smallest possible edit that
would make it passable for me is to replace "the" with "in which
case," but possibly the whole sentence should be rephrased somehow.

+ * If the user is giving us the schema name, it must exist already if
+ * the extension does not want to own the schema

This could be made clearer.

+ errmsg("schema \"%s\" already exists but the extension needs to create it",
+ schemaName),

I don't really find this an improvement over: ERROR:  schema
"test_ext_owned_schema" already exists.

+ else if (control->owned_schema)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_SCHEMA),
+ errmsg("schema \"%s\" already exists but the extension needs to create it",
+ schemaName),
+ errhint("Drop schema \"%s\" or specify another schema using CREATE
EXTENSION ... SCHEMA ...", schemaName)));
+ }
+
  }
- else if (!OidIsValid(schemaOid))
+ else if (control->owned_schema)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_SCHEMA),
+ errmsg("no schema has been selected to create in")));
+ }
+ else

It certainly seems worth asking whether this if-nest should be
rephrased in some way to make it clearer. But even if it's best to
keep it as it is, I find the absence of comments hard to justify. Who
is going to read the bit that emits "no schema has been selected to
create in" and find that self-explanatory?

I would like to see some improvements in AlterExtensionNamespace. In
the context of the patch, it's possible to puzzle out what is
happening, but I think the picture is not going to be clear to later
readers. It seems to me that this either needs some restructuring to
make the logical flow clearer, or a few well-written comments.

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

On Mon, Aug 11, 2025 at 1:55 PM Robert Haas <robertmhaas@gmail.com> wrote:
> [ some review ]

Another thing that's occurring to me here is that nothing prevents
other objects from making their way into the owned schema. Sure, if we
create a new schema with nobody having any permissions, then only the
creating role or some role that has its privileges can add anything in
there. But that could happen by accident, or privileges could later be
granted and somebody could add something into the extension schema
after that. I wonder whether we should lock this down tighter somehow
and altogether forbid creating objects in that schema except from an
extension create/upgrade script for the owning extension.

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

On Mon Aug 11, 2025 at 9:23 PM CEST, Robert Haas wrote:
> On Mon, Aug 11, 2025 at 1:55 PM Robert Haas <robertmhaas@gmail.com> wrote:
>> [ some review ]

Attached is a patch that addresses your comments I think. I restructured
the schema creation code, and added more comments to the
AlterExtensionNamespace code (I couldn't find a way to make the
structure clearer).


> Another thing that's occurring to me here is that nothing prevents
> other objects from making their way into the owned schema. Sure, if we
> create a new schema with nobody having any permissions, then only the
> creating role or some role that has its privileges can add anything in
> there. But that could happen by accident, or privileges could later be
> granted and somebody could add something into the extension schema
> after that. I wonder whether we should lock this down tighter somehow
> and altogether forbid creating objects in that schema except from an
> extension create/upgrade script for the owning extension.

I think that's an interesting idea, and I started with a change to try
this out, that I intend to finish soon. It doesn't seem strictly
necessary, though.

On Mon Sep 1, 2025 at 4:44 PM CEST, Jelte Fennema-Nio wrote:
> On Mon Aug 11, 2025 at 9:23 PM CEST, Robert Haas wrote:
>> On Mon, Aug 11, 2025 at 1:55 PM Robert Haas <robertmhaas@gmail.com> wrote:
>>> [ some review ]
>
> Attached is a patch that addresses your comments I think.

Attached is a version that doesn't fail the tests.

On Tue, 2 Sept 2025 at 02:03, Julien Rouhaud <rjuju123@gmail.com> wrote:
> One not too uncommon scenario is an extension in a dedicated schema that creates additional objects dynamically, for
instancecreating new partitions using triggers on one of the extension table.
 

Interesting. I didn't know there were extensions that did that. That
definitely doesn't seem like a very common pattern though.

But I don't think that's a problem for this idea. In the
implementation I'm working on, superuser would still be allowed to
create objects in such locked down owned schemas. So as long as the
extension upgrades its permissions to superuser during these DDLs it
should still be fine. (easy to do with SECURITY DEFINER or by
temporarily changing permissions from C)

On Tue, Sep 02, 2025 at 09:37:31AM +0200, Jelte Fennema-Nio wrote:
> On Tue, 2 Sept 2025 at 02:03, Julien Rouhaud <rjuju123@gmail.com> wrote:
> > One not too uncommon scenario is an extension in a dedicated schema that creates additional objects dynamically,
forinstance creating new partitions using triggers on one of the extension table.
 
>
> Interesting. I didn't know there were extensions that did that. That
> definitely doesn't seem like a very common pattern though.

I think that there are way more extensions that dynamically create objects than
what you think.  Some years ago I was working on such an extension at work, and
pgtt is also creating some objects under the hood.  That's already 3 extensions
that I know on top of my head without having to think about it.

> But I don't think that's a problem for this idea. In the
> implementation I'm working on, superuser would still be allowed to
> create objects in such locked down owned schemas. So as long as the
> extension upgrades its permissions to superuser during these DDLs it
> should still be fine. (easy to do with SECURITY DEFINER or by
> temporarily changing permissions from C)

Requiring superuser permission seems like a big penalty, especially since the
last few years have been all about *not* requiring superuser privileges.  Note
also that not all extensions embeds compiled code, some are just doing plain
plpgsql and work just fine.

Why not requiring schema owner privileges?

On Tue, Sep 2, 2025 at 5:02 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> Requiring superuser permission seems like a big penalty, especially since the
> last few years have been all about *not* requiring superuser privileges.  Note
> also that not all extensions embeds compiled code, some are just doing plain
> plpgsql and work just fine.
>
> Why not requiring schema owner privileges?

I agree with you that requiring superuser privileges is undesirable.
However, requiring schema owner privileges isn't really requiring
anything above and beyond what the permissions system would require
anyway, since at the start, nobody else will have privileges on that
schema. And that's where what Jelte was proposing -- and also what you
propose here -- seems very accident-prone to me. It would be quite
easy for the user who created the extension, and who therefore also
owns the schema IIUC, to accidentally put other stuff there, or allow
others to do so, and that might undermine the safe search_path
guarantee that is the purpose of this patch. I am not fixed on any
particular method of making that guarantee more robust, but I am in
favor of trying to come up with some way of making it more robust.

The first thought that popped into my head was that maybe your
extension should make the objects it creates part of the extension,
but I think that doesn't actually work, because it would mean that
they would not be dumped. I think what you've got is something
intermediate between full extension membership (where installing the
extension creates the objects) and non-membership (where the objects
are created by the user and unaffiliated with the extension) but we
have no concept of that in the system today. Should we?

Another thing to consider here is what happens when somebody tries to
drop the extension. As Jelte coded it in the last version I read, it
just blows away the schema. I don't remember whether it used RESTRICT
(in which case the presence of unrelated objects in the schema would
result in failure) or CASCADE (in which case they would be silently
blown away) but I don't like either option very much. The former would
be pretty inconvenient in a use case like yours, and the latter would
be terrible if there were valuable, unrelated objects in the schema. I
think CASCADE is the right answer as long as there's something that
makes it unlikely that the schema has any unanticipated contents.

Here are a few possible ways forward:

(1) Invent, as proposed above, an intermediate level of extension
membership where objects are dropped with the extension but are still
dumped, and find a way for extensions to create objects in an owned
schema only if the new object will be either fully owned by the
extension or at least at this intermediate level.

(2) Decide that extensions that create additional objects in the
schema shouldn't use owned schemas, and document this.

(3) Add a mechanism to track when an extension-owned function is
executing, and only allow objects to be created in an extension-owned
schema when that's the case.

(4) Add a GUC that overrides the usual prohibition on creating objects
in an extension-owned schema, and let users or the extension set it if
they wish.

(5) Decide I'm wrong and that we should just let objects be created
freely in the owned schema. Document the consequences of this and
blame any problems on user error.

(6) Your superior idea goes here!

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

Hi,

On Tue, Sep 02, 2025 at 09:35:45AM -0400, Robert Haas wrote:
> On Tue, Sep 2, 2025 at 5:02 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > Requiring superuser permission seems like a big penalty, especially since the
> > last few years have been all about *not* requiring superuser privileges.  Note
> > also that not all extensions embeds compiled code, some are just doing plain
> > plpgsql and work just fine.
> >
> > Why not requiring schema owner privileges?
>
> I agree with you that requiring superuser privileges is undesirable.
> However, requiring schema owner privileges isn't really requiring
> anything above and beyond what the permissions system would require
> anyway, since at the start, nobody else will have privileges on that
> schema. And that's where what Jelte was proposing -- and also what you
> propose here -- seems very accident-prone to me. It would be quite
> easy for the user who created the extension, and who therefore also
> owns the schema IIUC, to accidentally put other stuff there, or allow
> others to do so

Requiring schema owner privilege wouldn't allow the user who created the
extension to allow other users to mess up with the extension's private schema?
At least not with a simple GRANT on the schema.

I'm also wondering how much you can prevent the owner from doing changes in the
owned schema without leading to unhelpful behavior.  Would the owner still be
allowed to create extra indexes on extension owned table for instance, change
the TOAST setting, move them to other tablespace or ...?

> The first thought that popped into my head was that maybe your
> extension should make the objects it creates part of the extension,
> but I think that doesn't actually work, because it would mean that
> they would not be dumped.

Arguably there is pg_extension_config_dump() for that, assuming that this would
become allowed in scenario like the one I described (and modified to also emit
the DDL in such case).  But it would be hard for extension authors to use as
you would have to call it only with some major versions.

We could do it automatically, but extensions may not need to dump all tables
and/or all rows.  For instance in powa we have some unlogged tables that are
use transiently during a snapshot, and those tables shouldn't be dumped.  Right
now if such tables are created by the extension they're always dumped, but it
would be good to make it configurable if dynamically created tables become part
of the extension, one way or another.

On Sat, 6 Sept 2025 at 02:17, Julien Rouhaud <rjuju123@gmail.com> wrote:
> Requiring schema owner privilege wouldn't allow the user who created the
> extension to allow other users to mess up with the extension's private schema?
> At least not with a simple GRANT on the schema.

I think that sounds like reasonable change to Roberts initial
proposal: Allowing the schema owner and superusers to add objects in
the schema, but disallow all other users (even if they have CREATE
privileges on the schema).

I think this seems reasonable from a security perspective. The thing
owned_schema protects against, is accidentally executing code with
permissions of the extension script runner. The owner of the schema is
always the same user as the extension script runner. But it protects
users from the somewhat easy to make mistake of GRANT ALL ON SCHEMA
(instead of GRANT USAGE ON SCHEMA).

Note that this means that even with trusted=true, a non-superuser
extension owner would still not be able to the schema. For that
superuser=false is needed in the control file.

The only thing I'm wondering is if we should allow changing the schema
owner with ALTER SCHEMA OWNER TO. Because that would break this
assumption:
> The owner of the schema is always the same user as the extension script runner.

But that command seems unlikely to be run by accident. But on the
other hand, I don't really see a usecase for changing the schema
owner, except for breaking this protection. So I'm leaning towards
disallowing ALTER SCHEMA OWNER TO on the schema, probably even for
superusers.

On Sat, Sep 6, 2025 at 3:35 AM Jelte Fennema-Nio <me@jeltef.nl> wrote:
> I think that sounds like reasonable change to Roberts initial
> proposal: Allowing the schema owner and superusers to add objects in
> the schema, but disallow all other users (even if they have CREATE
> privileges on the schema).

I don't know, I'm not really convinced. I feel like this isn't really
a security issue but more of a could-be-an-unpleasant-surprise issue.
What the patch does (IIRC) is make it so that dropping the extension
just cascade-drops the schema. If the schema contains anything
unrelated to the extension, that's going to remove stuff that it
shouldn't remove. In Julien's examples, the other stuff that gets
introduced into the schema is logically part of the extension even if
it doesn't formally have membership in the extension, but somebody
could equally well just install an unrelated extension in the same
schema and then drop the first extension and, whoops.

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

On Thu, 11 Sept 2025 at 15:02, Robert Haas <robertmhaas@gmail.com> wrote:
> What the patch does (IIRC) is make it so that dropping the extension
> just cascade-drops the schema.

You recall incorrectly ;) It only does that when you do:
DROP EXTENSION ... CASCADE

Otherwise you get errors like this:

 DROP EXTENSION test_ext_owned_schema;
 ERROR:  cannot drop extension test_ext_owned_schema because other
objects depend on it
 DETAIL:  function test_owned_schema_defaults.new_owned() depends on
schema test_owned_schema_defaults

> but somebody
> could equally well just install an unrelated extension in the same
> schema and then drop the first extension and, whoops.

To be clear, that could only happen when that unrelated extension does
not have owned_schema=true. Because creating such an extension
requires the schema to not exist yet. (And even then as explained
above the accidental drop only happens when the user uses CASCADE.)

On Thu, Sep 11, 2025 at 9:29 AM Jelte Fennema-Nio <me@jeltef.nl> wrote:
> You recall incorrectly ;) It only does that when you do:
> DROP EXTENSION ... CASCADE
>
> Otherwise you get errors like this:
>
>  DROP EXTENSION test_ext_owned_schema;
>  ERROR:  cannot drop extension test_ext_owned_schema because other
> objects depend on it
>  DETAIL:  function test_owned_schema_defaults.new_owned() depends on
> schema test_owned_schema_defaults

OK. Perhaps that needs some associated tests?

To be honest, I'm kind of leaning at this point toward saying we
shouldn't impose any special restrictions here. If the DROP doesn't
cascade, then the worst thing that can happen is that you make it hard
for yourself to drop your own extension cleanly. I think letting the
superuser and the schema owner do things and other people not is too
weird -- it basically boils down to ignoring GRANT sometimes, and I
think users will find it confusing. If we were going to have
special_tinkering_mode=true|false that affected everyone equally, that
would make sense to me, but it sounds like nobody else really likes
that, so it's probably just a bad idea.

> > but somebody
> > could equally well just install an unrelated extension in the same
> > schema and then drop the first extension and, whoops.
>
> To be clear, that could only happen when that unrelated extension does
> not have owned_schema=true. Because creating such an extension
> requires the schema to not exist yet. (And even then as explained
> above the accidental drop only happens when the user uses CASCADE.)

Sure.

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