DBUS Should Support "Session Groups" (pam_group.so,/etc/security/groups.conf)

Bug #75602 reported by Jonathan Anderson
22
This bug affects 1 person
Affects Status Importance Assigned to Milestone
D-Bus
Invalid
High
dbus (Ubuntu)
Triaged
Wishlist
Unassigned

Bug Description

Binary package hint: dbus

In my lab, there are lots of users in an LDAP database. These users are assigned to the groups audio,video,plugdev,etc. by pam_group (using the /etc/security/groups.conf file). The problem is, DBUS doesn't recognize users as being in groups unless they're in /etc/group, which isn't practical for our setup. So, there are permission problems with USB sticks, kpowersave, etc. (well, there *will* be, once I upgrade from Dapper to a more version of Kubuntu).

On my laptop, I can use (the kludge) "adduser $uname plugdev"... this won't work in the lab.

Revision history for this message
In , Hp-pobox (hp-pobox) wrote :

dbus just uses the normal C library functions to get the user's groups; it
isn't manually groping around in /etc/group or anything.

Maybe you need to configure libc to use nscd or something like that?

I don't know how to fix this on the dbus level, certainly dbus should not be
in the business of understanding NIS, LDAP, PAM, /etc/group, and so forth by
hand...

Revision history for this message
In , Jonathan Anderson (jonathan-anderson) wrote :

Okay, so I did some digging in the DBUS source... I think that there are two
problems, one libc-related, the other DBUS-related.

The first is that getgrouplist() does not return "dynamic" groups assigned by
pam_group. This is not a DBUS issue, so I guess it'll have to be a libc/PAM
wishlist item.

The second is that (if I'm reading this correctly) DBusUserDatabase *system_db
is built when the daemon starts up, so it can only use static group
assignments. If getgrouplist() were modified (or an alternative found), could
DBUS be modified to allow group assignments to be "freshened"?

Revision history for this message
In , Hp-pobox (hp-pobox) wrote :

There is a patch in CVS now that allows you to build in a mode where groups
aren't cached. The problem is that if you're using the read-/etc-group libc
implementation instead of nscd this will be absurdly performance-intensive.
That is, every time someone sends a message over the bus we'd load and
parse /etc/group and /etc/passwd.

What we'd like someone to do is write a small test program to benchmark group-
based bus security policy checks with and without caching the user/group
information, and then run that benchmark
a) with current caching
b) with regular "read the file" libc implementation
c) with nscd implementation

Then we can discuss the right course of action.

More discussion can be found in the mailing list archives.

Revision history for this message
Jonathan Anderson (jonathan-anderson) wrote :

Binary package hint: dbus

In my lab, there are lots of users in an LDAP database. These users are assigned to the groups audio,video,plugdev,etc. by pam_group (using the /etc/security/groups.conf file). The problem is, DBUS doesn't recognize users as being in groups unless they're in /etc/group, which isn't practical for our setup. So, there are permission problems with USB sticks, kpowersave, etc. (well, there *will* be, once I upgrade from Dapper to a more version of Kubuntu).

On my laptop, I can use (the kludge) "adduser $uname plugdev"... this won't work in the lab.

Changed in dbus:
status: Unknown → Confirmed
Revision history for this message
Jonathan Anderson (jonathan-anderson) wrote :

Calling this "Confirmed", since it's been confirmed upstream.

Changed in dbus:
status: Unconfirmed → Confirmed
Revision history for this message
In , Jerome-koumbit (jerome-koumbit) wrote :

I haven't found much about this bug in the mailing list archive... Was a workaround found? Otherwise, has there been any progress on this front?

Using Debian, I find it somewhat tedious to have to add every user to groups such as audio, plugdev and netdev in /etc/group on a multi-user desktop machine, to enable basic functionality like sound, automounting and network-manager access.

Revision history for this message
In , Hp-pobox (hp-pobox) wrote :

I don't know of any progress. The situation last I knew remained that with /etc/group it is absurdly expensive not to cache the info from libc; with nscd, I'm not sure any profiling has been done. I don't know how the common distributions configure by default in recent versions.

