Fine-grained DBus sandboxing

[kris7t]

This PR adds the dbus-user and dbus-system options to individually control access to the session and system DBus buses, as per #3184. Access policies to the buses can be allow, which completely allows the bus, filter, which runs xdg-dbus-proxy, and none, which disables access. The nodbus options, which is equivalent to dbus-user none dbus-system none is kept for compatibility.

Filter rules for xdg-dbus-proxy can be specified with the dbus-user.talk, dbus-user.own, dbus-system.talk, and dbus-system.own options. While xdg-dbus-proxy implements finer-grained rules, these four should hopefully be enough to convert filter rules from Flatpak manifests.

On the implementation side:

• The new filter rules are added to the profile similarly to blacklist and whitelist commands.
• xdg-dbus-proxy runs outside the sandbox namespace (to maintain access to the original DBus sockets), but contained by a modified version of sbox_run. It is linked to the parent firejail process by a pipe fd, closing which triggers its exit, so it (hopefully) quits when the parent firejail process exits for any reason.
• Filtered DBus sockets are in /run/firejail/dbus and are bind-mounted to their usual locations inside the sandbox. Even the system DBus socket is owned the user running the sandbox, not root, because if we chown it to root, xdg-dbus-proxy cannot clean them up when it exits (an the parent firejail process might not be around to clean the up instead). So inside the sandbox, the system DBus socket is owned by a normal user. This does not cause any problems when connecting to the bus, but might be used to detect the sandbox.

This PR does not contain any profile changes, so all profiles remain to constrain DBus access in a coarse-grained way. Hopefully, in the future, profiles can be updated to take advantage of DBus filtering, but that might require a hard dependency on xdg-dbus-proxy, or some auto-detection mechanism.

[kris7t]

I have been using this for a couple of days with local profiles and it seems to work fine. For example,

dbus-user filter
dbus-user.own org.mpris.MediaPlayer2.spotify
dbus-user.talk org.freedesktop.Notifications
dbus-system none

Lets Spotify display notifications and be controller over MPRIS.

However, the situation is not as rosy with e.g. Electron based apps. They need a rather permissive

dbus-user.own org.kde.*

because they need to own names in the form org.kde.StatusNotifierItem-40-819 (with the two numbers variable) to display tray icons -- tray icons and Chromium are quite wonky, anyways, since it writes they tray icons directly into /tmp (and thus cannot be run with private-tmp).

I also tried to use desktop portals with

dbus-user.talk org.freedesktop.portal.*

and GTK_USE_PORTAL=1, but had no success yet. Perhaps I am missing some components on the DE side.

[Vincent43]

However, the situation is not as rosy with e.g. Electron based apps. They need a rather permissive

dbus-user.own org.kde.*

because they need to own names in the form org.kde.StatusNotifierItem-40-819 (with the two numbers variable) to display tray icons -- tray icons and Chromium are quite wonky, anyways, since it writes they tray icons directly into /tmp (and thus cannot be run with private-tmp).

Hm, I knew about those tray icon issues for qt apps but electron apps (which are more gtk than qt) in flatpak are able to show tray icon with just --talk-name=org.kde.StatusNotifierWatcher

The /tmp issue may be fixed with TMPDIR env variable. Perhaps pointing it to /run/user/xxxx aka $RUNUSER would work most widely as it's shared with host most of the time.

I also tried to use desktop portals with

dbus-user.talk org.freedesktop.portal.*

and GTK_USE_PORTAL=1, but had no success yet. Perhaps I am missing some components on the DE side.

