Date: Sat, 8 Feb 2020 08:54:30 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Julien Rouhaud <rjuju123@gmail.com>,
	Darafei =?utf-8?Q?Kom=D1=8Fpa?= Praliaskouski <me@komzpa.net>,
	PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Marking some contrib modules as trusted extensions
Message-ID: <20200208135429.GP3195@tamriel.snowman.net>
References: <32315.1580326876@sss.pgh.pa.us>
 <CAC8Q8tKiedws=GNF_M50tV7qw7TJ0y_=+k+WJHPNc=VqScZr5A@mail.gmail.com>
 <CAOBaU_YcmiKnLvb8kpLNA59Di9WA02V3_TCfJ8pAMriPfeRq_w@mail.gmail.com>
 <5280.1580333933@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="fIcxWlDF0GCpXOou"
Content-Disposition: inline
In-Reply-To: <5280.1580333933@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--fIcxWlDF0GCpXOou
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Julien Rouhaud <rjuju123@gmail.com> writes:
> >>> Probably NO, if only because you'd need additional privileges
> >>> to use these anyway:
> >>> pg_stat_statements
>=20
> > 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.
>=20
> 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 =3D is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_ST=
ATS);

> 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. =20

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

--fIcxWlDF0GCpXOou
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJePr2VAAoJEO1sijiDR2RV2psP/1lt1Qkm1Myp2ef3tAUmWX9p
wcMM2vKdlLqnbmjruRyol1vKLCJiateeEXvwot5ZEtaTsMgd9pYNl6Uj3oqFRUMC
OY1Zbd6EQNXQLvBUh+SZsoqAyXmyko6Rg5i1t5pJKe5JgA9MtgCuJtt1PKXW4vkU
QjMlwiztMrbouySLnBkDtqY3pOeKcCNdBK6YXzefB6rLk2GL3E7wfFsqYpusAEpP
9RQUFq9T61+tG3Ayzxq7EVvLio5aT5uAAVL+clSsufLVf6UiH8X7VessuIYW9Y8t
K2D2XrzXHhEz4kpda3MqCxwwc3i3TbsA2uxY4AIaTw4b/HEOeE+xLiZ4Fp9h0lHq
M9KvVVPqeRREVcSmtCHlQwZAqCLTtw47xBwzngMUmmUKfLXtOGmYCZ1s/lGq2i+P
9HgF45gUXBfooLUdYl9JLk30K3nOvSzGkR63iOi3V8qHGjLSxTwUxIF5zzkB36Fd
bu/wKEzRF/c572ZBmsV73O1isduwU87pW18v+8/B3OgZEv7UBsVCh5eAjLLjLjxk
slOdkdNUX2IDs8JoV6zIc7hTpRv4Txzb/PH0vSj4DQTS8hCye1ksARSlUbXG5vbg
fnEIlo6D645ay+e12DFL8xiBkZlG808ZaAhPEbiNgPxPOyyGiN8vRXeeq9GnVqRx
jJCuRrNnyVssFqJkPeO1
=g4PY
-----END PGP SIGNATURE-----

--fIcxWlDF0GCpXOou--