Bug 58891 - refuses all actions if user is member of a large number of groups
Summary: refuses all actions if user is member of a large number of groups
Status: RESOLVED MOVED
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: David Zeuthen (not reading bugmail)
QA Contact: David Zeuthen (not reading bugmail)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-30 14:43 UTC by Michael Biebl
Modified: 2018-08-20 21:37 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH] Fix get_groups_for_user() for large number of users (2.41 KB, patch)
2012-12-30 14:44 UTC, Michael Biebl
Details | Splinter Review

Description Michael Biebl 2012-12-30 14:43:11 UTC
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=696989

PolicyKit refuses all actions for my own user account, but works fine
for other accounts:

=== Begin SSH session as sascha.silbe ===
sascha.silbe@twin:~$ cat /etc/polkit-1/localauthority/50-local.d/90-sudo-allow-everything.pkla
[AllowEverythingToSudoGroup]
Identity=unix-group:sudo
Action=*
# from within active ConsoleKit sessions
ResultActive=yes
# from within inactive ConsoleKit sessions
ResultInactive=yes
# from within non-local ConsoleKit sessions
ResultAny=yes
sascha.silbe@twin:~$ id -u
8193
sascha.silbe@twin:~$ getent group sudo
sudo:x:27:sascha.silbe,bine
sascha.silbe@twin:~$ pkcheck --action-id org.freedesktop.udisks.filesystem-mount --process $$
Not authorized.
=== End SSH session as sascha.silbe ===

=== Begin SSH session as bine ===
bine@twin:~$ pkcheck --action-id org.freedesktop.udisks.filesystem-mount --process $$ ; echo $?
0
=== End SSH session as bine ===


Apparently polkitd chokes on the large number of groups my account is
a member of:

=== Begin ===
root@twin:~# /usr/lib/policykit-1/polkitd -r
Entering main event loop
Connected to the system bus
Registering null backend at priority -10
Using authority class PolkitBackendLocalAuthority
Acquired the name org.freedesktop.PolicyKit1

** (polkitd:20969): WARNING **: skipping unknown tag <_description> at line 15

** (polkitd:20969): WARNING **: skipping unknown tag <_message> at line 16

** (polkitd:20969): WARNING **: Error looking up groups for uid 8193: Numerical result out of range

=== End ===

Checking the source
(src/polkitbackend/polkitbackendlocalauthority.c:get_groups_for_user()),
there's even a TODO entry for this bug:

  gid_t groups[512];
  int num_groups = 512;
[...]
  /* TODO: should resize etc etc etc */

  if (getgrouplist (passwd->pw_name,
                    passwd->pw_gid,
                    groups,
                    &num_groups) < 0)
    {
      g_warning ("Error looking up groups for uid %d: %s", uid, g_strerror (errno));
      goto out;
    }


Once the account is a member of more than the hard-coded limit of 512
groups, PolicyKit will not recognise the user at all, therefore refuse
all actions for them.

This bug is still present in the latest development version (d6acecd),
now in
src/polkitbackend/polkitbackendjsauthority.c:subject_to_jsval().

The reason my user account is part of so many groups is that I'm using
the rainbow package extensively to run web browsers and the like with
less privileges than my user account and in isolation from each
other. For each isolated session, a group is created to enable
exchange of files between my user account and the session account (but
not between the sessions).

I've set the Severity to Important because PolicyKit refuses to work
at all (for this user) once the hard-coded limit is exceeded, rather
than just some part of PolicyKit not working as expected or only the
first few groups being evaluated to determine whether to grant access.
Comment 1 Michael Biebl 2012-12-30 14:44:39 UTC
Created attachment 72304 [details] [review]
[PATCH] Fix get_groups_for_user() for large number of users

get_groups_for_user() used a hard-coded limit for the number of
groups before, barring all access to users with a large number of
groups.
.
Dynamically allocate the list instead, potentially resizing it based
on what getgroupslist() tells us.
Comment 2 David Zeuthen (not reading bugmail) 2013-01-09 19:02:53 UTC
(In reply to comment #1)
> Created attachment 72304 [details] [review] [review]
> [PATCH] Fix get_groups_for_user() for large number of users
> 
> get_groups_for_user() used a hard-coded limit for the number of
> groups before, barring all access to users with a large number of
> groups.
> .
> Dynamically allocate the list instead, potentially resizing it based
> on what getgroupslist() tells us.

We can do this but

 - the patch no longer applies to master

 - the patch does not follow the existing coding standards/conventions
   - no space between function name and beginning paranthesis
   - should use GLib memory allocation routines (g_new0/g_malloc/etc)
   - "if (!ptr)" should be "if (ptr == NULL)"
Comment 3 Michael Biebl 2013-01-09 19:44:58 UTC
(In reply to comment #2)
> We can do this but
> 
>  - the patch no longer applies to master
> 
>  - the patch does not follow the existing coding standards/conventions
>    - no space between function name and beginning paranthesis
>    - should use GLib memory allocation routines (g_new0/g_malloc/etc)
>    - "if (!ptr)" should be "if (ptr == NULL)"

Sascha, since you are reading this bug report and you are the original author of the patch, do you want to rework it according to David's comments?
Comment 4 GitLab Migration User 2018-08-20 21:37:28 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/polkit/polkit/issues/43.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.