Installing xdg-desktop-portal & xdg-desktop-portal-{gtk,kde} and enabling systemd user services (if they aren't already):

xdg-desktop-portal.service
xdg-document-portal.service
xdg-permission-store.service

should be sufficient. Did you tested apps that actually support portals? AFAIK chromium/electron doesn't. Note that for Qt apps you need to use SNAP=1 instead of GTK_USE_PORTAL=1

[kris7t]

Hm, I knew about those tray icon issues for qt apps but electron apps (which are more gtk than qt) in flatpak are able to show tray icon with just --talk-name=org.kde.StatusNotifierWatcher

I tried that, but it failed to display an icon & complained about not owning the well-known name for the icon. Maybe because I am trying to run Wavebox 10, which is not "really" an Electron app (more like a forked Chromium, with some Electron bits).

The /tmp issue may be fixed with TMPDIR env variable. Perhaps pointing it to /run/user/xxxx aka $RUNUSER would work most widely as it's shared with host most of the time.

Thanks for these pointers!

Did you tested apps that actually support portals? AFAIK chromium/electron doesn't.

Oh that's a pity.

[rusty-snake]

• manpage: example filter
• For what is dbus-* allow? Not using dbus-* at all will also allow D-Bus access.
• just noting: nodbus vs. dbus-* allow: the later will win
• future of HAS_NODBUS?

meld, rhytmbox, gnome-maps, gfeeds

diff --git a/etc/gfeeds.profile b/etc/gfeeds.profile
index d332c1bb..b68fd864 100644
--- a/etc/gfeeds.profile
+++ b/etc/gfeeds.profile
@@ -37,7 +37,6 @@ caps.drop all
 machine-id
 netfilter
 no3d
-#nodbus
 nodvd
 nogroups
 nonewprivs
@@ -57,3 +56,8 @@ private-bin gfeeds,python3*
 private-dev
 private-etc alternatives,ca-certificates,crypto-policies,dbus-1,dconf,fonts,gconf,group,gtk-3.0,host.conf,hostname,hosts,ld.so.cache,ld.so.conf,ld.so.conf.d,ld.so.preload,locale,locale.alias,locale.conf,localtime,machine-id,mime.types,nsswitch.conf,pango,passwd,pki,protocols,resolv.conf,rpc,services,ssl,X11,xdg
 private-tmp
+
+dbus-user filter
+dbus-user.own org.gabmus.gfeeds
+dbus-user.talk ca.desrt.dconf
+dbus-system none
diff --git a/etc/gnome-maps.profile b/etc/gnome-maps.profile
index 62350b86..2abef929 100644
--- a/etc/gnome-maps.profile
+++ b/etc/gnome-maps.profile
@@ -62,3 +62,16 @@ private-bin gjs,gnome-maps
 private-dev
 private-etc alternatives,ca-certificates,clutter-1.0,crypto-policies,dconf,drirc,fonts,gconf,gcrypt,gtk-3.0,host.conf,hostname,hosts,ld.so.cache,ld.so.conf,ld.so.conf.d,ld.so.preload,locale,locale.alias,locale.conf,localtime,mime.types,nsswitch.conf,pango,pkcs11,pki,protocols,resolv.conf,rpc,services,ssl,X11,xdg
 private-tmp
+
+dbus-user filter
+dbus-user.own org.gnome.Maps
+dbus-user.talk org.gnome.OnlineAccounts
+dbus-user.talk org.gnome.evolution.dataserver.AddressBook9
+dbus-user.talk org.gnome.evolution.dataserver.Calendar7
+dbus-user.talk org.gnome.evolution.dataserver.Sources5
+dbus-user.talk org.gnome.evolution.dataserver.Subprocess.Backend.*
+dbus-user.talk org.freedesktop.secrets
+dbus-user.talk ca.desrt.dconf
+dbus-system filter
+dbus-system.talk org.freedesktop.NetworkManager
+dbus-system.talk org.freedesktop.GeoClue2
diff --git a/etc/meld.profile b/etc/meld.profile
index 9a320c13..1e37ff9c 100644
--- a/etc/meld.profile
+++ b/etc/meld.profile
@@ -69,4 +69,9 @@ private-dev
 #private-etc alternatives,ca-certificates,crypto-policies,fonts,hostname,hosts,pki,resolv.conf,ssl,subversion
 private-tmp
 
+dbus-user filter
+dbus-user.own org.gnome.meld
+dbus-user.talk ca.desrt.dconf
+dbus-system none
+
 read-only ${HOME}/.ssh
diff --git a/etc/rhythmbox.profile b/etc/rhythmbox.profile
index ad8b1015..0f70b4e3 100644
--- a/etc/rhythmbox.profile
+++ b/etc/rhythmbox.profile
@@ -31,7 +31,6 @@ include whitelist-var-common.inc
 # apparmor - makes settings immutable
 caps.drop all
 netfilter
-# nodbus - makes settings immutable
 nogroups
 nonewprivs
 noroot
@@ -46,3 +45,15 @@ tracelog
 private-bin rhythmbox,rhythmbox-client
 private-dev
 private-tmp
+
+dbus-user filter
+dbus-user.own org.gnome.Rhythmbox3
+dbus-user.own org.gnome.UPnP.MediaServer2.Rhythmbox
+dbus-user.own org.mpris.MediaPlayer2.rhythmbox
+dbus-user.talk ca.desrt.dconf
+dbus-user.talk org.freedesktop.Notifications
+dbus-user.talk org.gnome.SettingsDaemon.MediaKeys
+dbus-user.talk org.gtk.vfs
+dbus-user.talk org.gtk.vfs.*
+dbus-system filter
+dbus-system.talk org.freedesktop.Avahi

EDIT: add gfeeds

[kris7t]

manpage: example filter

Thanks, good catch!

For what is dbus-* allow? Not using dbus-* at all will also allow D-Bus access.

It seemed to be the logical thing to do. On the other hand, maybe only --dbus-*=filter and --dbus-*=none should be supported (and DBUS_POLICY_ALLOW should only be the default value for the arg_dbus_* variables in the program, not something user selectable).

just noting: nodbus vs. dbus-* allow: the later will win

One way to fix this would be to force the policies to the always strengthened by subsequent profile entries, i.e., --nodbus --dbus-user=filter should throw an error. Any thoughts on whether this behavior would be preferable?

future of HAS_NODBUS?

Logically, a sandbox with blocked DBus HAS_NODBUS, but maybe there should be conditionals for other settings, too.

[rusty-snake]

More conditions? Sure. I mean what will/should be the behaviour for e.g.

dbus-user allow
dbus-system none
?HAS_NODBUS: blacklist /etc/fstab

[kris7t]

That's a good question. Searching through the firejail repo in order to find uses of ?HAS_NODBUS that are hopefully similar to uses in other profiles in the wild, I could basically find two:

• In https://github.com/netblue30/firejail/blob/df1c73a00f68b3ee2503b75d3220e65f99a7f760/etc/disable-common.inc, the ksocket is blacklisted when DBus is blacklisted to tighten the sandbox. In this case, we should probably apply the ?HAS_NODBUS condition if any kind of DBus filtering is in effect (since we don't filter the ksocket).
• In https://github.com/netblue30/firejail/blob/32c3669115a7168e5a7fa13347bd6f8daf838be0/etc/chromium-common.profile, ?HAS_NODBUS is used to fix a regression when DBus is not available. If we apply ?HAS_NODBUS even if there is some DBus access, at worst we apply an unnecessary workaround, but do not degrade functionality (compared to the case when DBus was not available at all).

To sum up, setting ?HAS_NODBUS if any DBus filtering or blocking is in effect is safe for profiles which use it for optionally tightening the sandbox, while does not degrade profiles that use it to apply workarounds. Profiles of the latter kind could be eventually updated to use fine-grained filters to avoid the regressions in the first place.

[smitsohu]

Naive question: Would it be possible to run the proxy early as a child of the sandbox process?

EDIT: There is actually not much of a difference, but running it inside all of the sandbox namespaces maybe could add a bit extra peace of mind.

[smitsohu]

Naive question: Would it be possible to run the proxy early as a child of the sandbox process?

Or maybe that's not a good idea. If it runs in the same pid namespace, it can be vulnerable to attacks with ptrace, which the current design avoids.

[kris7t]

I was originally trying to run the proxy inside the sandbox, but I had to realize that

1. It connects to the original DBus socket only when a client connects to the proxy, so bind-mounting the proxy socket over the original one creates an infinite loop once a client tries to connect to the proxy socket (which causes the proxy to connect to the original socket at the given path, but finds the bind-mounted socket there instead).
2. Even if I passed the a handle to the original socket to the proxy that does not get overwritten (an O_PATH fd combined with the bus address unix:path=/proc/self/fd/<fd> works, provided /proc is mounted), the proxy would have to run as the user running the sandbox, which means I cannot easily isolate the /proc/<pid>/fd/<fd> that is a link to the original socket.
3. As you noted, there is a possible attack with ptrace.

We could create a "nesting doll" construction, where a essentially an "outer" set of namespaces isolates the sandbox from the rest of the world, and an "inner" set of namespaces isolates the program being sandboxed from the sandbox implementation. The "outer" container would be a nice place to run not only xdg-dbus-proxy, but also stuff like Xephyr. On the other hand, this would be a significant architectural change that nets relatively little security improvement.

[kris7t]

I revised the options --dbus-user and --dbus-system:

• Not there is not allow policy, only filter and block.
• The policy cannot be relaxed, only tightened. If we set --nodbus, then --dbus-user=filter will fail. Nevertheless, a .local profile can override the built-in nodbus command by e.g. ignore nodbus and dbus-user filter, dbus-system block.
• Adding a filter rule requires the filter or block policy. The block policy in conjunction with filter rules will produce a warning.

I am still not entirely satisfied by this design: to relax a profile from filtering to DBus allowed in a .local profile, we not only need to ignore dbus-user filter, but also ignore all filter rules. I don't know whether that's a good thing. On the one hand, it's pretty convenient. On the other hand, this forces the user to review any changes to filter rules in the main profile (and decide whether leaving DBus completely open is still warranted).

[abranson]

Thanks for this work - this is exactly what I've been looking for! One note - some systemd based systems use a different path for the user dbus socket: /run/user/%d/dbus/user_bus_socket. These should have the correct path set in the DBUS_SESSION_BUS_ADDRESS env var. Perhaps this should be checked first, then if it's not set fallback to /run/user/%d/bus.

[smitsohu]

Nice!

[kris7t]

@rusty-snake @glitsj16 What do you think about the interface for profiles? With the changes (#3265 (comment)) it is hopefully a bit harder to misuse, and compatibility with nodbus profiles should still be preserved.

[rusty-snake]

What happen if xdg-dbus-proxy isn't installed (and dbus-user filter is set)?

[kris7t]

@rusty-snake Currently, firejail will exit(1) if /usr/bin/xdg-dbus-proxy is not found. Alternatively, we could do one of the following:

1. We could fall back to --*-dbus=none in that case, which is always safe (but could nevertheless break applications, if we update profiles without nodbus to e.g., dbus-user.talk).
2. We could fall back to ignoring --*-dbus, which is not safe, but won't break any profile that currently does not have nodbus.
3. In addition to 1 or 2, we could add a conditonal (e.g., HAS_DBUS_FILTER or NO_HAS_DBUS_FILTER) to control fallback behavior per profile. For example, if we chose blocking DBus access as the fallback behavior, we can add

?HAS_DBUS_FILTER: dbus-user filter
?HAS_DBUS_FILTER: dbus-user.talk some.name.*

to profiles that currently does not have nodbus.

However, 3 hints at the suboptimality of the current filter rule design: if e.g.). dbus-user.talk causes an error unless dbus-user filter is set, it is very inconvenient to conditionally enable filters (we must repeat ?HAS_DBUS_FILTER:). Maybe we should downgrade that error to a warning.

[glitsj16]

@kris7t Just a FYI, I'm following the work you have been doing here without explicitly participating in the discussion. As @rusty-snake is on to it, we should be fine to adjust profiles when this gets merged. Very much appreciated!

[smitsohu]

Apologies for coming back, just one more idea: Would it make sense to run dbus_proxy_start with dropped privileges and then start the proxy in its own Firejail sandbox?

[kris7t]

@smitsohu Yeah, I was thinking about that, since the proxy only needs access to /run/firejail/dbus/<uid>/, the original DBus sockets, and not much else. So sandboxing it would definitely be neat. Maybe we could even have a xdg-dbus-proxy.profile and just exec /proc/self/exe with some special arguments (adding the original sockets to --whitelist, and setting --net=none if neither of the original sockets is an abstract socket).

One compilcation is the handling of the file descriptors, though. Perhaps we could add a general-purpose --keep-fd= flag to pass open file descriptors to the sandbox (close them in the main firejail process, but keep them open in the forked namespace).

On the other hand, xdg-dbus-proxy is running within a seccomp sandbox already (via sbox_exec). I don't know whether having a dedicated namespace for the proxy improves security at all, since if you can take over the proxy in any way, you are already able to send any DBus message to session bus you want. That's plenty of tools to escape, e.g., in Gnome, you can spawn a new Terminal with any command, or run any javascript code in the Shell.

[smitsohu]

@kris7t

I don't know whether having a dedicated namespace for the proxy improves security at all, since if you can take over the proxy in any way, you are already able to send any DBus message to session bus you want. That's plenty of tools to escape, e.g., in Gnome, you can spawn a new Terminal with any command, or run any javascript code in the Shell.

You're right, I didn't think of that. The benefits of a full sandbox would be limited.

[rusty-snake]

@kris7t I'm with @glitsj16 once this is in master the profile can be modified.

As a follow-up: xdg-dbus-proxys --log argument. It logs all the dbus-traffic to stdout (needs to be redirected). Either as --dbus-log (just a simple (maybe raw) log) or better as --dbus-tracelog to match any traffic against the policy in the profile. This could be very help-full while writing profiles or in issue about broken profiles because e.g. gtk only throws a Failed to register: GDBus.Error:org.freedesktop.DBus.Error.ServiceUnknown: org.freedesktop.DBus.Error.ServiceUnknown and does not tell which path it can't register.

[rusty-snake]

@kris7t can you rebase.

[kris7t]

@rusty-snake done.

[Fred-Barclay]

Looks good! One minor request -- let's don't remove nodbus from man firejail-profile

Cheers!
Fred

[kris7t]

Big thanks to everyone for the reviews and comments!

As a next step, I'll work on adding DBus logging (#3265 (comment)) to help profile creation.