Обсуждение: CREATE ROLE bug?

This works in PG 15:

        CREATE ROLE service CREATEROLE;
        CREATE ROLE service1 WITH LOGIN IN ROLE service;
        SET SESSION AUTHORIZATION service;
        CREATE ROLE service2 WITH LOGIN IN ROLE service;

but generates an error in git master:

        CREATE ROLE service CREATEROLE;
        CREATE ROLE service1 WITH LOGIN IN ROLE service;
        SET SESSION AUTHORIZATION service;
        CREATE ROLE service2 WITH LOGIN IN ROLE service;
-->     ERROR:  must have admin option on role "service"

If I make 'service' a superuser, it works:

        CREATE ROLE service SUPERUSER;
        CREATE ROLE service1 WITH LOGIN IN ROLE service;
        SET SESSION AUTHORIZATION service;
        CREATE ROLE service2 WITH LOGIN IN ROLE service;

It is probably related to this discussion and change:

       https://www.postgresql.org/message-id/flat/CA+TgmobGds7oefDjZUY+k_J7p1sS=pTq3sZ060qdb=oKei1Dkw@mail.gmail.com

I am not sure if the behavior is wrong, the error message is wrong, or
it is working as expected.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

Embrace your flaws.  They make you human, rather than perfect,
which you will never be.

On Wed, Jan 25, 2023 at 8:29 AM Bruce Momjian <bruce@momjian.us> wrote:
> This works in PG 15:
>
>         CREATE ROLE service CREATEROLE;
>         CREATE ROLE service1 WITH LOGIN IN ROLE service;
>         SET SESSION AUTHORIZATION service;
>         CREATE ROLE service2 WITH LOGIN IN ROLE service;
>
> but generates an error in git master:
>
>         CREATE ROLE service CREATEROLE;
>         CREATE ROLE service1 WITH LOGIN IN ROLE service;
>         SET SESSION AUTHORIZATION service;
>         CREATE ROLE service2 WITH LOGIN IN ROLE service;
> -->     ERROR:  must have admin option on role "service"
>
> If I make 'service' a superuser, it works:
>
>         CREATE ROLE service SUPERUSER;
>         CREATE ROLE service1 WITH LOGIN IN ROLE service;
>         SET SESSION AUTHORIZATION service;
>         CREATE ROLE service2 WITH LOGIN IN ROLE service;
>
> It is probably related to this discussion and change:
>
>        https://www.postgresql.org/message-id/flat/CA+TgmobGds7oefDjZUY+k_J7p1sS=pTq3sZ060qdb=oKei1Dkw@mail.gmail.com
>
> I am not sure if the behavior is wrong, the error message is wrong, or
> it is working as expected.

It is indeed related to that discussion and change. In existing
released branches, a CREATEROLE user can make any role a member of any
other role even if they have no rights at all with respect to that
role. This means that a CREATEROLE user can create a new user in the
pg_execute_server_programs group even though they have no access to
it. That allows any CREATEROLE user to take over the OS account, and
thus also superuser. In master, the rules have been tightened up.
CREATEROLE no longer exempts you from the usual permission checks
about adding a user to a group. This means that a CREATEROLE user now
needs the same permissions to add a user to a group as any other user
would need, i.e. ADMIN OPTION on the group.

In your example, the "service" user has CREATEROLE and is therefore
entitled to create new roles. However, "service" can only add those
new roles to groups for which "service" possesses ADMIN OPTION. And
"service" does not have ADMIN OPTION on itself, because no role ever
possesses ADMIN OPTION on itself.

I wrote a blog about this yesterday, which may or may not be of help:

http://rhaas.blogspot.com/2023/01/surviving-without-superuser-coming-to.html

I think that the new behavior will surprise some people, as it has
surprised you, and it will take some time to get used to. However, I
also think that the changes are absolutely essential. We've been
shipping major releases for years and just pretending that it's OK
that having CREATEROLE lets you take over the OS account and the
superuser account. That's a security hole -- and not a subtle one. I
could have easily exploited it as a teenager. My goals in doing this
project were to (1) fix the security holes, (2) otherwise change as
little about the behavior of CREATEROLE as possible, and (3) make
CREATEROLE do something useful. We could have accomplished the first
goal by just removing CREATEROLE or making it not do anything at all,
but meeting the second and third goals at the same time require
letting CREATEROLE continue to work but putting just enough
restrictions on its power to keep it from being used as a
privilege-escalation attack. I hope that what's been committed
accomplishes that goal.

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

On Wed, Jan 25, 2023 at 08:47:14AM -0500, Robert Haas wrote:
> > I am not sure if the behavior is wrong, the error message is wrong, or
> > it is working as expected.
> 
> It is indeed related to that discussion and change. In existing
> released branches, a CREATEROLE user can make any role a member of any
> other role even if they have no rights at all with respect to that
> role. This means that a CREATEROLE user can create a new user in the
> pg_execute_server_programs group even though they have no access to
> it. That allows any CREATEROLE user to take over the OS account, and
> thus also superuser. In master, the rules have been tightened up.
> CREATEROLE no longer exempts you from the usual permission checks
> about adding a user to a group. This means that a CREATEROLE user now
> needs the same permissions to add a user to a group as any other user
> would need, i.e. ADMIN OPTION on the group.
> 
> In your example, the "service" user has CREATEROLE and is therefore
> entitled to create new roles. However, "service" can only add those
> new roles to groups for which "service" possesses ADMIN OPTION. And
> "service" does not have ADMIN OPTION on itself, because no role ever
> possesses ADMIN OPTION on itself.

So, how would someone with CREATEROLE permission add people to their own
role, without superuser permission?  Are we adding any security by
preventing this?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

Embrace your flaws.  They make you human, rather than perfect,
which you will never be.

On Wed, Jan 25, 2023 at 07:38:51AM -0700, David G. Johnston wrote:
> On Wed, Jan 25, 2023 at 7:35 AM Bruce Momjian <bruce@momjian.us> wrote:
> 
> 
>     So, how would someone with CREATEROLE permission add people to their own
>     role, without superuser permission?  Are we adding any security by
>     preventing this?
> 
> 
> 
> As an encouraged design choice you wouldn't.  You'd create a new group and add
> both yourself and the new role to it - then grant it the desired permissions.
> 
> A CREATEROLE role should probably be a user (LOGIN) role and user roles should
> not have members.

Makes sense.  I was actually using that pattern, but in running some
test scripts that didn't revert back to the superuser, I saw the errors
and was confused.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

Embrace your flaws.  They make you human, rather than perfect,
which you will never be.

On Wed, Jan 25, 2023 at 9:35 AM Bruce Momjian <bruce@momjian.us> wrote:
> So, how would someone with CREATEROLE permission add people to their own
> role, without superuser permission?  Are we adding any security by
> preventing this?

They can't, because a role can't ever have ADMIN OPTION on itself, and
you need ADMIN OPTION on a role to confer membership in that role.

The security argument here is complicated, but I think it basically
boils down to wanting to distinguish between accessing the permissions
of a role and administering the role, or in other words, being a
member of a role is supposed to be different than having ADMIN OPTION
on it. Probably for that reason, we've never allowed making a role a
member of itself. In any release, this fails:

rhaas=# grant bruce to bruce with admin option;
ERROR:  role "bruce" is a member of role "bruce"

If that worked, then role "bruce" would be able to grant membership in
role "bruce" to anyone -- but all those people wouldn't just get
membership, they'd get ADMIN OPTION, too, because *bruce* has ADMIN
OPTION on himself and anyone to whom he grants access to his role will
therefore get admin option too. Someone might argue that this is OK,
but our precedent is otherwise. It used to be the case that the users
implicitly enjoyed ADMIN OPTION on their own roles and thus could do
the sort of thing that you were proposing. That led to CVE-2014-0060
and commit fea164a72a7bfd50d77ba5fb418d357f8f2bb7d0. That CVE is, as I
understand it, all about maintaining the distinction between
membership and ADMIN OPTION. In other words, we've made an intentional
decision to not let ordinary users do the sort of thing you tried to
do here.

So the only reason your example ever worked is because the "service"
role had CREATEROLE, and thus, in earlier releases, got to bypass all
the permissions checks. But it turns out that letting CREATEROLE
bypass all the permissions checks is *also* a big security problem, so
that is now restricted as well.

I don't want to take the position that we couldn't find some way to
allow ordinary users to do this. I think that the desire to maintain
the distinction between membership and ADMIN OPTION makes sense as a
general rule, but if somebody wants to abolish it in a particular case
by making strange grants, would that really be that bad? I'm not
totally convinced that it would be. It probably depends somewhat on
how much you want to try to keep people from accidentally giving away
more privileges than they intended, and also on whether you think that
this is a useful thing for someone to be able to do in the first
place. However, it's the way we've designed the system and we've even
requested CVEs when we accidentally did something inconsistent with
that general principle. Similarly, I don't want to take the position
that the restrictions I put on CREATEROLE are the *only* way that we
could have plugged the security holes that it has had for years now. I
think they are pretty sensible and pretty consistent with the overall
system design, but somebody else might have been able to come up with
some other set of restrictions that allowed this case to work.

I think David is right to raise the question of how useful it would be
to allow this case. In general, I think that if role A creates role B,
it is more sensible to grant B's permissions to A than to grant A's
permissions to B. The former is useful because it permits the more
powerful user to act on behalf of the less powerful user, just as the
superuser is able to administer the whole system by being able to act
on behalf of any other user. But the latter makes you wonder why you
are even bothering to have two users, because you end up with A and B
having exactly identical privileges. That's a bit of a strange thing
to want, but if you do happen to want it, you can get it with this new
system: again, as David says, you should just create one role to use
as a group, and then grant membership in that group to multiple roles
that are used as users.

But it does seem pretty important to keep talking about these things,
because there's definitely no guarantee whatsoever that all of the
commits I've made to master in this area are without problems. If we
find important cases that can't be supported given the new
restrictions on CREATEROLE, or even important cases that never worked
but we wish they did, well then we should think about what to change.
I'm not too concerned about this particular case not working, but the
next one might be different.

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

On Wed, Jan 25, 2023 at 12:21:14PM -0500, Robert Haas wrote:
> But it does seem pretty important to keep talking about these things,
> because there's definitely no guarantee whatsoever that all of the
> commits I've made to master in this area are without problems. If we
> find important cases that can't be supported given the new
> restrictions on CREATEROLE, or even important cases that never worked
> but we wish they did, well then we should think about what to change.
> I'm not too concerned about this particular case not working, but the
> next one might be different.

Agreed, thanks for the details.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

Embrace your flaws.  They make you human, rather than perfect,
which you will never be.