Try harder to become root and speed up by skipping unnecessary sudo's #608

ngaro · 2021-02-27T19:39:36Z

All other scripts in Installer-Linux can be improved in the same way as below, but I'm not spending time on it if this doesn't get merged...

This PR:

Speeds up the script because you don't need to run sudo anymore if you are already root
Tries harder to run itself as root if this is not the case:
- It checks where sudo is located instead of assuming it is in $PATH (it uses which instead of env for this because env is a binary that isn't necessarily available. which on the other hand is also a shell-buildin that even shells as tiny as busybox have)
- It tries to use su if you don't have sudo and warns you that this is deprecated (also not assuming it's in $PATH)
- It quits and warns you that you'll have to become root manually if both sudo and su aren't available
- It gives the correct reason if something fails*
- Doesn't depend on the name of root (see Accounts for a root-user with a different name #606 for more info)

*The 'real' part of the script hasn't been improved yet, that's for another PR. So if one these commands fail you will obviously not see the correct reason yet

This PR conflicts with Accounts for a root-user with a different name #606 . Use that one if you want something easy and this one if you want something good.
This PR has been tested with multiple shells, including tiny ones, so it can be combined with No longer rely on the presence of bash #605

rusty-snake · 2021-02-28T10:51:50Z

Speeds up the script because you don't need to run sudo anymore if you are already root

I don't think performance matters for shell-scripts (they have no (JIT-)compiler and start new processes for everything they do). Anyway it allows to run the script on systems without sudo / su.

It checks where sudo is located instead of assuming it is in $PATH

To quote man 1 which:

Which takes one or more arguments. For each of its arguments it prints to stdout the full path of the executables that would have been executed when this argument had been entered at the shell prompt. It does this by searching for an executable or script in the directories listed in the environment variable PATH using the same algorithm as bash(1).

So it just searches for it in $PATH.

which on the other hand is also a shell-buildin that even shells as tiny as busybox have

It's not listed in man 1 builtins and LC_ALL=en_US.UTF8 builtin which shows bash: builtin: which: not a shell builtin. command on the other hand is a builtin (at least in bash).

It tries to use su if you don't have sudo and warns you that this is deprecated

It is deprecated? man 1 su does not say this, from where is this info?

It quits and warns you that you'll have to become root manually if both sudo and su aren't available

We could also check pkexec if we want.

ngaro · 2021-02-28T13:48:12Z

Speeds up the script because you don't need to run sudo anymore if you are already root

I don't think performance matters for shell-scripts (they have no (JIT-)compiler and start new processes for everything they do). Anyway it allows to run the script on systems without sudo / su.

I agree with performance not being important. Still, low-importance improvements are also improvements. Although every call to a binary (e.g. foobar) starts at least 1 process, sudo foobar and su -c foobar start at least 2 processes.

It checks where sudo is located instead of assuming it is in $PATH

To quote man 1 which:

Which takes one or more arguments. For each of its arguments it prints to stdout the full path of the executables that would have been executed when this argument had been entered at the shell prompt. It does this by searching for an executable or script in the directories listed in the environment variable PATH using the same algorithm as bash(1).

So it just searches for it in $PATH.

which on the other hand is also a shell-buildin that even shells as tiny as busybox have

It's not listed in man 1 builtins and LC_ALL=en_US.UTF8 builtin which shows bash: builtin: which: not a shell builtin. command on the other hand is a builtin (at least in bash).

It seems that i was a bit to hasty with my conclusion. I tried my code on the tiniest "system" i had (a alpine docker container that barely has any binaries) and because it was present i incorrectly assumed it was a builtin.
Sorry about that. Your conclusions are correct.

While writing this I'm suddenly thinking about a better way to check if sudo (or su) is present that doesn't require which or env. I'll fix in my next commit

It tries to use su if you don't have sudo and warns you that this is deprecated

It is deprecated? man 1 su does not say this, from where is this info?

True, officially it's not deprecated.
But in practice it is. All recent tutorials, books, ... use sudo. Probably because it does everything su can do with the advantage of working correctly on systems without a root password (which are extremely common nowadays). I'll change the warning in the next commit.

