Bug 2170876 - [clamav-data] %{homedir}/*.cvd files might be specified as %config(noreplace) resulting in *.rpmnew rather than overwriting possibly more recent files
Summary: [clamav-data] %{homedir}/*.cvd files might be specified as %config(noreplace)...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: clamav
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Orion Poplawski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-02-17 13:52 UTC by Jan Pokorný [poki]
Modified: 2023-03-11 03:11 UTC (History)
12 users (show)

Fixed In Version: clamav-1.0.1-4.fc38
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-03-11 03:11:51 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github Cisco-Talos clamav issues 845 0 None open freshclam 1.0.1 ignores daily.cld if daily.cvd is present 2023-02-20 23:00:15 UTC

Description Jan Pokorný [poki] 2023-02-17 13:52:09 UTC
(Gotcha ... just as I finished typing the summary, I've realized that it
may collide with the updated-solely-by-RPM approach to updates of ClamAV.
Will return to that shortly.)

Anyway, my point was to highlight an issue whereby updates by freshclam
(clamav-update package) may, in the current setup, collide with interim
updates by the mean of clamav-data RPM, as happened in my case:

> $ echo bar > foo
> $ clamscan foo
> LibClamAV Error: Can't load /var/lib/clamav/daily.cld: Can't verify database integrity
> LibClamAV Error: cli_loaddbdir: error loading database /var/lib/clamav/daily.cld  
> ERROR: Can't verify database integrity
> 
> ----------- SCAN SUMMARY -----------
> [...]

In this case, the fix was simple, as root:

> mv /var/lib/clamav/daily.cvd{,.rpmnew}
> mv /var/lib/clamav/daily.cvd{.rpmsave,}

IOW, I mimicked what using "%config(noreplace)" would achieve instead of plain
"%config" as it stands now.

But back to that gotcha...  Said modification would help with clamav-data updates
gradually merged into periodic updates independent of RPM delivery (said freshclam
way), however, it would as well prevent desired updates in case one does not use
freshclam/clamav-update as a means to fetch up-to-date signatures.  In that case,
it might hurt to offer a false sense of security (entangled with "periodically
updated signatures").

So I wonder:

1. would it change anything in the equations if clamav-data package bundled
   /var/lib/clamav/*.cld (in addition to existing *.cvd) files as well?
   (it might effectively be a packaging bug that these are not contained;
   perhaps these are something evolutionarily added once the specfile
   skeleton was already in use and not spotted later on)

2. could the scriptlet harness (%posttrans?) be made smart enough, using
   introspection (sigtool --info=/var/lib/clamav/...), so that clamav-data
   files are "appointed" only in case signature bundles delivered by that
   package are actually newer (i.e., the effect equivalent to "make 'noreplace'
   switch for %config directive context-dependent")?

I mean, sure, one likely don't want to have both clamav-data and clamav-update
installed (and active) at the same time, but I can imagine a scenario whereby
freshclam gets stuck for a reason or another, so the natural update path
for signature databases is broken, yet an occasional update via clamav-data
RPM would opportunistically help with a progressive move towards the latest
signatures, hence providing a slightly better/more reliable security.

Perhaps both 1. and 2. would deserve addressing on their own and I just
conflated these two rather orthogonal things.

Thanks for considering, nonetheless.

Comment 1 Jan Pokorný [poki] 2023-02-17 13:57:53 UTC
> [...] whereby freshclam gets stuck for a reason or another

For instance:

Feb 17 12:57:34 freshclam[467036]: FreshClam previously received error code 429 or 403 from the ClamAV Content Delivery Network (CDN).
Feb 17 12:57:34 freshclam[467036]: This means that you have been rate limited or blocked by the CDN.

Comment 2 Orion Poplawski 2023-02-19 00:27:55 UTC
I think this is a good suggestion.  Also, I don't think it's as complicated as you make it seem.  In the case where only clamav-data is installed (and not clamav-update) - updates of clamav-data will update the .cvd files.  %config(noreplace) means to not replace the file *if the file is modified from the original version* - if it isn't modified it is updated.  One concern that just came to me is whether or not our specifying %verify(not size md5 mtime) affects this calculation at all.  I'll have to run some tests.

Also, on a different topic - I think it's time to rename clamav-update to clamav-freshclam.  We would still have clamav-freshclam provide the clamav-update name.

Any other comments?

Comment 3 Orion Poplawski 2023-02-19 02:41:27 UTC
I also think we should have clamav-freshclam have "Supplements: clamd".  This changes the default install set for "clamd" to be:

clamd
clamav-lib
clamav-filesystem
clamav-freshclam

instead of having clamav-data.

Comment 4 Orion Poplawski 2023-02-19 02:51:44 UTC
PR to test out these changes: https://src.fedoraproject.org/rpms/clamav/pull-request/24

Comment 5 Jan Pokorný [poki] 2023-02-20 12:21:09 UTC
Thanks for looking into this.

I fully agree with s/update/freshclam/ and Supplements changes.

Yes, %config(noreplace) needs proper testing, will return to that when
I have more time at hand.

Just a quick remark wrt.

> 1. would it change anything in the equations if clamav-data package bundled
>    /var/lib/clamav/*.cld (in addition to existing *.cvd) files as well?
>    (it might effectively be a packaging bug that these are not contained;
>    perhaps these are something evolutionarily added once the specfile
>    skeleton was already in use and not spotted later on)

though:

I got a tiny bit further on this ... indeed, no *.cld seems to be provided
in readily downloadable form like with *.cvd.  However, the mere *.cld update
scheme as exercised with RPM-based update seems prone to this suboptimal
condition that I've hit per above, now with (hopefully) more precise assessment:

- daily.cld has a bigger priority when it exists at the filesystem level,
  so it gets processed first:
  https://github.com/Cisco-Talos/clamav/blob/clamav-1.0.1/libclamav/readdb.c#L5080-L5086

- daily.cld will indeed exist when freshclam updates exercised

- to be confirmed: when daily.cld is read and determined not in lockstep with
                   daily.cvd (conflicting update sources as discussed), clamscan
                   and friends bail out instantly (as per above), effectively
                   making it defunct

So unless we can get respective *.cld contained in clamav-data as well
(seems unlikely), or alternatively have the scriptlet rename pre-existing
*.cld (for relevant filename radices), the sketched introspection based
on version embedded in the signature file(s) looks like our best bet
(and generally best solution if additional complexity is tolerable).

Comment 6 Sergio Basto 2023-02-20 20:57:25 UTC
As general reply, I haven't read this bug report carefully and clamav-1.0.x is not yet my area. 

if you used freshclam you may remove clamav-data package and clamav-data shouldn't overwrite freshclam updates so noreplace is correct . 


(In reply to Orion Poplawski from comment #2)
> I think this is a good suggestion.  Also, I don't think it's as complicated
> as you make it seem.  In the case where only clamav-data is installed (and
> not clamav-update) - updates of clamav-data will update the .cvd files. 
> %config(noreplace) means to not replace the file *if the file is modified
> from the original version* - if it isn't modified it is updated.  One
> concern that just came to me is whether or not our specifying %verify(not
> size md5 mtime) affects this calculation at all.  I'll have to run some
> tests.
> 
> Also, on a different topic - I think it's time to rename clamav-update to
> clamav-freshclam.  We would still have clamav-freshclam provide the
> clamav-update name.
> 
> Any other comments?

clamav-freshclam LGTM

Comment 7 Orion Poplawski 2023-02-20 23:00:16 UTC
I'm somewhat lost on the whole .cld issue - but I wonder if it isn't being complicated by the fact that I shipped a bogus daily.cvd file with the 1.0.1/0.103.8-1 updates?  (In fact it was a daily.cld file instead).

The only thing I'm noticing with freshclam and 1.X is that it creates a daily.cld file and deletes the daily.cvd file.  On update of clamav-data the daily.cvd file will be re-created.  If I simulate this by reinstalling clamav-data and so get a slightly out of date daily.cvd, freshclam seems to use that:

Current working dir is /var/lib/clamav/
check_for_new_database_version: Local copy of daily found: daily.cvd.
query_remote_database_version: daily.cvd version from DNS: 26818
daily database available for update (local version: 26816, remote version: 26818)
Current database is 2 versions behind.
Downloading database patch # 26817...
....
updatedb: Running g_cb_download_complete callback...
download_complete_callback: Download complete for database : /var/lib/clamav/tmp.daaadde252/clamav-aef57e9a87d7d7725d97dbd708f8c73d.tmp-daily.cld
download_complete_callback:   fc_context->bTestDatabases   : 1
download_complete_callback:   fc_context->bBytecodeEnabled : 1
Testing database: '/var/lib/clamav/tmp.daaadde252/clamav-aef57e9a87d7d7725d97dbd708f8c73d.tmp-daily.cld' ...
Loading signatures from /var/lib/clamav/tmp.daaadde252/clamav-aef57e9a87d7d7725d97dbd708f8c73d.tmp-daily.cld
Properly loaded 2021302 signatures from /var/lib/clamav/tmp.daaadde252/clamav-aef57e9a87d7d7725d97dbd708f8c73d.tmp-daily.cld
Database test passed.
daily.cld updated (version: 26818, sigs: 2021302, f-level: 90, builder: raynman)
fc_update_database: daily.cld updated.


which seems sub-optimal.  Maybe this is the issue you were trying to raise?

What might make sense is a %post scriptlet to remove the daily.cvd file if a more recent daily.cld file exists?  I've updated the PR and https://copr.fedorainfracloud.org/coprs/orion/clamav-test/ to add that.

It also seems like a bug in freshclam to ignore the daily.cld file.  I've filed https://github.com/Cisco-Talos/clamav/issues/845

Comment 8 Sergio Basto 2023-02-24 16:24:06 UTC




(In reply to Sergio Basto from comment #6)
> As general reply, I haven't read this bug report carefully and clamav-1.0.x
> is not yet my area. 
> 
> if you used freshclam you may remove clamav-data package and clamav-data
> shouldn't overwrite freshclam updates so noreplace is correct . 
 
ah I read this report upside down , because I though in 0.10x.x. it already use noreplace  , when is just used %config 

I test in F37 and freshclam delete /var/lib/clamav/daily.cvd 

but aware 
rpm -ql clamav-data
/var/lib/clamav/bytecode.cvd
/var/lib/clamav/daily.cvd
/var/lib/clamav/main.cvd

rpm -ql clamav-update
(...)
/var/lib/clamav/bytecode.cld
/var/lib/clamav/daily.cld
/var/lib/clamav/main.cvd

files clamav-data are .cvd and files from freshclam are .cld


maybe we can provide clamav-data in a separated package to avoid unnecessary updates of the package, a sort of https://bugzilla.redhat.com/show_bug.cgi?id=1477777 

Also I though that we can think in one rule where clamav-data and clamav-update couldn't not be installed together ...

Comment 9 Orion Poplawski 2023-02-26 23:56:56 UTC
I've added a commit to make clamav-freshclam (nee -update) %ghost all of the *.cld and *.cvd files.  Initial download will be .cvd, updated will be .cld.

I don't think we should ship clamav-data as a separate package.  I don't think Fedora should a replacement for Cisco's clamav content distribution.  While it might provide some use for initial setup or CI/testing setups, that's really the only goal.  We should *not* be updating the clamav package simply to update the cvd files.

I think this latest version seems pretty complete to me and I expect to push these changes to f38/rawhide soon.

Comment 10 Fedora Update System 2023-02-28 14:21:56 UTC
FEDORA-2023-221c3d26f9 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-221c3d26f9

Comment 11 Fedora Update System 2023-03-01 01:50:41 UTC
FEDORA-2023-221c3d26f9 has been pushed to the Fedora 38 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-221c3d26f9

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 12 Fedora Update System 2023-03-11 03:11:51 UTC
FEDORA-2023-221c3d26f9 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.