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

On Wed, 19 Jun 2024 at 19:55, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Jelte Fennema-Nio <me(at)jeltef(dot)nl> writes:
> > On Wed, 19 Jun 2024 at 17:28, Robert Haas <robertmhaas(at)gmail(dot)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