Citeren Michael Orlitzky via clamav-users <clamav-users@lists.clamav.net>:

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.

That may be, but now you have replaced it with two processes that run in the foreground, one of them as unpriviledged user and one as root (probably to delete the PIDFile upon exit). I don't consider this progress.

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.

No, as stated before, systemd doesn't need the PIDFile at all. It keeps track of the processes it started without the help of a PIDFile. It *can* use a PIDFile if you provide it with one and the only thing it will do with that is to remove that file if the service doesn't do it itself upon exit. Nothing more, it is not used for process control. There is absolutely no need for a PIDFile in the clamd.service, even with Type=forking.

I suggest
/run/clamd.pid, or any other location that is writable only by root.

It's fine where it is now. See above: systemd doesn't care.

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.

No, it doesn't, see above. Even if I start clamd on the commandline (which should then daemonize, unless told by --foreground to stay in the foreground) without any of this. This is a regression and has nothing to do with systemd.

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.

Makes no difference at all. Even without using systemd, clamd doesn't daemonize anymore, it will always run in the foreground.


_______________________________________________

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

Reply via email to