Re: role self-revocation - Mailing list pgsql-hackers

On Thu, Mar 10, 2022 at 2:58 PM Stephen Frost <sfrost@snowman.net> wrote:
> It'd be useful to have a better definition of exactly what a
> 'mini-superuser' is, but at least for the moment when it comes to roles,
> let's look at what the spec says:

Gosh, I feel like I've spelled that out approximately 463,121 times
already. That estimate might be slightly off though; I've been known
to make mistakes from time to time....

> CREATE ROLE
>   - Who is allowed to run CREATE ROLE is implementation-defined
>   - After creation, this is effictively run:
>     GRANT new_role TO creator_role WITH ADMIN, GRANTOR "_SYSTEM"
>
> DROP ROLE
>   - Any user who has been GRANT'd a role with ADMIN option is able to
>     DROP that role.
>
> GRANT ROLE
>   - No cycles allowed
>   - A role must have ADMIN rights on the role to be able to GRANT it to
>     another role.
>
> ALTER ROLE
>   - Doesn't exist
>
> This actually looks to me like more-or-less what you're looking for, it
> just isn't what we have today because CREATEROLE brings along with it a
> bunch of other stuff, some of which we want and some that we don't, and
> some things that the SQL spec says ADMIN should be allowed to do (DROP
> ROLE) we don't allow today.

The above is mostly fine with me, except for the part about ALTER ROLE
not existing. I think it's always good to be able to change your mind
post-CREATE.

Basically, in this sketch, ADMIN OPTION on a role involves the ability
to DROP it, which means we don't need a separate role owner concept.
It also involves membership, meaning that you can freely exercise the
privileges of the role without SET ROLE. While I'm totally down with
having other possible behaviors as options, that particular behavior
seems very useful to me, so, sounds great.

> It's also not quite what I want because it requires that membership and
> ADMIN go together where I'd like to be able to have those be
> independently GRANT'able- and then some.
>
> I don't think we're that far from having all of these though.  To start
> with, we remove from CREATEROLE the random things that it does which go
> beyond what folks tend to expect- remove the whole 'grant any role to
> any other' stuff, remove the 'drop role' exception, remove the
> 'alter role' stuff.  Do make it so that when you create a role, however,
> the above GRANT is effectively done.  Now, for the items above where we
> removed the checks against have_createrole_privilege() we go back and
> add in checks using is_admin_of_role().  Of course, also remove the role
> self-administration bug.

What do you mean by the 'drop role' exception?

I don't like removing 'alter role'.

The rest sounds good.

> That's step #1, but it gets us more-or-less what you're looking for, I
> think, and brings us a lot closer to what the spec has.

Great.

> Step #2 is also in-line with the spec: track GRANTORs and care about
> them, for everything.  We really should have been doing this all along.
> Note that I'm not saying that an owner of a table can't REVOKE some
> right that was GRANT'd on that table, but rather that a user who was
> GRANT'd ADMIN rights on a table and then GRANT'd that right to some
> other user shouldn't have some other user who only has ADMIN rights on
> the table be able to remove that GRANT.  Same goes for roles, meaning
> that you could GRANT rights in a role with ADMIN option and not have to
> be afraid that the role you just gave that to will be able to remove
> *your* ADMIN rights on that role.  In general, I don't think this
> would actually have a very large impact on users because most users
> don't, today, use the ADMIN option much.

There are details to work out here, but in general, I like it.

> Step #3 starts going in the direction of what I'd like to see, which
> would be to break out membership in a role as a separate thing from
> admin rights on that role.  This is also what would help with the 'bot'
> use-case that Joshua (not David Steele, btw) brought up.

Woops, apologies for getting the name wrong. I also said Marc earlier
when I meant Mark, because I work with people named Mark, Marc, and
Marc, and Mark's spelling got outvoted by some distant corner of my
brain.

I think this is a fine long-term direction, with the caveat that
you've not provided enough specifics here for me to really understand
how it would work. I fear the specifics might be hard to get right,
both in terms of making it understandable to users and in terms of
preserving as much backward-compatibility as we can. However, I am not
opposed to the concept.

> Step #4 then breaks the 'admin' option on roles into pieces- a 'drop
> role' right, a 'reset password' right, maybe separate rights for
> different role attributes, etc.  We would likely still keep the
> 'admin_option' column in pg_auth_members and just check that first
> and then check the individual rights (similar to table-level vs.
> column-level privileges) so that we stay in line with the spec's
> expectation here and with what users are used to.

Same comments as #3, plus I wonder whether it really makes sense to
separate #3 and #4. But we can decide that when there's a fleshed-out
design for this.

> In some hyptothetical world, there's even a later step #5 which allows
> us to define user profiles and then grant the ability for a user to
> create a role with a certain profile (but not any arbitrary profile),
> thus making things like the 'bot' even more constrained in terms of
> what it's able to do (maybe it can then create a role that's a member of
> a role without itself being a member of that role or explicitly having
> admin rights in that role, as an example).

Right. I don't object to this either, hypothetically, but I think
we're a long way from understanding how to get there, and I don't want
step #1 to get blocked behind all the rest of this. Particularly the
part where we remove the role self-administration thing.

-- 
Robert Haas
EDB: http://www.enterprisedb.com