Created attachment 131736 [details] [review]
Return the dbus-daemon SELinux context when asiking about org.freedesktop.DBus

Currently when asked the SELinux context of the owner of
org.freedesktop.DBus, the dbus-daemon is returning an error.

In the same situation when asked about the Unix user or the PID, the
daemon would return its own user or pid.

This patch is doing the same for the SELinux context by returning the
daemon one.

Created attachment 131738 [details] [review]
Return the dbus-daemon SELinux context when asking about org.freedesktop.DBus

Currently when asked the SELinux context of the owner of
org.freedesktop.DBus, the dbus-daemon is returning an error.

In the same situation when asked about the Unix user or the PID, the
daemon would return its own user or pid.

This patch is doing the same for the SELinux context by returning the
daemon one.

I think we should also fix GetConnectionCredentials(), at least in 1.11, to keep the invariant that GetConnectionCredentials() is at least as useful as the individual methods.

(In reply to Simon McVittie from comment #3)
> I think we should also fix GetConnectionCredentials(), at least in 1.11

(Patches on the way for that)

Comment on attachment 131738 [details] [review]
Return the dbus-daemon SELinux context when asking about org.freedesktop.DBus

Review of attachment 131738 [details] [review]:
-----------------------------------------------------------------

This looks good to me, although the commit message should really explain why this is problematic for UpdateActivationEnvironment's call out to systemd. I'll mention that when I commit it.

Created attachment 131743 [details] [review]
sysdeps: Add a cached accessor for our own Linux security  label

Created attachment 131744 [details] [review]
driver: Get the Linux security label for the bus-daemon  itself

GetConnectionCredentials() should report everything that the legacy
getter methods do, including the LSM label.

Created attachment 131745 [details] [review]
test/dbus-daemon: Exercise the bus daemon's own  credentials

---

This might need the patch "test/dbus-daemon: Fix some memory leaks" from Bug #101257.

(In reply to Simon McVittie from comment #8)
> test/dbus-daemon: Exercise the bus daemon's own  credentials

I've tried this on a system with Ubuntu's patched Linux kernel (which has SO_PEERSEC for AppArmor, and it correctly reports "unconfined"), and on a system with a Debian kernel (which does not, and no LSM label is reported).

On SELinux, hopefully it will report the right thing.

Comment on attachment 131743 [details] [review]
sysdeps: Add a cached accessor for our own Linux security  label

Review of attachment 131743 [details] [review]:
-----------------------------------------------------------------

::: dbus/dbus-sysdeps-util-unix.c
@@ +1562,5 @@
> +      _dbus_fd_set_close_on_exec (socks[1]);
> +
> +      if (!_dbus_add_linux_security_label_to_credentials (socks[0],
> +                                                          credentials))
> +        {

I might be wrong here but with SELinux, a socket might have a different label than the process (the transition might happen using the policy) can't this be a problem?

(In reply to Laurent Bigonville from comment #10)
> I might be wrong here but with SELinux, a socket might have a different
> label than the process (the transition might happen using the policy) can't
> this be a problem?

What does this mean for SO_PEERSEC?

The goal here is that GetConnectionSELinuxSecurityContext() and GetConnectionCredentials(), when called for "org.freedesktop.DBus", should return the same things as if a client queried the dbus-daemon's identity directly by using SO_PEERCRED and SO_PEERSEC.

(One complicating factor here is that if dbus-daemon was started from dbus.socket, then the AF_UNIX socket to which clients connect was actually initiated by systemd, not by dbus-daemon - so SO_PEERCRED and SO_PEERSEC would actually give information about systemd in that case.)

Nevermind, I was wrong. It's obviously the context of the peer, not the context of the socket itself

Applied Laurent's patch, with an extended commit message. Not in dbus-1.10 yet.

Laurent, would you mind reviewing and testing the patches I attached here? I'd like to have feature parity between the method you patched (which is vaguely deprecated) and its more modern replacement.

In particular, if you run test/test-dbus-daemon in some non-trivial SELinux profile (sorry, I have no idea how you do that), the /own-creds test should report "LinuxSecurityLabel of message bus is foo:bar:baz" or whatever. This is a semi-manual test because I'm not sure how else it could work.

I just tried to run the test in some arbitrary domain and it seems to work

DBUS_TEST_DAEMON=$(pwd)/bus/dbus-daemon runcon system_u:system_r:geoclue_t:s0 test/test-dbus-daemon --verbose

GTest: run: /creds
(MSG: ProcessID of this process is 19535)
(MSG: UnixUserID of this process is 1000)
(MSG: LinuxSecurityLabel of this process is system_u:system_r:geoclue_t:s0)

Now the own-creds test:

GTest: run: /own-creds
(MSG: ProcessID of message bus is 10031)
(MSG: UnixUserID of message bus is 1000)
(MSG: LinuxSecurityLabel of message bus is system_u:object_r:unlabeled_t:s0)
GTest: result: OK

This doesn't sounds good.

I'll try again later with the patched version installed

This is what I get with the installed version:


bigon@valinor:~$ dbus-send --session --dest=org.freedesktop.DBus     --type=method_call --print-reply / org.freedesktop.DBus.GetConnectionCredentials 'string:org.freedesktop.DBus'
method return time=1497283592.377754 sender=org.freedesktop.DBus -> destination=:1.63 serial=3 reply_serial=2
   array [
      dict entry(
         string "ProcessID"
         variant             uint32 1955
      )
      dict entry(
         string "UnixUserID"
         variant             uint32 1000
      )
      dict entry(
         string "LinuxSecurityLabel"
         variant             array of bytes "system_u:object_r:unlabeled_t:s0" + \0
      )
   ]
bigon@valinor:~$ dbus-send --system --dest=org.freedesktop.DBus     --type=method_call --print-reply / org.freedesktop.DBus.GetConnectionCredentials 'string:org.freedesktop.DBus'
method return time=1497283612.510121 sender=org.freedesktop.DBus -> destination=:1.102 serial=3 reply_serial=2
   array [
      dict entry(
         string "ProcessID"
         variant             uint32 719
      )
      dict entry(
         string "UnixUserID"
         variant             uint32 105
      )
      dict entry(
         string "LinuxSecurityLabel"
         variant             array of bytes "system_u:object_r:unlabeled_t:s0" + \0
      )
   ]

The session dbus-daemon process is running as "unconfined_u:unconfined_r:unconfined_t":

unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 bigon 1955 0.0  0.0 54212 5032 ? Ssl 18:06   0:00 /usr/bin/dbus-daemon --session --address=systemd: --nofork --nopidfile --systemd-activation

The system one is running as "system_u:system_r:system_dbusd_t":

system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 message+ 719 0.2  0.0 55896 5724 ? Ssl 18:05   0:01 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation --syslog-only

The "object_r" role is most of the time used for passive objects like files or sockets. Active processes are often using the "system_r" or "unconfined_r" ones.

(In reply to Laurent Bigonville from comment #16)
>       dict entry(
>          string "LinuxSecurityLabel"
>          variant             array of bytes
> "system_u:object_r:unlabeled_t:s0" + \0
>       )

According to https://github.com/systemd/systemd/issues/6120#issuecomment-308216855 my trick with using a socketpair() in Attachment #131743 [details] can't work, because socketpair() doesn't have the same LSM hooks as the usual listen()+connect()+accept() sequence.

We could make Attachment #131743 [details] use a "real" (path-based or abstract) socket, but, ugh. (And I'm not sure it's possible to do that synchronously.) So GetConnectionCredentials might have to just not provide the dbus-daemon's own LSM label unless/until LSM people can agree on what is or isn't reasonable behaviour for an LSM, or have LSM-specific code (that knows how to normalize labels to what would appear in SO_PEERSEC) for any LSMs that want this. :-(

However, https://github.com/systemd/systemd/issues/6120 has fixed this from the systemd side anyway, so perhaps nothing more is needed and my remaining patches on this bug should just be abandoned?

`socketpair()` now has the correct security credentials in recent kernels: <https://github.com/torvalds/linux/commit/aae7cfcbb733cf16f3bc9cbb650673b94d5df75f>.

-- 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/dbus/dbus/issues/177.