-
Notifications
You must be signed in to change notification settings - Fork 529
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
"SUID sandbox helper binary" warning on debian 10 #222
Comments
Happens to me as well. Happens with the appimage as well.
|
If one is okay with with the possible risks, as temporary workaround: sudo sysctl kernel.unprivileged_userns_clone=1 Or to make the change survive reboot add kernel.unprivileged_userns_clone=1 to /etc/sysctl.conf (or /etc/sysctl.d ) and run sudo sysctl --system |
I just read about that, and I'm not okay with the additional risks. |
Looks like there's a packaging change that I need to make, based on electron/electron#17972 @quizilkend @StygianBlues can you share what the permissions are for |
Permissions on my /opt/GitHub Desktop/chrome-sandbox file are 755 (rwxr-xr-x). My machine's kernel: 5.4.0-4-rt-amd64 #1 SMP PREEMPT_RT Debian 5.4.19-1 (2020-02-13) x86_64 GNU/Linux. Thank you for all the work you do in keeping this available on Linux. |
Actually, this comment from inside the /**
* For Electron versions that support the setuid sandbox on Linux, changes the permissions of
* the `chrome-sandbox` executable as appropriate.
*
* The sandbox helper executable must have the setuid (`+s` / `0o4000`) bit set.
*
* This doesn't work on Windows because you can't set that bit there.
*
* See: https://github.com/electron/electron/pull/17269#issuecomment-470671914
*/ I believe this would change the permission to |
Just downloaded Debian 10 (GNOME) and tested @Q-codes:
I unfortunately got a Adding the following command afterwards worked for me:
|
@jfgordon2 I was hoping the change to |
Out of curiosity, is anyone able to get this to work wiht the
I've found some other threads that seem to suggest this works, e.g signalapp/Signal-Desktop#3536 (comment), but it's still not a great solution. |
@shiftkey I looked through the Ubuntu seems to enable |
Can confirm, no sandbox works with that kernel option disabled on debian 10 |
I've opened #254 to track the file mode change so please follow along with that PR. @jfgordon2 the user owning |
Correct |
The problem with |
@Q-codes it's not clear to me that we've got the full fix given the |
Unassigning myself for now because The
The latter would be ideal (catching all cases) if we can find a way to show some UI that won't crash, but it would require more integration into the codebase itself... |
Just tried
|
@Symbian9 oops, I guess |
I want use Is it possible? |
@Symbian9 I'm honestly not sure - it's been at least a year since I looked at anything related to AppImage |
As I remember I no need to launch |
@Symbian9 this might be related to the |
After building Electron from source and slipstreaming it into the current GitHub Desktop app I was able to trigger the
I can't spot anything obviously wrong here, but if you want to spelunk the source here I recommend entering the file into https://source.chromium.org/ |
This Zygote documentation is probably important: https://source.chromium.org/chromium/chromium/src/+/master:docs/linux/zygote.md;l=16?q=zygote&ss=chromium%2Fchromium%2Fsrc |
So I think I've stumbled into the root cause of problem, and it's part of Chromium itself electron/electron#21309
Two options:
EDIT: not sure what the fix looks like for the AppImage, unless that too is generating a parent directory containing spaces. EDIT 2: i've confirmed moving the app from |
@shiftkey I was taking a look at the electron-builder script, and it seems like the path name is dependent on the product name in the package.json file The productFilename variable is just a sanitized productName: Which would mean that you can do some creative things like:
Or I guess you could do some changes to the electron-builder to "sanitize" differently. |
Step 1 was how it was originally configured (you can override I've been planning to move away from I've tested |
I did what jfgordon2 said and it worked |
I've published @Symbian9 I see you encountered a similar problem with the AppImage installer - could you open a fresh issue to provide details about your setup and what the installer is doing (or not doing) so we can focus fresh on that? |
Could you point me? I'm not sure what you mean under "AppImage installer", especially "installer" term. |
@Symbian9 I'm referring to this comment: #222 (comment) |
This is how I had dealt with this annoying Electron/Debian/AppImage issue in another project: I'd be happy to hear whether there are better solutions. |
@probonopd there were two issues I found:
Given the AppImage can be run from anywhere, we might not be able to fix the latter if users run it from a directory with spaces in the path. But I wanted to first confirm we haven't also fixed this for AppImage... |
Thanks @shiftkey. An AppImage, when executed, gets mounted at a temporary mountpoint in
Are you saying that doing this will remove the need for |
Good to know. Then it may just be the file mode issue that actually affected the AppImage and we've conflated the two problems because they both mention
I believe so, based on this comment that also mentioned the AppImage from @quizilkend:
|
Some experimentation:
I don't know whether it would be possible (or desirable) to change something in FUSE so that it honors the setuid bit. |
Oof, that's rough. Thanks for digging in @probonopd! |
Describe the bug
Unable to run the program GitHubDesktop.
Version & OS
GitHubDesktop-linux-2.3.1-linux1.deb
Linux ma-pc 4.19.0-8-amd64 #1 SMP Debian 4.19.98-1 (2020-01-26) x86_64 GNU/Linux
Debian GNU/Linux 10 (buster)
Steps to reproduce the behavior
Expected behavior
A clear and concise description of what you expected to happen.
Actual behavior
A clear and concise description of what actually happened.
Screenshots
Add screenshots to help explain your problem, if applicable.
Logs
Attach your logs by opening the
Help
menu and selectingShow Logs...
, if applicable.Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: