Re: Bug in handling default privileges inside extension update scripts - Mailing list pgsql-bugs

From Mats Kindahl
Subject Re: Bug in handling default privileges inside extension update scripts
Date
Msg-id CA+14425MQ7ALXS9M-Ftwt-uX1w-L=+0TdgHaSm0m7SyWZi9RHg@mail.gmail.com
Whole thread Raw
In response to Re: Bug in handling default privileges inside extension update scripts  (Stephen Frost <sfrost@snowman.net>)
Responses Re: Bug in handling default privileges inside extension update scripts  (Stephen Frost <sfrost@snowman.net>)
List pgsql-bugs
On Mon, Apr 26, 2021 at 7:29 PM Stephen Frost <sfrost@snowman.net> wrote:
Greetings,

* Mats Kindahl (mats@timescale.com) wrote:
> On Thu, Apr 22, 2021 at 5:15 PM Stephen Frost <sfrost@snowman.net> wrote:
> > * Mats Kindahl (mats@timescale.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

pgsql-bugs by date:

Previous
From: Stephen Frost
Date:
Subject: Re: Bug in handling default privileges inside extension update scripts
Next
From: PG Bug reporting form
Date:
Subject: BUG #16985: ModifyWaitEvent function does not have pgsocket fd and void *user_data arguments