Re: Monitoring roles patch - Mailing list pgsql-hackers

Hi

> On 25 Mar 2017, at 15:55, Stephen Frost <sfrost@snowman.net> wrote:
>
> Robert,
>
> * Robert Haas (robertmhaas@gmail.com) wrote:
>>> On Fri, Mar 24, 2017 at 12:46 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>> On Fri, Mar 24, 2017 at 4:24 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>>> That's possible, but it really depends on the tool, not on core
>>>> PostgreSQL.  The tool should be the one providing the script, because
>>>> the tool is what knows its own permissions requirements.  Users who
>>>> care about security won't want to grant every privilege given by a
>>>> pg_monitor role to a tool that only needs a subset of those
>>>> privileges.
>>>
>>> The upshot of this would be that every time there's a database server
>>> upgrade that changes the permissions required somehow, the user has to
>>> login to every server they have and run a script. It is no longer a
>>> one-time thing, which makes it vastly more painful to deal with
>>> upgrades.
>>
>> So, I might be all wet here, but I would have expected that changes on
>> the TOOL side would be vastly more frequent.  I mean, we do not change
>> what a certain builtin permission does very often.  If we add
>> pg_read_all_settings, what is the likelihood that the remit of that
>> role is ever going to change?  I would judge that was is vastly more
>> likely is that a new version of some tool would start needing that
>> privilege (or some other) where it didn't before.  If that happens,
>> and the script is on the tool side, then you just add a new line to
>> the script.  If the script is built into initdb, then you've got to
>> wait for the next major release before you can update the definition
>> of pg_monitor - and maybe argue with other tool authors with different
>> requirements.
>
> The expectation here is that the pg_monitor role will include all of the
> reasonable privileges in a given release that a monitor tool should be
> using.

Exactly.

>
> While it's likely that monitoring tools will not all be able to work
> with every function they would then have access to on day 1, I'm sure
> the goal for all of them will be to reach the point where they are using
> all of the functions and tables they then have access to.

Right - and we have discussed here, in at least one developer meeting, and with the tools team at EDB what should be
included,so I'm confident we have a solid starting point that would be generally useful. 

>
> Moving forward, the privileges being added in future versions of PG (the
> ones you point out as happening at initdb-time) would be only those
> which are associated with new features in those later versions of PG.
> Those new features aren't going to be back-patched and therefore it
> wouldn't be possible for the tool to provide a script to GRANT access to
> those new features which would work on older versions of PG, meaning
> that the script you suggest would necessairly be PG-version specific.

Indeed.

Another issue with having the admin run a script is that any new databases created from template0 or the default
template1,may all need the script to be run at that point. Admin tools that can automatically and immediately start
monitoringnew databases may need manual admin/dbowner intervention which could be extremely painful in large
environments.

I believe this and other reasons we've described are exactly why other DBMS' do what we're proposing.

>
> In other words, I don't believe this is a valid concern.
>
>> So I'm fine with this:
>>
>> -    if (tblspcOid != MyDatabaseTableSpace)
>> +    if (tblspcOid != MyDatabaseTableSpace &&
>> +        !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
>>
>> But I don't like this:
>>
>> +GRANT EXECUTE ON FUNCTION pgstattuple(text) TO pg_read_all_stats;
>
> I believe we understood that to be the case.  I don't object to the
> clarification, of course.
>
>> My position is that execute permission on a function is already
>> grantable, so granting it by default to a built-in role is just
>> removing flexibility which would otherwise be available to the user.
>
> To remove flexibility would require that we remove the ability to
> independently GRANT that right.  We are not doing that.  Nor are we
> taking anything away from the user by added a new default role- we
> already claimed the pg_ namespace for roles in 9.6.

Right - and if a user doesn't like it, they can easily just not use the default role and create their own alternative
instead.

From a usability perspective, I cannot see any downsides to the default roles as proposed. They take nothing away from
theusers that don't want/need them, and add significant convenience for those that do.