Closed ubellavance closed 7 years ago
What you have looks pretty good at first glance. If you submit a Pull Request, I'll add some comments.
Thanks for your work on this! I think we will end up needing milter as well, at some point.
Thanks, I think it is almost complete, tests will confirm. I think I should be able to test and have a final product by the end of next week. However, I can see that there is a pull request that changes the way that the default parameters are set. Should I follow this path or yours?
Also, I'll be doing the work for RHEL7 firstm, then maybe 6 and 5. I may try to make it work on Ubuntu but all I have is my home PC, and I don't use Debian at all.
Using a milter is quite simple. You configure the milter and you tell it how to talk with clamd (UNIX or TCP socket), then you configure your MTA (postfix, in our case), to use it. Then all the email messages received are sent to clamd for scan through the milter.
Don't worry about the default parameter change from the other pull request. When that gets accepted, I / we will refactor your stuff at that time.
No worries on RHEL7 only. Just make it fail with a nice message in the milter class indicating that you only support that at this time. We can add others later. Example:
unless ($::osfamily == 'RedHat') and (versioncmp($::operatingsystemrelease, '7.0') >= 0) {
fail("OS family ${::osfamily}-${::operatingsystemrelease} is not supported. Only RedHat >= 7 is suppported.")
}
@ubellavance How goes it? Can I help with anything?
@edestecd Thanks for asking :). I have done most of the work, but I got stuck on SELinux/permissions issues. I didn't have time to sort it out so I committed my most recent code even though it's still not complete. I'm about 99% done but the remaining 1% may take some time. I won't be able to work on this until Monday. You can have a look at the code if you'd like.
It looks like the current selinux-policy doesn't allow clamd to write temporary files in /tmp. I created a bug (https://bugzilla.redhat.com/show_bug.cgi?id=1351752). I don't think it will be included quickly in RHEL so I guess I'll have to insert a custom TE in the policy . I'm using the camptocamp module so I think it should be feasible: https://github.com/camptocamp/puppet-selinux/blob/master/manifests/module.pp
I'll let you know hot it goes but for now, the code is working if we insert a selinux module created with this TE: http://www.alcancelibre.org/linux/secrets/clamav-milter.te
@edestecd Even though all the puppet code is ready (or could be ready with little effort), I realize it won't work until the selinux-policy package is updated with the TE, which could be very long. And once the new selinux-policy package is out, I'll have to make this version a minimum requirement in the module. Do you see another solution?
What was the issue with using the selinux module?
Have you seen this? https://linux-audit.com/install-clamav-on-centos-7-using-freshclam/
If you set that seboolean then clamav can access the entire filesystem.
setsebool -P antivirus_can_scan_system 1
Puppet natively supports sebools: https://docs.puppet.com/puppet/latest/reference/type.html#selboolean
Setting the boolean didn't fix it. The problem is that when clamd receives a message from the milter, it wants to write it into a temp file in /tmp, which is prevented by selinux policy. See the bug I created: https://bugzilla.redhat.com/show_bug.cgi?id=1351752
The only solutions that I think of is to disable this temp file writing (I don't know if it's possible - most likely not because if we don't set the parameter it will use the OS's default) or change clamd's temp directory to one that has the right selinux context, but I woudn't know what context to use. I don't have information about this.
I tried setting 'TemporaryDirectory /var/lib/clamav' and it didn't change anything. I don't know enough about clamd to figure out what's going on.
That seems odd to me. Why would selinux prevent writing to /tmp. Usually that dir is pretty open and everyone can write to it. Have to tried listing the selinx context of the dirs with ls -Z and maybe changing them to another context with chcon?
I opened a ticket with Red Hat and they've been very helpful. I don't know yet how they got this information but this is their answer: The correct label for the target path would be 'antivirus_tmp_t' instead of init_tmp_t. I'll test that out when I have some time.
Update: I found out that I had to set the TemporaryDirectory in the milter, not in clamd. So I created a directory, /tmp/clamav-milter, and labeled it antivirus_tmp_t:
semanage fcontext -a -t antivirus_tmp_t /tmp/clamav-milter
restorecon -v /tmp/clamav-milter
I then set the premissions to 777 on /tmp/clamav-milter/
Then I set TemporaryDirectory /tmp/clamav-milter in /etc/mail/clamav-milter.conf, then restarted the milter. Now I don't get any SELinux error, but I get this error:
clamav-milter: LibClamAV Error: cli_gentempfd: Can't create temporary file /tmp/clamav-milter/clamav-9ea1dfda284eb7b52992887d1b6f41c2.tmp: No such file or directory
Still working on it. Getting closer...
Awesome. Great work. IT sounds like we need to manage the TemporaryDirectory with a file resource in puppet. This way we can set the proper selinux context etc..
Thanks, things are going forward. Yes, we'd have to set the TemporaryDirectory config as default in the module, and create the directory with a file resource. I haven't used the seltype option yet. But I still have to find out why I'm getting the No such file or directory
error.
I managed to make it work using only TCP sockets. I'm currently testing on a fresh install to see how it goes. I think the code should be ready this week. I have no idea why I had this problem with the UNIX socket but I don't have any time to figure out. Does your module sets a TCP socket by default for clamd? It looks like you don't define a socket by default for RHEL7. Should we make the clamd socket a TCP socket by default on RHEL7, or somehow send a warning if clamav-milter is used and is configured with an UNIX socket?
I think we are doing unix socket as the defualt for clam on RHEL6 and that will likely be the default on others soon. Sockets are safer as defaults and are way faster if everything is local to one box. For this reason I usually prefer them as the default. However, its not a deal breaker. I'd rather have it work out of the box and have the milter stuff than nothing at all...
I had this issue for nodejs etc in the past. You may need to set the selinux context on the file. This example allows httpd/nginx. You may need to find the context that allows virus_scan or clamd...
file {'socket_path':
seltype: httpd_sys_rw_content_t
}
Selinux is fun right? I guess the extra security is worth it in the end, but it sucks in the mean time.
This time it's probably not SELinux but I can't figure what it is.
Good news @edestecd!
I have finished the code. It is working under these conditions:
smtpd_milters = inet:localhost:8890
. If the MTA is not postfix, it must be configured to use the clamav-milter socket at localhost:8890milter_port_t
: tcp 8890,8891, 8893Here is my puppet config:
class { 'clamav':
manage_repo => false,
clamd_options => {
'TCPSocket' => '3310',
'TCPAddr' => '127.0.0.1'
},
manage_clamd => true,
manage_user => false,
manage_freshclam => true,
manage_clamav_milter => true,
clamd_service_ensure => 'running',
freshclam_service_ensure => 'stopped',
}
}
You can have a look at the code. What's the next step?
Should we start working on the documentation? I'm afraid that it may be problematic if we don't add support for the milter for the other OSs that you support... it may be misleading for users. It should at least be in the documentation.
@edestecd, I think there is one thing left. Please see https://github.com/edestecd/puppet-clamav/issues/20#issuecomment-220131314
What should we do with that? Manage the file to simply empty it? Offer options?
The rpm provides /etc/cron.d/clamav-update, but if we leave the /etc/sysconfig/freshclam file as is, the cronjob throws this error:
WARNING: update of clamav database is disabled; please see
'/etc/sysconfig/freshclam'
for information how to enable the periodic update resp. how to turn
off this message.
Yes we need documentation in the README indicating also that only Redhat is supported at this time. We also need spec tests for the new class. Can you open a Pull Request with your changes?
As for the /etc/sysconfig/freshclam issue, we have a PR to fix that. #24 I just need to finish reviewing it and accept the PR. Last I remember it was pretty much complete, just waiting on me to get time...
I did a PR. I hope I did it OK.
@ubellavance I don't see your pull request here: https://github.com/edestecd/puppet-clamav/pulls
I do see it in your fork. Did you maybe click pull request under your fork?
Most likely. I did it again, I think I did it right this time. Please let me know how it goes.
@edestecd Do you see the PR correctly now?
Hi,
Would it be possible to add clamav-milter support to this module? I personnaly use clamav mostly for mail servers and this module only lacks clamav-support to be able to add virus scanning to postfix or sendmail.