User=0day considered harmful in systemd
User=0day considered harmful in systemd
Posted Jul 12, 2017 20:54 UTC (Wed) by davidstrauss (guest, #85867)In reply to: User=0day considered harmful in systemd by dskoll
Parent article: User=0day considered harmful in systemd
There isn't one, nor did I argue there was one. The remainder of your questions seem to assume that was my position, so I don't know how to address them.
My comment was about the security implications of using syscalls to validate input as a username or UID, specifically responding to the argument I quoted in my comment. I only mentioned fallback on lack of username/UID match as an aside -- and I agreed that fallback to root is bad.
Posted Jul 14, 2017 10:49 UTC (Fri)
by dskoll (subscriber, #1630)
[Link] (13 responses)
I asked the question to to criticize you, but to suggest that the desirable behaviour should be to always treat the User value as a user name and to always fail if that value is not found in the password database. It seems to me that's the safest approach.
Posted Jul 14, 2017 12:05 UTC (Fri)
by mchapman (subscriber, #66589)
[Link] (6 responses)
That is actually what happens... sort of.
systemd does call getpwnam() between the fork() and execve() when launching a command. That is the precise point at which it maps a username to a UID, a GID, supplementary GIDs, etc.
But getpwnam() can *not* be called at the point at which the unit is loaded into systemd. It is perfectly fine for the user not to exist at that particular time. Consider what happens during a package installation, for instance, where the unit file is dropped into the filesystem and then, during some post-installation script, adds the user to the system. There's a window of time where the unit can be loaded without the user actually existing. Or consider the case of the user being removed from the system after the unit has been loaded into systemd... systemd has no way to know that the user no longer exists, and even if it did it can't drop the unit as the service may still be running!
And finally, there are cases where it is intended that the system user database not contain the username specified in the unit. The DynamicUser= directive and the nss-systemd module interact together to provide dynamic users that exist for the duration of a service's runtime only.
In short, systemd already does getpwnam(), and it does it precisely where it should do it.
The problem that started this whole shemozzle off is that there were certain User= values that cause the entire directive to be ignored altogether. This meant that the default value, User=root, was used instead. getpwnam() called in the child process would look that up, map it to UID 0, and run the command with that persona.
With Jędrzejewski-Szmek recently merged pull requests, the User= values that would have caused the directive to be ignored now cause the unit to fail to load completely.
Posted Jul 14, 2017 12:19 UTC (Fri)
by mchapman (subscriber, #66589)
[Link]
Actually, it'd probably be more correct to say that the default behaviour, in the absence of a User= directive, is that systemd doesn't bother changing persona at all. systemd does not assume that a "root" user exists.
Posted Jul 14, 2017 14:30 UTC (Fri)
by dskoll (subscriber, #1630)
[Link] (4 responses)
My understanding was that systemd would not call getpwnam() for a username that's numeric. Or maybe another interpretation is that it would interpret it as a UID iff getpwnam() failed. Was this understanding wrong?
Posted Jul 14, 2017 14:53 UTC (Fri)
by dskoll (subscriber, #1630)
[Link] (2 responses)
Indeed, the systemd.exec man page (Debian Stretch) says "Takes a single user or group name, or numeric ID as argument.
Well, it shouldn't. It should insist on a name and not an ID.
The use case someone mentioned for an ID (that the user name is in a remote database) is not persuasive. How does it make more sense to manage a bunch of machines with "User=4242" in the unit files rather than create a local user on each machine with that ID and use "User=foouser"? Changing the ID is just as onerous in each case.
Posted Jul 14, 2017 15:09 UTC (Fri)
by mchapman (subscriber, #66589)
[Link] (1 responses)
Well, the use case isn't supported anyway. The user must be in the system's user database -- i.e. must be able to be returned by a getpw*() call -- whether a username or a UID is used to look it up. systemd will not execute a command for a user that isn't in the user database.
> How does it make more sense to manage a bunch of machines with "User=4242" in the unit files
If I had my way I would have User= only take a username, never a UID. I still wouldn't have it allow all-digit usernames though, because that's just plain weird.
But that kind of change would be hard to introduce now. I don't know why anybody would do it, but there's probably somebody relying on User=4242 doing something useful on their systems.
Posted Jul 14, 2017 15:12 UTC (Fri)
by mchapman (subscriber, #66589)
[Link]
Ah sorry, I misread your comment.
systemd should work just fine with a remote database for users. It's just calling the standard libc functions, after all, and the user database can be in LDAP or whatever.
Posted Jul 14, 2017 15:00 UTC (Fri)
by mchapman (subscriber, #66589)
[Link]
Oh, you were talking about all-digit User= values specifically? systemd will assume that if the value in User= only consists of digits, then it should be interpreted as a UID. While spawning the child process it will attempt to get the credentials for that UID using getpwuid(). It never calls getpwnam() in this case, so if you have an all-digit username on your system the only way to reference this in User= would be to use that user's UID.
So only getpw*() call is chosen, and if it returns an error the command is not executed. It doesn't try one then the other, nor is there any "fallback" to root.
Posted Jul 14, 2017 21:17 UTC (Fri)
by davidstrauss (guest, #85867)
[Link] (5 responses)
Two issues are still getting mixed together here: I think people are over-focusing on the former because it's related to the User=0day example (and would have handled that case), but the latter is more relevant to ensuring "fail safe" behavior. Even if systemd could read the mind of the person who wrote the unit file (i.e. always knowing whether to treat it as a username or numeric UID), the latter risk exists if the necessary user is missing. A broken heuristic just happens to make the latter risk more likely. It's also not possible to just "always treat the User value as a user name" (as you suggest) because of the existing semantics of User=, which thousands of packages (and countless sysadmin-configured units) assume includes support for numeric UIDs. Treating every User= configuration as a username would make handling of the latter case even more important because legacy User=$UID configurations would generally fail to match when treated as usernames. That's why systemd has addressed the more fundamental issue: failing safely when user lookup fails.
Posted Jul 15, 2017 0:08 UTC (Sat)
by anselm (subscriber, #2796)
[Link] (2 responses)
Whatever the case may be, systemd shouldn't execute stuff as root that is not meant to be executed as root. That seems to be sorted now, which is good.
One could argue that systemd also shouldn't try to be smarter than is good for it, e.g., by enforcing its own undocumented assumptions about what usernames ought to look like that may or may not be grounded in reality. There's no doubt that systemd is a nifty piece of software and a huge improvement on anything that came before it, but sometimes a little less would actually be more.
Posted Jul 15, 2017 0:43 UTC (Sat)
by rahvin (guest, #16953)
[Link] (1 responses)
Anyway, in my mind that part of the discussion is already settled, patch is already merged, just need the release to come out. The publicity on the bug helps, people can now look at their service files and see if they have this happening and they can evaluate it with a simple scripted check using standard tools.
Posted Jul 15, 2017 1:07 UTC (Sat)
by anselm (subscriber, #2796)
[Link]
I don't think so. Where systemd goes out on a limb, as far as I'm concerned, is by assuming that a username that starts with a digit is invalid (and that therefore the directive could be ignored altogether, such that in the absence of a User= directive the unit is by default executed as root). We've heard Lennart claim that this is the “least common denominator” among various Linux distributions, but we've also heard from people who aver that digits at the beginning of a username (or even all-digit usernames) are perfectly acceptable. The paragraph on User= in systemd.exec(5) certainly doesn't mention that restriction but it probably should.
Posted Jul 17, 2017 11:30 UTC (Mon)
by dskoll (subscriber, #1630)
[Link] (1 responses)
Anyone have any idea how many unit files rely on specifying a numerical UID for User= ? I bet few to none.
Systemd introduced a huge disruption to the Linux init system. Saying we have to keep supporting numerical UIDs in the User= directive seems pretty silly to me.
Posted Jul 18, 2017 0:35 UTC (Tue)
by neilbrown (subscriber, #359)
[Link]
certainly having an /etc/systemd/system.conf config option to disable numerical UIDS in User= would be welcome.
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
User=0day considered harmful in systemd
Has anyone seriously argued systemd should treat a username starting with a number as root?
User=0day considered harmful in systemd
User=0day considered harmful in systemd
Shouldn't take too many lines of code, and creating a pull request on github is fairly easy.