Re: SET ROLE and reserved roles - Mailing list pgsql-hackers
| From | David G. Johnston |
|---|---|
| Subject | Re: SET ROLE and reserved roles |
| Date | |
| Msg-id | CAKFQuwZZQXStVn6cw-iB=VBpDv=azM+JjtevZDv4=8L2zdyC-A@mail.gmail.com Whole thread Raw |
| In response to | Re: SET ROLE and reserved roles (Stephen Frost <sfrost@snowman.net>) |
| Responses |
Re: SET ROLE and reserved roles
(Stephen Frost <sfrost@snowman.net>)
|
| List | pgsql-hackers |
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > On Wednesday, April 13, 2016, Tom Lane <tgl@sss.pgh.pa.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.
pgsql-hackers by date: