dinitcheck: warn about non-absolute executable path #384
Conversation
mobin-2008
added
Enhancement/New Feature
src/dinitcheck.cc
Outdated
| if (command[0] != '/') { | ||
| report_service_description_err(name, | ||
| std::string("executable '") + command + "' is not an absolute path"); | ||
| } else if (fstatat(dirfd, command, &command_stat, 0) == -1) { |
There was a problem hiding this comment.
The standard formatting used in Dinit is to have the "else if" on the next line after the closing brace (see the CODE-STYLE document).
Using "else" here means that if the path is both non-absolute and refers to a non-existent (non-stat'able) file, only one warning will be printed. I think it should do both checks.
There was a problem hiding this comment.
Will fix the style.
Using "else" here means that if the path is both non-absolute and refers to a non-existent (non-stat'able) file, only one warning will be printed. I think it should do both checks.
I skip them because if executable is not specified in absolute form, dinitcheck may end up examining wrong files and print meaningless warnings.
For example,
$ doas dinitcheck
...
Checking service: seatd...
Service 'seatd': command executable 'seatd' is not executable by owner.
...
$ cat /usr/lib/dinit.d/seatd
type = process
command = seatd -n 3 -g seat
smooth-recovery = true
logfile = /var/log/seatd.log
ready-notification = pipefd:3
depends-on = rc.target
before = login.target
dinitcheck stats /lib/dinit.d/seatd instead of /usr/bin/seatd, the diagnose is probably meaningless (and misleading) if we aren't sure what the executable is.
dinit's behavior depends on PATH environment if a service contains command with non-absolute executable path. dinitcheck may not even find correct executables in this case. Such services may lead to security problems, systemd has been searching executables only in compilation-time specified paths. As similar features do not exist in dinit and aren't very meaningful, we just warn about dangerous usage. References: https://www.man7.org/linux/man-pages/man5/systemd.service.5.html#COMMAND_LINES Signed-off-by: Yao Zi <[email protected]>
ziyao233
force-pushed
the
dinitcheck-with-relative-path
branch
from
dd60064
to
271143d
Compare
|
By the way if you wish to add yourself to CONTRIBUTORS file, please just open another PR with that change :) |
dinit's behavior depends on PATH environment if a service contains command with non-absolute executable path. dinitcheck may not even find correct executables in this case.
Such services may lead to security problems, systemd has been searching executables only in compilation-time specified paths. As similar features do not exist in dinit and aren't very meaningful, we just warn about dangerous usage.
References: https://www.man7.org/linux/man-pages/man5/systemd.service.5.html#COMMAND_LINES