Re: CREATEROLE and role ownership hierarchies

Greetings,

* Mark Dilger (mark.dilger@enterprisedb.com) wrote:
> > On Jan 25, 2022, at 12:44 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > I agree that CREATEROLE is overpowered and that the goal of this should
> > be to provide a way for roles to be created and dropped that doesn't
> > give the user who has that power everything that CREATEROLE currently
> > does.
>
> I'm attaching a patch that attempts to fix CREATEROLE without any connection to role ownership.

Alright.

> >  The point I was making is that the concept of role ownership
> > isn't intrinsically linked to that and is, therefore, as you say, gravy.
>
> I agree, they aren't intrinsically linked, though the solution to one might interact in some ways with the solution
tothe other. 

Sure.

> > That isn't to say that I'm entirely against the role ownership idea but
> > I'd want it to be focused on the goal of providing ways of creating and
> > dropping users and otherwise performing that kind of administration and
> > that doesn't require the specific change to make owners be members of
> > all roles they own and automatically have all privileges of those roles
> > all the time.
>
> The attached WIP patch attempts to solve most of the CREATEROLE problems but not the problem of which role who can
dropwhich other role.  That will likely require an ownership concept. 

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.

> 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.  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.

> -- 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.

> Whether "WITH ADMIN OPTION" or "WITHOUT ADMIN OPTION" is implied hinges on whether the role is given CREATEROLE.
Thathackery is necessary to preserve backwards compatibility.  If we don't care about compatibility, I could change the
patchto 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.

Thanks,

Stephen