vmihalko / t2_polkit

Other
0 stars 0 forks source link

If a permission depends on group membership, polkit should explicitly check if calling user is a member of that group as well as trying to find all members of the group #28

Open vmihalko opened 10 years ago

vmihalko commented 10 years ago

In GitLab by @bugzilla-migration on Aug 27, 2014, 23:01

Submitted by Adam Williamson

Assigned to David Zeuthen @david

Link to original bug (#83164)

Description

I ran across an interesting corner case today. I manage my systems with FreeIPA. I'd been trying to 'centralize' admin user configuration, by creating a 'wheel' group in FreeIPA and adding my user account to it - the intent being that I'd be considered a member of 'wheel' (and hence an admin for PK purposes) on any of my FreeIPA-managed systems.

However, this turns out not to work: PK doesn't recognize that my user is a member of the wheel group, and just asks for the root password for authentication.

We figured it out, in the end. When a permission (like admin permissions, in this case) depends on group membership, what PK does is call getgrgid() to try and find out all the members of that group:

grp = getgrgid (gid); ... for (n = 0; grp->gr_mem != NULL && grp->gr_mem[n] != NULL; n++) { blah blah check if n is a policykit user and isn't root and return a list of n's blahblah }

from src/polkitbackend/polkitbackendinteractiveauthority.c .

The problem in this case turns out to be the behaviour of getgrgid(). I created my 'wheel' group in FreeIPA, but didn't remove the local wheel group on my systems, listed in /etc/group . FreeIPA intentionally configures client systems such that this kind of conflict is resolved in favour of the local definition. That is, in /etc/nsswitch.conf , it lists 'files sss' , not 'sss files'. getgrgid() will return the result from the first nss 'engine' (or whatever you call those things) that contains the group in question. So because there's a wheel group in /etc/group , the getgrgid() call just returns the members of the group so far as /etc/group is concerned - which doesn't include me, because I'm only a member of the group so far as FreeIPA is concerned.

So if you explicitly check what groups my user is a member of, you'll get 'wheel', because that check will go through the definition of my user held in FreeIPA (there's no competing local definition of my user account) and return 'wheel'. But if you just check what users are a member of the group 'wheel', you won't find my user, because the query doesn't make it to FreeIPA. This is easily checkable using getent - 'getent passwd adamw' lists me as a member of wheel, but 'getent group wheel' doesn't list adamw as a member of the group wheel.

Simo (who helped debug this) basically said that what I'm doing is kind of stupid and I should handle it another way, but still, we (myself and Simo and mitr) agreed that it'd be good for polkit to cope with this corner case in some way - it's possible, and it makes polkit a little more robust. So we thought that when a permission depends on membership of group 'foo', as well as checking for all members of group 'foo', polkit should explicitly check whether the username from which the request originated is a member of group 'foo'. 99% of the time the check would be redundant, of course - perhaps the 'get all members of group' call should happen first and the 'check if requesting user is in the group' call should happen only if the user isn't on the list returned by the initial call.

vmihalko commented 10 years ago

In GitLab by @bugzilla-migration on Aug 27, 2014, 23:03

:speech_balloon: Miloslav Trmac said:

To be precise, this is not truly about “permissions” but about the users allowed to respond to auth_admin* challenges.

vmihalko commented 10 years ago

In GitLab by @bugzilla-migration on Aug 28, 2014, 01:17

:speech_balloon: Adam Williamson said:

I was figuring the 'get_group_members' (or whatever it's called) function was kinda generic and probably used in other cases too...

vmihalko commented 8 years ago

In GitLab by @bugzilla-migration on May 28, 2016, 20:48

:speech_balloon: Alexander E. Patrakov said:

This will be fixed as a part of glibc-2.24

https://sourceware.org/git/?p=glibc.git;a=commit;h=ced8f8933673f4efda1d666d26a1a949602035ed

vmihalko commented 5 years ago

In GitLab by @CendioOssman on Sep 11, 2019, 17:12

Ehm... That will not fix things. It does not cover the case of the backend not allowing enumeration (common in centralised systems because of the load in incurs, and is/has been a default in sssd), and it doesn't cover the case where it is the users primary group.

The unfortunate fact is that Unix/Linux does not reliable support giving an answer to the question "Who is in the group FOO?".

It looks like polkit needs a redesign. The only reliable questions are "Which groups are user FOO in?" (via getgroups() or initgroups()), or "What groups does process FOO have?".

IOW, polkit needs to do a lookup on each request. And it cannot give suggestions on who other users that can authenticate.

vmihalko commented 5 years ago

In GitLab by @CendioOssman on Sep 26, 2019, 14:48

For reference, here is sssd's FAQ entry on this:

https://docs.pagure.org/SSSD.sssd/users/faq.html#when-should-i-enable-enumeration-in-sssd-or-why-is-enumeration-disabled-by-default

It also documents how to force sssd to do enumeration, but warns it can cause performance issues.

Red Hat also has a bug entry for this issue:

https://bugzilla.redhat.com/show_bug.cgi?id=1214026