Fix behavior when run as non-root. #141
Conversation
1. Use `set -Eeuo pipefail` to enable Bash "sane mode". 2. Fix usage of sudo when writing to protected directories by introducing the tee command. 3. Change exit code from 0 to 1 when script fails due to lack of root and lack of an available sudo command. 4. Remove trailing whitespace (which is stripped by GPG anyway).
|
|
|
I should have mentioned: the Action "Curl | Bash Test" is currently broken on this repo due to the Debian Buster archive migration I mentioned. These changes fix the issue by removing Debian Buster from the test matrix. All other tests are passing. Without this change, the passing tests are actually terminated so it wasn't possible to see their status. |
|
Here is the workflow history in my fork of the repo: https://github.com/JEHoctor/install.zerotier.com/actions Also, here are some references I consulted to make sure I got "sane mode" right in all its details: |
|
One more thing 😂 On the download page, there is a formatting issue. I couldn't find where to open a PR to fix this. Just search the page for |
Before this patch, if you run the install script without root/sudo on Ubuntu/Debian, then you can get into a broken situation that could be hard for some users to fix. This breakage would prevent any updates to the OS until resolved.
There's one main fix here: removing two instances of lines like:
And replacing each with this fix:
The issue here is that in order to access a privileged location like
/etc/apt/trusted.gpg.d/zerotier-debian-package-key.gpg, it is not enough to use sudo and a shell redirect because the shell will actually open the file, not sudo. The fix is to use sudo with the tee command, which can use its elevated privileges to open the file and accept input from GPG.Note that this use case, running the script as non-root, is not covered by the test cases in CI. Nevertheless, it is actually recommended as "foil hat mode" in the readme. It's also a perfectly sensible and convenient way to use the script.
The way this issue can block OS updates is as follows: if a user on Ubuntu or Debian runs the script as non-root, then writing the ZeroTier package key to /usr/share/keyrings will fail, but the subsequent operation that adds a source list depending on this key will succeed. This leads to a situation where
apt-get updatefails with a GPG error, blocking any updates.The above fix is sufficient to resolve this issue with the script, but I've also added
set -Eeuo pipefailto the script to defend against this type of issue in the future. This enables Bash "sane mode", which would have prevented this issue by failing the script on the first command failure, leading to a consistent APT configuration even though the install would still fail.Adopting sane mode does require consideration of the existing code in the script. I had to disable errexit temporarily in one code block to support the way that code handles error status explicitly. Note that this code block is not exercised by tests. I reviewed all code in the script for any similar issues with sane mode.
I ran the new version of the script through ShellCheck on "style" mode (all messages, warnings, etc enabled) and reviewed all its output. Incidentally I removed all trailing white space from the script. GPG removes this whitespace when it de-armors the script.
Finally, I wanted to ensure that CI passed in my fork before opening a PR. I removed "debian:10" (Buster) from the test matrix as its repos have been moved to the archive now that it is end-of-life. It might be possible to keep Debian Buster in the test matrix by adding some conditional logic in ci.yml to update the repo lists to point to the archive repos. I also removed the unnecessary apt step and enabled Bash's "x" flag so print debug info.
I'm open to any feedback. Please let me know if you'd like me to split these changes into separate PRs.