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

Can't access to files #33

Closed
Split7fire opened this issue Feb 27, 2019 · 54 comments · Fixed by #136 or #277
Closed

Can't access to files #33

Split7fire opened this issue Feb 27, 2019 · 54 comments · Fixed by #136 or #277
Labels

Comments

@Split7fire
Copy link

When adding files to messages, can't see any file. I think it is related to flatpak permissions.

@Mikaela
Copy link
Contributor

Mikaela commented Feb 28, 2019

It looks like only downloads folder can be accessed by the flatpak by default.

"--filesystem=xdg-download",

If you have specific directories you would like Riot to be allowed to access, you can use something like flatpak override --user --filesystem=~/Desktop/Screenshots im.riot.Riot which would allow it to access ~/Desktop/Screenshots where I store my screenshots and most often need to upload to Riot. (source: https://github.com/flathub/org.telegram.desktop/issues/23#issuecomment-435114564)

@Split7fire
Copy link
Author

I think that read/write access to home dir is mandatory for such chat application.

@Mikaela
Copy link
Contributor

Mikaela commented Mar 3, 2019

I haven't had issues with just allowing access to my screenshots folder and I think Riot has no reason to access rest of my home directory. Looking at the Telegram issue where I referred to, I think the real issue is probably flatpak/xdg-desktop-portal#99.

@Split7fire
Copy link
Author

I checked right now and there is no download dir available for me to choose from. Attached image, when I press "Send" button.
2019-03-03 21-02-51

@TingPing
Copy link
Member

TingPing commented Mar 3, 2019

The real issue is that Electron doesn't use GtkFileChooserNative yet. There is a patch for it but it needs work, maybe this decade.

@SISheogorath
Copy link
Collaborator

SISheogorath commented Mar 11, 2019

At the same time we could mention that the Slack flatpak, which is running proprietary software (just saying), has more capabilities (i.e. access the whole home directory). So for now an easy fix would be to allow Riot more access as we do this for other applications as well.

electron/electron#15293 <-- the mentioned electron issues/PR

@SISheogorath
Copy link
Collaborator

This not only breaks influences the upload button, but also drag and drop. Just had the issues that I couldn't send a PDF.

We should definitely consider to allow the entire home directory, even when it's not a great solution.

@Mikaela
Copy link
Contributor

Mikaela commented Mar 21, 2019

If you have to allow access to the entire home directory, could it at least be read-only? I think the option would be --filesystem=home:ro http://docs.flatpak.org/en/latest/sandbox-permissions.html#filesystem-access .

@MightyCreak
Copy link
Contributor

Drag'n'drop also behaves weirdly when the app desn't have access to the file. The drag'n'drop process works, but fails at the end, saying the file could not be sent (it doesn't mention a file access issue).

I ran this:

flatpak override --user --filesystem=home:ro im.riot.Riot

And my life is much much better now 😉
👍 I vote up for adding home as a read-only filesystem.

