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

profiles: discord-common: harden & allow notifications #5978

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

haarp
Copy link
Contributor

@haarp haarp commented Aug 27, 2023

Tested some more directives by removing the ignores. Also add directives necessary for dbus notifications (fixes #5971).

What works:

  • Basic functionality
  • Receiving notifications
  • Voice communication
  • Watching streams

What wasn't tested:

  • Casting streams
  • Opening links
  • Tracking/displaying "current activity" as status message
  • Apparmor

Notes:

  • Discord tries to access system dbus ([ERROR:bus.cc(399)] Failed to connect to the bus: Failed to connect to socket /run/firejail/mnt/dbus/system: Permission denied). I don't know what business it has with the system dbus, and didn't notice any problems due to that.
  • I had one crash after 2h of watching a stream. Probably unrelated.

Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for testing!

@glitsj16
Copy link
Collaborator

Discord tries to access system dbus ([ERROR:bus.cc(399)] Failed to connect to the bus: Failed to connect to socket /run/firejail/mnt/dbus/system: Permission denied). I don't know what business it has with the system dbus, and didn't notice any problems due to that.

Lots of applications act similarly. As you've noticed we can usually block access to the system bus without crippling functionality and harden the sandbox that way.

Opening links

You can add the below to support opening links. We use this in several profiles already and it would be a nice addition to do the same for discord.


# The lines below are needed to find the default Firefox profile name, to allow
# opening links in an existing instance of Firefox (note that it still fails if
# there isn't a Firefox instance running with the default profile; see #5352)
noblacklist ${HOME}/.mozilla
whitelist ${HOME}/.mozilla/firefox/profiles.ini

# allow D-Bus communication with firefox for opening links
dbus-user.talk org.mozilla.*

Tracking/displaying "current activity" as status message

Probably needs an additional dbus-user.own foo. You can see what D-Bus addresses discord offers by running d-feet (or similar tool) while discord i running and checking the session bus tab. Just a FYI, I'm not sure if people using discord with Firejail consider this a vital functionality. And we can add this kind of additional functionality later.

Apparmor

IMO we can enable apparmor. On a standard Linux distro that uses AppArmor it shouldn't break anything, unless a vital part of discord is installed under the user's home dir. And in the latter case we already have comments in the relevant AA profile on how to cover such setups.

All in all your testing allows to harden discord.profile rather nicely already. Thank you for taking the time to check!

@haarp
Copy link
Contributor Author

haarp commented Aug 28, 2023

You can add the below to support opening links. We use this in several profiles already and it would be a nice addition to do the same for discord.

Can you give me a quick howto or link documentation on opening links in general? I've been searching for this for the past weeks and couldn't make it work (for any app). There's a draft PR and accompanying discussion, which indicate that inter/extra-jail link opening is still in the planning stages.

As of now, even with your suggested directives, Discord will instead attempt to use xdg-open for links, which of course fails. I will happily add dbus link opening once I have it working.

Probably needs an additional dbus-user.own foo. You can see what D-Bus addresses discord offers by running d-feet (or similar tool) while discord i running and checking the session bus tab. Just a FYI, I'm not sure if people using discord with Firejail consider this a vital functionality. And we can add this kind of additional functionality later.

Will have a look at that later.

@glitsj16
Copy link
Collaborator

glitsj16 commented Aug 28, 2023

Can you give me a quick howto or link documentation on opening links in general? I've been searching for this for the past weeks and couldn't make it work (for any app). There's a #5574 and #5566, which indicate that inter/extra-jail link opening is still in the planning stages.

Besides the links you've already referenced I'd have a look at The firejail URL open game thread at #5364.

As of now, even with your suggested directives, Discord will instead attempt to use xdg-open for links, which of course fails. I will happily add dbus link opening once I have it working.

My ommission, apologies. I forgot that discord-common.profile uses private-bin. Try allowing Firefox by adding firefox to it.

For debugging these URL opening situations I sometimes use a basic wrapper script for xdg-open like the below to echo out what it receives from sandboxed apps. Might help to double-check what discord is doing when running Firejailed.

$ cat ~/bin/xdg-open
#!/bin/sh

### vars
_app="xdg-open"
_bin="/usr/bin/${_app}"
_debug_log="/home/${USER}/Downloads/${_app}.debug"
_msg="[${_app}] debug info"

### logic
_parent="$(ps -o args= "$PPID")"
{
    echo "# ${_msg}"
    echo "# $(date +%Y.%m.%d\ \@\ %H:%M:%S)"
    echo "# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -"
    echo
    echo "parent: ${_parent}"
    echo "params:"
    echo "\$@: ${@}"
} > "$_debug_log"

${_bin} "$@"

@glitsj16
Copy link
Collaborator

Some clarifications on enabling the apparmor directive.

As mentioned here I've installed discord and did some additional testing too. We can add a comment for users that want to enable apparmor, pretty much like we do in other relevant profiles.

If you don't feel comfortable adding it to this PR that's fine, I can always add it in a follow-up PR myself.

$ cat /etc/apparmor.d/local/firejail-default
# Site-specific additions and overrides for 'firejail-default'.
# For more details, please see /etc/apparmor.d/local/README.

# Here are some examples to allow running programs from home directory.
# Don't enable all of these, just pick a specific one or write a custom rule
# instead as done below for torbrowser-launcher.
#owner @HOME/** ix,
#owner @HOME/bin/** ix
#owner @HOME/.local/bin/** ix

# Uncomment to opt-in to apparmor for brave + ipfs
#owner @{HOME}/.config/BraveSoftware/Brave-Browser/oecghfpdmkjlhnfpmmjegjacfimiafjp/*/** ix,

# Uncomment to opt-in to apparmor for brave + tor
#owner @{HOME}/.config/BraveSoftware/Brave-Browser/biahpgbdmdkfgndcmfiipgcebobojjkp/*/** ix,

# Uncomment to opt-in to apparmor for discord
#owner @{HOME}/.config/discord/*/modules/** ix,

# Uncomment to opt-in to apparmor for firefox DRM (gmp-widevinecdm)
#owner @{HOME}/.mozilla/firefox/*/gm*/** ix,

# Uncomment to opt-in to apparmor for firefox native-messaging-hosts under ${HOME}
#owner @{HOME}/.mozilla/native-messaging-hosts/** ix,

# Uncomment to opt-in to apparmor for torbrowser-launcher
#owner @{HOME}/.local/share/torbrowser/tbb/{i686,x86_64}/tor-browser_*/Browser/** ix,
$ cat /etc/firejail/discord-common.profile
[...]

# Add 'apparmor' to your discord-common.local to enable AppArmor support.
# IMPORTANT: the relevant rule in /etc/apparmor.d/local/firejail-default will need
# to be uncommented too for this to work as expected.
#apparmor

HTH

@kmk3 kmk3 changed the title More directives for discord-common discord-common.profile: harden & allow notifications Aug 30, 2023
Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

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

Thanks for testing!

LGTM.

Note: I squashed the commits and added your notes to the commit message, as
they can be helpful for debugging. But feel free to edit it in any case.

What works:
- Basic functionality
- Receiving notifications
- Voice communication
- Watching streams

What wasn't tested:
- Casting streams
- Opening links
- Tracking/displaying "current activity" as status message
- Apparmor

Notes:
- Discord tries to access system dbus (`[ERROR:bus.cc(399)] Failed to
  connect to the bus: Failed to connect to socket
  /run/firejail/mnt/dbus/system: Permission denied`). I don't know what
  business it has with the system dbus, and didn't notice any problems
  due to that.
- I had one crash after 2h of watching a stream. Probably unrelated.

Fixes netblue30#5971.
@kmk3 kmk3 merged commit 9599851 into netblue30:master Sep 6, 2023
3 checks passed
@kmk3
Copy link
Collaborator

kmk3 commented Sep 6, 2023

Merged, thanks!

@kmk3 kmk3 changed the title discord-common.profile: harden & allow notifications profiles: discord-common: harden & allow notifications Sep 2, 2024
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.

discord: notifications are not shown
3 participants