Reconsider usage of sudo in uarch-bench.sh

[travisdowns]

Reconsider usage of sudo in uarch-bench.sh

[travisdowns]

@zingaburga - yup, the root thing bothers me a bit too. In fact, you can run uarch-bench (the raw binary) and perhaps uarch-bench.sh without root, but support isn't as good as I'd like and issue #31 talks about making that better.

About the use of sudo specifically in the script, there is a method to the madness. The idea is that if root isn't needed for anything, it doesn't ask. E.g,. it checks if it needs to install the kernel module before doing it, and it checks if it needs to set the performance governor to performance, but if none of those things need to be done it maybe never prompts you for credentials (actually it seems like this might be broken by get_turbo_state_string which unconditionally tries to sudo).

The other purpose of the behavior is that it only runs the minimal, necessary amount of stuff as root. In particular, it doesn't run the uarch-bench binary itself as root. So if you were concerned about giving this project root you could conceivably just audit the few uses of sudo in the small uarch-bench.sh script and not need to audio the entire C++ and asm application, which would be quite the task.

So I'm not totally sure how to proceed here without loosing the benefits above. One option would be to, as you suggest, just use the existing root-or-not status of the script without using sudo internally. If sudo was needed, it could fail with a message, and an option like --lenient or --no-root could be use to just proceed even without root (e.g., by default if the performance governor wasn't performance and the script wasn't running as root, it would fail since it needs root to change that, but with --lenient it would simply not change the governor, at the cost of some accuracy in the tests.

@zingaburga what do you think?

[zingaburga]

So if you were concerned about giving this project root you could conceivably just audit the few uses of sudo in the small uarch-bench.sh script and not need to audio the entire C++ and asm application, which would be quite the task.

You would, because the binary could be calling sudo too (it isn't, but if someone were that concerned, it would be something they'd have to check). Despite that, I suppose having sudo marked as such does give an indication to a user what root is needed for, so I can see a point there.

I wouldn't bother thinking about it too much. I'm sure anyone who uses this is smart enough to figure out how to get it working. It's just that if sudo isn't installed, you either have to install it (may be undesirable), or edit the script to remove all instances of it (which isn't too hard I suppose, but feels a little hacky). Just changing sudo to something like $sudo which is conditionally set if the current user isn't root should be sufficient.

Thanks for the response!

[travisdowns]

You would, because the binary could be calling sudo too (it isn't, but if someone were that concerned, it would be something they'd have to check).

@zingaburga You raise a really good point and one I hadn't considered in my day-to-day usage of the terminal and sudo. Actually it means it is really unsafe to run an untrusted process in the same terminal (or anywhere by the same user if you aren't using tty_tickets) when you have an active sudo ticket! So yeah the user would have to really audit everything that can run after the first sudo call.

So on Debian systems without sudo, a typical way of running processes as root is just to su to root and run them?

[zingaburga]

I'm not sure what's typical, but I personally just run everything as root. Others may say that's a bad idea, but my opinion on it is probably something like this.

The tool runs without root anyway - someone really worried about it will probably just run the changes (setting CPU governor etc) manually and then invoke the binary. For the rest, there's always a certain amount of trust involved - I think you're already upfront about it so I don't see much to worry about it.

Nonetheless, I think calling sudo is a nice way to do things, but it can be avoided if the user already is root.

[nemequ]

Something like

#!/bin/sh

if [ $(id -u) != 0 ]; then
    if [ "x$SUDO" = "x" ]; then
	SUDO="$(which sudo 2>/dev/null)"
	if [ $? != 0 ]; then
	    echo "WARNING: sudo not found.  Please set the $SUDO environment variable" >&2
	    echo "or run this script as root." >&2
	fi
    fi
fi

$SUDO do_something

?

[travisdowns]

@nemequ - yup, that's approximately the direction I'm leading. Basically it should have the following attributes:

1. If already running as root definitely don't require sudo and it should just work (everything will run as root in this case and too bad if I'm exfiltrating your bitcoins that way). This solves the OP's original complaint.
2. If not running as root, but something actually needs root and sudo exists, do the per-command sudo thing like today.
3. If not running as root, but something actually needs root and sudo doesn't exist, fail.
4. Offer a --no-root option that doesn't fail in (3) and doesn't do anything that would normally elevate in (2).

This still doesn't solve the thing that uarch-bench runs with a sudo ticket and could potentially elevate itself via sudo if it wanted. I'm not sure of a good solution there: I don't want to invalidate the ticket since the user might have gotten that themselves on the terminal and they wouldn't expect that to just disapear when they run uarch-bench.sh.

Maybe I'm over-thinking this.

[nemequ]

I guess you could prompt to see if it's okay to run stuff via sudo, but TBH that seems a bit silly. If there is an open sudo ticket and uarch-bench were malicious it could exploit that… if you care about security you can't run untrusted code with an open sudo ticket, and there's nothing uarch-bench can do about that.

What might be more sensible is to ask permission before each task (e.g., prompt "We need to install a kmod, are you okay with that? [Y/n] " ANSWER.

If you're trying to protect against malicious benchmarks but do trust uarch-bench, then I guess you could sudo -K before running the benchmark, but that should probably be optional since it means people will have to reenter their password next time they use sudo, which could be pretty annoying if you use uarch-bench in a script you want to run unattended…

Maybe I'm over-thinking this.

I think so.