By hacking the dbus code you can trivially disable the caching of group information, but the question is just whether that will slow performance to a crawl. Remember the stuff dbus is doing happens for *every message* - it has to check what groups the user sending the message is in. Opening a file or making a blocking network request for every message will be too expensive.

This may or may not be hard to fix, or may or may not even be a real problem with "normal" configurations shipped by distributions these days. All I really know for sure is that when originally implementing this feature (on some years-old Fedora or Red Hat flavor), the caching was necessary to avoid parsing /etc/groups over and over.

So, what we need is research first of all.

If it is still too slow to turn off the cache, perhaps a fix would be to expire the cache after some period of time. However I would not spend time on that without first just documenting the situation (what libc is doing, and how fast it is, in typical configurations).

Revision history for this message
In , Dariem Pérez Herrera (dariemp) wrote :

I've configured an Ubuntu box to authenticate using Microsoft Active Directory through pam_winbind.so. Domain users can use pluggable devices through dbus/hal because I've used pam_group.so to assign such users to plugdev group. It worked just fine. Now I'm using Gentoo and I can't do the same, it's not working. Is it possible that some Ubuntu developer have solved the problem? My partial solution in Gentoo was to configure /etc/dbus-1/system.d/hal.conf to assign priviledges to group "domain users" and to modify /etc/init.d/dbus to start after samba/winbind. But there is a problem: any short-timed failure on network connection prevents dbus for knowing who are "domain users", so this users can't automount pluggable devices after any of these failures, they have to wait for my presence so I can restart dbus daemon, and consequently, all GNOME environment. I think this issue should be solved as soon as possible, because is fatal for remote authentication mechanism (LDAP, Kerberos, Active Directory) when network users can't use something as common as a pendrive.

James Westby (james-w)
Changed in dbus:
importance: Undecided → Wishlist
Changed in dbus (Ubuntu):
status: Confirmed → Triaged
Revision history for this message
In , ceg (ceg) wrote :

From comment #6 i got the idea that /etc/group is cached and read only once during start up or simmilar.

Would it be possible to refresh the cache when a request is made for a new user, that previously wasn't queried before?

Or maybe only rely on chache for new users after some amount of seconds has passed without changes (due to session scripts).

Revision history for this message
In , Colin Walters (walters) wrote :

Note applications should migrate to PolicyKit for privileged operations, and avoid using the DBus security policy.

In the specific case of HAL, work is ongoing already to replace it.

That all said, my guess is that the easiest fix here is to enable nscd and disable dbus' built in caching. (Maybe the latter could be a flag in the init script or we somehow detect nscd running)

Changed in dbus:
status: Confirmed → In Progress
Changed in dbus:
importance: Unknown → High
Changed in dbus:
importance: High → Unknown
Changed in dbus:
importance: Unknown → High
Revision history for this message
In , Simon McVittie (smcv) wrote :
Download full text (3.3 KiB)

The problem here is that the Unix semantics of groups are rather non-obvious. Each Unix process has a primary group ID and an array of of supplementary group IDs; these are *normally* the group ID and supplementary groups of the process's owning user, but things like pam_group cause that not to be true.

dbus-daemon accesses the system group database via libc. On a GNU system, that means nsswitch (a module of glibc); you can access that database from the command line by running "getent groups" (which returns something that looks remarkably similar to /etc/group, but also contains information from LDAP or whatever).

When login(1), su(1) or whatever switches uid from root to a user, it reads from the system's group database, and assigns those groups to the process. This has some interesting side-effects; for instance, if you remove a user from, say, the audio group, but they have a background process like a screen(1) session, that process still has audio rights. Similarly, if you add a user to the audio group, any of their processes that are already running won't automatically get audio rights (until they run sg or newgrp or something).

pam_group doesn't add users to groups - it adds extra groups to processes. dbus-daemon starts from the uid and reads directly from the system group database to determine the groups, so it can't see those extra groups.

