Fix #820 - add new option --fail-when-outdated

[rzvncj]

When passed, causes clamscan to exit with a non-zero return code after printing "The virus database is older than 7 days!".

[candrews]

I'm really excited for this PR!

[micahsnyder]

The functions with prototypes in clamav.h (basically all cl_* functions) are a part of the public API and cannot be changed in this way. It would break the ABI such that all programs that use libclamav will have to be changed to use the new parameter.

One option is to wrap the existing cl_load and cl_cvdverify with new functions that have the extra parameters, like cl_load_ex and cl_verify_ex. I have a second idea though.

Instead, maybe a new function should be added to clamav.h along the lines of

cl_error_t cl_cvd_get_age(char *path, time_t **age_seconds);

My idea is to allow path to either be a database directory, OR the path to a database file (CVD or CLD).

• If it is a directory, you could iterate all of the files in the directory and find age of the youngest CVD (or CLD) in the directory and return that -- where age == s_time - cvd.stime or else 0 if "in the future".
• If it is a CVD/CLD file, then just get the age of that exact file.

Instead of having the option be --fail-when-outdated, let's call it something like --fail-if-cvd-older-than. Make that an integer of the age in days, instead of a bool. If you have a more succinct name for the option, I'm all ears.

Then, IF the --fail-if-cvd-older-than option is used, you can use this function before the calls to cl_load() inside clamscan and clamd to get that CVD age and compare it with the value from the option times the number of seconds in a day.

Also, if adding a new option to clamscan we should add the option to the clamscan manpage: https://github.com/Cisco-Talos/clamav/blob/main/docs/man/clamscan.1.in

Also, if clamscan gets a new --fail-if-cvd-older-than option, perhaps clamd.conf should get a matching FailIfCvdOlderThan option. I'm not sure why anyone would want it to fail, since clamd may reload after an update. (?) But maybe. What do you think? @candrews ?

Also, If we do add a FailIfCvdOlderThan option to clamd.conf, that of course means adding the example option to the etc/clamd.conf.sample and to the clamd.conf manpage: https://github.com/Cisco-Talos/clamav/blob/main/docs/man/clamd.conf.5.in

This second idea has a few benefits:

1. it mean only having to add 1 function to the clamav.h public API, and no confusing "_ex" wrappers for existing functions.
2. should mean you have to change less code, and don't have to touch cli_cvdload at all, except to copy some file access and time check logic. Of course, you'll have to write the new logic to check for the age of the youngest CLD/CVD in the database directory.
3. means the user can decide how old is acceptable before clamscan (or clamd) fail.

What do you think overall?

[rzvncj]

Thanks for the review!

The functions with prototypes in clamav.h (basically all cl_* functions) are a part of the public API and cannot be changed in this way. It would break the ABI such that all programs that use libclamav will have to be changed to use the new parameter.

Yeah, I did worry about that when writing the code and expected this objection during the review, but thought it'd be best to go ahead and write the code anyway so that we'd have something to start this conversation. Noted.

Instead of having the option be --fail-when-outdated, let's call it something like --fail-if-cvd-older-than. Make that an integer of the age in days, instead of a bool.

[...]

Then, IF the --fail-if-cvd-older-than option is used, you can use this function before the calls to cl_load() inside clamscan and clamd to get that CVD age and compare it with the value from the option times the number of seconds in a day.

[...]

What do you think overall?

I think that's a good approach. I'll wait for @candrews's input, and if it sounds good I'll get started.

[candrews]

Also, if clamscan gets a new --fail-if-cvd-older-than option, perhaps clamd.conf should get a matching FailIfCvdOlderThan option. I'm not sure why anyone would want it to fail, since clamd may reload after an update. (?) But maybe. What do you think? @candrews ?

I can think of a use for a FailIfCvdOlderThan option in clamd.conf for an architecture that I may use in the near future. I have a clamav mirror, and I have short running containers (run using the clamav/clamav:<version>_base docker image) that use clamd+clamdscan (I can't simply use clamscan because it doesn't do multithreading like clamdscan -m does). It's important for the scan to fail if the database is out of date (presumably due to the mirror not being up to date). If I were to use clamscan, the new --fail-if-cvd-older-than would be fantastic - but I don't think that option would be available for clamdscan; therefore, the FailIfCvdOlderThan option in clamd.conf would be necessary.

I think that's a good approach. I'll wait for @candrews's input, and if it sounds good I'll get started.

I also think this is a good approach, thank you!

[rzvncj]

Please ignore the latest update of the branch, I simply clicked GitHub's "Update branch" button, which is not doing a proper rebase by default. I'll update it properly once I finish the actual work.

Sorry for the noise!

[rzvncj]

New work moved to #867