Date: Wed, 22 Mar 2017 09:15:10 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Dave Page <dpage@pgadmin.org>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Monitoring roles patch
Message-ID: <20170322131510.GC9812@tamriel.snowman.net>
References: 
 <CA+OCxoyRdsc1xyLfF9s698gUGyPXBs4CvJ+0Gwo8U65NmYJ7pw@mail.gmail.com>
 <CA+TgmobveXzVPytFOO3AkmsM+Y=YEG+UXcCvgwAt+SgYj85OCA@mail.gmail.com>
 <CA+OCxox9c9-1=bXxA6-gr-iWZbD5zdHYMz=cpC034RKB7zX40Q@mail.gmail.com>
 <CA+TgmoZ5zs50mEzALJqp3qBMC+CdT0ccnjvNJt1Ojiu9rz35zw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="kqmcYZU0IC3nN4rH"
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmoZ5zs50mEzALJqp3qBMC+CdT0ccnjvNJt1Ojiu9rz35zw@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


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

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Mar 22, 2017 at 7:48 AM, Dave Page <dpage@pgadmin.org> wrote:
> >> I'd be inclined to skip the rest of
> >> this.  If an individual user wants to grant that bundle of privileges
> >> to a role, they can do it with or without pg_monitor.
> >
> > GRANT cannot be used in all cases, as some of the functions changed
> > have hard-coded superuser checks. In those cases, I've added
> > pg_monitor membership to the permission checks in the C code.
>=20
> Oh.  Well, why not just control access using the permissions checking
> mechanism entirely, without hardcoding any OID?

I've not had the chance yet to review this (though I have been planning
to), but at least in most cases where we have hard-coded checks it's
because the GRANT system doesn't allow the kind of fine-grained access
we've implemented at the C level.  pg_stat_activity is a great example
of this where, if you aren't a superuser or a member of the other role,
you can still query the table but you can't see the 'query' column for
those other connections.

I did specifically ask for explicit roles to be made to enable such
capability and that the pg_monitor role be GRANT'd those roles instead
of hacking the pg_monitor OID into those checks, but it seems like
that's not been done yet.

> > The reason for having the role is to minimise the amount of work
> > required by the user to setup a role for the purposes of monitoring
> > the server. This is a practice which is seen in other DBMSs for
> > usability.
> >
> > With the patch, complex monitoring systems can easily be setup with
> > something like:
> >
> > CREATE ROLE monitoring_user LOGIN;
> > GRANT pg_monitor TO monitoring_role;
> >
> > Without, the users setting up their monitoring system will have to run
> > a much more complex set of GRANTs, almost certainly requiring
> > scripting.
>=20
> But that script is easy to provide, probably not very long, and could
> be bundled in an extension if it's helpful.  I'm wary of committing
> ourselves to a specific decision about what pg_monitor includes; that
> seems like it could result in endless future litigation every time
> somebody wants to make a change.  In contrast, nobody's going to have
> any question at all about the remit of pg_read_all_settings.

I tend to agree with Dave on this.  While I understand this concern and
risk of litigation, I believe that comes down to, really, making sure
that we spell out exactly what pg_monitor is intended for and that the
privileges it has will change over time.  If users are not comfortable
with that, then they can use provided script to create their own
'monitor' role- that is specifically why I want to have nothing
hard-coded to pg_monitor though, because otherwise you couldn't have
such a script able to GRANT out a subset of what pg_monitor allows.

Thanks!

Stephen

--kqmcYZU0IC3nN4rH
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

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

iQIcBAEBCgAGBQJY0njeAAoJEO1sijiDR2RVLiEP/i3mZV8LQ4VTMMwjBCu325tB
ssYeqRz/Imp3YUZUGqeeqS1V1GnWY/mN6xTXrT4GWdhWBxEoFKZ/uV4V+rV8KrR4
zJozguXUPY7A7eCnk4wfFXR+oQqL3FV16Iw7r+Dg0sTsqmqRdRhkO6J78oLzCJ5t
/DoGGbkL8f9GegGsSZn8q6fDaC4w4Wzuyu9S0yT+pNNSuQj+rzTAonyll1+sNPzf
rhjbwiE45K6YGK7DDx2Y1IZypJOS1y8r+GWcpKaMGbQRjoC5yvmvS6yNxMifaHOu
fkf/nYzRiOwZscrJlmxph1rK5Ecqeu8hqD/HS5PUc+PFLNnwb2smjhqB5RggsGhx
FiZczlmZZokMQWtuSDlVWvNOS5FUo01WrB4aacHYVofpDd8qgRwVy8p7RZMJYhU/
vocFgw6AQaEid2WPQIn9N8QFJJ3hjB5BeGGuIoMKcYsxRtFFfF36dnOPdjlrb6hL
lXZt0MrboygyOMeHYLmj9qhZlyu06QIwmGxOIg99yYTLufYwlpCezribi6zgUvaN
7o9JSVDC4/1rCr57ONyJprQEOnc1hsI6sK6unNsjfZ1dT9Eqvn6pCVquhmp8FUkz
iAyuUjsw4HxNLa92+YmzGYsvdBYJhnYqNdqh7Sq53NFoFTncmBERZXtQOrmOJDEP
TZbQj64ViYGUeHVg9m58
=U02a
-----END PGP SIGNATURE-----

--kqmcYZU0IC3nN4rH--