flathub / org.qbittorrent.qBittorrent

https://flathub.org/apps/details/org.qbittorrent.qBittorrent
9 stars 12 forks source link

Change "--filesystem=host" to "--filesystem=xdg-download" #104

Closed magical-heyrovsky closed 1 year ago

magical-heyrovsky commented 1 year ago

There is no reason why qBittorrent should have access to the entire host. I changed it to xdg-download on my computer and it works without any problems. Only give the permissions really needed, --filesystem=host is not one of them.

Chocobo1 commented 1 year ago

There is no reason why qBittorrent should have access to the entire host.

Some users have their own dedicated/complex directory structure which may not be rooted in xdg-download. For example an usb disk or a network drive.

Maybe there really is a way to use more restrictive paths than host without sacrificing program usability.

Chocobo1 commented 1 year ago

There is no reason why qBittorrent should have access to the entire host.

Actually it is not unrestricted as one would thought, see: https://docs.flatpak.org/en/latest/sandbox-permissions.html#filesystem-access

Maybe there really is a way to use more restrictive paths than host without sacrificing program usability.

Since host is already restrictive enough, I doubt there is room to improve it. Closing issue.

therealmate commented 1 year ago

There is no reason why qBittorrent should have access to the entire host.

Some users have their own dedicated/complex directory structure which may not be rooted in xdg-download. For example an usb disk or a network drive.

Maybe there really is a way to use more restrictive paths than host without sacrificing program usability.

qbt has support for portals, which solves this issue

FOSSProponent9436 commented 1 year ago

I disagree that host is restrictive enough. The app can access the entire home directory which has all the user's personal files and such. When really it only needs to access one folder. Also the ideal manner of sandboxing is to only give the app what it needs, so it is better if the rules are tightened if no functionality is lost.

FOSSProponent9436 commented 1 year ago

There is no reason why qBittorrent should have access to the entire host.

Some users have their own dedicated/complex directory structure which may not be rooted in xdg-download. For example an usb disk or a network drive. Maybe there really is a way to use more restrictive paths than host without sacrificing program usability.

qbt has support for portals, which solves this issue

But would it retain access to files/folders it is not normally given access to after a restart of the app? That is necessary for seeding/resuming downloads.

Erick555 commented 1 year ago

If you remove host permission then app will lost access to user files after update until they re-add them. It's quite disruptive change.

Chocobo1 commented 1 year ago

Also the ideal manner of sandboxing is to only give the app what it needs, so it is better if the rules are tightened if no functionality is lost.

Basically I agree but what about other paths I mentioned: For example an usb disk or a network drive? Will --filesystem=xdg-download (or any other setting) allows it?

But would it retain access to files/folders it is not normally given access to after a restart of the app?

I suppose it won't. qbt doesn't ask the user again for permission for the folders/paths, it just access them directly (and nor does it have concepts of portals it only uses Qt library).

Erick555 commented 1 year ago

Qt itself is aware of portals. Accessing usb disk or network drive is no different than any other directory. You can remove host permission and access any path chosen from file dialog right now. Only the pre-existing torrents outside xdg-download won't be available - they would need to be re-added.

Chocobo1 commented 1 year ago

Qt itself is aware of portals. Accessing usb disk or network drive is no different than any other directory. You can remove host permission and access any path chosen from file dialog right now.

Thanks for explaining! Another thing worth mention is, libtorrent only use OS API for accessing files so it won't know about portals and not every operation will re-ask user for permission.

Only the pre-existing torrents outside xdg-download won't be available - they would need to be re-added.

If you remove host permission then app will lost access to user files after update until they re-add them. It's quite disruptive change.

This is especially disruptive if one has many torrents placed in the affected folder/path which means qbt became unwieldy for those big seeders and this would drive them away. IMO users that wishes to tighten their security should override this setting themselves instead of enforcing it on everyone as it clearly affect normal program operation in some conditions.

FOSSProponent9436 commented 1 year ago

Perhaps whitelist directories that are often used for mounting (e.g. /media) could work but yeah, I think it is probably not possible to tighten permissions right now without breakage. Keepassxc came to same conclusion and also has host permission.

zefr0x commented 3 weeks ago

Another thing worth mention is, libtorrent only use OS API for accessing files so it won't know about portals and not every operation will re-ask user for permission.

libtorrent doesn't need to know about portals, the portal is just a GUI thing, under it there is just a normal OS direcoty/file that is mapped to the the actual directory/file. When the GUI's file selector is used, it will pass to libtorrent a normal direcoty like /run/user/1000/doc/xxxxxxxx/Downloads that is mapped to /home/xxx/Downloads under the hood.

This is not a problem, and it just work (tested). We just need some GUI restrictions to always use the file selector rather than enabling the user to edit the directory string manually.


So the only real problem now is the technical debt one.

We should find a solution for it with the Flatpak team. There should be a backward compatible way of removing permissions.