Using or not using the libdbus "userdb cache" won't fix this, because it's still reading from nsswitch and not looking at the requesting process's credentials directly.

Using or not using ncsd won't fix this either, because ncsd returns the same things as nsswitch (albeit perhaps an out-of-date version).

"Fixing" this would be not at all trivial. The D-Bus wire protocol normally uses a single Unix credential-passing message (on first connection to the bus) to get the user ID, but credential-passing can normally only carry one user ID and one group ID - so dbus-daemon never finds out the list of supplementary groups, only the primary group.

I'm inclined to say this is WONTFIX, because:

* using PolicyKit solves this for recent versions of HAL, upower, udisks,
  etc. as used in current GNOME (and presumably also KDE) distributions,
  removing the need for plugdev, powerdev, etc. group membership

* not directly related to D-Bus, but using ConsoleKit and udev also solves
  this for device-node ownership: on my Debian sid laptop, logging in to
  a local GNOME session automatically gives me rw access to appropriate
  device nodes via a POSIX ACL, for instance:

  # file: dev/snd/pcmC0D0p
  # owner: root
  # group: audio
  user::rw-
  user:smcv:rw-
  group::rw-
  mask::rw-
  other::---

* using pam_group for temporary group membership is generally insecure,
  in the sense that there's trivial privilege escalation from temporary
  to permanent group membership if users can write to any location that
  supports setgid executables:

  * log in at the console and be placed in the audio group

  * cd to anywhere that supports setgid executables (home directories
    are usually sufficient)

  * cp /bin/bash audiobash && chgrp audio audiobash && chmod g+s audiobash

  * log out

  * ssh in while another...

Read more...

Revision history for this message
In , ceg (ceg) wrote :

Thank you for the summary.

