Bug 864613

Created attachment 624219 [details]
add debugging statements to clarify behavior of polkit_backend_local_authorization_store_ensure function

Description of problem:
While trying to allow udisks mounts for one unix-group and deny for everyone else (see someone else's Bug #812684), I found that the order of authorizations is unpredictable, for two reasons: (a) because the list of authorizations read is reversed every time an entry is added; and (b) it doesn't appear that the list of PolicyKit local authority files in a given directory is sorted by name, so the files may be read in any order.

Version-Release number of selected component (if applicable):
polkit-0.96.2.el6.1


How reproducible:
1. Apply the attached add-debug patch to the polkit sources. It adds more g_debug calls that make it clear what is happening. Build polkit the same way the official .spec file does.

2. Place in /etc/polkit-1/localauthority/90-mandatory.d at least four PolicyKit config files. For example, I've got one to cause all NetworkManager actions to require admin_auth, one to cause all PackageKit actions to require admin_auth, one for udisks to require admin_auth, and another for udisks where Identity=unix-group:my-special-group that will allow mount and unmount actions. You need at least four, and they should not be created in lexical sort order: i.e., the output of ls -f should not equal the output of ls.

3. Run polkitd with --debug.

4. As a normal user, plug in a USB disk and try to mount it with udisks --mount. Watch the debug output of polkitd.

5. The debug messages shown as each file is read should make it clear that the files are not being processed in sorted order.

6. The debug messages should also make clear that every time a value is prepended to the list of authorizations, the list is reversed.

As a concrete example, for four files FA FD FB FC, each containing a heading (INI file section) A D B C respectively, I believe the intended behavior is that the files should be read in sorted order, FA FB FC FD. The section in each is prepended to the list of authorization entries. With no reversals, the resultant list would be in the order D C B A. The list is reversed, to yield A B C D, and authorizations are checked in this order. But the files are read in directory order, FA FD FB FC, and each time an authorization entry is prepended the list is reversed: (<> means "reverses to")

A <> A
D A <> A D
B A D <> D A B
C D A B <> B A D C

and the final order in which the authorization entries are processed is B A D C.

The reverse is in src/polkitbackend/polkitbackendlocalauthorizationstore.c, function polkit_backend_local_authorization_store_ensure, at line 641 in the patched source, 636 in the unpatched source.

* * *

To fix: 
(a) Move the call to g_list_reverse outside the for loop it is in.
(b) Sort the list of files before iterating over it.