Desktop: Resolves #9136: Install script: Work around unprivlidged user namespace restrictions by adding the --no-sandbox flag to the launcher

[personalizedrefrigerator]

Summary

Updates the Linux install script to add the --no-sandbox flag on Ubuntu 23.10. See the discussion on the forum.

According to this article on Chromium's sandbox on Linux, there are legacy sandboxes that don't use the unprivileged user namespace capabilities of the kernel. However, I don't see a way to disable just the unprivileged user namespace restriction and enable one of the legacy sandboxing approaches (or any notes about whether this is a bad idea).

Testing

Using this, I have manually tested the script on Ubuntu 23.10 by running

$ echo 1 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns

then

$ bash Joplin_install_and_update.sh --force

(--force because I already have Joplin 2.12.19 from previous runs of the script), then attempting to launch Joplin.

Notes

This pull request is a draft mostly intended for discussion. Note that the Electron docs state,

Disabling Chromium's sandbox (testing only)

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.

Note that the sandbox: true option will still disable the renderer's Node.js environment.

[[ emphasis added ]]

An alternative would be distributing a custom .deb, but it seems that Electron Builder doesn't support this (we would likely have to switch to Electron Forge).

[personalizedrefrigerator]

The original conditional didn't work in my shell (bash). Note that

TEST="test"
YES="yes"
NO="no"
if [[ $TEST = "test" ]] || [[ $YES = "no" ]] && [[ $NO = "yes" ]] ; then
   echo "if"
fi

produces no output (the arguments to && need to be grouped). See https://unix.stackexchange.com/a/717776

[tessus]

Uaing [[ can be tricky, but @personalizedrefrigerator's change is sound.

[personalizedrefrigerator]

Suggested change

	then
	SANDBOXPARAM="--no-sandbox"
	fi
	then
	SANDBOXPARAM="--no-sandbox"
	print "${COLOR_YELLOW}WARNING${COLOR_RESET} Electron sandboxing disabled. Avoid loading untrusted notes/PDFs/audio/video files."
	fi

If we do merge this as is, it may make sense to include a warning.

[tessus]

A warning is not a bad idea. I would go for it.

[laurent22]

Any idea who could review this pull request? Maybe @tessus or James Carroll (but I don't know his GitHub handle)?

[tessus]

Please see my inline comments.

Please move the PR out of the Draft status, unless you never intend to merge it. Update: I saw that you meant it as a discussion but I think your reasoning makes sense.

[tessus]

Uaing [[ can be tricky, but @personalizedrefrigerator's change is sound.

[tessus]

A warning is not a bad idea. I would go for it.

[tessus]

No extra blank line needed. Only EOL at EOF.

[personalizedrefrigerator]

It may also make sense to link to a forum post (or FAQ entry) that gives a high-level summary of what this means (and how it affects Joplin's security).

[tessus]

Yes, such info is always helpful and might put people at ease. However, I am not sure if we already have any forum posts or a FAQ entry in the docs.

[laurent22]

I think a link would be better and the message "Avoid loading untrusted notes/PDFs/audio/video files" could be removed because that's a bit vague and probably always true.

But a link with more information would clarifies the issue.

[laurent22]

@personalizedrefrigerator, could you link to one of the existing forum posts? I'm not sure which one is the best. There's no need to create a FAQ entry for now

[personalizedrefrigerator]

I've added a link to https://discourse.joplinapp.org/t/possible-future-requirement-for-no-sandbox-flag-for-ubuntu-23-10/32160.

Of the search results for no-sandbox on the forum, it and a snap-related post seemed most relevant.

[laurent22]

Thanks, could you set it to https://discourse.joplinapp.org/t/32160/5 please? As it's less likely to wrap in the terminal

[personalizedrefrigerator]

$DISTVER = "Ubuntu23.10". We may want to update this to include future versions of Ubuntu as well (depending on whether they plan to keep this restriction enabled or not).

[personalizedrefrigerator]

Note that I am still able to launch AppImages on my Ubuntu 23.10 system.

I plan to test a fresh install of Ubuntu 23.10 to see if this has been enabled for new installs some time in the next few hours.

[personalizedrefrigerator]

Note that I am still able to launch AppImages on my Ubuntu 23.10 system.

I plan to test a fresh install of Ubuntu 23.10 to see if this has been enabled for new installs some time in the next few hours.

It does not seem to be enabled yet on a new install. It does look like they still plan to enable restrictions on unprivileged user namespaces for 23.10:

As we collect more feedback from you throughout the first couple of weeks of the 23.10 lifetime, we will build more AppArmor profiles for more apps, and then turn this feature on by default on 23.10, through the SRU process (Stable Release Updates).¹

Footnotes

1. https://canonical.com/blog/ubuntu-23-10-restricted-unprivileged-user-namespaces ↩

[JGCarroll]

The patch to me looks fine, but from my perspective this is less of a technical problem ( there's solutions, some better than others ) but also the human factor.

If we assume the restrictions are to be turned on in 23.10, then there's already people running with the sandbox turned on, who'll find that a system upgrade just breaks Joplin, and their first thought likely won't be to rerun the installer script ( and that's assuming they even use the installer script, rather than executing the AppImage raw. )

Similarly, there might be people who jump from 22.04 to 24.04 skipping 23.10, if the feature isn't turned on in 23.10 it almost certainly would be in 24.04 ( there's a reason it's been designed and implemented, it's presumably a matter of when not if ), who'd then also find the app breaks. In this situation I imagine they'd be more likely to reinstall though, since the correlation would be clearer than the 23.10 update, but the script would eventually simplify to checking whether the OS is Ubuntu, rather than whatever specific version of Ubuntu it is. It's also not impossible that this would find it's way in downstreams, such as Mint or PopOS.

And the people this effects would probably all run to the forums/GitHub for support.

Essentially to me this looks like a ticking time bomb of making a choice when to apply a solution. If the feature never gets turned on at all, then it's pointless having tried and weakens security for no gain. On the other hand, if it's an inevitability, then there's no reason to delay because the problem only gets worse as time goes on and the user base gets bigger.

While I understand Ubuntu's position on this, I can't imagine this won't cause havoc elsewhere. It'd potentially be worth raising this to Electron upstream, because Joplin isn't the only Electron app that runs on Ubuntu without a proper package.

And even "proper packages" might get hit. I'd assume this change would even hit the Flatpak for example, because from the kernels perspective the Flatpak wouldn't have an AppArmor policy. The Flatpak is in a better position though because they could push an update to disable the sandbox reliably, Joplins AppImage can't.

As far as I see it, the viable options are basically:

1. Create a proper .deb package so that AppArmor profiles can be deployed in a safe and consistent manner. Using PPAs via Launchpad might benefit this.

2. Use no-sandbox as above and like Debian, where the (almost) same policy applies.

3. Encourage people to revert the restriction. While the restriction is done for security, the main beneficiaries are server deployments in finely controlled environments, where having everything as a proper package could be reasonably expected. Reasonably for home/desktop use, this change could be considered on the lower end of beneficial.

4. Petition Electron to allow disabling the sandbox more granularly. It's only the user namespace that's problematic, ideally this wouldn't be an all or nothing switch for the other components.