Did find this thread about the issue:
http://web.archiveorange.com/archive/v/2mxmYMz9oyt4896NAcKZ
(http://lists.freedesktop.org/archives/dbus/2008-August/010191.html)

Concluding, it looks like there is some code/patch available to allow to check for the correct full list of groups (incl. dynamicly added) of a process at connect time.

Revision history for this message
In , Simon McVittie (smcv) wrote :

From that mail:
> The solution is a bit hairy, because it does require a changed kernel
> (at least I haven't found any other way to test the group of another
> process efficiently).

There's also an interesting attack based on a race condition, if dbus-daemon performs access control by asking the kernel for a process's credentials by pid:

* attacker has (say) uid 1001, gid 11111 and pid 12345

* attacker sends a D-Bus message which is only allowed for group 22222
  (or for uid 0, for that matter)

* [race] attacker immediately uses exec() to replace itself with any
  setuid-0 or setgid-22222 binary

* [race] dbus-daemon asks the kernel "what is the gid of pid 12345?"
  (or for the uid 0 case, "what is the uid of pid 12345?"

* if the dbus-daemon wins the race, the kernel replies "11111"
  (or 1001) and permission is denied

* if the attacker wins the race, the kernel replies "22222" (or 0)
  and permission is granted!

... so, don't do that.

For local Unix sockets on Linux, the credentials exchange step at the beginning of the connection means that dbus-daemon can validate the connection's uid, pid and one of its groups - but there's no way to get the other groups, unless the connecting process sends one message per group. This would require modifications to dbus-daemon and all client libraries wishing to use this mechanism (libdbus, QtDBus, GDBus, etc.), and wouldn't work for non-local connections, or on platforms where this credentials-passing doesn't work.

A new syscall of the form "what are the uid, gid and all supplementary groups at the other end of this socket?" or "does the other end of this socket have these credentials?" would also solve this, albeit in a completely non-portable way.

Revision history for this message
In , Krzysztof-konopko (krzysztof-konopko) wrote :

Here's a mailing list thread that alluded to this bug:
http://lists.freedesktop.org/archives/dbus/2013-February/015505.html

The idea is to at least try harder on Linux where relevant user/group information can be extracted from procf status file.

I'll be sending some patches here for review to see if it's plausible to take this approach.

Revision history for this message
In , Krzysztof-konopko (krzysztof-konopko) wrote :

Created attachment 75714
A patch for _dbus_file_get_contents() to support procfs files that appear to be empty

This is the first patch in hopefully a series of patches that will try to remedy the problem described in this bug at least on Linux.

First thing to do is to support files in procfs that appear to be empty. Unfortunately _dbus_file_get_contents() ignores such files upfront.

The patch has been inspired by a patch used in a production system:
https://github.com/kkonopko/dbus/commit/7af563d558808fc91d181b5bf9fe24543a44df4c#L8R148

This one uses _dbus_read() instead of plain read. It's also tempting to fold the logic from the last `else' statement into the first one. And even further the logic that closes the file (and reverts the output DBusString size) and returns the status could also be folded. This will require writing some tests for this function first.

The patch is to get a brief opinion and views.

Revision history for this message
In , Krzysztof-konopko (krzysztof-konopko) wrote :

Created attachment 76224
Required plumbing for reading process credentials from procfs

Revision history for this message
In , Simon McVittie (smcv) wrote :
Download full text (5.8 KiB)

Comment on attachment 76224
Required plumbing for reading process credentials from procfs

Review of attachment 76224:
-----------------------------------------------------------------

This is a much more intrusive change than you actually need, and appears to aim to change functions' semantics in ways that make their names no longer make sense.

A general comment for anyone who wants to alter D-Bus access-control: be aware of the attack involving exec() described in <https://bugs.freedesktop.org/show_bug.cgi?id=47581#c3>.

::: dbus/dbus-auth.c
@@ +549,5 @@
> }
>
> + if (!_dbus_credentials_add_from_user (auth->desired_identity,
> + data,
> + _dbus_credentials_get_unix_pid (auth->credentials)))

This is in DBUS_COOKIE_SHA1 authentication, which is used if we don't have enough credentials-passed information for EXTERNAL authentication. Why would it ever know the pid?

(Also, if it can legitimately know the pid, the same comments as below apply.)

@@ +1080,5 @@
> else
> {
> if (!_dbus_credentials_add_from_user (auth->desired_identity,
> + &auth->identity,
> + _dbus_credentials_get_unix_pid (auth->credentials)))

I think this may be the only place where you actually need the pid...

Extending _dbus_credentials_add_from_user() is inappropriate, given the name of the function. Instead, you should add _dbus_credentials_add_from_other_process (DBusCredentials *credentials, dbus_pid_t pid) or something. Make sure it can return "I don't know", as it usually will on non-Linux. If it does, this caller should fall back to some other strategy, for instance calling _dbus_credentials_add_from_user() like it does now.

Here's an interesting design question: a connecting process sends us "I am { uid 1000, pid 1234 }" via credentials-passing, and uses the EXTERNAL SASL mechanism to say "I am uid 1000". We look in /proc/1234 to find out its groups. It turns out to have uid 2000 (which must mean it made use of CAP_SETUID, or exec()'d a setuid process). What should happen? What are its credentials now?

I don't think "its uid is assumed to be 2000" is an acceptable answer: during the SASL handshake, it explicitly told us that it wanted to authenticate as uid 1000!

It's possible that the "we have /proc" code path might need to use the username/uid as well (because it's the SASL identity). Perhaps it needs to call _dbus_credentials_add_from_user() *and* _dbus_credentials_add_from_other_process().

Another interesting design question: suppose a connecting process sends us "I am { uid 1000, pid 1234 }" via credentials-passing, and uses the EXTERNAL SASL mechanism to say "I am uid 1000". /etc/passwd and /etc/group say that uid 1000 is a member of groups 100 and 101. When we inspect /proc/1234 we discover that it has uid 1000 and groups 100 and 200. What are the connections's credentials? Current libdbus would say { uid=1000, pid=1234, group 100, group 101 }. They definitely include uid=1000, pid=1234. This bug report wants them to include { group 100, group 200 } but do they al...

Read more...

Revision history for this message
In , Krzysztof-konopko (krzysztof-konopko) wrote :
Download full text (3.5 KiB)

Comment on attachment 76224
Required plumbing for reading process credentials from procfs

Review of attachment 76224:
-----------------------------------------------------------------

Thanks for your really valuable comments and your time. I admit this PID thing is sprinkled everywhere. I didn't want to spend too much effort without knowing your views first and this review proves that I would have gone astray. I'll revisit this patch and address all your concerns focusing only on EXTERNAL authentication and supplementary groups. My other goal is also try to not even touch /etc/{passwd,group} at all if not absolutely necessary.

Please see also my specific comment wrt design/security.

::: dbus/dbus-auth.c
@@ +1080,5 @@
> else
> {
> if (!_dbus_credentials_add_from_user (auth->desired_identity,
> + &auth->identity,
> + _dbus_credentials_get_unix_pid (auth->credentials)))

I'll address only specific questions here and defer the rest to the generic comment.

If an account has access to setuid/setgid executables and is available to potentially rogue users, then the system is flawed, not D-Bus. IMHO the vulnerability here are setuid/setgid programs that shouldn't exist at all or at least shouldn't be available to random users. It's a bit like saying "Everyone on the system has passwordless sudo access because a sysadmin/architect had a bad day. Oops! Let's defend!".

I don't understand your reasoning in one of your comments [1]:
* cp /bin/bash audiobash && chgrp audio audiobash && chmod g+s audiobash

The user must be a member of `audio' group in order to chgrp a file to that group. Why then ever bother with these tricks if the user is a *genuine* member of the group?

You also mentioned in your comment on some other bug [2]:
/* any setuid program will do */
exec ("/bin/ping", "example.com")

Again, no setuid/setgid should be available. In this instance `ping' program should have cap_net_admin and cap_net_raw capabilities set. No UID change.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=9328#c9
[2] https://bugs.freedesktop.org/show_bug.cgi?id=47581#c3

----
The reality is different though and no one wants to make libdbus the culprit. So I agree with you: UID should be received from the socket rather than from procfs.

WRT the other question where you consider different groups in /etc/group and in procfs, my opinion is that those in /etc/group should be ignored. My view is that someone knew better and since they were privileged enough to convince kernel to set those groups on the process, libdbus in this instance should respect that.

Now again you could argue that there's a race condition here and someone was able to gain group membership the same way as for UID. I think it's up to the system administrator/architect to provide or deny such opportunity. For example the system I work on has all D-Bus services/clients running in their individual sandboxes where one of the features is a separate mount namespace with all filesystems mounted either ro,nosuid or noexec.

So the conclusion is that this functionality could be optional ...

Read more...

Revision history for this message
In , Simon McVittie (smcv) wrote :
Download full text (5.3 KiB)

(In reply to comment #16)
> I'll revisit this patch and address all your concerns focusing
> only on EXTERNAL authentication and supplementary groups. My other goal is
> also try to not even touch /etc/{passwd,group} at all if not absolutely
> necessary.

Great. If you can get this working well and securely, I'm happy to review/merge.

Regarding setuid:

> If an account has access to setuid/setgid executables and is available to
> potentially rogue users, then the system is flawed, not D-Bus.

They don't have to be setuid executables that do anything inherently unsafe on their own. su, sudo or pkexec is enough for the attack I described: as long as its uid is 0 at the time that the dbus-daemon looks at it, it doesn't matter that all it was doing is sitting at an "enter password:" prompt.

Even if *your* system does not have any setuid executables, general-purpose Unix does - su, sudo, pkexec are easy examples - and D-Bus is meant to be functional and secure on general-purpose Unix, and in particular, general-purpose Linux.

Now, this attack is mostly about what happens if you check credentials per-message instead of per-connection, which you're not doing, so the most straightforward version of the attack is avoided. I'm working on the assumption that for bad things to happen, the client will have to send a message. If it exec()s a privileged executable, it "has lost control" and can no longer send "bad messages".

However, I'm concerned about the possibility of a client queueing up its side of the entire authentication handshake, the Hello message, and a "bad message" into the socket's buffer, then exec()ing; is it possible that the server could perform the /proc check, see the exec()'d executable's credentials, and then act on the rest of the pre-queued data from the *original* executable?

One way to avoid this would be to avoid the /proc stuff, and instead extend the authentication handshake with a new verb similar to NEGOTIATE_UNIX_FD:

    C: AUTH EXTERNAL 31303030
    S: OK
    C: I_AM_IN_GROUP 100
    S: OK
    C: I_AM_IN_GROUP 101
    S: OK
    C: BEGIN

where the first message of I_AM_IN_GROUP is accompanied by SCM_CREDENTIALS out-of-band data containing the corresponding gid. That's likely to be somewhat more difficult than rummaging in /proc, though.

> * cp /bin/bash audiobash && chgrp audio audiobash && chmod g+s audiobash
>
> The user must be a member of `audio' group in order to chgrp a file to that
> group. Why then ever bother with these tricks if the user is a *genuine*
> member of the group?

Please read Comment #9 if you're unfamiliar with Unix group semantics - they are not what you might think they are.

If a user is not (according to /etc/group) a member of audio, but their shell has the audio supplementary group due to pam_group, then they can escalate from "temporarily in audio group" to "permanently able to get into audio group" by copying a shell (or anything similar) and chmod'ing it g+s. This makes session groups not very useful as a security measure on general-purpose Unix (they can be secure on a system where users cannot write to any filesystem mounted without nosuid and/or noexec, but D-Bus is not restricte...

Read more...

Revision history for this message
In , Simon McVittie (smcv) wrote :

(In reply to comment #17)
> If you can get this working well and securely, I'm happy to
> review/merge.

On the other hand, I still think a more appropriate solution to the problem you outlined on the mailing list and in your git repository (dynamically-allocated users/groups without editing /etc) would be to install a nss module that understands those users/groups; or if your libc doesn't have pluggable nss, modify your libc to know about them directly, like Android's Bionic libc.

If this feature request can't be satisfied in a way that is clearly secure, I'm afraid I would prefer not to satisfy it at all.

Revision history for this message
In , Krzysztof-konopko (krzysztof-konopko) wrote :

Created attachment 76666
Required plumbing for reading process credentials from procfs v. 2

OK, here's another attempt with the PID plumbing :)

This time I tried to focus on EXTERNAL authentication and took your advice from the previous review. My main goal is to avoid touching /etc/{passwd,group} if possible.

Here are the highlights:
- if the data sent along with AUTH message is a numeric UID, then try to use it instead of looking in /etc/passwd.
See handle_server_data_external_mech(). The desired_identity is checked against the UID from the socket so no risk here.

- the next step is determining whether the client can connect
bus_policy_allow_unix_user() takes additionally PID as an argument and tries to get groups from /proc first (_dbus_unix_groups_from_pid() not implemented yet) and falls back to the lookup based on UID.

- a new API function introduced to get the PID form the connection and pass it to client authentication: dbus_connection_get_unix_process_id_unauth()
As opposite to dbus_connection_get_unix_process_id(), it doesn't try to authenticate the user internally which at this stage would end up in the endless loop.

Unfortunately got stuck with bus_policy_create_client_policy()
This function calls _dbus_unix_user_is_at_console() which unfortunately needs a username so ends up looking in /etc/passwd (for UIDs without the entry in /etc/passwd _dbus_user_database_lookup() fails with assertion). Need to think how to avoid touching /etc/passwd here.
The goal here is to let D-Bus find out about the process from it's state (socket credentials, procfs if possible) rather than pesky /etc/passwd. And a natural way of doing it is to try the authentication with UIDs not present in /etc/passwd.
I guess the priority in this bug is to focus on groups. So if I can't come up with something reasonable for the problem above, I'll carry on with groups only.

Changed in dbus:
status: In Progress → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.