Re: CREATEROLE and role ownership hierarchies

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Shinya Kato <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com>
Subject: Re: CREATEROLE and role ownership hierarchies
Date: 2022-03-01 20:03:48
Message-ID: CA+TgmoZ-0QOZa4=fzXjUuhrTFeyuH_ZXZGurGTtWF_EZeC-5pw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 28, 2022 at 2:09 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> I'm generally in support of changing CREATEROLE to only allow roles that
> the role with CREATEROLE is an admin of to be allowed as part of the
> command (throwing an error in other cases). That doesn't solve other
> use-cases which would certainly be nice to solve but it would at least
> reduce the shock people have when they discover how CREATEROLE actually
> works (that is, the way we document it to work, but that's ultimately
> not what people expect).

So I'm 100% good with that because it does exactly what I want, but my
understanding of the situation is that it breaks the userbot case that
Joshua is talking about. Right now, with stock PostgreSQL in any
released version, his userbot can have CREATEROLE and give out roles
that it doesn't itself possess. If we restrict CREATEROLE in the way
described here, that's no longer possible.

Now it's possible that you and/or he would take the position that
we're still coming out ahead despite that functional regression,
because as I now understand after reading Joshua's latest email, he
doesn't want the userbot to be able to grant ANY role, just the
'employees' role - and today he can't get that. So in a modified
universe where we restrict the privileges of CREATEROLE, then on the
one hand he GAINS the ability to have a userbot that can grant some
roles but not others, but on the other hand, he's forced to give the
userbot the roles he wants it to be able to hand out. Is that better
overall or worse?

To really give him EXACTLY what he wants, we need a way of specifying
administration without membership. See my last reply to the thread for
my concerns about that.

> I don't think it's a great plan to limit who is allowed to DROP roles to
> be just those that a given role created. I also don't like the idea of
> introducing a single field for owner/manager/tenant/whatever to the role
> system- instead we should add other ways that roles can be associated to
> each other by extending the existing system that we have for that, which
> is role membership. Role membership today is pretty limited but I don't
> see any reason why we couldn't improve on that in a way that's flexible
> and allows us to define new associations in the future. The biggest
> difference between a 'tenant' or such as proposed vs. a role association
> is in where the information is tracked and what exactly it means.
> Saying "I want a owner" or such is easy because it's basically punting
> on the complciated bit of asking the question: what does that *mean*
> when it comes to what rights that includes vs. doesn't? What if I only
> want some of those rights to be given away but not all of them? We have
> that system for tables/schemas/etc, and it hasn't been great as we've
> seen through the various requests to add things like GRANT TRUNCATE.

Well, there's no accounting for taste, but I guess I see this pretty
much opposite to the way you do. I think GRANT TRUNCATE is nice and
simple and clear. It does one thing and it's easy to understand what
that thing is, and it has very few surprising or undocumented side
effects. On the other hand, role membership is a mess, and it's not at
all clear how to sort that mess out. I guess I agree with you that it
would be nice if it could be done, but the list of problems is pretty
substantial. Like, membership implies the right to SET ROLE, and also
the right to implicitly exercise the privileges of the role, and
you've complained about that fuzziness. And ADMIN OPTION implies
membership, and you don't like that either. And elsewhere it's been
raised that nobody would expect to have a table end up owned by
'pg_execute_server_programs', or a user logged in directly as
'employees' rather than as some particular employee, but all that
stuff can happen, and some of it can't even be effectively prevented
with good configuration. 'toe' can be a member of 'foot' while, which
makes sense to everybody, and at the same time, 'foot' can be a member
of 'toe', which doesn't make any sense at all. And because both
directions are possible even experienced PostgreSQL users and hackers
get confused, as demonstrated by Joshua's having just got the
revoke-from-role case backwards.

Of those four problems, the last two are clearly the result of
conflating users with groups - and really also with capabilities - and
having a unified role concept that encompasses all of those things. I
think we would be better off if we had not done that, both in the
sense that I think the system would be less confusing to understand,
and also in the sense that we would likely have fewer security bugs.
And similarly I agree with you that it would be better if the right to
administer a role were clearly separated from membership in a role,
and if the right to use the privileges of a role were separated from
the ability to SET ROLE to it. However, unlike you, I see the whole
'role membership' concept as the problem, not the solution. We
conflate a bunch of different kinds of things together and call them
all 'roles' and a bunch of other things together and call them
'membership' and then we end up with an awkward mess. That's how I see
it, anyway.

> The ability of a role to revoke itself from some other role is just
> something we need to accept as being a change that needs to be made, and
> I do believe that such a change is supported by the standard, in that a
> REVOKE will only work if you have the right to make it as the user who
> performed the GRANT in the first place.

Great. I propose that we sever that issue and discuss it on a new
thread to avoid confusion. I believe there is some debate to be had
about exactly what we want the behavior to be in this area, but if we
can reach consensus on that point, this shouldn't be too hard to knock
out. I will take it as an action item to get that thread going, if
that works for you.

> > So that leads to these questions: (2A) Do you care about restricting
> > which roles the userbot can drop? (2B) If yes, do you endorse
> > restricting the ability of roles to revoke themselves from other
> > roles?
>
> As with Joshua, and as hopefully came across from the above discussion,
> I'm also a 'yes and yes' on these two.

Great.

> > I think that we don't have any great problems here, at least as far as
> > this very specific issue is concerned, if either the answer to (2A) is
> > no or the answer to (2B) is yes. However, if the answer to (2A) is yes
> > and the answer to (2B) is no, there are difficulties. Evidently in
> > that case we need some new kind of thing that behaves mostly likes a
> > group of roles but isn't actually a group of roles -- and that thing
> > needs to prohibit self-revocation. Given what I've written above, you
> > may be able to guess my preferred solution: let's call it a TENANT.
> > Then, my pseudo-super-user can have permission to (i) create roles in
> > that tenant, (ii) drop roles in that tenant, and (iii) assume the
> > privileges of roles in that tenant -- and your userbot can have
> > privileges to do (i) and (ii) but not (iii). All we need do is add a
> > roltenant column to pg_authid and find three bits someplace
> > corresponding to (i)-(iii), and we are home.
>
> Where are those bits going to go though..? I don't think they should go
> into pg_authid, nor do I feel that this 'tenant' or such should go there
> either because pg_authid is about describing individual roles, not about
> role associations. Instead, I'd suggest that those bits go into
> pg_auth_members in the form of additional columns to describe the role
> associations. That is, instead of the existance of a row in
> pg_auth_members meaning that one role has membership in another role, we
> give users the choice of if that's the case or not with a separate
> column. That would then neatly give us a way for a role to have admin
> rights over another role but not membership in that role. We could then
> further extend this by adding other columns to pg_auth_members for other
> rights as users decide they need them- such as the ability for a role to
> DROP some set of roles.

What I had in mind is to add a pg_tenant catalog (tenid, tenname) and
add some columns to the pg_authid catalog (roltenant, roltenantrights,
or something like that). See above for why I am not excited about
piggybacking more things onto role membership.

> => revoke joshua from admin;
> REVOKE ROLE
>
> =*> \du
> List of roles
> Role name | Attributes | Member of
> -----------+------------------------------------------------------------+-------------
> admin | Cannot login | {}
> employees | Cannot login | {}
> joshua | | {employees}
> sfrost | Superuser, Create role, Create DB, Replication, Bypass RLS | {}
>
> Even though, in this case, it was 'sfrost' (a superuser) who GRANT'd
> joshua to admin.

Quite so.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2022-03-01 20:05:23 Re: Postgres restart in the middle of exclusive backup and the presence of backup_label file
Previous Message Greg Stark 2022-03-01 20:01:20 Re: Commitfest 2022-03 Patch Triage Part 1a.i