Re: Additional role attributes && superuser review

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Thu, Jan 28, 2016 at 11:04 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> >> So, this seems like a case where a built-in role would be
> >> well-justified. I don't really believe in built-in roles as a way of
> >> bundling related permissions; I know you do, but I don't. I'd rather
> >> see the individual function permissions granted individually. But
> >> here you are talking about a variable level of access to the same
> >> function, depending on role. And for that it seems to me that a
> >> built-in role has a lot more to recommend it in that case than it does
> >> in the other. If you have been granted pg_whack, you can signal any
> >> process on the system; otherwise just your own. Those checks are
> >> internal to pg_terminate_backend/pg_cancel_backend so GRANT is not a
> >> substitute.
> >
> > If we extend this into the future then it seems to potentially fall
> > afoul of Noah's concern that we're going to end up with two different
> > ways of saying GRANT EXECUTE X ON Y. Consider the more complicated case
> > of pg_stat_activity, where values are shown or hidden based on who the
> > current role is. The policy system only supports whole-row, today, but
> > the question has already come up, both on the lists and off, of support
> > for hiding individual column values for certain rows, exactly as we do
> > today for pg_stat_activity. Once we reach that point, we'll have a way
> > to GRANT out these rights and a default role which just has them.
>
> Well, I'm not saying that predefined rolls are the *only* way to solve
> a problem like this, but I think they're one way and I don't clearly
> see that something else is better. However, my precise point is that
> we should *not* have predefined rolls that precisely duplicate the
> result of GRANT EXECUTE X ON Y. Having predefined rolls that change
> the behavior of the system in other ways is a different thing. So I
> don't see how this falls afoul of Noah's concern. (If it does,
> perhaps he can clarify.)

Apologies if it seems like what I'm getting at is obtuse but I'm trying
to generalize this, to provide guidance on how to handle the larger set
of privileges.

If I'm following correctly, having default roles for cases where the
role is specifically for something more than 'GRANT EXECUTE X ON Y' (or
a set of such commands..?) makes sense. Going back to the list of
roles, that would mean that default roles:

pg_monitor

Allows roles granted more information from pg_stat_activity. Can't be
just a regular non-default-role right as we don't, currently, have a
way to say "filter out the values of certain columns on certain rows,
but not on others."

(There's a question here though- for the privileges which will be
directly GRANT'able, should we GRANT those to pg_monitor, or have
pg_monitor only provide unfiltered access to pg_stat_activity, or..?
Further, if it's only for pg_stat_activity, should we name it
something else?)

pg_signal_backend

Allows roles to signal other backend processes beyond those backends
which are owned by a user they are a role member of. Can't be a
regular non-default-role right as we don't, currently, have any way to
GRANT rights to send signals only to backends you own or are a member
of.

pg_replication

Allows roles to use the various replication functions. Can't be a
regular non-default-role right as the REPLICATION role attribute
allows access to those functions and the GRANT system has no way of
saying "allow access to these functions if they have role attribute
X."

Make sense, as these are cases where we can't simply write GRANT
statements and get the same result, but we don't need (or at least,
shouldn't have without supporting GRANT on catalog objects and agreement
on what they're intended for):

pg_backup

pg_start_backup(), pg_stop_backup(), pg_switch_xlog(), and
pg_create_restore_point() will all be managed by the normal GRANT
system and therefore we don't need a default role for those use-cases.

pg_file_settings

pg_file_settings() function and pg_file_settings view will be managed
by the normal GRANT system and therefore we don't need a default role
for those use-cases.

pg_replay

pg_xlog_replay_pause(), and pg_xlog_replay_resume() will be managed
by the normal GRANT system and therefore we don't need a default role
for those use-cases.

pg_rotate_logfile

pg_rotate_logfile() will be managed by the normal GRANT system and
therefore we don't need a default role for that use-cases.

> > Personally, I don't have any particular issue having both, but the
> > desire was stated that it would be better to have the regular
> > GRANT EXECUTE ON catalog_func() working before we consider having
> > default roles for same. That moves the goal posts awful far though, if
> > we're to stick with that and consider how we might extend the GRANT
> > system in the future.
>
> I don't think it moves the goal posts all that far. Convincing
> pg_dump to dump grants on system functions shouldn't be a crazy large
> patch.

I wasn't clear as to what I was referring to here. I've already written
a patch to pg_dump to support grants on system objects and agree that
it's at least reasonable. When I was talking about moving goalposts, I
was saying that if we don't want default roles until the only thing
they're doing is being GRANT'd rights which administrators could GRANT
or create policies for themselves (which may one day be the case) then
that would imply a much larger amount of effort (support for hiding
columns based on policies, and perhaps even some way to GRANT access to
functions to operate only against other backends which are controlled by
a role of which you are a member).

> > What got me thinking along these lines was a question posed by Bruce
> > (Bruce, feel free to chime in if I've misunderstood) to me at SCALE
> > where we were chatting a bit about this, which was- how could we extend
> > GRANT to support the permissions that we actually wish
> > pg_terminate_backend() and pg_cancel_backend() to have, instead of using
> > a default role? I didn't think too much on it at the time as it strikes
> > me as a pretty narrow use-case and requiring quite a bit of complexity
> > to get right, but there again, I'd certainly view a system where the
> > user is allowed to have pg_start_backup() rights but not
> > pg_stop_backup() as being quite a small use-case also, yet that's the
> > direction we're largely going in with this discussion.
>
> Well, sure, in that largely artificial example it's not hard to say
> ha, ha, silly, but the actual examples we looked at upthread were much
> less obviously silly. There was plenty of room for argument about the
> precise contours of each predefined role.

I'm sure I'm looking at this through rosy colored goggles, but going
back over this thread, which started in October of 2014, and the other
one which I started, the questions which have come up around the rights
granted the default roles have been:

People would really like a default role that allowed pg_dump to always
work and maybe we should name 'pg_backup' something else, since it's
different from that.

Shouldn't we restrict access to the various pg_*_xlog_location()
functions? (I continue to agree that we should, but that's really an
independent discussion..)

The only specific discussions or questions about the permissions
included in the default roles, based on my ~1h review of the threads,
were from Michael regarding pg_switch_xlog(), who ultimately agreed that
it was fine as part of pg_backup, and from Fujii who suggested that
pg_monitor also have access to pgstattuple(), which is a contrib module
(and hence, I hadn't really thought about it, to be frank, tho it could
certainly be modified to work with a pg_monitor role). The other
concerns raised have not included specific issues or questions about the
privileges included but rather contended that there may be issues.

That's not to say that there couldn't be issues raised, but I guess I
don't see that many possible combinations for the sets of rights
included, which are the only ones sensible to consider based on my
review of the "if (!superuser()) ereport()" calls in the backend (from:
http://www.postgresql.org/message-id/20141015052259.GG28859@tamriel.snowman.net)

Thanks!

Stephen