MIME-Version: 1.0
References: 
 <CAGECzQQzDqDzakBkR71ZkQ1N1ffTjAaruRSqppQAKu3WF+6rNQ@mail.gmail.com>
 <dbd3cc5a5edde04941c8624dde7e4bd2a063f366.camel@j-davis.com>
 <CAGECzQTe3y6xwP9aMbFBCxKnQEEn3G3=cEDqoD5xaOwPwypd_A@mail.gmail.com>
 <CAGECzQTAgtTp3G=Em3xEY=T=uiKfyu5xLYri9By=sfNBS5C_9A@mail.gmail.com>
 <CAKFQuwaT4_n=e0YKBZAyox1CQUra2ka0cySs+3pGZR5p50pn-g@mail.gmail.com>
 <CAGECzQTOJrnnJkmMe9nems0jouiKUbFcEb1rb9kE_svsAZiGQg@mail.gmail.com>
 <585e996c-a5c6-4e61-acc4-d92b7a1458ea@vondra.me>
 <CAGECzQS02M6YPDXemo36tShO-ZYObjqnyTJyVttua1PGyN4xRw@mail.gmail.com>
 <CADE6LvjnX8MbZPSTkrdoRGwf+_q6deNN1+-N_bmjdUgvFNy+fQ@mail.gmail.com>
In-Reply-To: 
 <CADE6LvjnX8MbZPSTkrdoRGwf+_q6deNN1+-N_bmjdUgvFNy+fQ@mail.gmail.com>
From: Jelte Fennema-Nio <me@jeltef.nl>
Date: Mon, 28 Jul 2025 10:01:38 +0200
Message-ID: 
 <CAGECzQRFa-RLf2FMsL51TqXbtrbJjtC=SCMo50ZO7Pf_e0O7Sg@mail.gmail.com>
Subject: Re: Extension security improvement: Add support for extensions with
 an owned schema
To: Sadeq Dousti <msdousti@gmail.com>
Cc: Tomas Vondra <tomas@vondra.me>,
 "David G. Johnston" <david.g.johnston@gmail.com>,
	Jeff Davis <pgsql@j-davis.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
	"David E. Wheeler" <david@justatheory.com>,
 Artem Gavrilov <artem.gavrilov@percona.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAGECzQRFa-RLf2FMsL51TqXbtrbJjtC%3DSCMo50ZO7Pf_e0O7Sg%40mail.gmail.com>
Precedence: bulk

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 to be mentioned in the document=
ation. In addition, an extra confirmation (e.g., "This will drop schema nnn=
n 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 kee=
ps 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 we=
ll.

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 author=
s.
>
> While "ALTER EXTENSION UPDATE" is mentioned as a minefield, the patch doe=
s not fix it (only ALTER EXTENSION ... SET SCHEMA is affected AFAICS). A po=
ssible 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 EXT=
ENSION command? Something like:
>
> CREATE EXTENSION xyz
> WITH owned_schema=3Dtrue

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 explicitl=
y 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 extension X installation depend=
s on other extensions Y & Z to be installed. I was thinking if X calls a fu=
nction from Y during installation, and Y does not have owned_schema, the se=
arch_path confusion attack can be transitively applied. It could make sense=
 that X refuses to install, unless both Y & Z (=3D all required extensions)=
 are marked as owned_schema=3Dtrue. Although in favor of backwards compatib=
ility, this can be overridable by an option in CREATE EXTENSION, such as "W=
ITH transitive_owned_schema=3Dfalse".

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.