Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

3.17.1.AppImage does not start linux #1213

Closed
Ruslan5h opened this issue Nov 26, 2019 · 11 comments · Fixed by #1214
Closed

3.17.1.AppImage does not start linux #1213

Ruslan5h opened this issue Nov 26, 2019 · 11 comments · Fixed by #1214

Comments

@Ruslan5h
Copy link

linux Patchwork-3.17.1.AppImage does not start

@black-puppydog
Copy link
Contributor

black-puppydog commented Nov 26, 2019

Confirmed. This is due to the sandboxing issue.

[29971:1126/200605.885487:FATAL:setuid_sandbox_host.cc(157)] The SUID sandbox helper binary was found, but is not configured correctly. Rather than run without sandboxing I'm aborting now. You need to make sure that /tmp/.mount_Patchws3H8fi/chrome-sandbox is owned by root and has mode 4755.

IIRC there's a step that needs to be taken during build to fix this, this is the root cause:
electron/electron#17972
I am heading out right now. I'll have a look later tonight though

@christianbundy
Copy link
Contributor

@Ruslan5h Could you please try:

sudo sysctl kernel.unprivileged_userns_clone=1

@christianbundy
Copy link
Contributor

Previous discussion: #1199 (comment)

@black-puppydog
Copy link
Contributor

Thank you Christian.

Keeping in mind that this is not of your making (indeed you were the one to first raise the issue upstream, thanks for that too) I have two issues with this:

  1. This stackexchange answer and the comments following up to it seem to suggest that unprivileged_userns_clone has in the past (and supposedly continues to be) a potential risk to the system integrity. This really goes out to the electron devs more than anyone else, but are we trading off "app security" (by sandboxing electron apps) against "system security" (by allowing arbitrary user code to interact with low-audit kernel code)?
  2. This assumes the user has sudo rights on their machine. I would guess that that's a sane assumption (statistically speaking) for now, but even then, it would have prevented me from running patchwork for my office identity. Long-term I'd say it's not viable.

The interesting bit is... am I misunderstanding this, or is this the same situation for any electron app? Do they all cease to be useful for non-priviledged users who can't get their admins to disable a security feature?

@christianbundy
Copy link
Contributor

Yeah, the options seem to be:

  • setuid: Root privileges to chrome-sandbox? I'd feel very uncomfortable with this. 😱
  • unprivileged_userns_clone: Potential security problems, I'm more comfortable with this.
  • --no-sandbox: maybe better? I feel bad about this too.

@nornagon I'd love input from you on this, if you have thoughts/feels.

@nornagon
Copy link
Contributor

unprivileged_userns_clone is what Chrome prefers, though I'm not familiar with AppImage to know if that would be allowed through whatever sandboxing layers AppImage applies. This is the best option, but it depends on the feature being enabled in the kernel (& allowed by the AppImage sandboxing layer maybe?).

chrome-sandbox being setuid root is what Chrome falls back to when unpriviliged userns_clone isn't available. Electron ships the same exact binary that Chrome does, unmodified. You can audit it here. You could even just copy one from Chrome if you'd prefer. IMO it's safe to setuid root for this binary.

I'd advise against --no-sandbox as it would prevent you from isolating sites from each other if that becomes necessary (e.g. imagine wanting to isolate a youtube.com iframe from the rest of Patchwork).

@black-puppydog
Copy link
Contributor

Honestly, we try to keep everything online out of this app anyhow. It's supposed to be offline-first, and loading e.g. images from the web instead of from a local source has been considered a privacy issue in the past. So if we're sure we don't even load remote content (sure as in we have means in place to completely prevent it) then I'd say --no-sandbox is acceptable, no?

It also seems to be the only option for people running debian without sudo. Which seems to be a non-empty set of people.

@christianbundy
Copy link
Contributor

So if we're sure we don't even load remote content (sure as in we have means in place to completely prevent it) then I'd say --no-sandbox is acceptable, no?

I know that we load "remote" content from localhost, like when we serve blobs, and I think there's a remote HTTP request to check for Electron updates, but otherwise I don't think that we'll be reaching out to remote sites. Maybe we should add a document with all of the above options and point to that in the release notes?

@nornagon
Copy link
Contributor

I'm not sure that's the sort of thing that an end user should be configuring :)

Does AppImage itself sandbox the app at all?

@black-puppydog
Copy link
Contributor

No, it is just able to "run in a sandbox like Firejail."
But I agree that a real solution to this would make the issue disappear for the end user. Sadly, I think given the constraints from upstream here, I don't really see a way where we don't either disable sandboxing (on debian systems with namespaces disabled) or make the user jump through some extra hoops.

@christianbundy
Copy link
Contributor

Yeah, the best option I've seen is electron-userland/electron-builder#4364 but I think it works on .deb rather than AppImage. I don't want to ask users to configure anything, but since we're using electron-builder and people are using AppImages it doesn't seem like we have an easy solution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants