Re: CREATEROLE and role ownership hierarchies

Hi,

Am Montag, dem 31.01.2022 um 09:18 -0800 schrieb Mark Dilger:
> On Jan 31, 2022, at 12:43 AM, Michael Banck <
> michael(dot)banck(at)credativ(dot)de> wrote:

> Ok, sure. I think this topic is hugely important and as I read the
> patch anyway, I added some comments, but yeah, we need to figure out
> the fundamentals first.

Right.

Perhaps some background on this patch series will help.  
[...]

Thanks a lot!

If we don't go with backwards compatibility, then CREATEROLE would only
allow you to create a new role, but not to give that role LOGIN, nor
CREATEDB, etc.  You'd need to also have admin option on those things. 
To create a role that can give those things away, you'd need to run
something like:

CREATE ROLE michael
        CREATEROLE WITH ADMIN OPTION    -- can further give away
"createrole"
        CREATEDB WITH ADMIN OPTION    -- can further give away
"createdb"
        LOGIN WITH ADMIN OPTION    -- can further give away "login"
        NOREPLICATION WITHOUT ADMIN OPTION    -- this would be implied
anyway
        NOBYPASSRLS WITHOUT ADMIN OPTION    -- this would be implied anyway

        CONNECTION LIMIT WITH ADMIN OPTION    -- can specify connection
limits
        PASSWORD WITH ADMIN OPTION    -- can specify passwords
        VALID UNTIL WITH ADMIN OPTION    -- can specify expiration

Those last three don't work for me:

postgres=# CREATE ROLE admin3 VALID UNTIL WITH ADMIN OPTION;
ERROR: syntax error at or near "WITH"

postgres=# CREATE ROLE admin3 PASSWORD WITH ADMIN OPTION;
ERROR: syntax error at or near "WITH"

postgres=# CREATE ROLE admin3 CONNECTION LIMIT WITH ADMIN OPTION;
ERROR: syntax error at or near "WITH"

> (I'm on the fence about the phrase "WITH ADMIN OPTION" vs. the phrase
> "WITH GRANT OPTION".)
>
> Even then, when "michael" creates new roles, if he wants to be able
> to further administer those roles, he needs to remember to give
> himself ADMIN membership in that role at creation time.  After the
> role is created, if he doesn't have ADMIN, he can't give it to
> himself.  So, at create time, he needs to remember to do this:
>
> SET ROLE michael;
> CREATE ROLE mark ADMIN michael;

What would happen if ADMIN was implicit if michael is a non-superuser
and there's no ADMIN in the CREATE ROLE statement? It would be
backwards-compatible, one could still let somebody else be ADMIN, but
ISTM a CREATEROLE role could no longer admin a role already existing
previously/it didn't create/got assigned admin for (e.g. the predefined
roles).

I.e. (responding what you wrote much further below), the CREATEROLE
role would no longer be ADMIN for all roles, just automatically for the
ones it created.

> But that's still a bit strange, because "ADMIN michael" means that
> michael can grant other roles membership in "mark", not that michael
> can, for example, change mark's password.

Yeah, changing a password is one of the important tasks of a delegated
role admin, if no superusers are around.

> If we don't want CREATEROLE to imply that you can mess around with
> arbitrary roles (rather than only roles that you created or have been
> transferred control over) then we need the concept of role
> ownership.  This patch doesn't go that far, so for now, only
> superusers can do those things.  Assuming some form of this patch is
> acceptable, the v9 series will resurrect some of the pre-v7 logic for
> role ownership and say that the owner can do those things.

> > Ok, so what I would have needed to do in the above in order to have
> > "admin2" and "admin" be the same as far as creating login users is (I
> > believe):
> >
> > ALTER ROLE admin2 CREATEROLE LOGIN WITH ADMIN OPTION;
>
> Yes, those it's more likely admin2 would have been created with these
> privileges to begin with, if the creator intended admin2 to do such
> things.

Right, maybe people just have to adjust to the new way. It still feels
strange that whatever you do at role creation time is more meaningful
than when altering a role.

>
> > By the way, is there now even a way to add admpassword to a role
> > after it got created?
> >
> > postgres=# SET ROLE admin2;
> > SET
> > postgres=> \password test
> > Enter new password for user "test":
> > Enter it again:
> > ERROR:  must have admin on password to change password attribute
> > postgres=> RESET ROLE;
> > RESET
> > postgres=# ALTER ROLE admin2 PASSWORD WITH ADMIN OPTION;
> > ERROR:  syntax error at or near "WITH"
> > UPDATE pg_authid SET admpassword = 't' WHERE rolname = 'admin2';
> > UPDATE 1
> > postgres=# SET ROLE admin2;
> > SET
> > postgres=> \password test
> > Enter new password for user "test":
> > Enter it again:
> > postgres=>
>
> I don't really have this worked out yet.  That's mostly because I'm
> planning to fix it with role ownership, but perhaps there is a better
> way?

Well see above, maybe the patch is just broken/unfinished with respect
to PASSWORD and the others? It works for REPLICATION e.g.:

postgres=# ALTER ROLE admin2 REPLICATION WITH ADMIN OPTION;
ALTER ROLE

> > However, the next thing is:
> >
> > postgres=# SET ROLE admin;
> > SET
> > postgres=> CREATE GROUP testgroup;
> > CREATE ROLE
> > postgres=> GRANT testgroup TO test;
> > ERROR:  must have admin option on role "testgroup"
> >
> > First off, what does "admin option" mean on a role?
>
> From the docs for "CREATE ROLE",
> https://www.postgresql.org/docs/14/sql-createrole.html
>
>   The ADMIN clause is like ROLE, but the named roles are added to the
> new role WITH ADMIN OPTION, giving them the right to grant membership
> in this role to others.

Hrm, I see; I guess I never paid attention to that part so far. The
CREATEROLE thing or SUPERUSER was all I ever needed so far.

And with that I guess I should really bow out of this thread and start
reading from the beginning.

> > I then tried this:
> >
> > postgres=# CREATE USER admin3 CREATEROLE WITH ADMIN OPTION;
> > CREATE ROLE
> > postgres=# SET ROLE admin3;
> > SET
> > postgres=> CREATE USER test3;
> > CREATE ROLE
> > postgres=> CREATE GROUP testgroup3;
> > CREATE ROLE
> > postgres=> GRANT testgroup3 TO test3;
> > ERROR:  must have admin option on role "testgroup3"
> >
> > So I created both user and group, I have the CREATEROLE priv (with or
> > without admin option), but I still can't assign the group. Is that
> > (tracking who created a role and letting the creator do more thing) the
> > part that got chopped away in your last patch in order to find a common
> > ground?
>
> You need ADMIN on the role, not on CREATEROLE.  To add members to a
> target role, you must have ADMIN on that target role.  To create new
> roles with CREATEROLE privilege, you must have ADMIN on the
> CREATEROLE privilege.

Right ok. Maybe it's just me, but I feel a lot of people will need to
learn a lot more than they'd like to know about the ADMIN thing after
this patch goes in.

>
> > I
> > feel this (next to creating users/groups) is the primary thing those
> > CREATEROLE admins are supposed to do/where doing up to now.
>
> Right.  In the past, having CREATEROLE implied having ADMIN on every
> role.  I'm intentionally breaking that.

Right; I commented on that above.

> > The way the adm* privs are now somewhere in the middle of the rol*
> > privs also looks weird for the end-user and there does not seems to be
> > some greater scheme behind it:
>
> Because they are not variable length nor nullable, they must come
> before such fields (namely, rolpassword and rolvaliduntil).  They
> don't really need to come before rolconnlimit, but I liked the idea
> of packing twelve booleans together, since with "bool" typedef'd to
> unsigned char, that's twelve contiguous bytes, starting after oid (4
> bytes) and rolname (64 bytes) and likely fitting nicely without
> padding bytes on at least some platforms.  If I split them on either
> side of rolconnlimit (which is 4 bytes), there'd be seven bools
> before it and five bools after, which wouldn't pack nicely.

Hrm ok, but it's a user-visible column ordering, so I'm wondering
whether that should trump efficiency here.

Michael

--
Michael Banck
Teamleiter PostgreSQL-Team
Projektleiter
Tel.: +49 2166 9901-171
E-Mail: michael(dot)banck(at)credativ(dot)de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Geoff Richardson, Peter Lilley

Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz