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

clamav-milter #22

Closed
ubellavance opened this issue Jun 22, 2016 · 26 comments
Closed

clamav-milter #22

ubellavance opened this issue Jun 22, 2016 · 26 comments

Comments

@ubellavance
Copy link

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.

@edestecd
Copy link
Owner

edestecd commented Jun 23, 2016

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.

@ubellavance
Copy link
Author

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.

@edestecd
Copy link
Owner

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.")
}

@edestecd
Copy link
Owner

@ubellavance How goes it? Can I help with anything?

@ubellavance
Copy link
Author

ubellavance commented Jun 29, 2016

@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.

@ubellavance
Copy link
Author

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

@ubellavance
Copy link
Author

@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?

@edestecd
Copy link
Owner

edestecd commented Jul 5, 2016

What was the issue with using the selinux module?

@edestecd
Copy link
Owner

edestecd commented Jul 5, 2016

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

@ubellavance
Copy link
Author

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.

@ubellavance
Copy link
Author

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.

@edestecd
Copy link
Owner

edestecd commented Jul 5, 2016

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?

@ubellavance
Copy link
Author

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.

@ubellavance
Copy link
Author

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...

@edestecd
Copy link
Owner

edestecd commented Jul 8, 2016

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..

@ubellavance
Copy link
Author

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.

@ubellavance
Copy link
Author

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?

@edestecd
Copy link
Owner

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.

@ubellavance
Copy link
Author

This time it's probably not SELinux but I can't figure what it is.

@ubellavance
Copy link
Author

ubellavance commented Jul 12, 2016

Good news @edestecd!

I have finished the code. It is working under these conditions:

  • The default is to use a tcp socket for clamd, listening on localhost (3310 is the default)
  • Once the configuration is applied, this line must be added to postfix's main.cf: smtpd_milters = inet:localhost:8890. If the MTA is not postfix, it must be configured to use the clamav-milter socket at localhost:8890
  • Another port can be used but the SELinux policy defines only these ports as milter_port_t: tcp 8890,8891, 8893

Here 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.

@ubellavance
Copy link
Author

@edestecd, I think there is one thing left. Please see #20 (comment)

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.

@edestecd
Copy link
Owner

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...

@ubellavance
Copy link
Author

I did a PR. I hope I did it OK.

@edestecd
Copy link
Owner

@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?

@ubellavance
Copy link
Author

Most likely. I did it again, I think I did it right this time. Please let me know how it goes.

@ubellavance
Copy link
Author

@edestecd Do you see the PR correctly now?

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

No branches or pull requests

2 participants