Re: Bug in handling default privileges inside extension update scripts

On Mon, Apr 26, 2021 at 7:29 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> Greetings,
>
> * Mats Kindahl (mats(at)timescale(dot)com) wrote:
> > On Thu, Apr 22, 2021 at 5:15 PM Stephen Frost <sfrost(at)snowman(dot)net>
> wrote:
> > > * Mats Kindahl (mats(at)timescale(dot)com) wrote:
> > > > * To be able to read the configuration tables, "reader" need to have
> > > > SELECT privileges.
> > > >
> > > > * Since the new role is added by the user and not by the extension,
> > > > the grants have to be dumped as well. Otherwise, a restore of the
> > > > data will have wrong privileges.
> > > >
> > > > * Since new configuration tables could be added by an update of the
> > > > extension, it is necessary to make sure that these privileges are
> > > > added to new tables when updating. Typically, this means changing
> > > > the default privileges on the schema for the configuration files.
> > >
> > > If the extension is updated, I think it's entirely reasonable to expect
> > > an admin to have to go in and update the relevant permissions on any
> new
> > > tables that have come into existance and, as I've said elsewhere, I
> > > don't think that schema-level default privs should be applied to tables
> > > created by extensions. Sadly, no one else seems to have an opinion
> > > regarding that and so there hasn't been a change in that, yet, but
> > > that's the source of the issue imv.
> >
> > That is a different way to solve it, but I think that is a little
> > unintuitive. I am actually proposing to still assign default privileges,
> > but not add them to initprivs, to make sure that they are treated the
> same
> > way before and after an update.
>

Before delving into the discussion below, my main issue originally is that
the default privileges are used to set the initprivs for objects created by
the extension, causing them to not be dumped after an update of the
extension. From the thread that you mentioned above, it seems like you
agree that this should indeed not be the case for the reasons I outlined.
Did I understand it correctly? If so, I can create a patch for that part at
least.

> Yes, I understood your suggestion, but I did, and still do, disagree
> with that approach- how is an admin supposed to correctly guess what
> permissions would be appropriate for new tables being added during an
> upgrade of an extension?

I am not really sure in what situation an admin will not be able to decide
on the grants for extension objects (beyond what the extension decides are
required).

The example I gave is because a non-superuser should be able to back up the
tables and I think that the statements are quite clear in what they mean:
assigning grants to tables directly is for the purpose of the currently
existing objects, and default privileges are for future objects.

In the example given, the reason adding additional grants and/or default
privileges is because the user needs to read the objects for backup
purpose. It is quite clearly expressed using the statements in the example.
To help me understand what problem you see, do you have an example where
this is not as clear and where the water is muddier about what privileges
to assign?

> Not to mention that extensions routinely get
> added to existing schemas and I don't think it's at all obvious to
> users that tables, functions, etc, added by an extension into a schema
> should get the default privileges for that schema (and that could even
> lead to security issues, I suspect...),

Well... a user that wants to assign specific privileges to extension
objects and keep them "under control" is probably going to use a dedicated
schema for the extension(s) to separate user data from extension data (for
example, in a hosted environment). Sure, users *can* use the public schema
for all extensions (I think that is what you meant by "extensions routinely
gets added to existing schemas"), but that will quickly be unmanageable for
precisely the reason that it is hard to separate extension data from user
data, so not sure if that is a good reason to not allow default privileges
on extension objects. It feels like optimizing for the rare case.

I am not sure how this could cause a security issue though. My reasoning is
this: escalations of privileges can happen for objects created by the
extension if you have default privileges on the schema, so this would mean
that some role can get access to new objects created by the extension, but
this was the purpose of assigning default privileges on the schema in the
first place so it cannot be unexpected.

Do you have some ideas or suspicions for how unexpected escalations could
occur?

not to mention that you have to
> wonder if the privileges installed by the extension should be applied
> *first*, and default privs after, or if the default privileges should
> be first and the extension's privileges after.

Forgive my ignorance, but I am not sure how that makes a difference. Could
you elaborate?

As it's currently the
> latter, it's rather complicated as the extension has no idea what to
> expect the privileges on the object to be and so how can it sensibly set
> privileges on it..?
>

Again, forgive my ignorance, but isn't the expectation of the extension
that it has superuser privileges to the database where it is being created?

Best wishes,
Mats

> More and more it looks clear to me that this is really just broken and
> we need to stop applying default privs to objects created by extensions.
>
> Thanks,
>
> Stephen
>