Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbus filter profiles (1) #3326

Merged
merged 5 commits into from
May 2, 2020

Conversation

rusty-snake
Copy link
Collaborator

@rusty-snake rusty-snake commented Apr 7, 2020

Opening as PR so we can disscuss about them. The most are just extracted from flathub, see #3265 (comment).

EDIT: everyone fell free to push more profiles here.

@rusty-snake
Copy link
Collaborator Author

BTW: firejail --noprofile --dbus-user=filter --audit | grep D-Bus MAYBE: D-Bus socket /run/firejail/mnt/dbus/user is available.

Copy link
Collaborator

@kris7t kris7t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, thanks for creating the DBus profiles!

In the previous weeks, I was running the DBus filter with much tighter rules. As I use sway, not Gnome, desktop integration wasn't a problem, so I may be overly vigilant. Nevertheless, below I made some comments regarding the filter rules from flathub.

Comment on lines 52 to 53
dbus-user.talk org.gtk.vfs.*
dbus-user.talk org.gtk.vfs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is allowing Gvfs access safe? Could for example a malicious application access any Gvfs mount (and potentially e.g., spread ransomware to an unsecured samba mount, using the credentials already provided to Gvfs by the user)?

Also, I don't think you need org.gtk.vfs here, org.gtk.vfs.* covers it as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved other org.gtk.vfs commend in order to have one thread.

Your right, the most programs don't need it.

TODO: can it be used to bypass disable-mnt/blacklist/whitelist.

etc/rhythmbox.profile Outdated Show resolved Hide resolved
etc/feedreader.profile Outdated Show resolved Hide resolved
# - In order to make dconf work (if it is used by the app) you need to allow
# 'ca.desrt.dconf' even if it is not allowed by flatpak.
#dbus-user filter
#dbus-user.own com.github.netblue30.firejail
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing any firejailed application to own the name com.github.netblue30.firejail (and potentially impersonate firejail, if we ever add any custom DBus interfaces) sounds a bit risky.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats just an example, it would not pass any review. However we can change it to com.example.appname.

# 'ca.desrt.dconf' even if it is not allowed by flatpak.
#dbus-user filter
#dbus-user.own com.github.netblue30.firejail
#dbus-user.talk ca.desrt.dconf
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether unrestricted DConf access is always a good idea. A malicious application could, for example, change stuff under org.gnome.system.proxy to MITM traffic.

Luckily, users could just ignore dbus-user.talk ca.desrt.dconf in globals.local, and selectively enable it for applications that absolutely need it.

I wonder what's the flatpak solution for this vulnerability.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what's the flatpak solution for this vulnerability.

https://docs.flatpak.org/en/latest/sandbox-permissions.html#dconf-access, a lot of flatpaks still use direct access. gnome-builder suggests it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A malicious application could, for example, change stuff under org.gnome.system.proxy to MITM traffic.

Or even worser: /org/gnome/terminal/legacy/profiles:/<UUID>/custom-command

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only added it there as an example since it is common used.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A sandboxed dconf similar to flatpak's would be a logical next step after DBus

dbus-user.own org.gnome.FeedReader.ArticleView
dbus-user.talk org.freedesktop.Notifications
dbus-user.talk org.gtk.vfs.*
dbus-user.talk org.freedesktop.secrets
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the access policy of the secret service? I think at least Gnome keyring only asks for the user password once (at most, never if PAM unlocks it), and provides secrets to application without any confirmation required. Although arguably, that's a vulnerability in the secret service implementation (I'm pretty sure keepassxc can be set to ask for confirmation before releasing credentials), not in the sandbox.

etc/ghostwriter.profile Outdated Show resolved Hide resolved
@kris7t
Copy link
Collaborator

kris7t commented Apr 7, 2020

BTW: firejail --noprofile --dbus-user=filter --audit | grep D-Bus MAYBE: D-Bus socket /run/firejail/mnt/dbus/user is available.

Well, that socket is the proxy socket, so it should be present. But yeah, I should fix the audit so it does not emit a warning, it makes no sense to warn for a properly running filter.

@@ -28,5 +28,11 @@ include whitelist-usr-share-common.inc
# private-etc must first be enabled in firefox-common.profile
#private-etc firefox

dbus-user filter
dbus-user.own org.mozilla.firefox.*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting with Firefox 75, there should probably also be a dbus-user.own org.mpris.MediaPlayer2.firefox.* for hardware media key support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to start minimal, there a lot more path for firefox + extensions. The policy used by the inoffical firefox-flatpak:

dbus-user filter
dbus-user.own org.mozilla.Firefox
dbus-user.own org.mozilla.firefox.*
dbus-user.talk org.freedesktop.FileManager1
dbus-user.talk org.freedesktop.Notifications
dbus-user.talk org.freedesktop.ScreenSaver
dbus-user.talk org.gnome.SessionManager
dbus-system filter
dbus-system.talk org.freedesktop.NetworkManager

Copy link
Collaborator

@kris7t kris7t Apr 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoah, org.freedesktop.NetworkManager sounds quite evil in this context (MITM via adding a VPN, or leaking IP and MAC addresses of interfaces otherwise isolated with --net=).

@Fred-Barclay
Copy link
Collaborator

@rusty-snake I was going to replace all nodbus in profiles with

dbus-system none
dbus-user none

as a general starting point

Ok to push here or should I open a new PR? It's probably lots of files so didn't want to clutter up this PR.

@rusty-snake
Copy link
Collaborator Author

@Fred-Barclay if you push upstream, I can rebase.

Nitpick: I used dbus-user before dbus-system.

@Fred-Barclay
Copy link
Collaborator

@rusty-snake I'm usually bad at alphabetising... in this case though for consistency's sake I'd guess dbus-system before dbus-user. What do you think?

@rusty-snake
Copy link
Collaborator Author

The must profiles will disable dbus or look like the example below. For my eyes it was nicer to have a dbus-system none at the bottom instead of the top.

dbus-user filter
dbus-user.own com.foo.bar
dbus-user.talk ca.desrt.dconf
dbus-user.talk org.freedesktop.smile
dbus-system none

The addresses (grouped by own and talk) should be alphabetical sure.

Fred-Barclay added a commit that referenced this pull request Apr 7, 2020
See
- 07fac58 for new dbus filters
- #3326 (comment)

Except for ocenaudio, access/restrictions on dbus options should
be unchanged

Ocenaudio profile: dbus filters were sandboxed (initially `nodbus`
was enabled) since comments indicated blocking dbus meant
preferences were broken
@Fred-Barclay
Copy link
Collaborator

@rusty-snake done, see 3848b98

There are a few small changes left, like how to handle ?HAS_NODBUS, but that should affect >5 files.

dbus-user.own org.mozilla.firefox.*
dbus-user.talk org.freedesktop.Notifications
dbus-system none
ignore nodbus
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get why we need to have ignore nodbus here. Does that mean we can remove it from firefox-common-addons.inc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now needs to be ignore dbus-user none.

firefox-common.profile is used by firefox, thunderbird, waterfox, ... and disables D-Bus access. Of course we can't add the own rule to firefox-common. Because some program throw errors messages if D-Bus isn't available, but hardfail on D-Bus error due to work policy, I added this for now only to firefox.

@glitsj16
Copy link
Collaborator

glitsj16 commented Apr 8, 2020

Suppose a profile has dbus-user.talk org.freedesktop.Notifications and a user wants to override that, and only that. Is ignore still the correct/preferred way to do this with regards to the DBus filters? If so, I'll add an example to man firejail-profile to avoid potential confusion.

glitsj16 added a commit that referenced this pull request Apr 8, 2020
@rusty-snake
Copy link
Collaborator Author

I created a short list about some of the D-Bus addresses.

Legend:
# |0| => harmless
# |!| => Danger sandbox escape possible
# |!!| => Very dangerous
# |$| => sensitive

# |0| MediaKeys
org.gnome.SettingsDaemon.MediaKeys
# |!| Dconf
ca.desrt.dconf
# |0| NightMode/Screen temperaur
org.gnome.SettingsDaemon.Color
# |!!| Native notifications
org.freedesktop.Notifications
# |$| Keyring
org.freedesktop.secrets
# |$|
org.gnome.OnlineAccounts
# |0|
org.mpris.MediaPlayer2.APP
# |!!| Filesystem access (blacklist/whitelist esacape)
org.freedesktop.FileManager1
# |$| Unlock screen
org.freedesktop.ScreenSaver
# |$| reboot(?)
org.gnome.SessionManager
# |!|
org.freedesktop.NetworkManager
# |$|
org.gnome.evolution.dataserver.*
# |$|
org.freedesktop.GeoClue2
# |!|
org.gnome.Shell
# |!!|
org.gnome.Shell.*
# |!|
org.freedesktop.login1
# |!|
org.gtk.vfs.*


Things that `org.freedesktop.Notifications` can do:

# Screenshots

firejail --noprofile --dbus-user=filter --dbus-user.talk=org.freedesktop.Notifications dbus-send --session --print-reply --dest=org.freedesktop.Notifications /org/gnome/Shell/Screenshot org.gnome.Shell.Screenshot.Screenshot boolean:false boolean:false string:screenshot.png

# Unlock your screen

