public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Jelte Fennema-Nio <[email protected]>
To: David G. Johnston <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Extension security improvement: Add support for extensions with an owned schema
Date: Fri, 27 Sep 2024 14:00:17 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAGECzQTOJrnnJkmMe9nems0jouiKUbFcEb1rb9kE_svsAZiGQg@mail.gmail.com>
References: <CAGECzQQzDqDzakBkR71ZkQ1N1ffTjAaruRSqppQAKu3WF+6rNQ@mail.gmail.com>
<[email protected]>
<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>
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
view thread (26+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Extension security improvement: Add support for extensions with an owned schema
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox