Re: Marking some contrib modules as trusted extensions

Greetings,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Julien Rouhaud <rjuju123(at)gmail(dot)com> writes:
> >>> Probably NO, if only because you'd need additional privileges
> >>> to use these anyway:
> >>> pg_stat_statements
>
> > But the additional privileges are global, so assuming the extension
> > has been properly setup, wouldn't it be sensible to ease the
> > per-database installation? If not properly setup, there's no harm in
> > creating the extension anyway.
>
> Mmm, I'm not convinced --- the ability to see what statements are being
> executed in other sessions (even other databases) is something that
> paranoid installations might not be so happy about.

Of course, but that's why we have a default role which allows
installations to control access to that kind of information- and it's
already being checked in the pg_stat_statements case and in the
pg_stat_activity case:

/* Superusers or members of pg_read_all_stats members are allowed */
is_allowed_role = is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS);

> Our previous
> discussions about what privilege level is needed to look at
> pg_stat_statements info were all made against a background assumption
> that you needed some extra privilege to set up the view in the first
> place. I think that would need another look or two before being
> comfortable that we're not shifting the goal posts too far.

While you could maybe argue that's true for pg_stat_statements, it's
certainly not true for pg_stat_activity, so I don't buy it for either.
This looks like revisionist history to justify paranoia. I understand
the general concern, but if we were really depending on the mere
installation of the extension to provide security then we wouldn't have
bothered putting in checks like the one above, and, worse, I think our
users would be routinely complaining that our extensions don't follow
our security model and how they can't install them.

Lots of people want to use pg_stat_statements, even in environments
where not everyone on the database server, or even in the database you
want pg_stat_statements in, is trusted, and therefore we have to have
these additional checks inside the extension itself.

The same goes for just about everything else (I sure hope, at least) in
our extensions set- none of the core extensions should be allowing
access to things which break our security model, even if they're
installed, unless some additional privileges are granted out. The act
of installing a core extension should not create a security risk for our
users- if it did, it'd be a security issue and CVE-worthy.

As such, I really don't agree with this entire line of thinking when it
comes to our core extensions. I view the 'trusted extension' model as
really for things where the extension author doesn't care about, and
doesn't wish to care about, dealing with our security model and making
sure that it's followed. We do care, and we do maintain, the security
model that we have throughout the core extensions.

What I expect and hope will happen is that people will realize that, now
that they can have non-superusers installing these extensions and
therefore they don't have to give out superuser-level rights as much,
there will be asks for more default roles to allow granting out of
access to formerly superuser-only capabilities. There's a bit of a
complication there since there might be privileges that only make sense
for a specific extension, but an extension can't really install a new
default role (and, even if it did, the role would have to be only
available to the superuser initially anyway), so we might have to try
and come up with some more generic and reusable default role for that
case. Still, we can try to deal with that when it happens.

Consider that you may wish to have a system that, once installed, a
superuser will virtually never access again, but one where you want
users to be able to install and use extensions like postgis and
pg_stat_statements. That can be done with these changes, and that's
fantastic progress- you just install PG, create a non-superuser role,
make them the DB owner, and then GRANT things like pg_read_all_stats to
their role with admin rights, and boom, they're good to go and you
didn't have to hack up the PG source code at all.

> The bigger picture here is that I don't want to get push-back that
> we've broken somebody's security posture by marking too many extensions
> trusted. So for anything where there's any question about security
> implications, we should err in the conservative direction of leaving
> it untrusted.

This is just going to a) cause our users to complain about not being
able to install extensions that they've routinely installed in the past,
and b) make our users wonder what it is about these extensions that
we've decided can't be trusted to even just be installed and if they're
at risk today because they've installed them.

While it might not seem obvious, the discussion over on the thread about
DEFAULT PRIVILEGES and pg_init_privs is actually a lot more relevant
here- there's extensions we have that expect certain functions, once
installed, to be owned by a superuser (which will still be the case
here, thanks to how you've addressed that), but then to not have EXECUTE
rights GRANT'd to anyone (thanks to the REVERT FROM PUBLIC in the
installation), but that falls apart when someone's decided to set
up DEFAULT PRIVILEGES for the superuser. While no one seems to want to
discuss that with me, unfortunately, it's becoming more and more clear
that we need to skip DEFAULT PRIVILEGES from being applied during
extension creation.

Thanks,

Stephen