(The perfect behavior would be, like on Android, to ask the user the rights access when the app doesn't have it, but that is more something to do on the Flatpak/DE side).

@SISheogorath
Copy link
Collaborator

One possible problem that we should take into consideration is that those directories will appear in the save-dialogs but won't be writable. This is more than contra intuitive and bad UX.

@MightyCreak
Copy link
Contributor

MightyCreak commented Apr 9, 2019

That is a very good point.

Actually then -- and I know I might sound heretic -- why not enable r/w access right on home?

Maybe it would be smarter to set the same restrictions* as if I were to install the app from a repo. IMHO it would be smarter because users would have a better experience out-of-the-box when using Flatpak and, indirectly, Flathub (that is until there is a proper Flatpak API and a proper UI in the DEs to handle rights in a more user-friendly manner). Setting the same rights as in any other "regular" app will certainly create less friction to the users.

* if access rights are given to home, then it's still better than a regular app since it won't be able to access the entire host

@Erick555
Copy link
Contributor

Erick555 commented Apr 9, 2019

@MightyCreak home access gives app ability to read or steal all user secrets, host doesn't give much more. This isn't much different than using app from your distro repos in term of security. Overrides exist to fit for specific user needs without forcing everyone to turn down the sandbox.

@MightyCreak
Copy link
Contributor

Yeah that's the point I'm trying to make actually 😉
Since repo apps already have access to all the host, it would create less friction if the flatpak app has (at least) access to home

@Erick555
Copy link
Contributor

Erick555 commented Apr 9, 2019

The premise of flatpak was to provide sandboxed apps for users. Keep in mind that distro apps are maintained by limited circle of maintainers who needed to earn they their status somewhat. Flatpaks can be maintained by any random person so they in general aren't same trustworthy as distro apps and sandbox was some sort of guarantee that even if you get malware it will be contained. Take a look at android store where they have tons of malware apps but they ARE sandboxed. Now imagine if they aren't. Flatpak without sandbox doesn't look like improvement for delivering apps in linux.

@MightyCreak
Copy link
Contributor

Indeed... 🤔
But I'm afraid that until Flatpak's rights management is easy to use, users will consider it not as good as their distro repo (unless for very special apps where sandboxing is not an issue for the UX). I presume this will be an important problem for Flatpak adoption.

I'm afraid I got no solution to provide here, and that's what scaring me the most for the future of Flatpak :/

@SISheogorath
Copy link
Collaborator

The solution is the portal implementation on electron side. I already liked a PR above, sadly no one picked up the work which would benefit all electron-based projects in Flatpak. If someone is good at CPP and may previously contributed to electron it would be overly awesome to see that person picking up and complete the work to use portals instead of Filesystem-access.

@Mikaela
Copy link
Contributor

Mikaela commented Apr 13, 2019

Matrix.org/Riot.im infra was recently compromised entirely including package signing keys, so I am again more hesistant about giving Riot or other apps more permissions than they need, so I would stay with having access only to Downloads and my screenshots directory.

According to the attacker one issues making the compromise possible was everyone/everything given more permissions than they needed.

@MightyCreak
Copy link
Contributor

@Mikaela It's not exactly the same. The matrix.org server (and only this server) was compromised because of a security issue with Jenkins, unrelated to Matrix ecosystem.

The fact that they had Jenkins and private keys on their production server is indeed a concern, but it is not related to Matrix or Riot code bases.

Anyway, I explained the issue here: as of today, for an average user, apps seem to have issues when they're packaged with Flatpak, while they seem to work well when installed through a regular repository. They feel simply clunkier. Make no mistake, I completely understand the security reasons behind all that, but, until this issue is fixed somehow, I don't foresee a mass adoption to Flatpak since it doesn't make the life easier.

@Erick555
Copy link
Contributor

Erick555 commented Apr 13, 2019

On the other hand if every app will be allowed to bypass the sandbox then there would be no incentive for apps developers to change their code ever. Even if they change the code, removing existing permission will certainly break someone use case, causing regressions which will create pressure for maintainers to keep them forever.

If flatpaks would behave the same way as distro packages then what's the point of using them in first place? Instead of creating solution for desktop app security we would create more of the same problems and need a brand new package format to fix it again. It's much better to educate users about xdg standard and how to use it in a way that doesn't collide with sandboxing.

@Pointedstick
Copy link
Member

Man, this is really annoying. What is the solution to attaching files and drag-and-drop for files not in ~/Downloads? If the answer is "none" then people are going to install the un-sandboxed distro package instead of the more secure Flatpak packaging to work around this defect, totally defeating the point of the extra security. If security is annoying, people bypass it.

@MightyCreak
Copy link
Contributor

@Pointedstick use override permissions as written here: #33 (comment)

@SISheogorath
Copy link
Collaborator

You can also use flatseal to grant access to more filesystem directories if you don't want to use a CLI.

As pointed out in earlier statements. There is no good answer here. If we just open up the home filesystem, then escaping becomes trivial, which means from a security perspective there is no benefit by using flatpak. The alternative would be to share Pictures, Documents, Music and Videos only, but then you run into the same problem when you use a non-standard directory like, lets say, Nextcloud. Also there is no technical reason for Riot to have access to those directories.

To me the answer is the portal: electron/electron#19159

If you want this to be solved, I suggest to help moving this PR forward.

@MightyCreak
Copy link
Contributor

I didn't know about flatseal, thank you so much!

@MightyCreak
Copy link
Contributor

MightyCreak commented May 26, 2020

@SISheogorath I installed flatseal and I would argue (for this issue) that a lot of flathub applications have far more permissions than Riot.

For instance:

  • Fedora Media Writer has complete access to the host
  • GIMP has complete access to the host
  • VLC has complete access to the host
  • Audacity has complete access to the host
  • OBS has complete access to the host
  • Remmina has complete access to the home directory
  • GNOME Games has read-only access to the host

While I agree complete access to the host is certainly not a good solution, I do think xdg-download and home:ro is a good compromise (it is basically better than any of the aforementioned applications).

@SISheogorath
Copy link
Collaborator

For all interested in this topic, please try the build from #136 It would be very useful to know if it's running properly for our user-base or not. If this is successful, we no longer need any special filesystem permissions.

@hdoupe
Copy link

hdoupe commented Sep 30, 2020

@SISheogorath Thanks for the fix! Would you mind posting a link to directions for testing the patch?

@SISheogorath
Copy link
Collaborator

@hdoupe You should be able to use the flatpak install instruction form flathubbot. If not I might trigger a new build.

#136 (comment)

@hdoupe
Copy link

hdoupe commented Sep 30, 2020

Ah, thanks for the link @SISheogorath! I got an error when I tried to run it:

$ flatpak install --user https://dl.flathub.org/build-repo/27334/im.riot.Riot.flatpakref
error: Can't load uri https://dl.flathub.org/build-repo/27334/im.riot.Riot.flatpakref: Server returned status 404: Not Found

I saw that maybe I needed to add a flathub repo, but that didn't help:

$ flatpak remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo

Sorry about this! I'm new to flatpak.

@salim-b
Copy link

salim-b commented Oct 24, 2020

Ehm, I still can't share files from outside the Flatpak sandbox (e.g. from ~/Downloads/) without manually giving additional permissions (e.g. in Flatseal to ~/:ro).

I think I have the latest version of the Element Flatpak:

$ flatpak info im.riot.Riot

Element - Create, share, communicate, chat and call securely, and bridge to
other apps

          ID: im.riot.Riot
         Ref: app/im.riot.Riot/x86_64/stable
        Arch: x86_64
      Branch: stable
     Version: 1.7.10
     License: Apache-2.0
      Origin: flathub
  Collection: org.flathub.Stable
Installation: system
   Installed: 233.5 MB
     Runtime: org.freedesktop.Platform/x86_64/20.08
         Sdk: org.freedesktop.Sdk/x86_64/20.08

      Commit: 9542b17d5bbab3cb31db860220c63fc640d1cf30edd11922085327c0d619cc6a
      Parent: fda9fd67665f06cf783c3a3b04ac0539ae26b8eb9b658df5fb50accff65ef680
     Subject: Update element-desktop_1.7.9_amd64.deb to 1.7.10 (7f30e583)
        Date: 2020-10-22 16:03:43 +0000

I'm on Ubuntu 20.04. What am I missing?

@SISheogorath
Copy link
Collaborator

SISheogorath commented Oct 27, 2020

Re-opening as element-hq/element-web#136 didn't work out as expected. We'll wait for the new zybak version to become stable.

Reverted the previous changed with fcabb98

@vchernin
Copy link
Contributor

vchernin commented Jun 15, 2021

Thanks to electron/electron#19159, this issue should be fixed with the release of Electron 14. The file chooser XDG desktop portal will be used instead of the traditional file chooser. The portal can choose any file without giving Element entire file system access.

Electron 14 is scheduled for release on August 31st 2021. It might take some time for Element to update to Electron 14 after it is released (usually they’re pretty quick though).

When that is released we shouldn’t need to grant any file system access to Element Flatpak, since the portal makes any file system access redundant. Drag and drop of any file should also work with other Electron 14 and GTK4 apps (I believe the portal hasn’t been backported to GTK 3).

@colans
Copy link

colans commented Jun 17, 2021

Until that gets released, where can we actually find our downloaded files? I searched all of ~/.var/app/im.riot.Riot, and they weren't in there. For now, I suppose I'll need to use the browser version to download files.

@vchernin
Copy link
Contributor

vchernin commented Jun 17, 2021

@colans Could you specify what a “downloaded file” is? Most of this thread is about uploading a file via the upload file button or drag and drop to send it. Do you mean if someone sends a picture or something and you try to download it? Because I’m not sure if that’s actually covered by electron/electron#19159.

@colans
Copy link

colans commented Jun 17, 2021

@vchernin Yes, exactly. I receive the file, decrypt it, and download it. I get a pop-up asking me where to put it so I say, /tmp. Then I go to /tmp locally, and it's not there, even though the app is telling me that it is. Is there actually a way to get into the sandbox to access the files? Where is it?

I'm happy to open a new bug report if this shouldn't be covered here.

@SISheogorath
Copy link
Collaborator

We are currently only exposing the xdg-downloads folder of your filesystem. By default that's probably ~/Downloads. If you safe a file anywhere else, it'll only survive in the overlay filesystem while Element is running.

@vchernin
Copy link
Contributor

@colans could you give an example procedure? I tried to reproduce your issue and I never got a prompt to save a shared file locally in Element. When I clicked the download button all it did was open the file URL in my web browser. Once the web browser is given the file it's out of scope for Element.

I tried with both a webp and a tar.gz file.

Screenshot
dog and file

@colans
Copy link

colans commented Jun 17, 2021

We are currently only exposing the xdg-downloads folder of your filesystem. By default that's probably ~/Downloads. If you safe a file anywhere else, it'll only survive in the overlay filesystem while Element is running.

Ah, yes! Saving there actually works. (I normally prefer /tmp because it's not persistent.)

When I clicked the download button all it did was open the file URL in my web browser.

For me, I get the save-file dialog chooser, which defaults to Downloads. I initially thought this was strange because it doesn't show all of my normal locations and bookmarks on the left like it normally does; I only see a few things:

Screenshot from 2021-06-17 13-56-05

I'm on Pop!_OS 20.10, which is mostly just Ubuntu 20.10, if that helps.

I would say this is a major UX bug (i.e. why ask me for a location when there's only one option?), but maybe it isn't generally and I'm on an edge case somehow.

@vchernin
Copy link
Contributor

In theory your use case (saving files) could be solved by xdg desktop portal like how electron/electron#19159 uses the portal for choosing files. But I don't see an explict mention that PR added support for saving files. So maybe more electron portal integration is needed so saving files also works without needing directory permissions given.

We could also add write-only permissions to more directories until there is better support for saving files with the portal?

@wioxjk
Copy link

wioxjk commented Jun 20, 2021

The flathub package should be considered broken until this is not fixed

Meanwhile, @Mikaela did suggest a solution - even thou temporary.

flatpak override --user --filesystem=home:ro im.riot.Riot

@vchernin
Copy link
Contributor

vchernin commented Jun 20, 2021

@colans I researched a bit and it turns out the fix for choosing files (supporting the file chooser portal) also should apply for saving files, since the portal seems to support it. So your issue (saving files to /tmp or anywhere else) should be fixed with Electron 14 like how choosing files should be fixed. At least this only applies if Electron is making full use of the portal, which would be the sensible way of implementing it. Maybe I’ll get around to testing it at some point.

https://flatpak.github.io/xdg-desktop-portal/portal-docs.html#gdbus-org.freedesktop.impl.portal.FileChooser

@vchernin
Copy link
Contributor

vchernin commented Sep 8, 2021

Unfortunately for drag and drop without file system access we will need electron/electron#30650 as well... the FileChooser portal in Electron 14 won't be enough.

@Peque
Copy link

Peque commented Oct 7, 2021

Cross-referencing element-hq/element-desktop#728, for those wanting to track Electron 14 integration. 😊

@vchernin
Copy link
Contributor

vchernin commented Oct 7, 2021

Sadly with electron 14 we might run into this: electron/electron#31258

I am not sure why that issue exists but I have not had time to investigate it properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet