Chrome SUID sandbox prevents AppImage from running

[joeyh]

joey@darkstar:~>bin/patchwork 
[3239:1128/104205.521035: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 /home/joey/tmp/.mount_patchwSfbcAw/chrome-sandbox is owned by root and has mode 4755.
Trace/breakpoint trap
 - exit 133

Previous versions have not had this problem.

Linux 5.2.0, Debian unstable

[joeyh]

electron/electron#17972

patchwork --no-sandbox works around this electron/debian incompatability. Or enable unprivileged user namespaces, but that weakens the system's security.

[christianbundy]

Thanks for the bug report. Do you have any recommendations on how we should resolve this?

There's a bullet point about this in the release notes and an active discussion about this in #1213, but we don't have a way to set --no-sandbox in the AppImage release and both of the other solutions require root. To make things harder, the SUID solution doesn't work in the AppImage release because it uses a temporary filesystem mounted in /tmp/.

Not sure what the right move is.

cc: @black-puppydog @nornagon maybe let's move the discussion here?

[joeyh]

One way would be to adjust the AppRun script inside the appimage to pass --no-sandbox, or add another wrapper shell script around the ssb-patchwork binary. I did it as a proof of concept: mkdir patchwork cd patchwork patchwork --appimage-extract cd squashfs-root # edited AppRun diff -u AppRun.orig AppRun --- AppRun.orig 2019-11-28 17:58:05.251883948 -0400 +++ AppRun 2019-11-28 17:58:15.699612748 -0400 @@ -40,7 +40,7 @@ { if [ $isEulaAccepted == 1 ] ; then if [ $NUMBER_OF_ARGS -eq 0 ] ; then - exec "$BIN" + exec "$BIN" --no-sandbox else exec "$BIN" "${args[@]}" fi ARCH=x86_64 ~/appimagetool -v . Resulting repacked appimage works here.

…

-- see shy jo

[joeyh]

BTW, I can't really recommend --no-sandbox in good conscience, since I don't fully understand what electron is accomplishing with its sandbox. Maybe it's really important for security, or maybe it's not relevant for patchwork, or would only be a useful line of defense if a bug allowed malicious javascript to be embedded in a ssb message and run in the browser.

…

-- see shy jo

[christianbundy]

Same. I'm thinking maybe we should just add some documentation on the workarounds and add it to the release notes? I don't feel very comfortable advocating for any of these workarounds, and it's hard because there are conflicting perspectives:

• Debian: Disallow unprivileged CLONE_NEWUSER by default
• Electron/Chromium: Depend on unprivileged CLONE_NEWUSER or root SUID on chrome-sandbox, discourage --no-sandbox

[nornagon]

FWIW, --no-sandbox gets you the same security you used to have. The only thing that’s sandboxes by default in Electron is the GPU process. However, if you want to sandbox more stuff in future then obviously --no-sandbox will get in your way :)

On Mon, Dec 2, 2019 at 08:22 Christian Bundy ***@***.***> wrote: Same. I'm thinking maybe we should just add some documentation on the workarounds and add it to the release notes? I don't feel very comfortable advocating for *any* of these workarounds, and it's hard because there are conflicting perspectives: - *Debian:* Disallow unprivileged CLONE_NEWUSER by default <https://sources.debian.org/patches/linux/3.16.56-1+deb8u1/debian/add-sysctl-to-disallow-unprivileged-CLONE_NEWUSER-by-default.patch/> - *Electron/Chromium:* Depend on unprivileged CLONE_NEWUSER or root SUID on chrome-sandbox, discourage --no-sandbox — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1217?email_source=notifications&email_token=AABKGADISDEEQRULIN45LSLQWUY2NA5CNFSM4JSVOQNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFUBDHY#issuecomment-560468383>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABKGAAM4NW4UE5VRPSFJK3QWUY2NANCNFSM4JSVOQNA> .

-- j

[christianbundy]

I've opened an issue upstream in electron-builder, but for the time being I've added some documentation in #1218. Would it be wise to automatically add --no-sandbox to the .deb install, since Debian in particular has disabled this feature? Looks like we can do that pretty easily, but it doesn't fix the AppImage.

[madduck]

BTW, I can't really recommend --no-sandbox in good conscience, since I don't fully understand what electron is accomplishing with its sandbox. Maybe it's really important for security, or maybe it's not relevant for patchwork, or would only be a useful line of defense if a bug allowed malicious javascript to be embedded in a ssb message and run in the browser.

Fair enough, though I have a hard time imagining what the heck the sandbox could be protecting from, which is mitigated by running code as root.