Re: SET ROLE and reserved roles - Mailing list pgsql-hackers

From David G. Johnston
Subject Re: SET ROLE and reserved roles
Date
Msg-id CAKFQuwaU13y7VBH7FUWPTaftki2EKc-AUHfTYea-bwUYhwocfA@mail.gmail.com
Whole thread Raw
In response to Re: SET ROLE and reserved roles  (Stephen Frost <sfrost@snowman.net>)
List pgsql-hackers
On Fri, Apr 15, 2016 at 8:56 AM, Stephen Frost <sfrost@snowman.net> wrote:
Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Apr 13, 2016 at 6:53 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > Requiring that SET ROLE be allowed will mean that many more paths must
> > be checked and adjusted, such as in all of the CreateObject statements
> > and potentionally many other paths that I'm not thinking of here, not
> > the least of which are all of the *existing* checks near
> > get_rolespec_oid/tuple which will require additional checks for the
> > CURRENT_USER case to see if the current user is a default role.
>
> I don't get it.  I think that these new roles should work just like
> any other roles, except for existing at initdb time.  I don't see why
> they would require checking or adjusting any code-paths at all.  It
> would presumably have made the patch you committed smaller and
> simpler.  The only thing you'd need to do is (approximately) not dump
> them.

Perhaps it would be helpful to return to the initial goal of these
default roles.

We've identified certain privileges which are currently superuser-only
and we would like to be able to be GRANT those to non-superuser roles.
Where our GRANT system is able to provide the granularity desired, we
have simply removed the superuser checks, set up appropriate default
values and, through the "pg_dump dump catalog ACLs" patch, allowed
administrators to make the changes to those objects via the
'GRANT privilege ON object TO role' system.

For those cases where our GRANT system is unable to provide the
granularity desired and it isn't sensible to extend the GRANT system to
cover the exact granularity we wish to provide, we needed to come up
with an alternative approach.  That approach is the use of special
default roles, where we can allow exactly the level of granularity
desired and give administrators a way to grant those privileges to
users.

​I didn't fully comprehend this before, but now I understand why additional checks beyond simple "has inherited the necessary grant" are needed.  The checks being performed are not for itemized granted capabilities 

Further, there seems to be no particular use-case which is satisfied by
allowing SET ROLE'ing to the default roles or for those roles to own
objects; indeed, it seems more likely to simply spark confusion and
ultimately may prevent administrators from making use of this system for
granting privileges as they are unable to GRANT just the specific
privilege they wish to.


​And I'm have trouble imaging what kind of corner-case could result from not allowing a role to assume ownership of a role it is a member of.  While we do have the capability to required SET ROLE it is optional: any code paths dealing with grants have to assume that the current role receives permission via inheritance - and all we are doing here is ensuring that is the situation.

It now occurs to me that perhaps we can improve on this situation in the
future by formally supporting a role attribute that is "do not allow SET
ROLE to this user" and then changing the default roles to have that
attribute set (and disallowing users from changing it).  That's just
additional flexibility over what the current patch does, however, but
perhaps it helps illustrate that there are cases where only the
privileges of the role are intended to be GRANT'd and not the ability to
SET ROLE to the role or to create objects as that role.

​That is where my first response was heading but it was definitely scope creep so I didn't bring it up.  Basically, an "INHERITONLY" ​modifier in addition to INHERIT and NOINHERIT.

I had stated the having a role that neither provides inheritance nor changing-to is pointless but I am now unsure if that is true.  It would depend upon whether a LOGIN role is one that is considered having been SET ROLE to or not.  If not, then a "LOGINONLY" combination would work such that a role with that combination would only have whatever grants are given to it and is not allowed to be changed to by a different role - i.e., it could only be used by someone logging into the system specifically as that role.  It likewise complements "NOLOGIN + LOGIN".

It wouldn't directly preclude said role from itself SET ROLE'ing to a different role though.​

And, for all that, I do realize I'm using these terms somewhat contrary to reality since INHERIT is done on the receiver and not the giver.  The wording would have to change to reflect the POV of the giver.  That is, INHERITONLY should be something done to a role that prevents it from being SET TO as opposed to NOINHERIT which forces a role to use SET TO to access the permissions of all roles in which it is a member.

​David J.

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: EXPLAIN VERBOSE with parallel Aggregate
Next
From: Robert Haas
Date:
Subject: Re: Odd system-column handling in postgres_fdw join pushdown patch