Re: SET ROLE and reserved roles

On Wed, Apr 13, 2016 at 3:53 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> > Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > > On Wednesday, April 13, 2016, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > >> If you want to prevent that, I think it needs to be done somewhere
> else
> > >> than here. What about "ALTER OWNER TO pg_signal_backend", for
> instance?
> >
> > > Checks are included in that code path to disallow it.
> >
> > If there are such low-level checks, why do we need to disallow SET ROLE?
> > It seems like the wrong place to be inserting such defenses.
>
> The low-level checks were placed all along the paths which used
> get_rolespace_oid/tuple. SET ROLE is the only place where a user could
> change to another role and then do things which result in objects being
> owned by that role without going through one of those paths.
>
> 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.
>
> > >> But perhaps more to the point, why is it a strange corner case for one
> > >> of these roles to own objects?
> >
> > > If the default roles can own objects and otherwise be used for other
> > > purposes then we can never, ever, be rid of any which we create.
> >
> > This argument seems quite bogus to me, because granted privileges are
> > pretty darn close to being objects. Certainly, if you have some
> > "GRANT pg_signal_backend TO joe_user" commands in a pg_dump script,
> > those will fail for lack of the role just as surely as ALTER OWNER
> > commands would. So we would need some kind of special hack in pg_dump
> > either way, unless we make it incumbent on users to clean up before
> > upgrading (which doesn't seem out of the question to me ...)
>
> I don't think we would have any concerns with a hack in pg_dump to
> remove those GRANTs if that default role goes away. There's certainly
> no way we're going to do that for tables which have data in them,
> however. Therefore, the user would have to address any such cases
> before being able to an upgrade, and I don't see why we want to allow
> such a case.
>
> As for present-vs-future cruft, we can't put this genie back in the
> bottle once we allow a default role to own objects, which is why I felt
> it was prudent to address that concern from the get-go.
>
> > I think you're replacing hypothetical future cruft with actual present
> > cruft, and it doesn't seem like a very good tradeoff.
>
> If we don't care about default roles being able to own objects, then we
> can remove the check in SET ROLE and simply accept that we can never
> remove those roles without user intervention. There's a number of other
> checks in the tree which we can debate (should a default role be allowed
> to have a USER MAPPING? What about additional privileges beyond those
> we define for it? Perhaps DEFAULT privileges?). If we're going to
> allow default roles to own objects, it seems like we could just about
> remove all those other checks also.
>
> If we don't want default roles to be able to own objects, but we do want
> users to be able to SET ROLE to them, then there's a whole slew of
> *additional* checks which have to be added, which is surely a
> net-increase in code.
>
> I'm happy to do my best to implement the concensus opinion, just wanted
> to be clear on what the options are. If I could get responses regarding
> everyone's preferences on the above, then we can get to what the
> desired behavior should be and I'll implement it.
>

From what I've read here I'm thinking Stephen has the right idea.

Lets be conservative in what we allow with these new roles​ and let
experience guide us as to whether we need to open things up more - or just
fix oversights.

We have chosen to give up "defense in depth" here since if by some other
means the value of current_user gets set to one of these roles having the
sole protection point at SET ROLE will seem like a bad choice. Aside from
that it will allow for fewer changes for code that chooses to rely on that
assertion instead of ensuring that it is true.

If we are wrong the obvious work-around is that all roles that would be
granted a given pg_* role would instead be granted a normal role which
itself would be granted the pg_* role. Any of those roles would be then be
able emulate SET ROLE pg_* by instead switching to the normal role.

I don't think we'd be inclined to make these login roles so most external
tools are likely going to simply rely on the fact that whatever user they
are told to login with already has the necessary grants setup.

Stephen's entire response is enough for me to want to require an
justification for why "INHERIT ONLY" (e.g., the third logical result after
NOINHERIT & INHERIT(+SET); the fourth being a role that neither inherits
nor can be set to - which would be pointless) should not be an acceptable
combination of ROLE attributes.

Arguing that the bootstrap user has some combination of properties doesn't
seem like a compelling line of argument. The whole point of this exercise
is to institute more fine-grained authorizations and to provide built-in
roles to access those built-in features. The bootstrap user still owns all
of the system objects but delegates access and usage of them to these newly
created roles. There isn't an internal need for the internal users to own
things - we have the bootstrap user - and while its quite possible other
people might devise a reason to utilize these roles in that way it doesn't
seem like a necessary thing but rather likely done out of convenience.
Call it parental involvement if you'd like but why allow situations to
arise where these system roles are doing anything more than providing
access to existing system objects thus increasign the number of unknowable
dependencies that these system roles have when doing things like pg_upgrade?

I have not seen a strong argument for this need and while we might want to
tighten up security by adding checks having the internals assume that the
pg_* roles are never "current_role" should result in a more secure system
and less code.

David J.