Re: Additional role attributes && superuser review

All,

* Stephen Frost (sfrost(at)snowman(dot)net) wrote:
> Alright, I've got an initial patch to do this for pg_start/stop_backup,
> pg_switch_xlog, and pg_create_restore_point. The actual backend changes
> are quite small, as expected. I'll add in the changes for the other
> functions being discussed and adapt the documentation changes from
> the earlier patch to make sense, but what I'd really appreciate are any
> comments or thoughts regarding the changes to pg_dump (which are generic
> to all of the function changes, of course).

So, I've tested this approach with extensions and binary upgrade and
things appear to all be working correctly, but there's a few issues
remaining to discuss:

The functions pg_start_backup and pg_stop_backup can currently be run by
replication roles but those roles won't have any permissions on those
functions with the new approach unless we GRANT those rights during
pg_dump and/or pg_upgrade. Note that this isn't an issue for tools
which use the replication protocol (eg: pg_basebackup) since the
replication protocol calls do_pg_start_bacup() directly. As such, my
first question is- do we want to worry about this? We should certainly
mention it in the release notes but I'm not sure if we really want to do
anything else.

The second question is in regards to pg_stat_activity. My first
suggestion for how to address that would be to have the function return
everything and then have the view perform the filtering to return only
what's shown today for users. Admins could then grant access to the
function for whichever users they want to have access to everything.
That strikes me as the best way to avoid changing common usage while
still providing the flexibility. Another option would be to still
invent the MONITOR role attribute and keep the rest the same. Again,
we'd want to mention something in the release notes regarding this
change.

Lastly, there is the question of pg_cancel_backend and
pg_terminate_backend. My thinking on this is to create a new
'pg_signal_backend' which admins could grant access to and leave the
existing functions alone (modulo the change for has_privs_of_role as
discussed previously). We'd rename the current 'pg_signal_backend' to
something else (maybe '_helper'); it's not exposed anywhere and
therefore renaming it shouldn't cause any heartache.

For pg_create_restore_point, pg_switch_xlog, pg_xlog_replay_pause,
pg_xlog_replay_resume, pg_rotate_logfile, we can just remove the
if(!superuser()) ereport() checks and REVOKE rights on them during the
initdb process, since there isn't anything complicated happening for
those today.

One other question which I've been thinking about is if we want to try
and preserve permission changes associated with other catalog objects
(besides functions), or if we even want to change how functions are
handled regarding permission changes. We don't currently preserve any
such changes across dump/restore or even binary upgrades and this would
be changing that. I don't recall any complaints about not preserving
permission changes to objects in pg_catalog, but one could argue that
our lack of doing so today is a bug.

Thoughts?

Thanks!

Stephen