On Mon, Jan 31, 2022 at 1:50 PM Stephen Frost <sfr...@snowman.net> wrote: > > Greetings, > > * Mark Dilger (mark.dil...@enterprisedb.com) wrote: > > > On Jan 31, 2022, at 8:53 AM, Stephen Frost <sfr...@snowman.net> wrote: > > > Yeah, we do need to have a way to determine who is allowed to drop > > > roles and role ownership seems like it's one possible approach to that. > > > > Which other ways are on the table? Having ADMIN on a role doesn't allow > > you to do that, but maybe it could? What else? > > Supporting that through ADMIN is one option, another would be a > 'DROPROLE' attribute, though we'd want a way to curtail that from being > able to be used for just any role and that does lead down a path similar > to ownership or just generally the concept that some roles have certain > rights over certain other roles (whether you create them or not...). > > I do think there's a lot of value in being able to segregate certain > rights- consider that you may want a role that's able to create other > roles, perhaps grant them into some set of roles, can lock those roles > (prevent them from logging in, maybe do a password reset, something like > that), but which *isn't* able to drop those roles (and all their > objects) as that's dangerous and mistakes can certainly happen, or be > able to become that role because the creating role simply doesn't have > any need to be able to do that (or desire to in many cases, as we > discussed in the landlord-vs-tenant sub-thread). >
This is precisely the use case I am trying to accomplish with this patchset, roughly: - An automated bot that creates users and adds them to the employees role - Bot cannot access any employee (or other roles) table data - Bot cannot become any employee - Bot can disable the login of any employee Yes there are attack surfaces around the fringes of login, etc but those can be mitigated with certificate authentication. My pg_hba would require any role in the employees role to use cert auth. This would adequately mitigate many threats while greatly enhancing user management. > Naturally, you'd want *some* role to be able to drop that role (and one > that doesn't have full superuser access) but that might be a role that's > not able to create new roles or take over accounts. I suspect some kind of web backend to handle manual user pruning. I don't expect Bot to automatically drop users because mistakes can happen, and disabling the login ability seems like an adequate tradeoff. > Separation of concerns and powers and all of that is what we want to be > going for here, more generically, which is why I was opposed to the > blanket "owners have all rights of all roles they own" implementation. > That approach doesn't support the ability to have a relatively > unprivileged role that's able to create other roles, which seems like a > pretty important use-case for us to be considering. Agreed. > The terminology seems to also be driving us in a certain direction and I > don't know that it's necessarily a good one. That is- the term 'owner' > implies certain things and maybe that's where some of the objection to > my argument that owners shouldn't necessarily have all rights of the > roles they 'own' comes from (ok- I'll also put out there for general > consideration that since we're talking about roles, and login roles are > generally associated with people, that maybe 'owner' isn't a great term > to use for this anyway ...). I feel like the 'owner' concept came from > the way we have table owners and function owners and database owners > today rather than from a starting point of what do we wish to > specifically enable. > > Perhaps instead of starting from the 'owner' concept, we start from the > question about the kinds of things we want roles to be able to do and > perhaps that will help inform the terminology. > > - Create new roles > - Drop an existing role > - Drop objects which belong to a role > - Lock existing roles > - Change/reset the PW of existing roles > - Give roles to other roles > - Revoke access to some roles from other roles > - Give select role attributes to a role > - Revoke role attributes from a role > - Traditional role-based access control (group memberships, SET ROLE) > > Certain of the above are already covered by the existing role membership > system and with the admin option, though there's definitely an argument > to be made as to if that is as capable as we'd like it to be (there's no > way to, today at least, GRANT *just* the admin option, for example, and > maybe that's something that it would actually be sensible to support). > > Perhaps there is a need to have a user who has all of the above > capabilities and maybe that would be an 'owner' or 'manager', but as I > tried to illustrate above, there's definitely use-cases for giving > a role only some of the above capabilities rather than all of them > together at once. > > > >> The main idea here is that having CREATEROLE doesn't give you ADMIN on > > >> roles, nor on role attributes. For role attributes, the syntax has been > > >> extended. An excerpt from the patch's regression test illustrates some > > >> of that concept: > > >> > > >> -- ok, superuser can create a role that can create login replication > > >> users, but > > >> -- cannot itself login, nor perform replication > > >> CREATE ROLE regress_role_repladmin > > >> CREATEROLE WITHOUT ADMIN OPTION -- can create roles, but cannot > > >> give it away > > >> NOCREATEDB WITHOUT ADMIN OPTION -- cannot create db, nor give it > > >> away > > >> NOLOGIN WITH ADMIN OPTION -- cannot log in, but can give it > > >> away > > >> NOREPLICATION WITH ADMIN OPTION -- cannot replicate, but can give > > >> it away > > >> NOBYPASSRLS WITHOUT ADMIN OPTION; -- cannot bypassrls, nor give it > > >> away > > >> > > >> -- ok, superuser can create a role with CREATEROLE but restrict > > >> give-aways > > >> CREATE ROLE regress_role_minoradmin > > >> NOSUPERUSER -- WITHOUT ADMIN OPTION is implied > > >> CREATEROLE WITHOUT ADMIN OPTION > > >> NOCREATEDB WITHOUT ADMIN OPTION > > >> NOLOGIN WITHOUT ADMIN OPTION > > >> NOREPLICATION -- WITHOUT ADMIN OPTION is implied > > >> NOBYPASSRLS -- WITHOUT ADMIN OPTION is implied > > >> NOINHERIT WITHOUT ADMIN OPTION > > >> CONNECTION LIMIT NONE WITHOUT ADMIN OPTION > > >> VALID ALWAYS WITHOUT ADMIN OPTION > > >> PASSWORD NULL WITHOUT ADMIN OPTION; > > > > > > Right, this was one of the approaches that I was thinking could work for > > > managing role attributes and it's very similar to roles and the admin > > > option for them. As I suggested at least once, another possible > > > approach could be to have login users not be able to create roles but > > > for them to be able to SET ROLE to a role which is able to create roles, > > > and then, using your prior method, only allow the attributes which that > > > role has to be able to be given to other roles. > > > > I'm not sure how that works. If I have a group named "administrators" > > which as multiple attributes like BYPASSRLS and such, and user "stephen" is > > a member of "administrators", then stephen can not only give away bypassrls > > to new users but also has it himself. How is that an improvement? (I mean > > this as a question, not as criticism.) > > That's not how role attributes work though- "stephen" only has the > 'bypassrls' role attribute after a 'set role administrators'. This has > been one of the issues with role attributes in general as there's no way > to change that (unlike the 'inherit' option for roles themselves) but in > this particular case it might be to our advantage. > > > > That essentially makes > > > a role be a proxy for the per-attribute admin options. There's pros and > > > cons for each approach and so I'm curious as to which you feel is the > > > better approach? I get the feeling that you're more inclined to go with > > > the approach of having an admin option for each role attribute (having > > > written this WIP patch) but I'm not sure if that is because you > > > contempltaed both and felt this was better for some reason or more > > > because I wasn't explaining the other approach very well, or if there > > > was some other reason. > > > > I need more explanation of the other option you are contemplating. My > > apologies if I'm being thick-headed. > > Hopefully the above helps. Note that in order to not allow the > "stephen" role simply alter itself to have the bypassrls role attribute, > we'd need to consider roles to not have 'ownership' (or whatever) over > themselves, which leads into the prior complaint I made around roles > having 'admin' rights on themselves which I generally don't feel is > correct either. > > > >> -- fail, having CREATEROLE is not enough to create roles in privileged > > >> roles > > >> SET SESSION AUTHORIZATION regress_role_minoradmin; > > >> CREATE ROLE regress_nosuch_read_all_data IN ROLE pg_read_all_data; > > >> ERROR: must have admin option on role "pg_read_all_data" > > > > > > I would say not just privileged roles, but any roles that the user > > > doesn't have admin rights on. > > > > Yes, that's how it works. But this portion of the test is only checking > > the interaction between CREATEROLE and built-in privileged roles, hence the > > comment. > > But.. predefined roles aren't actually different in this regard from any > other role, so I disagree that such a test of explicitly predefined > roles makes sense..? > > > >> Whether "WITH ADMIN OPTION" or "WITHOUT ADMIN OPTION" is implied hinges > > >> on whether the role is given CREATEROLE. That hackery is necessary to > > >> preserve backwards compatibility. If we don't care about compatibility, > > >> I could change the patch to make "WITHOUT ADMIN OPTION" implied for all > > >> attributes when not specified. > > > > > > Given the relative size of the changes we're talking about regarding > > > CREATEROLE, I don't really think we need to stress about backwards > > > compatibility too much. > > > > Yeah, I'm leaning pretty strongly that way, too. > > Great. > > Thanks, > > Stephen
