Drop unnecessary privileges in node

[mrcnski]

The node could drop privileges if it finds out it is running as root, it's a good security measure by itself.

Originally posted by @s0me0ne-unkn0wn in #1685 (comment)

Preliminary Design

• Detect if running as privileged user, and not in a container
• Display an error message if so
• Allow bypassing with a flag?

Pseudo-code

is_root = getuid() == 0 || geteuid() == 0

is_container = ??

root_block = is_root && !is_container

TODO: Check for admin on Windows?

Open questions

There doesn't seem to be a reliable way to check is_container (and I doubt this could be made future-proof). Should there be some new flag such as --is-container or --allow-root?

Alternatives

There was an idea of starting the node with root privileges (using a suid bit), then doing some things at startup requiring root, and finally dropping the privileges. This still needs to be discussed because it carries some risk. It would need to be audited at the very least.

This design went a different route: we detect privileges, instead of unconditionally dropping them. This seemed more useful right now because privileges probably indicates some issue with how a validator is running the node, that we can communicate to them

Dropping privileges can still be done in the future if the need arises; see here.

Related

"Consider starting node with root privileges": #2324

[mrcnski]

@eskimor @bkchr What do you think about this proposal? In order to avoid false positives we would need a new flag for containers and CI. I considered simply dropping privileges instead, but that would require having a user ID to switch to. Looking at prior art like tcpdump, they either get the unprivileged user from a CLI flag, or they magically get it at compile-time (!?).

[s0me0ne-unkn0wn]

nobody is the default username of a user with no special privileges. I'm curious if some standard defines it, but it's common across the UNIX ecosystem; beyond Linuxes, I remember using it on several BSDs, Solaris, and AIX. However, its uid is not stable, and getpwnam() should be called to find one. It can be used to drop privileges in general cases. Creating a separate user account for the service during the installation process is a good practice, but it only makes sense if we're going to run as root or with suid bit set, so it should be considered if we decide to go that way.

[s0me0ne-unkn0wn]

I had a brief look into tcpdump source; no magic involved there. configure.ac defines AC_DEFINE_UNQUOTED(WITH_USER, "$withval", [define if should drop privileges by default]), so it just becomes a configure flag. When building a .deb or an .rpm or whatever, build script provides a default username to drop privileges with ./configure --with-user=someuser. It could be nobody, or the build script could provide a separate username if it adds the user in its pre-install script. For example, looking into Fedora RPM spec, it builds with configure --with-user=tcpdump and in %pre section it has /usr/bin/getent passwd tcpdump >/dev/null || /usr/sbin/useradd -u 72 -g 72 -s /sbin/nologin -M -r -d / tcpdump 2> /dev/null

[mrcnski]

Appreciate the info! I wonder if nobody is available inside containers? It should be tested, but if so, then we can just unconditionally drop to the nobody user and error if we're not able to do so. (Of course we should have a flag to bypass this requirement.)

[s0me0ne-unkn0wn]

I wonder if nobody is available inside containers?

Just looked into a couple of docker overlay images in my Podman setup. nobody is available in both, so I hope it's also a common case for containers. Of course, it can depend on what base image and setup is used to create the final container image, but I'd expect all of them to have nobody available except for some special "very, very minimal setup" containers, maybe.

[mrcnski]

I did a bit of research which just confirmed what you said: the best practice is to set the suid bit, let the process set up a new user specific to the process, and then drop privileges to that user. I am just hesitant about the suid bit because it is a change in how the binary is run and could be perceived as scary/insecure. We would need @bkchr to give this approach the green light. This seems like the best approach, but we'd still need a CLI flag to disable this step in case it's not possible on some nonstandard configuration.

[s0me0ne-unkn0wn]

I am just hesitant about the suid bit because it is a change in how the binary is run

Not only that. Right now, it is possible to cargo build the whole thing from source and just run it. It wouldn't be the case if the suid bit is required, you'd have to either set it manually or require root privileges for cargo install (I don't know if cargo can even handle that, tbh).

So the main question is, do we really need the suid bit? What could we do with it that we cannot do without it? To enable audit monitoring at least, but that's an auxiliary feature and is not worth requiring suid by itself. What else could we achieve?

[ggwpez]

The node could drop privileges if it finds out it is running as root, it's a good security measure by itself.

I remember some programs just exit immediately and refuse to run when they detect root privileges.
That could be easier than trying to switch users. It could also reveal an incorrect setup on the operator side.

[mrcnski]

@ggwpez Yeah, the concern is Docker or other containers. We'd need a new flag:

There doesn't seem to be a reliable way to check is_container (and I doubt this could be made future-proof). Should there be some new flag such as --is-container or --allow-root?

Indeed this is a simple solution to the problem. But, following the best practice (suid, setting up a new user, and changing to it) seems better in the long-run. It would let us do more things at start-up.