Skip to content

ClamAV: add a warning for the internal message size limit#3341

Merged
georglauterbach merged 1 commit intodocker-mailserver:masterfrom
ap-wtioit:master-fix_clamav_sanity_check
May 15, 2023
Merged

ClamAV: add a warning for the internal message size limit#3341
georglauterbach merged 1 commit intodocker-mailserver:masterfrom
ap-wtioit:master-fix_clamav_sanity_check

Conversation

@ap-wtioit
Copy link
Copy Markdown
Contributor

@ap-wtioit ap-wtioit commented May 15, 2023 •
edited
Loading

Description

ClamAV stops scanning files larger than or equal to 2GiB add a warning for that.

When checking the 4000M limit for CLAMAV_MESSAGE_SIZE_LIMIT in #3332 (comment) i discovered that ClamAV stops scanning at a much lower limit.

Fixes #

Type of change

  • Improvement (non-breaking change that does improve existing functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Info @wt-io-it

@georglauterbach georglauterbach self-requested a review May 15, 2023 09:16
@georglauterbach georglauterbach added area/scripts service/security/clamav kind/improvement Improve an existing feature, configuration file or the documentation labels May 15, 2023
@georglauterbach georglauterbach added this to the v13.0.0 milestone May 15, 2023
@georglauterbach georglauterbach changed the title add a warning for the internal message size limit of clamav ClamAV: add a warning for the internal message size limit May 15, 2023
@georglauterbach
Copy link
Copy Markdown
Member

georglauterbach commented May 15, 2023

Thanks a lot for this follow-up 🚀 ❤️

I have two questions:

  1. I wasn't sure whether ClamAV uses SI (MB) or IEC (MiB) as well; now our scripts use a mix. Could you please clarify
  2. When ClamAV does not scan 2G(i?)B files (or bigger), we don't need the warning for 4GB, right? Can you elaborate on why this was kept explicitly? I am completely fine with issuing a warning for 2G(i?)B, and I don't see value in also emitting one for 4GB, but I may have misunderstood something :)

@ap-wtioit
Copy link
Copy Markdown
Contributor Author

ap-wtioit commented May 15, 2023

ad 2) The reason for keeping both:
The 4000M / 4G warning is reflecting the ClamAV warning that you cannot configure a value higher than 4000M. Otherwise in my tests i got the following warning WARNING: Numerical value for option max-filesize too high, resetting to 4G from clamscan.

The 2GiB warning is different and somehow is only documented in the blog post (at least i could not find any other reference during running my tests).

So they are different and for users to make informed decisions we should tell them what's happening.

ad 1) Well, i wasn't sure either. ;-) I thought it was MiB, but it seems that for config limits they use SI (M) --max-filesize=4000M. However that internal limit is clearly at the 2GiB border. (bs=1024 count=2097152 (2GiB) fails, and bs=1024 count=2097151 (2GiB - 1KiB)) succeeds). And to make it clear that one is M (SI) and the other is Gi (IEC) i thought for the code to be easier (if it ever can be easy) to understand the units in the script should reflect this. Using float's of either M or GiB didn't seem to make it clear what's happening.
The full tests (to also document it here) i did were:

docker run --rm mailserver/docker-mailserver:latest bash -c 'dd if=/dev/zero bs=1024 count=2097152 of=zero.txt && du -hs zero.txt && clamscan --max-filesize=4000M --max-scansize=4000M --alert-exceeds-max zero.txt' 2>&1 | awk '{print "# " $0}'
# 2097152+0 records in
# 2097152+0 records out
# 2147483648 bytes (2.1 GB, 2.0 GiB) copied, 1.88814 s, 1.1 GB/s
# 2.0G	zero.txt
# LibClamAV Warning: **************************************************
# LibClamAV Warning: ***  The virus database is older than 7 days!  ***
# LibClamAV Warning: ***   Please update it as soon as possible.    ***
# LibClamAV Warning: **************************************************
# /zero.txt: Heuristics.Limits.Exceeded.MaxFileSize FOUND
# 
# ----------- SCAN SUMMARY -----------
# Known viruses: 8664779
# Engine version: 0.103.8
# Scanned directories: 0
# Scanned files: 1
# Infected files: 1
# Data scanned: 0.00 MB
# Data read: 2048.00 MB (ratio 0.00:1)
# Time: 10.142 sec (0 m 10 s)
# Start Date: 2023:05:15 12:28:48
# End Date:   2023:05:15 12:28:58
docker run --rm mailserver/docker-mailserver:latest bash -c 'dd if=/dev/zero bs=1024 count=2097151 of=zero.txt && du -hs zero.txt && clamscan --max-filesize=4000M --max-scansize=4000M --alert-exceeds-max zero.txt' 2>&1 | awk '{print "# " $0}'
# 2097151+0 records in
# 2097151+0 records out
# 2147482624 bytes (2.1 GB, 2.0 GiB) copied, 1.88303 s, 1.1 GB/s
# 2.0G	zero.txt
# LibClamAV Warning: **************************************************
# LibClamAV Warning: ***  The virus database is older than 7 days!  ***
# LibClamAV Warning: ***   Please update it as soon as possible.    ***
# LibClamAV Warning: **************************************************
# /zero.txt: OK
# 
# ----------- SCAN SUMMARY -----------
# Known viruses: 8664779
# Engine version: 0.103.8
# Scanned directories: 0
# Scanned files: 1
# Infected files: 0
# Data scanned: 4362.23 MB
# Data read: 2048.00 MB (ratio 2.13:1)
# Time: 375.698 sec (6 m 15 s)
# Start Date: 2023:05:15 12:29:17
# End Date:   2023:05:15 12:35:33

georglauterbach
georglauterbach requested changes May 15, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

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

I see, this is reasonable. If you agree, I'd like to apply some minimal feedback to the changes :)

Comment thread target/scripts/startup/setup.d/security/misc.sh Outdated
Comment thread target/scripts/startup/setup.d/security/misc.sh Outdated
Comment thread target/scripts/startup/setup.d/security/misc.sh Outdated
@ap-wtioit ap-wtioit force-pushed the master-fix_clamav_sanity_check branch 2 times, most recently from 111a0fa to 4ab3bf0 Compare May 15, 2023 13:25
@ap-wtioit ap-wtioit force-pushed the master-fix_clamav_sanity_check branch from 4ab3bf0 to 62b6fbf Compare May 15, 2023 13:26
@georglauterbach georglauterbach merged commit ec330a3 into docker-mailserver:master May 15, 2023
@casperklein casperklein mentioned this pull request May 15, 2023
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@georglauterbach georglauterbach georglauterbach approved these changes

Assignees

@ap-wtioit ap-wtioit

Labels

area/scripts kind/improvement Improve an existing feature, configuration file or the documentation service/security/clamav

Projects

Rspamd Integration
Archived in project

Milestone

v13.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@ap-wtioit @georglauterbach