Re: Monitoring roles patch - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: Monitoring roles patch
Date
Msg-id 20170325155520.GW9812@tamriel.snowman.net
Whole thread Raw
In response to Re: Monitoring roles patch  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: Monitoring roles patch  (Dave Page <dpage@pgadmin.org>)
List pgsql-hackers
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.

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.

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.

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.

Thanks!

Stephen

pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: increasing the default WAL segment size
Next
From: ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Subject: Re: PL/Python: Add cursor and execute methods to plan object