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

Sandbox this harder (instead of filesystem=host permissions by default)? #76

Closed
nekohayo opened this issue Mar 9, 2022 · 23 comments
Closed

Comments

@nekohayo
Copy link

nekohayo commented Mar 9, 2022

It was brought to my attention that poppler and ghostscript can be security concerns, and that Evince should therefore be run under sandboxes.

Looking at it with flatseal, I see that:

  • Evince has filesystem=host permissions by default. Shouldn't it be restricted to access only xdg-download and xdg-documents by default, or at best filesystem=home?
  • It has "socket=pulseaudio"... why would it need that?
@hadess
Copy link
Collaborator

hadess commented Mar 9, 2022

  • Evince has filesystem=host permissions by default. Shouldn't it be restricted to access only xdg-download and xdg-documents by default, or at best filesystem=home?

That's for the "recent files view" that you get when running "evince" to still work correctly.

  • It has "socket=pulseaudio"... why would it need that?

Because there can be videos in PDFs. That's implemented in libview/ev-media-player.c.

@gpoo
Copy link
Collaborator

gpoo commented Mar 22, 2022

FWIW, the flatpak version of Evince does not ship the backends for DVI (tetex-live) nor PS (ghostshipt).

@gpoo
Copy link
Collaborator

gpoo commented Mar 22, 2022

Quoting what I wrote in #61 (comment)

I dislike having filesystem=host enabled, but IIUC, that is the one the currently gives Evince access to neighbour files. That is, until there is a fix for flatpak/xdg-desktop-portal#463.

@Mikenux
Copy link
Contributor

Mikenux commented Mar 30, 2022

Neighbor files will work in most cases, but not all. This will not work if you have a summary pdf and then folders where other pdf files are located if you first open a pdf located in one of those folders. If you do this, the opened pdf file will not be able to access the summary because it will not be in the same directory.

Example:

  • GeneralSummary.pdf
  • Issue1Folder/issue1-summary.pdf
  • Issue2Folder/issue2-summary.pdf

Directly open issue1-summary.pdf in Issue1Folder. Grant access to neighboring files in this folder. If issue1-summary has an entry to access GeneralSummary.pdf, it won't work.

There is another question: If I open GeneralSummary, then open issue-1-summary from it, then close GeneralSummary, will access to GeneralSummary still be available or will it be forgotten?

However, I don't think this is a common case.

@hadess
Copy link
Collaborator

hadess commented Mar 31, 2022

However, I don't think this is a common case.

I've never ever seen a PDF that linked to other PDFs on the local filesystem. I've seen it on the web...

@Mikenux
Copy link
Contributor

Mikenux commented Mar 31, 2022

However, I don't think this is a common case.

I've never ever seen a PDF that linked to other PDFs on the local filesystem. I've seen it on the web...

https://gitlab.gnome.org/GNOME/evince/-/issues/48. It was on DVD, but the author of the issue said it took up hard drive space. This was an old issue, but in the comments someone else used a manual and he used Fedora 35. As for the DVD, you can copy the files to the hard drive for storage.

Audio files can also be in a folder (see https://gitlab.gnome.org/GNOME/evince/-/issues/1584). Do the folders fit into the neighboring files case?

@mwleeds
Copy link

mwleeds commented Apr 5, 2022

  • Evince has filesystem=host permissions by default. Shouldn't it be restricted to access only xdg-download and xdg-documents by default, or at best filesystem=home?

That's for the "recent files view" that you get when running "evince" to still work correctly.

Why though? I just tested removing filesystem=host using Flatseal, opened a PDF using the "Open..." button, closed Evince, opened Evince again, and the recent document is there and works. The file access is persistent:

$ flatpak documents org.gnome.Evince 
ID
f77477f2

@hadess
Copy link
Collaborator

hadess commented Apr 5, 2022

Why though?

In my memory, the recent files view showed all the PDFs, not just the ones opened by Evince. That might not be accurate (anymore?).

@Mikenux
Copy link
Contributor

Mikenux commented Apr 5, 2022

In my memory, the recent files view showed all the PDFs, not just the ones opened by Evince. That might not be accurate (anymore?).

The recent view only shows files recently opened by Evince (I tested removing the filesystem=host permission and two files, and both files are shown in the recent view). Thus, access can be limited. I guess if flatpak goes with file types/extensions globally (for all files in home/user), it would be an improvement even without the restriction with neighboring files - at least if it's doable.

gnomesysadmins pushed a commit to GNOME/evince that referenced this issue Apr 5, 2022
This should not be needed, since Evince uses the file chooser to open
files and the recent files view works since access to the files opened
with the file chooser portal is persistent.

flathub/org.gnome.Evince#76
@mwleeds
Copy link

mwleeds commented Apr 5, 2022

Why though?

In my memory, the recent files view showed all the PDFs, not just the ones opened by Evince. That might not be accurate (anymore?).

Looks like Evince ignores recent items from other applications, since at least 2014: https://gitlab.gnome.org/GNOME/evince/-/blob/b4bdbc420c8d7bec29a710a43deb8e8368275b8d/shell/ev-recent-view.c#L717

Here's an MR to remove the filesystem=host permission: https://gitlab.gnome.org/GNOME/evince/-/merge_requests/462

@mwleeds
Copy link

mwleeds commented Apr 5, 2022

I guess if flatpak goes with file types/extensions globally (for all files in home/user), it would be an improvement even without the restriction with neighboring files - at least if it's doable.

I don't understand what you mean, could you try rephrasing?

@Mikenux
Copy link
Contributor

Mikenux commented Apr 6, 2022

@mwleeds:

There is discussion about limiting access to specific file types (e.g. images, videos) and file extensions that are in a given folder (for example, automatically open a subtitle file when a video file is opened; see flatpak/xdg-desktop-portal#463).

What I said is that this access limitation could also be applied in general, i.e. when the application has access to all folders.

@Mikenux
Copy link
Contributor

Mikenux commented May 7, 2022

If the neighboring files functionality is designed to be simple (i.e. without giving the reason for accessing these files), I think it doesn't make sense to have a dialog asking for it when, for example, a PDF file is opened... The question is whether users will click "Allow" if they don't know why Evince wants to access neighboring files when opening their PDF file (is it obvious when a user has a catalog of PDFs or has downloaded a PDF with additional resources?).

@Mikenux
Copy link
Contributor

Mikenux commented May 17, 2022

@hadess:

Given what I've read and commented on in another issue of Evince, I think neighboring files should (also) be available as a filesystem restriction to have regular filesystem names − and not the sandboxed ones, needed for "Open Containing Folder".

@hadess
Copy link
Collaborator

hadess commented May 23, 2022

Given what I've read and commented on in another issue of Evince, I think neighboring files should (also) be available as a filesystem restriction to have regular filesystem names − and not the sandboxed ones, needed for "Open Containing Folder".

I can't parse this, sorry.

@Mikenux
Copy link
Contributor

Mikenux commented May 23, 2022

If filesystem=host is specified, when we click "Open Containing Folder", Evince shows the file in its original folder (e.g. /home/user/Documents/MyFiles - the "normal path") .

If filesystem=host is disabled (via Flatseal), Evince is more sandboxed, and clicking "Open Containing Folder" shows the file in a sandboxed folder (e.g. /run/user/1000/doc/ a1b23c4d - the "sandbox path").

So if the neighboring files feature creates a sandbox for the files, Evince will have a sandbox path for them. And I think we want the normal path when we click "Open Containing Folder".

@hadess
Copy link
Collaborator

hadess commented May 23, 2022

I honestly don't know what you want me to do with this information. Is it a bug report, or ideas for a design for a portal feature that doesn't yet exist?

@mwleeds
Copy link

mwleeds commented May 23, 2022

Sounds to me like a comment that should be added to flatpak/xdg-desktop-portal#463 so it can then either be taken into account in the design of that portal, or moved to a separate issue on the xdg-desktop-portal repo for later follow-up.

@Mikenux
Copy link
Contributor

Mikenux commented May 23, 2022

It's just that if the neighboring files portal creates a sandbox, and if Evince uses it, "Open Containing Folder" will open a sandboxed folder instead of showing the correct location.

This was mentioned on the neighboring files issue when we thought it would be nice to have it as an improvement over "filesystem=host". But it probably wasn't clear. It's hard to say something when we don't know how this portal will be developed...

@Mikenux
Copy link
Contributor

Mikenux commented May 23, 2022

Given that this is the case when Evince does not use filesystem=host and in Firefox, I decided to open a bug (flatpak/xdg-desktop-portal#807).

@hadess
Copy link
Collaborator

hadess commented May 24, 2022

  • Evince has filesystem=host permissions by default. Shouldn't it be restricted to access only xdg-download and xdg-documents by default, or at best filesystem=home?

I just remembered that the host filesystem was probably so that evince could access system-provided documentation. That should still work through the document portal though.

@rugk
Copy link

rugk commented Jul 7, 2022

@gpoo
Copy link
Collaborator

gpoo commented Mar 14, 2023

#88 fixed the issue.

@gpoo gpoo closed this as completed Mar 14, 2023
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 a pull request may close this issue.

6 participants