Re: CREATEROLE and role ownership hierarchies

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Joshua Brindle <joshua(dot)brindle(at)crunchydata(dot)com>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, 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-02-17 17:40:09
Message-ID: CA+Tgmob_rJtHdjMOqiOcGtk0vU6XXjOqQ0LSvVpUJjvdY341rw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jan 31, 2022 at 1:57 PM Joshua Brindle
<joshua(dot)brindle(at)crunchydata(dot)com> wrote:
> 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.

So, where do we go from here?

I've been thinking about this comment a bit. On the one hand, I have
some reservations about the phrase "the use case I am trying to
accomplish with this patchset," because in the end, this is not your
patch set. It's not reasonable to complain that a patch someone else
wrote doesn't solve your problem; of course everyone writes patches to
solve their own problems, or those of their employer, not other
people's problems. And that's as it should be, else we will have few
contributors. On the other hand, to the extent that this patch set
makes things worse for a reasonable use case which you have in mind,
that's an entirely legitimate complaint.

After a bit of testing, it seems to me that as things stand today,
things are nearly perfect for the use case that you have in mind. I
would be interested to know whether you agree. If I set up an account
and give it CREATEROLE, it can create users, and it can put them into
the employees role, but it can't SET ROLE to any of those accounts. It
can also ALTER ROLE ... NOLOGIN on any of those accounts. The only gap
I see is that there are certain role-based flags which the CREATEROLE
account cannot set: SUPERUSER, BYPASSRLS, REPLICATION. You might
prefer a system where your bot account had the option to grant those
privileges also, and I think that's a reasonable thing to want.

However, I *also* think it's reasonable to want an account that can
create roles but can't give to those roles membership in roles that it
does not itself possess. Likewise, I think it's reasonable to want an
account that can only drop roles which that account itself created.
These kinds of requirements stem from a different use case than what
you are talking about here, but they seem like fine things to want,
and as far as I know we have pretty broad agreement that they are
reasonable. It seems extremely difficult to make a convincing argument
that this is not a thing which anyone should want to block:

rhaas=> create role bob role pg_execute_server_program;
CREATE ROLE

Honestly, that seems like a big yikes from here. How is it OK to block
"create role bob superuser" yet allow that command? I'm inclined to
think that's just broken. Even if the role were pg_write_all_data
rather than pg_execute_server_program, it's still a heck of a lot of
power to be handing out, and I don't see how anyone could make a
serious argument that we shouldn't have an option to restrict that.

Let me separate the two features that I just mentioned and talk about
them individually:

1. Don't allow a CREATEROLE user to give out membership in groups
which that user does not possess. Leaving aside the details of any
previously-proposed patches and just speaking theoretically, how can
this be implemented? I can think of a few ideas. We could (1A) just
change CREATEROLE to work that way, but IIUC that would break the use
case you outline here, so I guess that's off the table unless I am
misunderstanding the situation. We could also (1B) add a second role
attribute with a different name, like, err, CREATEROLEWEAKLY, that
behaves in that way, leaving the existing one untouched. But we could
also take it a lot further, because someone might want to let an
account hand out a set of privileges which corresponds neither to the
privileges of that account nor to the full set of available
privileges. That leads to another idea: (1C) implement an in-database
system that lets you specify which privileges an account has, and,
separately, which ones it can assign to others. I am skeptical of that
idea because it seems really, really complicated, not only from an
implementation standpoint but even just from a user-experience
standpoint. Suppose user 'userbot' has rights to grant a suitable set
of groups to the new users that it creates -- but then someone creates
a new group. Should that also be added to the things 'userbot' can
grant or not? What if we have 'userbot1' through 'userbot6' and each
of them can grant a different set of roles? I wouldn't mind (1D)
providing a hook that allows the system administrator to install a
loadable module that can enforce any rules it likes, but it seems way
too complicated to me to expose all of this configuration as SQL,
especially because for what I want to do, either (1A) or (1B) is
adequate, and (1B) is a LOT simpler than (1C). It also caters to what
I believe to be a common thing to want, without prejudice to the
possibility that other people want other things.

Joshua, what is your opinion on this point?

2. Only allow a CREATEROLE user to drop users which that account
created, and not just any role that isn't a superuser. Again leaving
aside previous proposals, this cannot be implemented without providing
some means by which we know which CREATEROLE user created which other
user. I believe there are a variety of words we could use to describe
that linkage, and I don't deeply care which ones we pick, although I
have my own preferences. We could speak of the CREATEROLE user being
the owner, manager, or administrator of the created role. We could
speak of a new kind of object, a TENANT, of which the CREATEROLE user
is the administrator and to which the created user is linked. I
proposed this previously and it's still my favorite idea. There are no
doubt other options as well. But it's axiomatic that we cannot
restrict the rights of a CREATEROLE user to drop other roles to a
subset of roles without having some way to define which subset is at
issue.

Now, my motivation for wanting this feature is pretty simple: I want
to have something that feels like a superuser but isn't a full
superuser, and can't interfere with accounts set up by the service
provider, but can do whatever they want to the other ones. But I think
this is potentially useful in the userbot case that you (Joshua)
mention as well, because it seems like it could be pretty desirable to
have a certain list of users which the userbot can't remove, just for
safety, either to limit the damage if somebody gets into that account,
or just to keep the bot from going nuts and doing something it
shouldn't in the event of a programming error. Now, if you DON'T care
about the userbot being able to access this functionality, that's fine
with me, because then there's nothing left to do but argue about what
to call the linkage between the CREATEROLE user and the created user.
Your userbot need not participate in whatever system we decide on, and
things are no worse for that use case than they are today.

But if you DO want the userbot to be able to access that
functionality, then things are more complicated, because now the
linkage has to be special-purpose. In that scenario, we can't say that
the right of a CREATEROLE user to drop a certain other role implies
having the privileges of that other role, because in your use case,
you don't want that, whereas in mine, I do. What makes this
particularly ugly is that we can't, as things currently stand, use a
role as the grouping mechanism, because of the fact that a role can
revoke membership in itself from some other role. It will not do for
roles to remove themselves from the set of roles that the CREATEROLE
user can drop. If we changed that behavior, then perhaps we could just
define a way to say that role X can drop roles if they are members of
group G. In my tenant scenario, G would be granted to X, and in your
userbot scenario, it wouldn't. Everybody wins, except for any people
who like the ability of roles to revoke themselves from any group
whatsoever.

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?

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.

Thoughts?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sadhuprasad Patro 2022-02-17 17:55:25 Re: Per-table storage parameters for TableAM/IndexAM extensions
Previous Message Jacob Champion 2022-02-17 17:29:15 Re: [PATCH] Accept IP addresses in server certificate SANs