Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the NotifyClamd path to the new freshclam.conf path #59

Merged

Conversation

christianbumann
Copy link
Contributor

@christianbumann christianbumann commented Oct 17, 2024

Freshclam now notifies the correct configuration about the database defininiton update.
Before this change the old path /etc/clamav/clamd.conf was is inside the freshclam.conf.

https://linux.die.net/man/5/freshclam.conf
NotifyClamd
Notify a running clamd(8) to reload its database after a download has occurred. The path for clamd.conf file must be provided. Default: The default is to not notify clamd. See clamd.conf(5)'s option SelfCheck for how clamd(8) handles database updates in this case.

Fixes #56

@christianbumann christianbumann force-pushed the topic/bch/56-fix-notify-path branch 2 times, most recently from 9f0ccc1 to 35f5bc1 Compare October 17, 2024 06:53
@@ -5,10 +5,15 @@ mkdir -p /clamav/data
mkdir -p /clamav/tmp
cp /etc/clamav/* /clamav/etc/

# Replace values in freshclam.conf
sed -i 's/^#\?NotifyClamd .*$/NotifyClamd \/clamav\/etc\/clamd.conf/g' /clamav/etc/freshclam.conf
Copy link
Contributor Author

@christianbumann christianbumann Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotifyClamd has no # at the beginning, so this ensures that both, with and without # replaces the value inside the config.
Imho we should improve this for the other settings too in a separate pull request to make this more reliable against changes to the defaults from the clamav and freshlam config settings. Whats your opinion @davosian, does it make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davosian Ready for review again. I will inform you tomorrow around 11:30 CET if it's working, as then a newer version of an antivirus database should be available.

Copy link
Contributor Author

@christianbumann christianbumann Oct 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reloading after 120s

2024-10-20 16:19:19 Sun Oct 20 16:19:19 2024 -> Self checking every 600 seconds.
2024-10-20 16:19:19 Sun Oct 20 16:19:19 2024 -> Set stacksize to 1048576
2024-10-20 16:19:20 Clamd version: "ClamAV 1.2.2/27429/Wed Oct 16 10:34:11 2024"
2024-10-20 16:19:20 Connected to clamd on tcp://localhost:3310
2024-10-20 16:20:40 RELOADING
2024-10-20 16:20:40 Sun Oct 20 16:20:40 2024 -> Reading databases from /clamav/data
2024-10-20 16:21:18 Sun Oct 20 16:21:18 2024 -> Database correctly reloaded (8699158 signatures)
2024-10-20 16:21:18 Sun Oct 20 16:21:18 2024 -> Activating the newly loaded database...

http://localhost:9000/version

{
    "Clamav": "1.2.2",
    "Signature": "27433",
    "Signature_date": "Sun Oct 20 10:46:57 2024"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next antivirus definition

2024-10-21 11:02:10 Received signal: wake up
2024-10-21 11:02:10 ClamAV update process started at Mon Oct 21 11:02:10 2024
2024-10-21 11:02:10 daily database available for update (local version: 27433, remote version: 27434)
2024-10-21 11:02:14 Testing database: '/clamav/data/tmp.98b9096e0f/clamav-ff2d2d9d28fa91a48f3b7a5a940e7f82.tmp-daily.cld' ...
2024-10-21 11:02:33 Database test passed.
2024-10-21 11:02:33 daily.cld updated (version: 27434, sigs: 2067364, f-level: 90, builder: raynman)
2024-10-21 11:02:33 main.cvd database is up-to-date (version: 62, sigs: 6647427, f-level: 90, builder: sigmgr)
2024-10-21 11:02:33 bytecode.cvd database is up-to-date (version: 335, sigs: 86, f-level: 90, builder: raynman)
2024-10-21 11:02:33 Clamd successfully notified about the update.
2024-10-21 11:02:33 Mon Oct 21 11:02:33 2024 -> Reading databases from /clamav/data
2024-10-21 11:03:17 Mon Oct 21 11:03:17 2024 -> Database correctly reloaded (8699158 signatures)
2024-10-21 11:03:17 Mon Oct 21 11:03:17 2024 -> Activating the newly loaded database...
2024-10-21 11:13:22 Mon Oct 21 11:13:22 2024 -> SelfCheck: Database status OK.

http://localhost:9000/version

{
    "Clamav": "1.2.2",
    "Signature": "27434",
    "Signature_date": "Mon Oct 21 10:49:31 2024"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davosian seems to work correct now

@christianbumann christianbumann force-pushed the topic/bch/56-fix-notify-path branch from 35f5bc1 to 5b98848 Compare October 17, 2024 06:57
@christianbumann
Copy link
Contributor Author

christianbumann commented Oct 17, 2024

I'll better test the application if updates are working correctly

@christianbumann
Copy link
Contributor Author

Hm this seems still not working. I will investigate again to get it working

@davosian
Copy link
Contributor

Thanks for fighting through this challenge, @christianbumann

@christianbumann
Copy link
Contributor Author

christianbumann commented Oct 20, 2024

Thanks for fighting through this challenge, @christianbumann

I just called clamscan --version and this provides database information from the original directory.
This led me to believe that the update wasn't working.

This returns the correct string clamscan --database=/clamav/data --version.

Anyway, calling the url http://localhost:9000/version still returns the old value.
I think I finally found the reason.

freshclam and clamd are starting almost at the same time, so clamd picks the old version and freshclam updates the database. So ClamAv returns the old version in the console log and also calling the web-api returns the old version.

I believed that the version check itself would notify ClamAv (fix in this pr) about the new version, but that only happens when a new version is available. So around 11:xx every day.
So you have to wait until the next new version after which it should be properly updated.
Tomorrow I'll see if my guess was correct.

To read the correct version directly, a timeout between freshclam and clamav start would help, but this is an ugly hack :)

I will try to notify the socket to reload the db after the ClamAv has been started.

@arizon-dread
Copy link
Collaborator

arizon-dread commented Oct 20, 2024

@christianbumann Could one way to circumvent this be to change line 6 in the entrypoint.sh:

# cp /etc/clamav/* /clamav/etc/
mv /etc/clamav/* /clamav/etc/
rm -r /etc/clamav
ln -s /clamav/etc/ /etc/clamav

So instead of copying the files and leaving the originals in the original place, move them to the new location, remove the old folder and creat a symlink in the old folder location pointing to the new location? That way, we can keep the files nice and orderly under /clamav/ and still only have one copy of the files, so nothing old is used or referenced unintentionally.

I haven't tested if this works.

@christianbumann
Copy link
Contributor Author

christianbumann commented Oct 20, 2024

@christianbumann Could one way to circumvent this be to change line 6 in the entrypoint.sh:

# cp /etc/clamav/* /clamav/etc/
mv /etc/clamav/* /clamav/etc/
rm -r /etc/clamav
ln -s /clamav/etc/ /etc/clamav

So instead of copying the files and leaving the originals in the original place, move them to the new location, remove the old folder and creat a symlink in the old folder location pointing to the new location? That way, we can keep the files nice and orderly under /clamav/ and still only have one copy of the files, so nothing old is used or referenced unintentionally.

I haven't tested if this works.

Yes this could work to get the clamscam --version working but imho it doesn‘t fix the main issue. An alternative could be to keep the original folder for data. Imho only cloning, changing and using the .conf files should be enough.

Reason for .conf cloning
#44

@christianbumann christianbumann force-pushed the topic/bch/56-fix-notify-path branch 6 times, most recently from 60658f3 to 83a8fc1 Compare October 20, 2024 14:13
@christianbumann christianbumann marked this pull request as ready for review October 20, 2024 14:13
@christianbumann christianbumann force-pushed the topic/bch/56-fix-notify-path branch from 83a8fc1 to f79c48a Compare October 20, 2024 14:17
Freshclam now notifies the correct configuration about the database defininiton update.
Before this change the old path `/etc/clamav/clamd.conf` was is inside the `freshclam.conf`.

https://linux.die.net/man/5/freshclam.conf
NotifyClamd
Notify a running clamd(8) to reload its database after a download has occurred. The path for clamd.conf file must be provided.
Default: The default is to not notify clamd. See clamd.conf(5)'s option SelfCheck for how clamd(8) handles database updates in this case.

Fixes ajilach#56
@christianbumann christianbumann force-pushed the topic/bch/56-fix-notify-path branch from f79c48a to 4712807 Compare October 20, 2024 14:27
@christianbumann
Copy link
Contributor Author

@davosian Do you have some time to review the changes? Many thanks

Copy link
Collaborator

@arizon-dread arizon-dread left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pulled the code and built it locally and verified that it works, the version updates as it should when selfchecking and updating the database, the /version endpoint then reflects the new version.

curl http://localhost:9000/version
{ "Clamav": "1.2.2", "Signature": "27455" , "Signature_date": "Mon Nov 11 10:58:33 2024" }
# After the running container triggers a database update and it finishes:
curl http://localhost:9000/version
{ "Clamav": "1.2.2", "Signature": "27456" , "Signature_date": "Tue Nov 12 10:49:43 2024" }

@davosian

Copy link
Contributor

@davosian davosian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that these changes work as expected. Thanks for all the hard work everybody.

@davosian davosian merged commit 5366cee into ajilach:master Nov 13, 2024
1 check passed
@christianbumann
Copy link
Contributor Author

I can confirm that these changes work as expected. Thanks for all the hard work everybody.

Many thanks for your review, and many thanks @arizon-dread to confirm that this is working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix configuration settings so that clamd is notified about the antivirus database update
3 participants