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

[Flatpak] Reduce filesystem permissions #413

Closed
jannuary opened this issue Jul 30, 2021 · 46 comments · Fixed by #460
Closed

[Flatpak] Reduce filesystem permissions #413

jannuary opened this issue Jul 30, 2021 · 46 comments · Fixed by #460

Comments

@jannuary
Copy link
Contributor

(Moved from com.usebottles.bottles#120)

Currently, Bottles has a whopping 12 different filesystem permissions. Now that we use portals, it wouldn't hurt to go through different permission and reduce ones that aren't needed.

@jannuary jannuary added the Bug label Jul 30, 2021
@jannuary jannuary changed the title Reduce filesystem permissions [FlatpakReduce filesystem permissions Jul 30, 2021
@jannuary jannuary changed the title [FlatpakReduce filesystem permissions [Flatpak] Reduce filesystem permissions Jul 30, 2021
@mirkobrombin mirkobrombin added this to Todo in 2021.8.14 Jul 30, 2021
@mirkobrombin mirkobrombin moved this from Todo to Done in 2021.8.14 Jul 30, 2021
@mirkobrombin mirkobrombin moved this from Done to Todo in 2021.8.14 Jul 30, 2021
@sonnyp
Copy link
Contributor

sonnyp commented Aug 5, 2021

With #436 - all filesystem permissions that are related to letting a user open an arbitrary file can be removed. As far as Bottles is concerned.

Regarding Windows app that open/save files;

I see 3 options

  1. Make Bottles request rw permission on ~
  2. Remove all filesystem permissions and instruct users to use Flatseal if they need a permission for a Windows app
  3. Let users pick folders (via the portal) which are exposed to a Bottle via a symbolic link in the wine prefix (is that even possible?)

WDYT?

@mirkobrombin
Copy link
Member

With #436 - all filesystem permissions that are related to letting a user open an arbitrary file can be removed. As far as Bottles is concerned.

Regarding Windows app that open/save files;

I see 3 options

  1. Make Bottles request rw permission on ~
  2. Remove all filesystem permissions and instruct users to use Flatseal if they need a permission for a Windows app
  3. Let users pick folders (via the portal) which are exposed to a Bottle via a symbolic link in the wine prefix (is that even possible?)

WDYT?

Surely the second and third points are the best. What I don't understand is how it is possible that in my tests, even removing the permissions I am able to browse and pick files from the system using wine explorer.

@mirkobrombin
Copy link
Member

image

Maybe the sandbox is disabled by GNOME Builder or flatpak-builder?

@sonnyp
Copy link
Contributor

sonnyp commented Aug 5, 2021

Maybe the sandbox is disabled by GNOME Builder or flatpak-builder?

Exactly

@mirkobrombin
Copy link
Member

Maybe the sandbox is disabled by GNOME Builder or flatpak-builder?

Exactly

This explains A LOT of things. It is time for me to start using more Flatpak packages.

@sonnyp
Copy link
Contributor

sonnyp commented Aug 5, 2021

Yes, it took me a while to understand the subtleties of flatpak when I started targeting it. It was quite frustrating. I think the developer experience could be better there.

When I want to test/share the real deal, I build a bundle

flatpak-builder --user  --force-clean --repo=repo --install-deps-from=flathub flatpak com.usebottles.bottles.json
flatpak build-bundle repo Bottles.flatpak com.usebottles.bottles --runtime-repo=https://flathub.org/repo/flathub.flatpakrepo
flatpak --user install Bottles.flatpak

GNOME Builder also offers this IIRC

EDIT:

Builds have the --allow=devel and --allow=multiarch permissions that regular flatpak runs don't have by default. This allows limits the syscall filtering that is normally done so development tools like debuggers work.

https://docs.flatpak.org/en/latest/flatpak-builder-command-reference.html#idm1400

@mirkobrombin
Copy link
Member

Yes, it took me a while to understand the subtleties of flatpak when I started targeting it. It was quite frustrating. I think the developer experience could be better there.

When I want to test the real deal, I build a bundle

flatpak-builder --user  --force-clean --repo=repo --install-deps-from=flathub flatpak com.usebottles.bottles.json
flatpak build-bundle repo Bottles.flatpak com.usebottles.bottles --runtime-repo=https://flathub.org/repo/flathub.flatpakrepo
flatpak --user install Bottles.flatpak

GNOME Builder also offers this IIRC

EDIT:

Builds have the --allow=devel and --allow=multiarch permissions that regular flatpak runs don't have by default. This allows limits the syscall filtering that is normally done so development tools like debuggers work.

https://docs.flatpak.org/en/latest/flatpak-builder-command-reference.html#idm1400

Thanks for the informations I understand their decision to add the devel flag but this is a big limitation for debugging :S

@sonnyp
Copy link
Contributor

sonnyp commented Aug 5, 2021

We agree 😄

Also, it was unclear to me that devel flag involve full fs access. It should at least be explicit.

Please do create an issue - I'll happily share my similar experience there as well https://github.com/flatpak/flatpak-builder/issues/new

@mirkobrombin
Copy link
Member

We agree

Also, it was unclear to me that devel flag involve full fs access. It should at least be explicit.

Please do create an issue - I'll happily share my similar experience there as well https://github.com/flatpak/flatpak-builder/issues/new

Great idea, I'll do it

@sonnyp
Copy link
Contributor

sonnyp commented Aug 5, 2021

#130 is a requirement to consider dropping the following permissions

        "--filesystem=~/.Bottles:ro",
        "--filesystem=~/.local/share/bottles:ro",

note that Bottles will probably need a 'migrator' so even after #130 - these permissions might be always required

@mirkobrombin what is --filesystem=~/wineprefixes:ro for?

@mirkobrombin
Copy link
Member

mirkobrombin commented Aug 5, 2021

#130 is a requirement to consider dropping the following permissions

        "--filesystem=~/.Bottles:ro",
        "--filesystem=~/.local/share/bottles:ro",

note that Bottles will probably need a 'migrator' so even after #130 - these permissions might be always required

@mirkobrombin what is --filesystem=~/wineprefixes:ro for?

.Bottles, Games, .wine and wineprefixes are used by the wineprefix importer. From these can be safely removed .Bottles (old path where we stored bottles in v1), wineprefixes and .wine.

Btw I just saw that .PlayOnLinux is missing and it is needed by the importer.

@sonnyp
Copy link
Contributor

sonnyp commented Aug 5, 2021

Safely as in "app won't crash" or safely as in "won't break any feature"?

What is a user wants to impot a prefix from .wine ?

@mirkobrombin
Copy link
Member

Safely as in "app won't crash" or safely as in "won't break any feature"?

Safely as: "won't break anything and it is not necessary to import prefixes from there"

What is a user wants to impot a prefix from .wine ?

That is the default path of the prefix created by wine. The user might want to bring it to Bottles but even though we offer this functionality, it is not officially supported and there are very few cases where a 'bottled' prefix works.

@sonnyp
Copy link
Contributor

sonnyp commented Aug 5, 2021

From these can be safely removed .Bottles (old path where we stored bottles in v1)

What about https://github.com/bottlesdevs/Bottles#backward-compatibility-triumph ?


Screenshot from 2021-08-05 18-09-29

this is the importer right? If I had something to import it would automatically list it?

Do you think it would make sense to let users choose a folder to import via portal instead? Perhaps as a fallback.

If so, only "--filesystem=~/.local/share/bottles:ro" would be required for now.

@mirkobrombin
Copy link
Member

From these can be safely removed .Bottles (old path where we stored bottles in v1)

What about https://github.com/bottlesdevs/Bottles#backward-compatibility-triumph ?

Screenshot from 2021-08-05 18-09-29

this is the importer right? If I had something to import it would automatically list it?

Do you think it would make sense to let users choose a folder to import via portal instead? Perhaps as a fallback.

If so, only "--filesystem=~/.local/share/bottles:ro" would be required for now.

It has been a very long time and two major versions since I wrote about back compatibility. I think it could be the first step to retire v1 due the next coming major.

At the moment it list automatically but it might be a good idea to allow the user to select the path using the portals.

Also .local/share/bottles will be removed in the near future, as the Flatpak now store inside the .var/app/com.usebottles.bottles/data directory.

@sonnyp
Copy link
Contributor

sonnyp commented Aug 11, 2021

Ok then for now I suggest we remove all permissions (and break the importer) and add to the flathub repo README.md how to use the importer with Flatseal.

I will create a follow up ticket to change how the importer work.

WDYT?

@mirkobrombin
Copy link
Member

Ok then for now I suggest we remove all permissions (and break the importer) and add to the flathub repo README.md how to use the importer with Flatseal.

I will create a follow up ticket to change how the importer work.

WDYT?

I agree

@sonnyp
Copy link
Contributor

sonnyp commented Aug 11, 2021

@mirkobrombin can you assign me ?

@sonnyp
Copy link
Contributor

sonnyp commented Aug 11, 2021

@sonnyp
Copy link
Contributor

sonnyp commented Aug 16, 2021

The current situation isn't perfect but is temporary and will allow us to solve the problem once and for all.

See #413 (comment)

@BobyMCbobs
Copy link

The current situation isn't perfect but is temporary and will allow us to solve the problem once and for all.

See #413 (comment)

Thank you for your efforts and thank you for listening.
Keep up the good work ❤️

@mirkobrombin @sonnyp

@BrainBlasted
Copy link
Contributor

Reducing filesystem permissions does not work well with programs that bundle their own DLLs (or other assets). Say I have DoSomething.exe and it requires special_do_things.dll. This DLL is included in the same folder as the exe. When opening via the file chooser via the "Run Executable" button, which I assume uses GtkFileChooserNative, Bottles is only given access to the individual file handed to it by the portal. It is not given access to the directory, thus things in the sandbox cannot access related files in the same folder. If you give Bottles filesystem permissions (e.g. --filesystem=home) it can access said files without issue.

@mirkobrombin
Copy link
Member

mirkobrombin commented Oct 4, 2021

If we give the flatpak that permission again, the issue (#572) should be closed.

I don't think it is possible to give access to an entire folder through the GtkFileChooserNative because the user has to select the file and not the path. I believe that the lack of at least this permission is blocking the functioning of the main Bottles feature (starting windows executables), but I also think we shouldn't give in, to keep Bottles on the road to full-sandbox.

@sonnyp what is your opinion on this? Do you think there is an easy way to solve or do you think it is better to explain better how to give the user the responsibility to add permissions to Bottles?

As I thought Bottles, the target is the inexperienced user (despite everything), maybe a step by step tutorial should be done? Or maybe it is possible to integrate the permissions management in Bottles and act through flatpak-spawn in a controlled way?

Regarding the last proposal, we could show a dialog box with the exposed paths and a simple + to expose others. Since we need to execute commands on the host, we could use flatpak-spawn (requires permissions in manifest) to give Bottles new permissions. So Bottles at the first start is full sandboxed and the user can (at his own risk) expose path in a simple way (even if I would point out the risk with a warning).

@tim77
Copy link
Contributor

tim77 commented Oct 4, 2021

Bottles shouldn't rely only on portals and --filesystem=home either under any circumstance. Any Wine launcher shouldn't never use --filesystem=home and --filesystem=home since this disable FS sandbox entirely. Bottles already have finely tuned FS permissions and i have no idea why some decide to broke everything and remove them all.

@mirkobrombin
Copy link
Member

mirkobrombin commented Oct 4, 2021

Bottles shouldn't rely only on portals

I agree and on what you say I propose another solution: to introduce the user to the world of sandboxes. So, since the problem here is that the executable (with its dlls) are out of the sandbox, it would be possible to explain to the end user that these files must be moved inside the sandbox, perhaps taking advantage of the functionality of this issue: #572

So each bottle has its homedir (maybe sharable in the future) inside the bottle itself and the user place its executables in it, then the runner will have all the required permissions.

@BrainBlasted
Copy link
Contributor

From a user perspective, I don't think that will be good for the inexperienced user. The inexperienced user is more likely to give up and look for a different program instead of moving their files around. So, I can think of a few options that may work:

  • Work with the xdg-desktop-portal team on a way for the portal to give access to related files when opening a directory. This is useful for multiple different use cases, such as an image browser.
  • Have users use GtkFileChooserNative to give Bottles access to directories. This is possible, and I believe GtkFileChooserNative gives permanent access when using it's functionality to pick directories.
  • Give filesystem permissions to Bottles. This is the simplest solution, and what I'd recommend as a stopgap so that users can just use the app as expected out of the box until the portal issue is resolved.

@gasinvein
Copy link

FYI there is a feature request to add an option to allow access to the whole directory wile opening a file flatpak/xdg-desktop-portal#463
I guess it could've solve the Bottles sandbox dilemma.

@mirkobrombin
Copy link
Member

FYI there is a feature request to add an option to allow access to the whole directory wile opening a file flatpak/xdg-desktop-portal#463 I guess it could've solve the Bottles sandbox dilemma.

This would be the perfect solution to the problem. I'll keep an eye on it, thanks!

@Termuellinator
Copy link

* Give filesystem permissions to Bottles. This is the simplest solution, and what I'd recommend as a stopgap so that users can just use the app as expected out of the box until the portal issue is resolved.

Wouldn't this negate the whole sandbox? I'm specifically currently trying to find out if bottles flatpak would provide a suitable sandbox to make origin usable, cause the Lutris Flatpak is still not mature, so i would vote against this approach ;)

@mirkobrombin
Copy link
Member

mirkobrombin commented Jul 12, 2022

These are current perms

finish-args:
- --allow=devel
- --allow=multiarch
- --share=network
- --share=ipc
- --socket=x11
- --socket=wayland
- --socket=pulseaudio
- --device=all
- --system-talk-name=org.freedesktop.UDisks2
- --system-talk-name=org.freedesktop.Flatpak
- --talk-name=org.freedesktop.Notifications
- --filesystem=xdg-documents/Bottles:create # know as the Box path in Bottles, users should place non-standalone executables in this path
- --env=LD_LIBRARY_PATH=/app/lib:/app/lib32
- --env=PATH=/app/bin:/app/utils/bin:/usr/bin:/usr/lib/extensions/vulkan/MangoHud/bin/:/usr/bin:/usr/lib/extensions/vulkan/OBSVkCapture/bin/
- --require-version=1.1.2

@gasinvein
Copy link

@mirkobrombin Why --system-talk-name=org.freedesktop.Flatpak, btw? First, org.freedesktop.Flatpak lives on session bus, so this perm is no-op. And second, it's used to escape sandbox (e.g. run flatpak-spawn --host), I guess Bottles doesn't need that?

@mirkobrombin
Copy link
Member

mirkobrombin commented Jul 12, 2022

@mirkobrombin Why --system-talk-name=org.freedesktop.Flatpak, btw? First, org.freedesktop.Flatpak lives on session bus, so this perm is no-op. And second, it's used to escape sandbox (e.g. run flatpak-spawn --host), I guess Bottles doesn't need that?

There is an experimental feature in Bottles which aim to provide a per-bottle sandbox. It uses flatpak-spawn (with no --host) inside the flatpak and bwrap outside.

@gasinvein
Copy link

@mirkobrombin Why --system-talk-name=org.freedesktop.Flatpak, btw? First, org.freedesktop.Flatpak lives on session bus, so this perm is no-op. And second, it's used to escape sandbox (e.g. run flatpak-spawn --host), I guess Bottles doesn't need that?

There is an experimental feature in Bottles which aim to provide a per-bottle sandbox. It uses flatpak-spawn (with no --host) inside the flatpak and bwrap outside.

flatpak-spawn without --host doesn't need access to org.freedesktop.Flatpak.

@mirkobrombin
Copy link
Member

@mirkobrombin Why --system-talk-name=org.freedesktop.Flatpak, btw? First, org.freedesktop.Flatpak lives on session bus, so this perm is no-op. And second, it's used to escape sandbox (e.g. run flatpak-spawn --host), I guess Bottles doesn't need that?

There is an experimental feature in Bottles which aim to provide a per-bottle sandbox. It uses flatpak-spawn (with no --host) inside the flatpak and bwrap outside.

flatpak-spawn without --host doesn't need access to org.freedesktop.Flatpak.

oh ok thanks for the tip

@Perkolator
Copy link

Related to this topic? #2990

@Perkolator
Copy link

#2999 (Flatpak portal problem with mounted SMB / CIFS share folder set as a Drive in Bottles)

@bayazidbh
Copy link

It's been a while since I used Bottles, but does this:

   - --filesystem=xdg-documents/Bottles:create  # know as the Box path in Bottles, users should place non-standalone executables in this path 

Already have a notice exposed in the app? I know that there is a notice the first time you try to run an executable, but in my opinion, the message is still rather unclear and has jargons that can confuse new users. IMO drawing direct parallels to Android/iPhone folder limitation would probably feel more understandable to new users (especially those on Steam Deck) than the long notice Bottles have right now.

Just say something like "By default, similar to many phone apps, Bottles is restricted to opening apps and files in /home//Documents/Bottles" or somewhere along that line would be clearer to most users not familiar with the terms and provides a non-hidden folder that they can just easily drop their games and apps into.

I think that solution would be nice as, while flatpak/xdg-desktop-portal#463 will be great, there are apps with weird folder structures like Hitman 3, which have executable in a subfolder of the game's folder. I like the idea of just telling users to straight-up use ~/Documents/Bottles better.

@jannuary jannuary closed this as not planned Won't fix, can't repro, duplicate, stale Aug 18, 2023
@Perkolator
Copy link

Why was this closed? About 2 weeks ago @mirkobrombin replied to me on another issue this:

Hi, thanks for reporting. As you pointed this is not something we can fix and should be managed by the portal itself. Closing, we can continue the discussion in the #413 for the permissions part.

@sonnyp sonnyp removed their assignment Aug 22, 2023
@MasterKia
Copy link

I don't think it is possible to give access to an entire folder through the GtkFileChooserNative [...] I believe that the lack of at least this permission is blocking the functioning of the main Bottles feature (starting windows executables)

https://docs.flatpak.org/en/latest/portals.html#portal-support-in-gtk:

Use GtkFileChooserNative (or GtkFileChooserButton) to open files. As of xdg-desktop-portal-gtk 1.7.1 it can also open directories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
2021.8.14
  
Postponed
2021.8.28
  
Postponed
Development

Successfully merging a pull request may close this issue.