Re: Additional role attributes && superuser review

On Mon, Jan 04, 2016 at 12:55:16PM -0500, Stephen Frost wrote:
> * Noah Misch (noah(at)leadboat(dot)com) wrote:
> > On Tue, Dec 29, 2015 at 08:35:50AM -0500, Stephen Frost wrote:

> I'm approaching this largely from a 3rd-party application perspective.
> There are two examples off-hand which I'm considering:
>
> check_postgres
> pgbackrest
>
> I'd like to be able to include, in both of those, a simple set of
> instructions for granting the necessary rights to the user who is
> running those processes. A set of rights which an administrator can go
> look up and easily read and understand the result of those grants. For
> example:
>
> check_postgres:
>
> Most check_postgres sub-commands can be run without superuser
> privileges by granting the pg_monitor role to the monitoring user:
>
> GRANT pg_monitor TO monitor;
>
> For information regarding the pg_monitor role, see:
> http://www.postgresql.org/docs/current/static/roles/database-roles.html

Stop. Even if PostgreSQL introduces pg_monitor, check_postgres will do itself
a favor by not using it. The moment the two projects' sense of monitoring
privileges falls out of lockstep, benefits from using pg_monitor go negative.
check_postgres should instead furnish a script that creates a role holding
required privileges and/or SECURITY DEFINER helpers. If check_postgres starts
using an object, say pgstattuple, it will wish to use it in all versions.
Since PostgreSQL will change pg_monitor privileges in major releases only,
check_postgres would wait 6-18 months to use a privilege in just one of five
supported versions. If PostgreSQL hackers ever disagree with check_postgres
hackers about whether a privilege belongs in the pg_monitor role, then
check_postgres will wish it had never used pg_monitor. For a sample
controversy, some monitoring tool may well call pg_read_file, but that's
arguably too much power to be giving _every_ monitoring tool.

By the way, the pg_monitor role you were ready to commit does not cover
today's check_postgres needs. Among restricted objects, check_postgres uses
at least pg_ls_dir, pg_stats, pg_settings, and pg_stat_activity. Having said
that, it remains premature to design predefined roles. It would be a waste to
mobilize such a design effort with GRANT's limitation clouding the issue.

> and, to reiterate what I said above, I'd rather have one abstraction for
> these kinds of permissions instead of a mish-mash of instructions. The
> difference I can imagine being between:
>
> For backups and monitoring, you can use default roles:
>
> GRANT pg_backup,pg_monitor to new_admin;
>
> but for other regular privileges such as rotating logfiles, or sending
> signals to other processes, you have to explicitly GRANT permissions:
>
> GRANT EXECUTE ON FUNCTION pg_rotate_logfile() TO new_admin;
> GRANT EXECUTE ON FUNCTION pg_signal_backend() TO new_admin;

I don't mind having those two ways to transfer privilege. If I have to settle
for one or the other, I pick the latter.

> > > Further, I'm not convinced that adding support for dumping ACLs or, in
> > > general, encouraging users to define their own ACLs on catalog objects
> > > is a good idea. We certainly have no mechanism in place today for those
> > > ACLs to be respected by SysCache and encouraging their use when we won't
> > > actually respect them is likely to be confusing.
> >
> > What's this problem with syscache? It sounds important.
>
> CREATE TABLE and DROP TABLE aren't going to care one bit if you have
> access to pg_class or not. The same goes for basically everything else.
>
> If we really want to support ACLs on the catalog, we'd have to either
> caveat that none of the internal lookups will respect them or revamp
> SysCache and any other direct catalog access to do permission checks
> first, which I don't think we really want to do.

Oh, that. Having internal lookups ignore the ACLs is more good than bad, and
users have little cause to expect something different. You don't need INSERT
on pg_attribute to add a column, so why expect lack of SELECT on pg_attribute
to prevent dropping one? I might document it like so:

While GRANT can modify the privileges of a system catalog table, that
affects only queries that address the catalog as an SQL table. Internal,
system access to the same underlying data will proceed normally. For
example, "REVOKE SELECT ON pg_proc FROM PUBLIC" does not preclude calling
functions or even preclude passing them to pg_get_functiondef. It does
block queries that name pg_proc in a FROM clause.

> This entire discussion of privileges-on-catalog-objects should really
> also consider the ongoing discussion about providing policies for the
> catalog via RLS. If we start pg_dump'ing the ACLs of catalog objects
> then we'd, presumably, also want to pg_dump out any policies defined
> against catalog objects.

I would have no qualms supporting ACL changes while not supporting added
policies, indexes, triggers, rules, inheritance children, extra columns, etc.
Both Oracle and SQL Server invite system object GRANTs, but they don't invite
system object mutations generally.

nm