Re: Additional role attributes && superuser review

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Robert Haas <robertmhaas(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Gavin Flower <GavinFlower(at)archidevsys(dot)co(dot)nz>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Adam Brightwell <adam(dot)brightwell(at)crunchydatasolutions(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Additional role attributes && superuser review
Date: 2016-01-04 17:55:16
Message-ID: 20160104175516.GC3685@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Noah,

* Noah Misch (noah(at)leadboat(dot)com) wrote:
> On Tue, Dec 29, 2015 at 08:35:50AM -0500, Stephen Frost wrote:
> > * Noah Misch (noah(at)leadboat(dot)com) wrote:
>
> > The one argument which you've put forth for adding the complexity of
> > dumping catalog ACLs is that we might reduce the number of default
> > roles provided to the user.
>
> Right. If "GRANT EXECUTE ON FUNCTION pg_rotate_logfile() TO mydba" worked as
> well as it works on user-defined functions, the community would not choose to
> add a pg_rotate_logfile role holding just that one permission.

I understand that's your position, but I disagree with your conclusion.

If we're going to provide default roles, which I continue to feel is a
good approach, then I would suggest we use them as an abstraction for
the permissions which we view as senible sets of grantable rights. I
dislike the idea of having default roles and then making an exception
for single-permission cases.

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

pgbackrest:

To run pgbackrest as a non-superuser and not the 'postgres' system
user, grant the pg_backup role to the backrest user and ensure the
backrest system user has read access to the database files (eg: by
having the system user be a member of the 'postgres' group):

GRANT pg_backup to backrest;

For information regarding the pg_backup role, see:
http://www.postgresql.org/docs/current/static/roles/database-roles.html

I can see similar bits of documentation being included in pgAdmin or
other tools. For the pg_rotate_logfile permission, specifically, we
were asked by a client about that permission with the use case being a
logrotate-type of tool, which only has access to the log files but needs
to be able to perform a rotation. This particular client is pretty tech
savvy and I don't think they'd have a problem using GRANT EXECUTE if
that was the only option, but I can see a similar use-case with
logrotate or pgAdmin or even for regular non-superuser admins using psql
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 disagree that we would. Having a single
> > set of default roles which provide a sensible breakdown of permissions
> > is a better approach than asking every administrator and application
> > developer who is building tools on top of PG to try and work through
> > what makes sense themselves, even if that means we have a default role
> > with a small, or even only an individual, capability.
>
> The proposed pg_replication role introduces abstraction that could, as you
> hope, spare a DBA from studying sets of functions to grant together. The
> pg_rotate_logfile role, however, does not shield the DBA from complexity.

I disagree with the above statement. Having to understand only one
level of abstraction (the default roles) does reduce the complexity and
means that the DBA does not have to work out if the specifc GRANT
requested by the end user would result in some other access or if there
are any unexpected issues to encounter with issuing GRANTs directly on
catalog objects- something we don't currently support, so such concern
is certainly reasonable.

> Being narrowly tied to a specific function, it's just a suboptimal spelling of
> GRANT. The gap in GRANT has distorted the design for these predefined roles.
> I do not anticipate a sound design discussion about specific predefined roles
> so long as the state of GRANT clouds the matter.

I'm loathe to encourage any direct modification of catalog objects,
even if it's just ACLs. I've seen too many cases, as I imagine others
have, of users destroying their databases or running into unexpected
results when modifying the catalog. The catalog modifications supported
should be explicitly provided through other means rather than direct
commands against the catalog objects. I see the default roles approach
as being similar to having:

ALTER DATABASE db WITH CONNECTION LIMIT 5;

instead of suggesting users issue:

UPDATE DATABASE SET datconnlimit = 5 WHERE datname = 'db';

There is little difference between the two, technically, but I'm a whole
lot more comfortable with the ALTER DATABASE than with the user issuing
an UPDATE against a catalog table. With 9.5, we are adding
ALLOW_CONNECTIONS and IS_TEMPLATE also and I don't recall any particular
concern that those are overly redundant with the equivilant UPDATE
statement.

> > > To summarize, I think the right next step is to resume designing pg_dump
> > > support for system object ACLs. I looked over your other two patches and will
> > > unshelve those reviews when their time comes.
> >
> > To be clear, I don't believe the two patches are particularly involved
> > with each other and don't feel that one needs to wait for the other.
>
> Patch 2/3 could stand without patch 3/3, but not vice-versa. It's patch 2/3
> that makes pg_dumpall skip ^pg_ roles, and that must be in place no later than
> the first patch that adds a predefined ^pg_ role.

Apologies for not being clear on this- I was referring to pg_dump
support for GRANT on catalog objects vs. the default roles patch in my
statement by way of summary.

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

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 wonder if we may end up causing ourselves
trouble going with that approach though if we start providing a set of
default policies which SysCache knows how to work with but which users
can change.

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2016-01-04 17:56:08 Re: dynloader.h missing in prebuilt package for Windows?
Previous Message Pavel Stehule 2016-01-04 17:27:03 Re: custom function for converting human readable sizes to bytes