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

Upstream the Flatpak sandbox code #337

Open
DemiMarie opened this issue Nov 15, 2023 · 27 comments
Open

Upstream the Flatpak sandbox code #337

DemiMarie opened this issue Nov 15, 2023 · 27 comments

Comments

@DemiMarie
Copy link

DemiMarie commented Nov 15, 2023

The Chromium team is probably in a better shape than it was when #43 (comment) was written. Furthermore, this would allow Electron apps to work normally under flatpak without requiring Zypack. I suspect this is what would be needed for apps like Element to ship an official flatpak.

@cjao
Copy link

cjao commented May 17, 2024

Here's an interesting thread about Chromium's sandbox strength when modified to work with flatpak. Note in particular:

Pls check the namespace structure of Flatpak’s Chromium vs the official native Chrome with lsns -T. You will see a noticable difference and weakened site isolation. The native Chrome namespace structure is more fine-grained. Thus leading to better horizontal isolation.

Is this accurate? If so, is it a limitation of flatpak or just of this particular adaptation of Chromium to Flatpak?

@refi64

@user1-github
Copy link

I'm also all in favor of upstreaming the Flatpak sandbox code.

However, since we're started talking about the Flatpak sandbox quality, here is another interesting comment by Cromite browser dev questioning the sandbox quality of the patch after comparing it to the non Flatpak version of the browser. The next few comments are also interesting.

Basically, their main finding is that the browser's renderer process isn't properly sandboxed with the Flatpak sandbox patch. Is this a limitation of running Chromium browsers in Flatpak, or are they interpreting their findings wrong and the renderer process is actually properly sandboxed?

@DemiMarie
Copy link
Author

Chromium’s sandbox is designed for Chromium and so is stronger than what Flatpak currently offers. This could be fixed on the Flatpak side.

@cjao
Copy link

cjao commented Jul 17, 2024

Is it enough to provide a flatpak option to allow user namespace creation?

@cjao
Copy link

cjao commented Jul 18, 2024

Basically, their main finding is that the browser's renderer process isn't properly sandboxed with the Flatpak sandbox patch. Is this a limitation of running Chromium browsers in Flatpak, or are they interpreting their findings wrong and the renderer process is actually properly sandboxed?

Here is another interesting thread discussing how putting Chromium in flatpak may weaken its sandbox.

@DemiMarie
Copy link
Author

Is it enough to provide a flatpak option to allow user namespace creation?

This would be a security vulnerability in Flatpak. Flatpak portals check /proc/$PID/root to determine which flatpak made a D-Bus API call. It would also massively increase kernel attack surface.

The proper solution is for Flatpak to provide a sandbox that is as tight as Chromium wants it to be. This will need to be opt-in on the part of applications, but also only very few applications can run with a sandbox that is as tight as that of the Chromium render process.

@DemiMarie
Copy link
Author

Further thought: Could Chromium use Landlock to sandbox itself adequately? Landlock requires no privileges to use at all, nor does it rely on namespaces. It is much more similar to how macOS sandboxing works.

@cjao
Copy link

cjao commented Jul 18, 2024

Is it enough to provide a flatpak option to allow user namespace creation?

This would be a security vulnerability in Flatpak. Flatpak portals check /proc/$PID/root to determine which flatpak made a D-Bus API call. It would also massively increase kernel attack surface.

The proper solution is for Flatpak to provide a sandbox that is as tight as Chromium wants it to be. This will need to be opt-in on the part of applications, but also only very few applications can run with a sandbox that is as tight as that of the Chromium render process.

Would it be a greater security vulnerability than --filesystem=host or the other holes that one can already poke in the sandbox? Those options support major use cases:

  • Apps that can but have yet to opt into the portal APIs
  • Apps for which the portal APIs are insufficient
  • Apps that are not practical to be confined, such as IDEs which generally expect full access to the host system (especially if e.g. one is doing low-level development).

An option like --allow-userns would cater to a fourth category:

  • Apps that integrate their own custom sandboxes.

For comparison, Ubuntu snap provides a custom security profile specifically designed to support browsers that bring their own sandbox. In particular, this profile whitelists user namespaces. This allows them to ship Chromium using snap without modifying Chromium's battle-tested sandbox.

@DemiMarie
Copy link
Author

Is it enough to provide a flatpak option to allow user namespace creation?

This would be a security vulnerability in Flatpak. Flatpak portals check /proc/$PID/root to determine which flatpak made a D-Bus API call. It would also massively increase kernel attack surface.
The proper solution is for Flatpak to provide a sandbox that is as tight as Chromium wants it to be. This will need to be opt-in on the part of applications, but also only very few applications can run with a sandbox that is as tight as that of the Chromium render process.

Would it be a greater security vulnerability than --filesystem=host or the other holes that one can already poke in the sandbox? Those options support major use cases:

  • Apps that can but have yet to opt into the portal APIs
  • Apps for which the portal APIs are insufficient
  • Apps that are not practical to be confined, such as IDEs which generally expect full access to the host system (especially if e.g. one is doing low-level development).

I would argue that IDEs can practically be confined, and that Qubes OS does this all the time. Of course, it probably helps that that the confinement is implemented by giving the IDE a whole VM.

An option like --allow-userns would cater to a fourth category:

  • Apps that integrate their own custom sandboxes.

For comparison, Ubuntu snap provides a custom security profile specifically designed to support browsers that bring their own sandbox. In particular, this profile whitelists user namespaces. This allows them to ship Chromium using snap without modifying Chromium's battle-tested sandbox.

This is an acceptable stopgap, but it means that the browser process is either unconfined or much less confined, which means that the system is exactly as secure as if a flatpak was not used. Providing a sandbox that is as strong as Chromium’s is the best solution, but is also the most work.

@cjao
Copy link

cjao commented Jul 18, 2024

This is an acceptable stopgap, but it means that the browser process is either unconfined or much less confined, which means that the system is exactly as secure as if a flatpak was not used.

This is true. However, Chromium is a bit of a special case because it's basically the gold standard for application sandboxing, whereas the builtin Flatpak sandbox is designed for programs that don't already have internal security mechanisms.

Even if Flatpak does not add a security boundary, the ability to bundle dependencies would still have independent value. In fact, some blog posts early in the development of flatpak suggest that bundling was the initial focus.

@Erick555
Copy link
Contributor

Chromium has per-process sandboxing which protect processes from each other. Flatpak sandbox protects host from the whole app environment. They're different layers and orthogonal which is why flatpak will never replace chromium sandboxing. The problem is flatpak also blocks creating of chromium own sandbox which needs workaround.

@DemiMarie
Copy link
Author

Chromium has per-process sandboxing which protect processes from each other. Flatpak sandbox protects host from the whole app environment. They're different layers and orthogonal which is why flatpak will never replace chromium sandboxing. The problem is flatpak also blocks creating of chromium own sandbox which needs workaround.

On Linux, a sandboxed process will not generally have the privileges to create sub-sandboxes. That’s why I think it is better for Flatpak to have a special case that creates an ultra-strict sandbox, even if Chromium is the only one to use it.

Leaving the browser process unsandboxed is reasonable (if suboptimal) for Chromium itself, but not for Electron apps.

@Erick555
Copy link
Contributor

For chromium you need many sandboxes not one and there is no one ultra-strict sandbox that will work for all chromium process (i.e. some require network access and some don't).

Without flatpak chromium parent process creates sandboxes for its child processes. In flatpak chromium parent process must ask flatpak to crate sandbox for child on its behalf.

Flatpak can't manage chromium processes and chromium parent can't create subsandboxes so they need to cooperate i.e. through flatpak-spawn api which is done atm. There is no replacement one by the other in cards.

@DemiMarie
Copy link
Author

Can flatpak-spawn be made flexible enough for Chromium’s needs?

@Erick555
Copy link
Contributor

Erick555 commented Jul 20, 2024

I don't know. Maybe. This is something to delve into when someone starts upstreaming.

@cjao
Copy link

cjao commented Aug 25, 2024

The lead developer of cromite recently investigated the Flatpak sandbox adapter used by Chromium and concluded that

that patch is highly insecure and should not be used.

Of course, flatpak is effective in the process and disk sandbox. but, somehow, it seems to reduce the rendering process sandbox, the most important one in chromium. the objective of chromium is to reduce the possibilities for websites, in the event of security holes, to tamper with the user's browsing, or to steal it. this is why it is important that the sandbox remain as defined by the chromium security team, or improve it.
From what it seems to me, but mind you, it seems to me, flatpak is fine for isolating applications that do not have a sandbox of their own, but perhaps not suitable for use in more sophisticated applications.

@user1-github
Copy link

user1-github commented Aug 25, 2024

The lead developer of cromite recently investigated the Flatpak sandbox adapter used by Chromium and concluded that

That's the comment I linked to in my previous post about a month ago.

I agree with rany2's comment in that thread - as you can see, different devs interpret the results differently. That's why I think it's worth trying to submit upstream the Flatpak sandbox patch, so that Chromium devs who actually understand how browser sandboxing should work will properly review and scrutinize the sandbox patch.

@rusty-snake
Copy link

that patch is highly insecure and should not be used.

I concluded that over two years ago, madaidans-insecurities/madaidans-insecurities.github.io#62, so it is really something we know and life with.

Also see flatpak/flatpak#5879 (comment)

@DemiMarie
Copy link
Author

DemiMarie commented Aug 25, 2024

Could Landlock be a solution to this? “Running in a filesystem namespace with no access to anything” should be equivalent to “running with a Landlock policy that blocks all filesystem operations.” The latter requires no privileges at all. Landlock cannot currently reject all filesystem operations.

@rusty-snake
Copy link

rusty-snake commented Aug 25, 2024

Well, the difference between an empty mount-namespace (what I guess you mean with filesystem namespace) and blocking all filesystem related syscalls (seccomp) is not huge.

@DemiMarie
Copy link
Author

@rusty-snake Indeed so. Flatpak imposes no restrictions on seccomp or Landlock, and if Chromium was able to adequately sandbox itself without namespaces the problem would go away.

@rusty-snake
Copy link

To me this sounds like adding a --add-seccomp-fd argument to flatpak-spawn (flatpaks sub-sandbox interface/tool) like flatpak-spawn --sandbox --no-network --add-seccomp-fd={FD} would significantly increase the sub-sandboxing capabilities.

  • user,pid,ipc,mnt,net namespaces are unshared
  • NNP is set and the capability bounding-set is cleared
  • filesystem permissions are reduced to app and runtime only (you could provide a custom sub-runtime/sub-app)
  • no D-Bus access
  • custom seccomp-bpf

@refi64
Copy link
Collaborator

refi64 commented Aug 27, 2024

I posted a reply on the cromite thread. With regard to the comments here...most of this literally already exists, as-is Chromium's layer 2 sandbox uses seccomp, and there is work towards using landlock.

Basically everything outlined under --add-seccomp-fd is also already a thing that's actively used.

@rusty-snake
Copy link

So what's missing then in flatpaked chromium browsers?

@DemiMarie
Copy link
Author

I posted a reply on the cromite thread. With regard to the comments here...most of this literally already exists, as-is Chromium's layer 2 sandbox uses seccomp, and there is work towards using landlock.

Did you mean to link to something else? The link you posted points back at this issue.

@refi64
Copy link
Collaborator

refi64 commented Aug 28, 2024

once again I prove to myself that writing issue comments at 9pm is a mistake, correct link to the landlock support issue.

@robertwthomson
Copy link

there is work towards using landlock.

More like the start of a discussion than work, though

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

No branches or pull requests

7 participants