On 2020-08-21 08:11, Arjen de Korte via clamav-users wrote: > Citeren Michael Orlitzky via clamav-users <clamav-users@lists.clamav.net>: > >> On 2020-08-21 04:45, Arjen de Korte via clamav-users wrote: >>> >>> It is not clear to me what problem this patch intends to solve (for a >>> systemd service it is absolute not required from a security point of >>> view). The PIDFile should be writable by vscan user only anyway. >>> >> >> With a Type=forking service, systemd will send SIGTERM to the contents >> of the PID file as root. > > Not unconditionally. See the following from 'man 5 systemd.service': > > "The PID file does not need to be owned by a privileged user, but if it > is owned by an unprivileged user additional safety restrictions are > enforced: the file may not be a symlink to a file owned by a different > user (neither directly nor indirectly), and the PID file must refer to > a process already belonging to the service." >
That's good to hear (and is news to me), and maybe they've found another way to prevent this vulnerability in systemd. >> If the "vscan" user can put whatever he wants >> in the PID file, then he can kill root processes. > > See above: you're trying to fix a problem that doesn't exist. However, systemd isn't the only service manager, and the problem still exists in all of the other ones. Systemd is able to avail itself of platform-specific features in brand-new Linux kernels. SysV init, OpenRC, and others must stick to real or de-facto standard tools, and there is no standard way to implement what systemd says they've done. >> Are you using the upstream systemd service? > > No, we're using "Type=forking" since the clamd.service can take > several minutes to start and we don't want to start services that > depend on it before it actually finished starting up. Creating the > socket beforehand is not a solution, as clamd won't start serving any > requests until it has actually finished starting up. That's fine, now you just need to synchronize the PIDFile and PidFile entries in your systemd service and clamd.conf, respectively. I suggest /run/clamd.pid, or any other location that is writable only by root. >> It defaults to Type=simple, and runs clamd in the foreground. > > See above. Actually, with this patch clamd wil always run in the > foreground, as daemonizing is now completely broken. Up to and > including 0.102.4, starting clamd on the commandline without any > further options would just start the daemon and return. Now, it never > returns. That's not true, your service file just needs to know where the PID file lives. It always did, but somehow it managed to not crash in the past. >> In that case, your clamd daemon >> shouldn't be creating a PID file at all -- systemd should take care of >> it when it shoves the process into the background. PidFile should be >> left unset in clamd.conf. > > There is no PIDFile in the clamd.service file as systemd doesn't need > that here (even when running as Type=forking). The same goes for > freshclam.service. Systemd has other ways to keep track of which > processes it has started and will not use the PIDFile unless you tell > it to do so (with the above mentioned restrictions). Well empirically that's not true, because it isn't working. Add PIDFile entries to your service files when using Type=forking, and synchronize them with the PidFile lines in clamd.conf and freshclam.conf. _______________________________________________ clamav-users mailing list clamav-users@lists.clamav.net https://lists.clamav.net/mailman/listinfo/clamav-users Help us build a comprehensive ClamAV guide: https://github.com/vrtadmin/clamav-faq http://www.clamav.net/contact.html#ml