firejail --noprofile --dbus-user=filter --dbus-user.talk=org.freedesktop.Notifications dbus-send --session --print-reply --dest=org.freedesktop.Notifications /org/gnome/ScreenSaver org.gnome.ScreenSaver.SetActive boolean:false

# Install gnome-shell-extensions

# /org/gnome/Shell org.gnome.Shell **Eval** any code (gjs)

# /org/gnome/Mutter/ScreenCast org.gnome.Mutter.ScreenCast

Record your screen

# /org/gnome/keyring/Prompter org.gnome.keyring.internal.Prompter

IDK what it can, but keyring sounds not good.

# /org/gnome/SessionManager/EndSessionDialog org.gnome.SessionManager.EndSessionDialog

Logout; Poweroff?; Reboot?

# /org/gnome/Mutter/RemoteDesktop org.gnome.Mutter.RemoteDesktop

forward desktop

# /org/gnome/Mutter/DisplayConfig

turnoff the monitor

# force you to hard-poweroff

@Fred-Barclay
Copy link
Collaborator

@rusty-snake Awesome, care to toss it in the wiki?

@rusty-snake
Copy link
Collaborator Author

👍 Definitely something for the wiki. with more comments/descriptions and better formating.


What policy should be apply for org.freedesktop.Notifications?

@glitsj16
Copy link
Collaborator

What policy should be apply for org.freedesktop.Notifications?

That's one of the reasons I wanted to add an example to the firejail-profile man page. I admit not spending a whole lot of time on it and decided to add an example on how to override a given setting. But for this I would have liked a dbus-system.talk org.freedesktop.Notifications none option (or something similar). Probably not easily - if at all - implementable, but I for one will be scanning future profiles that allow it and do whatever I can to disable. Just a personal opinion...

@rusty-snake
Copy link
Collaborator Author

flatpak like: dbus-user.no-talk

I would say making it opt-in for whitelisting profiles.

@kris7t
Copy link
Collaborator

kris7t commented Apr 11, 2020

something like dbus-user.no-talk is potentially tricky (originally, I considered it, but decided not to implement for now) due to the presence of .* patterns in talk rules. Basically, what should happen if we have (fictitious example, obvious not a good idea to apply these exact rules):

dbus-user.talk org.freedesktop.*
dbus-user.no-talk org.freedesktop.Notifications

The challenge here is that, as far as I could find, there is no way to pass negative rules to xdg-dbus-proxy. We could resort to an approach similar to filesystem whitelisting, i.e., allow all names below org.freedesktop current present on the bus except Notifications, but that is complicated (we have to connect to the bus and list all names) and wrong (new names may appear all the time).

Alternatively, we could just throw an error if a no-talk rule is superseded by a talk rule (i.e., a no-talk with no corresponding talk rule or a talk rule which would whitelist that exact pattern of bus name(s) is ok, but anything more general than that is an error). That's pretty much what an ignore command does, tho.

I should really look into how Flatpak handles this.

On the other hand, xdg-dbus-proxy can disable dbus interfaces and methods in a quite fine-grained manner, so the Notifications issue (with the rest of Gnome Shell being practically exposed) can be handled more easily.

@rusty-snake
Copy link
Collaborator Author

I would say dbus-user.no-talk can act like (no)blacklist, path/address of the no command must match the path/address of the blacklist/talk command.

@glitsj16
Copy link
Collaborator

@kris7t Very informative ^. I might have missed it, but have there been suggestions on how to deal with org.freedesktop.portal.* (see here) yet?

@rusty-snake
Copy link
Collaborator Author

@kris7t
Copy link
Collaborator

kris7t commented Apr 13, 2020

@glitsj16 Unfortunately, I don't know much about xdg-portals. I think Gnome has some kind of settings dialog for at least flatpak app permissions (filesystem access, webcam, microphone, etc.). I don't know whether that's just black/whitelisting the D-Bus names of portals, or something implemented inside the portals themselves.

@rusty-snake
Copy link
Collaborator Author

From my side we could merge.

@glitsj16
Copy link
Collaborator

glitsj16 commented May 2, 2020

From my side we could merge.

@rusty-snake LGTM. Great work and a big thank you to all involved!

@rusty-snake rusty-snake merged commit 8744e08 into netblue30:master May 2, 2020
@rusty-snake rusty-snake deleted the dbus-filter-profiles branch May 2, 2020 18:05
glitsj16 added a commit that referenced this pull request May 4, 2020
* use the new dbus format in chromium-common.profile

* use new dbus format in firejail.config

Now that #3326 landed I think it might be less confusing to keep using the --nodbus wording. Couldn't come up with a better alternative (yet), so this might need future improvements.

* block dbus system bus

Blocking the system bus shouldn't affect password functionality etc, as that uses the session bus.
@matu3ba matu3ba mentioned this pull request Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants