Definitely a good suggestion.  Re: deprecating/removing the PidFile config options -- not sure of any use case for it, but I don't have anything against it either, other than we'd have to make up a OnAccessPidFile option if we wanted feature parity for clamonacc, which at present seems to have no PID file feature.

It occurred to me that removing the PidFile option from ClamD would break Freshclam's NotifyClamd features, which uses the clamd.conf path to look up the clamd PidFile setting. 

One of the other users has suggested the idea of having clamonacc use the clamd PID file to filter out clamd process access-events by PID, which would laso rely on the PidFile option.

In an ideal world, ClamAV would be _less_ configurable and would work out-of-the-box a whole lot more than it does.  If I had my way, the location of the PID file would be hard-coded into each of the programs, and the PID file would be always-enabled so that clamonacc could depend on it.  I have a feeling such a change would upset a few people though.  Food for thought. 

Perhaps another solution would be for it to use a hard-coded path for the PID files by default, or use the --pid <path> if specified by the user, in which case clamonacc could have a --clamd-pid <path> option in the event that clamd is run with a non-default PID file path.  

Anyways, so long as some of the daemons have the --pid option I don't see any reason not to have it in all of the daemons.

(In reply to Micah Snyder from comment #2)
> It occurred to me that removing the PidFile option from ClamD would break
> Freshclam's NotifyClamd features, which uses the clamd.conf path to look up
> the clamd PidFile setting. 

That's technically true, but the NotifyClamd option takes the path to clamd.conf as an argument, and the path to clamd.conf is just a proxy for the PID file location. You're still telling freshclam where to look, but in a roundabout way.

And regardless... wouldn't everyone be happier if freshclam just ran "clamdscan --reload" instead? That's a much better API for reloading in my opinion.

> One of the other users has suggested the idea of having clamonacc use the
> clamd PID file to filter out clamd process access-events by PID, which would
> laso rely on the PidFile option.

Likewise, there's got to be a better API for this. Or maybe those users should be ignoring accesses that the "clamav" user performs instead.


> In an ideal world, ClamAV would be _less_ configurable and would work
> out-of-the-box a whole lot more than it does.... I have a
> feeling such a change would upset a few people though.

I feel your pain there...


> Perhaps another solution would be for it to use a hard-coded path for the
> PID files by default, or use the --pid <path> if specified by the user, in
> which case clamonacc could have a --clamd-pid <path> option in the event
> that clamd is run with a non-default PID file path.  
> 
> Anyways, so long as some of the daemons have the --pid option I don't see
> any reason not to have it in all of the daemons.

You can add the flags without immediately hurting anything, which is nice. It would introduce some theoretical weirdness with e.g. freshclam looking in the clamd.conf file for a PidFile directive that's no longer accurate because the user passed --pidfile=<wherever> on the command-line, overriding the conf file. But so long as you don't encourage anyone to do both, people would have to try pretty hard to shoot themselves in the feet.

Long term, you can slowly talk people out of weird configurations with better documentation and working defaults. If @runstatedir@ and friends are used in the service scripts, or as compiled-in default values for the pidfile location, then everything will "just work" out-of-the-box. And after things "just work" for long enough, relatively few people will remain who have changed the defaults for no good reason.

> Likewise, there's got to be a better API for this. Or maybe those users should > be ignoring accesses that the "clamav" user performs instead.

Having worked with commercial AM, running clamd as root seems to be a must in order to achieve feature parity.

Maybe my imagination is lacking, but I can't think of a reason why clamd would ever need to run as root. You're probably safer without an antivirus than with one that runs as root and scans anything anyone can feed to it.

It has to *start* as root, to drop privileges. But after that, clamd should be receiving either file descriptors or the file data from someone who has privileges to read it. The clamd daemon itself needs no special privileges in that situation.

(In reply to Hanspeter Gosteli from comment #4)
> > Likewise, there's got to be a better API for this. Or maybe those users should > be ignoring accesses that the "clamav" user performs instead.
> 
> Having worked with commercial AM, running clamd as root seems to be a must
> in order to achieve feature parity.

Can you explain what you mean a little more?  I agree with Michael that clamd should should use the User option so it switches to run as a less-privileged user after started as root.

On that topic -- for clamd to work as a less-privileged user, you either need to use fd-passing, or you need to start clamd with CAP_DAC_READ_SEARCH privelege (see https://github.com/Cisco-Talos/clamav-devel/pull/135).

fd-passing works for clamdscan, but is broken for clamonacc. We just merged a fix for clamonacc fd-passing that will go into 0.103.1 in mid/late January: https://github.com/Cisco-Talos/clamav-devel/commit/15927034582150fd1384c18118bcdbbff0d043ad

Btw, I was mistaken about freshclam using the PidFile option.  It actually uses clamd.conf for the socket information so it can send the reload command.

I was not aware of fd-passing. The concept of passing root's capabilities to lesser-priviledged users via file descriptor was new to me. Clamonacc clamd-pid-filtering is not needed at this point when OnAccessExcludeUname can be used.

Some commercial AM allows for filtering by process-name/binary-path, which is nice to have (From Application Owner's Perspective) and could have been built upon pid-filtering. Thinking about this i'd prefer OnAccessExcludeUname to Pid-Filtering. 

Thanks to Michael and Micah for taking the time, I highly appreciate it. Is PID used at all? Do you support me creating a PR:

1. removing OnAccessPidFile
2. removing -p from freshclam

Apologies, i aim to remove PidFile (not OnAccessPidFile). A PR should realize within days.

If above succeeds, removing -p will be attempted on a successive PR as no usage could be found in Fedora/EPEL packaging. Please let me know if you're aware of others using it or how i can track them down.

The -p flag can still be useful for "dumb" init systems like SysV init and (to a lesser extent) OpenRC. In a perfect world, the init scripts for those service managers would all be a part of the ClamAV source tree, and when you build clamav with e..g

  ./configure --runstatedir=/run

then the PID file locations would be compiled into both the daemon and the init scripts as /run/clamd.pid, /run/freshclam.pid, et cetera. That works only if you remove the PidFile settings from the configuration files, however.

If you leave the PidFile settings in the conf files, then the init system still needs to know where the PID file winds up, and it can't possibly parse your configuration files to find out. The next best thing it can do is override the config file with e.g. "clamd -p /run/clamd.pid", so that when the service manager later wants to stop clamd, it knows where to find the PID file.

Removing PidFile may be tricky because it could break working configurations. So on the assumption that the ClamAV wouldn't want to do that, I actually think *adding* the -p flag to the other daemons would be nice (see the title of this bug report), since it would let SysV init and OpenRC manage the location of the PID files and find them by a method more reliable than the current prayer-based model.

The proposed extension of the codebase would certainly work. It would be easy to implement and bring further options to users. 

On the other hand a reduction of the codebase is favorable, if it is possible to move pid-handling to service management.

Openrc should be able to do this using command_background as mentioned here:

https://github.com/OpenRC/openrc/blob/master/service-script-guide.md

Please comment: https://github.com/Cisco-Talos/clamav-devel/pull/146

(In reply to Hanspeter Gosteli from comment #11)
> The proposed extension of the codebase would certainly work. It would be
> easy to implement and bring further options to users. 
> 
> On the other hand a reduction of the codebase is favorable, if it is
> possible to move pid-handling to service management.
> 
> Openrc should be able to do this using command_background as mentioned here:

You are right about OpenRC, but SysVinit doesn't have the ability to force a process into the background, and I believe the rc.d system on all of the BSDs is the same. A PID file remains necessary on those systems, so it has to at least be *possible* to create one.


> https://github.com/OpenRC/openrc/blob/master/service-script-guide.md

I wrote that document =)

To contrast with the systemd approach, I personally prefer not to use the fancier features of OpenRC if the old way works just as well. When the OpenRC service script looks a lot like the BSD scripts, duplication is reduced, and bug fixes from one can be incorporated into the other. It also helps our BSD friends by ensuring that no "linux only" changes get incorporated upstream accidentally, now that most linux people are using systemd...

i am all in favor of keeping things simple, backwards compatible and supporting the broadest possible range of os. what do you think about introducing <service> & echo $! > /var/run/program.pid to the service script?

(In reply to Hanspeter Gosteli from comment #13)
> i am all in favor of keeping things simple, backwards compatible and
> supporting the broadest possible range of os. what do you think about
> introducing <service> & echo $! > /var/run/program.pid to the service script?

When programs daemonize themselves, they do more than just run in the background. They also close any inherited file descriptors, disconnect themselves from the controlling terminal, and switch their working directory to the root.

So, the problem is:

1. If you just run "clamd &" and let clamd daemonize itself, you're going to get the wrong PID in the file, because clamd calls fork().

2. If you run "clamd --foreground &", you'll get the right PID, but none of that other stuff I just mentioned will happen. (Systemd and OpenRC do it all themselves.)

Thanks, i am learning an amazing amount from you. Shouldn't there be any priviledge-drop / fork at all? Isn't that something that the service manager should take care of as well? Can we deprecate the User-option also? I found the user option confusing, because i remember the user was mentioned as clamav in one place and clamscan in another in doc. I would like to investigate deprecating the user or can you explain to me why we would want a priviledge-drop/fork?

(In reply to Hanspeter Gosteli from comment #15)
> Thanks, i am learning an amazing amount from you. Shouldn't there be any
> priviledge-drop / fork at all? Isn't that something that the service manager
> should take care of as well? Can we deprecate the User-option also? I found
> the user option confusing, because i remember the user was mentioned as
> clamav in one place and clamscan in another in doc. I would like to
> investigate deprecating the user or can you explain to me why we would want
> a priviledge-drop/fork?

You could probably eliminate it from the config file, but not altogether. The reasons are similar to those for keeping the PID file. The "dumb" service managers, which all run as root, simply don't know how to drop privileges on their own. They rely on clamd to do it during the daemonization process, so there has to be some way for clamd to know which user/group it's supposed to switch to.

Whether or not it can "politely" be removed from the config file (but left as a command-line flag) is not totally clear to me. For contrast, the PidFile option is "easy" to remove from the config file because no interactive human users care about it. The only "person" who should care about it is the service manager, who has no reason not to pass "-p <foo>" when starting the daemon. An interactive human user could just run "ps aux" or a similar command to find the PID if the need arises. He doesn't need the PID file if he's sitting there; but he probably does still want the daemon to drop its privileges without having to pass "-u clamav" every time.

On the other hand, if you're using a local socket, then a private directory needs to be set up for the clamd user to write to. In that case it makes sense that whoever created the directory should pass "-u <whatever>" to the daemon instead of relying on the contents of the config file to magically agree with the directory just created. And knowing the user ahead of time might improve security since you could (maybe, I haven't checked!) drop privileges before parsing the config file.

So... I can make an argument for it, but regardless, a lot more people are going to complain about that change. In an earlier comment I suggested a way around that problem:

1. Make it possible to do things right
2. Document the right way to do them
3. Wait a long time
4. Drop support for the wrong way, and hope nobody complains

Hi Michael and Hanspeter,

Regarding https://github.com/Cisco-Talos/clamav-devel/pull/146

It doesn't make sense to remove the PidFile option entirely, and certainly doesn't make sense to remove the --pid option or the clamd.conf User option. I love the idea of simplifying ClamAV configuration, but this change will break things for a lot of users.  As Michael noted:

> You are right about OpenRC, but SysVinit doesn't have the ability to force a process into the background, and I believe the rc.d system on all of the BSDs is the same. A PID file remains necessary on those systems, so it has to at least be *possible* to create one.

Per the response from the clamav-users mailing list (https://lists.archive.carbon60.com/clamav/users/80849) and comments from Michael here, even removing the PidFile config option will upset a bunch of folks who would have to change their scripts as a consequence, with very little benefit.

We should keep `--pid` and the `PidFile` config options, and add the `--pid` option to `clamd` and `clamonacc`. 

Hanspeter:  If you want to help out, perhaps you can replace https://github.com/Cisco-Talos/clamav-devel/pull/146 with a new PR that adds the `--pid=FILE` option to clamonacc and clamd instead, which will help for OpenRC (see this work-in-progress: https://github.com/Cisco-Talos/clamav-devel/pull/132). You could update the documentation in clamd.conf.sample and the clamd.conf manpage to recommend the using the `clamd --pid` option over the clamd.conf PidFile option.  We could even mark the PidFile option as "Deprecated". 

In the meantime, I'm going to go ahead and close https://github.com/Cisco-Talos/clamav-devel/pull/146