Suggest using firejail profile instead of wrapper.

cubercsl commented 1 year ago

This is my profile copied from /etc/firejail/telegram.profile and put into /etc/firejail/tencent-qq.profile or ~/.config/firejail/tencent-qq.profile

``` include tencent-qq.local # Persistent global definitions include globals.local noblacklist ${HOME}/.config/QQ # Allow opening hyperlinks include allow-bin-sh.inc include disable-common.inc include disable-devel.inc include disable-exec.inc include disable-interpreters.inc include disable-programs.inc include disable-shell.inc include disable-xdg.inc mkdir ${HOME}/.config/QQ whitelist ${HOME}/.config/QQ whitelist ${DOWNLOADS} include whitelist-common.inc include whitelist-runuser-common.inc include whitelist-usr-share-common.inc include whitelist-var-common.inc apparmor caps.drop all netfilter nodvd noinput nonewprivs noroot notv protocol unix,inet,inet6,netlink seccomp seccomp.block-secondary shell none disable-mnt private-bin bash,sh,tencent-qq,xdg-open private-cache private-dev private-etc alsa,alternatives,ca-certificates,crypto-policies,fonts,group,ld.so.cache,ld.so.preload,localtime,machine-id,os-release,passwd,pki,pulse,resolv.conf,ssl,xdg private-opt QQ private-tmp dbus-user filter dbus-user.talk org.freedesktop.Notifications # Input Method dbus-user.talk org.freedesktop.portal.Fcitx dbus-user.talk org.freedesktop.portal.IBus ?ALLOW_TRAY: dbus-user.talk org.kde.StatusNotifierWatcher dbus-user.talk org.gnome.Mutter.IdleMonitor dbus-user.talk org.freedesktop.ScreenSaver dbus-system none ```

And then ln -s /usr/bin/firejail /usr/local/bin/tencent-qq to run with the firejail profile.

A patched the desktop entry can do this. (Should be repored to upstream)

-Exec=/usr/bin/tencent-qq %U
+Exec=tencent-qq %U

Or use firecfg(1) to fixed this.

TL;DR suggest just package a profile and the wrapper config can be done by firecfg(1) and the application can just install the original linuxqq-new.

BeautyyuYanli commented 1 year ago

Thank you for your suggestions. I'm not yet familiar with Firejail so I want to ask: does the profile forbid the access to

most directories under /
HOME except for Downloads and .config/QQ

cubercsl commented 1 year ago

Yes, but there are some limits --- we can not open any link from QQ. Because I have set the private-bin, as a workaround, I have comment it out and make some small change

-seccomp
+seccomp !chroot

Some reference: https://github.com/netblue30/firejail/issues/2946

and you can see some profiles in the /etc/firejail for more references.

Also I am not yet familiar with it, so I can only offer some suggestions that might be useful.

BeautyyuYanli commented 1 year ago

Since the guidelines say that "do not rely on transitive dependencies", are there any proper way to add linuxqq as a dependency?

glitsj16 commented 1 year ago

Hello! As an Arch Linux user I noticed linuxqq-firejail and linuxqq-new-firejail were recently added to the AUR. I'm also part of the Firejail team, so these firejail-related packages caught my attention. I've been reading tencent-qq.profile and a few things came up that you might be interested in.

We're preparing a new release and there are some newly added options, of which restrict-namespaces is the most important one. You might not be aware of this. The reason for contacting you here is I wanted to share some concerns about the custom profile:

linuxqq being an Electron app, it would make more sense to refactor it as a redirect to the electron.profile
if you add noblacklist ${HOME}/.config/QQ that implies that somewhere in the Firejail profiles this path is blacklisted, so other applications can't see it (which obviously is important in a sandboxing environment)
seccomp is tricky with Electron - we have chromium-common-hardened.inc.profile for implementing that safely
support for opening hyperlinks needs additional options

Besides these points, I actually find the profile very well designed and also very tight. Would you be interested in adding such a refactored version to upstream Firejail? It would be another nice and welcome addition. On top of that it would also make it a lot easier for non-AUR users to get sandbox protection when using linuxqq.

Please don't see this as an attempt on pressuring you into anything, the decision is ofcourse yours. But seeing constructs like firejail --private=~/.linuxqq --noprofile /opt/QQ/qq in linuxqq-new-firejail I got a bit worried and wanted to reach out :-). Let me explain. The --noprofile option does NOT offer any protection and is meant as a debug tool.

There's a lot of extra hoops in the mentioned AUR packages that can be dropped if there was a linuxqq.profile in Firejail. I've created a first draft of such a profile here. Feel free to test it, make changes, etcetera. I'm available to answer any questions there might be.

Regardless of your decision, I wish you all the best with this project and a safe, healthy and productive 2023!

BeautyyuYanli commented 1 year ago

@glitsj16 Thank you for your suggestions!

linuxqq-new-firejail will be deprecated soon and I have made a legacy branch for it.

I'm willing to see Firejail includes the profile directly. While QQ is a proprietary software with poor Linux support for a long time before refactored using Electron, and its name is still unclear (the AUR package renamed from linuxqq-new to linuxqq just yesterday, as well as the binary name from tencent-qq to linuxqq. And the official deb package use qq).

Also the users may have different expectations to the profile. I'm willing to blacklist the whole HOME, but it may be inconvenient if it's directly included in Firejail.

Also, wish you have a good year!

glitsj16 commented 1 year ago

I'm willing to see Firejail includes the profile directly.

Great!

Thanks for the input regarding the different executable names QQ can have on different distributions. We deal with that all the time and we can create alias profiles for each one.

Also the users may have different expectations to the profile. I'm willing to blacklist the whole HOME, but it may be inconvenient if it's directly included in Firejail.

I understand. But in fact, as this and electron are both designed as so called whitelisting profiles, only the bare minimum needed to run QQ with all (or most) of its functionality is exposed in the sandbox ${HOME}. In this context I assume users will want to save some files to disk (chat logs, shared files, etcetera). So I disabled private and added a comment to inform users how to harden the profile further in their linuxqq.local if they don't need/want that.

BeautyyuYanli commented 1 year ago

Another problem I'm not sure if it is a bug from QQ itself. It is always "download error". Seems it always try to write into $HOME instead of $DOWNLOADS even I've set so.

glitsj16 commented 1 year ago

It is always "download error". Seems it always try to write into $HOME instead of $DOWNLOADS even I've set so.

Sounds like QQ's issue. But I've also whitelisted ${DESKTOP} just now. Some apps are determined to open a filepicker dialog there. It might help.

glitsj16 commented 1 year ago

It is always "download error". Seems it always try to write into $HOME instead of $DOWNLOADS even I've set so.

Should have asked before, but if you're unsure about this being a QQ bug, try comparing download behaviour with running without firejail.

BeautyyuYanli commented 1 year ago

@glitsj16 I can't reproduce the bug today. It may not be a problem when running with correct profile.

glitsj16 commented 1 year ago

Opened a PR for the new firejail profiles just now. Feel free to add any remarks there so we can get those in for the upcoming 0.9.72 firejail release.

Thanks for all the suggestions and testing!

BeautyyuYanli / linuxqq-firejail

Suggest using firejail profile instead of wrapper. #1