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

Forcing --no-sandbox is not a solution #3573

Open
1 task done
SISheogorath opened this issue Sep 7, 2019 · 12 comments
Open
1 task done

Forcing --no-sandbox is not a solution #3573

SISheogorath opened this issue Sep 7, 2019 · 12 comments

Comments

@SISheogorath
Copy link

  • I have searched open and closed issues for duplicates

Bug Description

Slapping --no-sandbox into the desktop file, and therefore disabling all sandboxing, is not a proper fix. Electron Version 5 improved security for electron-based applications. It doesn't help anyone when those features are disabled entirely.

Problem was introduced in this commit:

1ca0d82

Problem could be resolved in a similar way to this commit:

element-hq/element-web@56674ea

I just hope I can bring this successfully to your attention, since I would consider it a serious problem when a software like Signal simply dismisses the security gains by proper Sandboxing.

@kierun
Copy link

kierun commented Feb 6, 2020

Running 1.30.1 on Cent OS 7's latest kernel still requires --no-sandbox to run. I am not sure if this is relevant or not… ☺

@moppman
Copy link

moppman commented Sep 16, 2020

Setting the SUID bit on chrome-sandbox via chmod 4755 chrome-sandbox was not working for me (Debian 10). I had to enable user namespaces sandboxing (see e.g. here for reference) via sysctl kernel.unprivileged_userns_clone=1.

@pasko
Copy link

pasko commented May 3, 2021

On Debian 11 the kernel.unprivileged_userns_clone is set to 1 by default, it seems. I tested dropping --no-sandbox from command line arguments on Debian 11 (Bullseye) and signal-desktop started without an error.

Setting the SUID bit on the helper is deprecated in Chromium, the aim is to replace it with the namespace sandbox (which is the default right now): explainer. All maintained kernel versions already support the namespace sandbox.

I think an ideal solution right now would be to have a helper that adds --no-sandbox only if /proc/sys/kernel/unprivileged_userns_clone (and its variants from other distros) reads as 0. Would this work?

@iabraham
Copy link

I just remove it from the .desktop file whenever I do an update and Signal seems to start just fine (Ubuntu 20.04.3 LTS).

@stale
Copy link

stale bot commented Dec 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 14, 2021
@pasko
Copy link

pasko commented Dec 14, 2021

seems like a serious security/privacy issue, no?

@stale stale bot removed the stale label Dec 14, 2021
@stale
Copy link

stale bot commented Mar 14, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 14, 2022
@SISheogorath
Copy link
Author

Stalebot is great :)

@iridos
Copy link

iridos commented Oct 23, 2022

stale-bot is the only one interested on "working" on the ticket.

IIRC, aving kernel.unprivileged_userns_clone=1 has had multiple security problems / right escalations in the past.

signal-desktop only needs the capability at start-up, but temporarily enabling capabilities for that time span also needs root and that doesnt seem like a proper workaround either.

@korg91
Copy link

korg91 commented Dec 31, 2024

This is a security vulnerability and it has not been addressed yet. The Electron documentation explicitly recommends against this:

You can also disable Chromium's sandbox entirely with the --no-sandbox CLI flag, which will disable the sandbox for all processes (including utility processes). We highly recommend that you only use this flag for testing purposes, and never in production.

I found out about this by chance as I was looking at running processes and I noticed the --no-sandbox flag. So not only is this the default behavior, but it is also not transparent to the user.

I find it quite concerning that a security-oriented app like Signal goes against "highly recommended" security practices and seems to give little consideration to complaints for over 5 years. Messaging apps can and do be targeted as an attack vector to break into the OS -- see Pegasus. I'm not a sandbox expert, but I'd expect sandboxing to provide at least some degree of protection against that kind of threat.

The Signal app seems to work perfectly on Ubuntu without the --no-sandbox flag. As a workaround, I just copied the .desktop file to the local folder and removed the flag:

cp /usr/share/applications/signal-desktop.desktop ~/.local/share/applications/

This automatically overrides the Signal entry in the app menu. But this is clearly not the right solution for the average user and also means that any future (potentially important) changes to the packaged-owned .desktop file are overridden too.

@trevor-signal
Copy link
Contributor

Hi @korg91, the next beta release of Signal Desktop (7.38) removes the --no-sandbox CLI flag and also includes a recent electron-builder update that supports Chromium's user namespaces sandbox. There is still more work to do, but we would appreciate your help testing the latest beta on Linux when it is released.

@WhyNotHugo
Copy link
Contributor

Do these upstream updates also cancel the need for mounting /tmp/ without noexec? As, in: #6897

If user namespaces are used, then no need to write executable ont the host's /tmp/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

10 participants