It quits and warns you that you'll have to become root manually if both sudo and su aren't available

We could also check pkexec if we want.

The name vaguely rings a bell, but I've never used it. I'll take a look at it and use it in a 3rd commit if it it's a extra feature.

rusty-snake · 2021-02-28T13:52:59Z

While writing this I'm suddenly thinking about a better way to check if sudo (or su) is present that doesn't require which or env.

If you want to only use builtins (is echo always a builtin?):

if ! command -v sudo >&-; then
	echo "no sudo"
fi

ngaro · 2021-02-28T14:37:36Z

Changes after this commit:

Improved the warning about the more-or-less deprecated state of su
The presence of which is no longer necessary. Checking for the returncode 127 is also a method to check if the command is present. (This is also faster because the extra call to which is no longer needed.)

I've tested the returncode 127-rule in bash, zsh and busybox so it's safe to assume that it works everywhere.
This doesn't stop developers of choosing to exit their code with 127 themselves but this isn't the case for sudo and su. (And it's probably never the case in well-known programs because it's just 'evil')

Note:
Returncode 127 only checks for shell builtins and binaries in $PATH so it could be that the binaries are at a unexpected place and we are not using them although we could have been.
But checking this would be extremely slow because commands like locate or find would be necessary. Also, a sudo (or su) at a 'strange' place could just as well be a completely different command that just happens to have the same name.

rusty-snake

👍

rusty-snake · 2021-02-28T14:43:36Z

Installer-Linux/linux-hosts-installer.sh

+    sudo $0 && exit 0
+    if [ $? -eq 127 ] ; then
+        echo "I can't find 'sudo', I'll try to use 'su' to become root."
+	echo "(Remember that 'su' is considerd deprecated (it needs a root password), I strongly recommend installing 'sudo'.)"


tab -> 8 spaces

Suggested change

echo "(Remember that 'su' is considerd deprecated (it needs a root password), I strongly recommend installing 'sudo'.)"

echo "(Remember that 'su' is considerd deprecated (it needs a root password), I strongly recommend installing 'sudo'.)"

I also noticed that i wrote considerd instead of considered . I'll fix both things in the next commit (after taking a look at pkexec and including it if we can use it)

(Also fixes typo 'considerEd' typo and incorrect indenting

ngaro · 2021-02-28T16:20:19Z

indenting corrected
typo fixed
pkexec support
warnings/errors to STDERR because that's the standard policy . This is a separate commit because I only thought about it after the previous commit

According to it's man-page pkexec can return 127 which breaks using 127 to check for a non-existent pkexec 😒
So in case of a problem I'm just checking for 126 (authorization-related problems) or any other non-zero code (all other problems)

We could extend the code even more by:

Using the 127 trick to check the existence of which
Falling back to env in case of non-existence of which and also doing a 127 check of env
Mentioning we don't know the cause of pkexec's 127 if env is also unavailable
Using which (or env if we couldn't find which) to check the existence of pkexec
Use this info to report the reason of the pkexec's 127

But according to me that's overkill, certainly because unless you have a really strange configuration, you will see a 'command-not-found'-type of error anyway and it will cause running which or env which will be a slowdown.

funilrys · 2021-05-02T07:43:51Z

Hi everyone, this looks marvelous. About your "We could extend the code [...]" part, I agree that it's overkill. People who manage "strange configuration" should be able to handle such thing.

Anything to add @rusty-snake ?

cc @mitchellkrogza.

rusty-snake

Anything to add @rusty-snake ?

Exit-codes don't work like exceptions. The don't tell you the reason why some thing failed. The don't even tell you if something failed. IMHO it would be better to use command (or which) to check if sudo/su/pkexec are present.

Furthermore this has duplicated and nested code. Here are two ideas to improve this.

become_root() {
        if command -v "$1" >/dev/null; then
                "$@" && exit 0
                echo "Something went wrong with trying to use '$1' to run this script" > /dev/stderr
        else
                return 1
        fi
}       

echo "This script needs to be ran as root, let's switch to root:" > /dev/stderr
become_root sudo "$0" && exit 0
echo "I can't find 'sudo', I'll try to use 'su' to become root." > /dev/stderr
echo "(Remember that 'su' is considered deprecated (it needs a root password), I strongly recommend installing 'sudo'.)" > /dev/stderr
become_root su -c "$0" && exit 0
echo "I also can't find 'su', I'll try to use 'pkexec' to run this as root..." > /dev/stderr
become_root pkexec "$0" && exit 0
echo "I can't give you authorization to run this as root, you'll need to find another way to become root..." > /dev/stderr
exit 1

echo "This script needs to be ran as root, let's switch to root:" > /dev/stderr
if command -v sudo >/dev/null; then
        sudo "$0" && exit 0
        echo "Something went wrong with trying to use 'sudo' to run this script" > /dev/stderr
        exit 1
elif command -v su >/dev/null; then
        su -c "$0"
elif command -v pkexec >/dev/null; then
        pkexec "$0" && exit 0
        echo "Something went wrong with trying to use 'pkexec' to run this script." > /dev/stderr
        exit 1
else    
        echo "I can't give you authorization to run this as root, you'll need to find another way to become root..." > /dev/stderr
        exit 1
fi

funilrys · 2021-05-02T09:47:22Z

Great. Thanks for your input and time @rusty-snake. This PR will stay a draft until the recommendation of @rusty-snake are applied.

Stay safe and healthy.

Try harder to become root and speed up by skipping unnecessary sudo's

d74738c

ngaro mentioned this pull request Feb 27, 2021

Accounts for a root-user with a different name #606

Closed

Use returncode 127 to check if commands are present

8d6c859

rusty-snake reviewed Feb 28, 2021

View reviewed changes

ngaro added 2 commits February 28, 2021 16:42

pkexec-support

9478ae7

(Also fixes typo 'considerEd' typo and incorrect indenting

All warnings/errors should be sent to STDERR

2c62c87

funilrys requested a review from rusty-snake May 2, 2021 07:43

funilrys mentioned this pull request May 2, 2021

No longer rely on the presence of bash #605

Closed

rusty-snake reviewed May 2, 2021

View reviewed changes

funilrys marked this pull request as draft May 2, 2021 09:46

funilrys assigned ngaro May 2, 2021

Merge branch 'Ultimate-Hosts-Blacklist:master' into try-root-harder

9b6eb93

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try harder to become root and speed up by skipping unnecessary sudo's #608

Try harder to become root and speed up by skipping unnecessary sudo's #608

ngaro commented Feb 27, 2021

rusty-snake commented Feb 28, 2021

ngaro commented Feb 28, 2021

rusty-snake commented Feb 28, 2021

ngaro commented Feb 28, 2021

rusty-snake left a comment

rusty-snake Feb 28, 2021

ngaro Feb 28, 2021

ngaro commented Feb 28, 2021 •

edited

Loading

funilrys commented May 2, 2021

rusty-snake left a comment

funilrys commented May 2, 2021

	echo "(Remember that 'su' is considerd deprecated (it needs a root password), I strongly recommend installing 'sudo'.)"
	echo "(Remember that 'su' is considerd deprecated (it needs a root password), I strongly recommend installing 'sudo'.)"

Try harder to become root and speed up by skipping unnecessary sudo's #608

Are you sure you want to change the base?

Try harder to become root and speed up by skipping unnecessary sudo's #608

Conversation

ngaro commented Feb 27, 2021

rusty-snake commented Feb 28, 2021

ngaro commented Feb 28, 2021

rusty-snake commented Feb 28, 2021

ngaro commented Feb 28, 2021

rusty-snake left a comment

Choose a reason for hiding this comment

rusty-snake Feb 28, 2021

Choose a reason for hiding this comment

ngaro Feb 28, 2021

Choose a reason for hiding this comment

ngaro commented Feb 28, 2021 • edited Loading

funilrys commented May 2, 2021

rusty-snake left a comment

Choose a reason for hiding this comment

funilrys commented May 2, 2021

ngaro commented Feb 28, 2021 •

edited

Loading