systemd error handling on user= is dangerous and makes no sense

[Joerg-rw]

Submission type

• Bug report

systemd version the issue has been seen with

systemd 232

Used distribution

Linux ubuntu 4.10.0-19-generic #21-Ubuntu SMP Thu Apr 6 17:04:57 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

In case of bug report: Expected behaviour you didn't see

systemd rejects invalid value to "user=" and throws bold warning, doesn't run the process

In case of bug report: Unexpected behaviour you saw

systemd considers a
user=_syntactically_invalid
like
foobar=
I.E. it ignores the correctly detected "user=" and runs the process with default root privileges.

This is highly surprising, unexpected (by the average user) and dangerous behavior.
Systemd should not bother at all about syntactical correctness of value at right-side of "user=" and simply leave it to the system to check if the particular system allows and knows a user of that name.

*1: It's not systemd's call to enforce anything about "syntactically correct names" here
*2: systemd MUST NOT run jobs as root when a "user=" been given, no matter what (except "root") is on the value side.

Rationale: it's considered highly dangerous to run a job as root when that job obviously is not meant to run as root, since admin gave "user=" parameter.
Syntax of user names gets enforced elsewhere, systemd must not duplicate any such policies unless it's creating a new user(name)

In case of bug report: Steps to reproduce the problem

see #6237

[Joerg-rw]

particularly refer to #6237 (comment)

[tixxdz]

Hi,

Fix is here #6300

Closing, thanks!

[Joerg-rw]

thanks! looks like it's targeting at least the *2: point of above.
how about *1: ""It's not systemd's call to enforce anything about "syntactically correct names" here"" ?

[tixxdz]

Hi @Joerg-rw for:

how about *1: ""It's not systemd's call to enforce anything about "syntactically correct names" here"" ?

I did not follow the whole issue but from my limited understanding it seems that systemd is not enforcing anything here. Most software and distros there do not support such names, if that was the case then maybe systemd can support them too, I think this was already pointed out by other systemd maintainers ? no ?

While we are at it, the POSIX standard allows to break things:

1. Breaks programs and scripts that directly call chown and maybe chmod

$ id
uid=1000(tixxdz) gid=1000(tixxdz) groups=1000(tixxdz),10(wheel)
$ ls -lha newfile
-rw-rw-rw-. 1 root root 0 Jul  9 01:50 newfile
$ sudo chown tixxdz.tixxdz newfile
$ ls -lha newfile
-rw-rw-rw-. 1 tixxdz tixxdz 0 Jul  9 01:50 newfile

Now
$ sudo useradd 0day.tixxdz
$ ls -lha newfile
-rw-rw-rw-. 1 root root 0 Jul  9 01:50 newfile
$ sudo chown 0day.tixxdz newfile
$ ls -lha newfile 
-rw-rw-rw-. 1 0day.tixxdz root 0 Jul  9 01:50 newfile

Your file is still owned by group root, what if it is a setgid root executable ? or another type of secure file and your scripts or program could not set the appropriate owner due to this behaviour ? This behaviour may break a well established way on how to use chown !

2. That POSIX standard will not work when you have a user X with a username:"10" and UID:"99" and a user Y with a username:"99" and UID:"10"

3. What will happen if you have a username 0 that is not root and your scripts and programs perform # chown 0 my_secrect_file ?

4. The standard is awful when used with Linux user namespaces. Linux supports a couple of special UIDs like the Unmapped user and group IDs that are part of user namespaces /proc/sys/kernel/overflowuid by default it is 65534 or the mapped valid UIDs, what if your username clashes with such ranges or is outside of these ranges. I think I am a human and I will for sure get confused if I see a number or maybe an ID ? that represents a username that is outside the range of valid user IDs of that user namespace ? It just does not make any sense.
   Reference:
   http://man7.org/linux/man-pages/man7/user_namespaces.7.html

That's why we are not interested in such cases, we won't introduce new complexity.

Thanks!

[Joerg-rw]

I'm asking to reduce complexity, by not doing syntactical checks on user names (In Expression: dicard the complete "user=" name value syntax check in systemd) since all of the above is not ti get handled or enforced in systemd, it's handled in useradd or whatever other tool the system provides to create new users. So systemd MUST NOT check for any syntax properties of the "user=" value provided, just a simple getpwnam_r() or whatever it already does, then act accordingly, that's sufficient.

A "syntactically invalid" username can't be in the password database, a name that is in that database can't be syntactically incorrect.
There's absolutely no benefit at all from systemd doing a syntax check for "user=" name value here - only existing added complexity cruft for nothing in systemd now, which indeed breaks stuff in niche cases.
I ask to remove that check from systemd. Can't be hard.

[Joerg-rw]

from my limited understanding it seems that systemd is not enforcing anything here

#6237 (comment) suggests the opposite

I'd re-open this ticket because of this, if I could find how to

[tixxdz]

@Joerg-rw
We may support that if it is the standard in other tools and used by other distros, right now it is not the case. So please start working with the community, patch the related tools and work with the relevant distributions to make what you propose the standard, and then if it is adopted send us a patch. Thanks!

[poettering]

I'm asking to reduce complexity, by not doing syntactical checks on user names (In Expression: dicard the complete "user=" name value syntax check in systemd) since all of the above is not ti get handled or enforced in systemd, it's handled in useradd or whatever other tool the system provides to create new users.

That's simply not true. systemd validates specified user names at two places. In the sysusers.d code and in the unit code. In both cases, we will potentially register users in the system. In the sysusers.d case that's the purpose of the entire excercise, and in the User=/Group= unit setting that's done as soon as DynamicUser=1 is specified. And yes, systemd should not be permitted to be a vehicle to pollute the users database with users that you couldn't register with "adduser".

Note that systemd-logind does not validate the user names passed to it the same way, as PAM/login do not validate their input either, and in this case we are just consumers of the data, and just integrate into some foreign ecosystem that PAM is.

There's absolutely no benefit at all from systemd doing a syntax check for "user=" name value here - only existing added complexity cruft for nothing in systemd now, which indeed breaks stuff in niche cases.
I ask to remove that check from systemd.

Even if systemd was only consuming user names for User=/Group= (which it isn't, see above), I still think there's great benefit in complaining about user names that are highly problematic, in particular as we want unit files to remain portable between systems and distros, and if you make use of a syntax that isn't supported widely, you work against that.