[clamav-users] ClamAV 0.103.0 release candidate - systemd service start fails

[Michael Orlitzky]

On 2020-08-21 08:11, Arjen de Korte via clamav-users wrote:
> Citeren Michael Orlitzky via clamav-users <clamav-users at 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.