Bug 557011 - Review Request: clamsmtp - A Daemon to virus scan mail using clamav
Summary: Review Request: clamsmtp - A Daemon to virus scan mail using clamav
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords: Reopened
: 555059 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-01-20 05:28 UTC by Nathanael Noblet
Modified: 2010-08-20 15:48 UTC (History)
4 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2010-08-20 15:48:03 UTC
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Nathanael Noblet 2010-01-20 05:28:37 UTC
Spec URL: http://www.gnat.ca/clamsmtp.spec
SRPM URL: http://www.gnat.ca/clamsmtp
Description:

ClamSMTP is an SMTP filter that allows you to check for
viruses using the ClamAV anti-virus software. It accepts
SMTP connections and forwards the SMTP commands and
responses to another SMTP server. The 'DATA' email body
is intercepted and scanned before forwarding.

It aims to be lightweight, reliable, and simple
rather than have a myriad of options. It's written in C
without major dependencies. If you need more options then
you could use something big like AMaViS which is written
in PERL and can do almost anything.

Written with the Postfix mail server in mind it can be
configured as a Postfix Content Filter. It can also
be used as a transparent proxy to filter an entire network's
SMTP traffic at the router.

Comment 1 Nathanael Noblet 2010-01-20 16:22:43 UTC
the rpmlint output and justification

clamsmtp.x86_64: W: non-standard-uid /var/run/clamsmtp clamsmtp
clamsmtp.x86_64: W: non-standard-uid /var/log/clamsmtp clamsmtp
clamsmtp.x86_64: W: non-standard-uid /var/lib/clamsmtp clamsmtp

This is so that the clamd instance and clamsmtp are isolated from someone on the system injecting/overriding the file being scanned.

clamsmtp.x86_64: W: dangling-relative-symlink /usr/sbin/clamd.clamsmtp ../../usr/sbin/clamd

Not sure why it complains here, the symlink works perfectly when installed

clamsmtp.x86_64: E: no-status-entry /etc/rc.d/init.d/clamd-clamsmtp
clamsmtp.x86_64: W: no-reload-entry /etc/rc.d/init.d/clamd-clamsmtp
clamsmtp.x86_64: E: subsys-not-used /etc/rc.d/init.d/clamd-clamsmtp
clamsmtp.x86_64: W: no-reload-entry /etc/rc.d/init.d/clamsmtpd

These are all pretty much bogus, the clamd initscript is a wrapper around /usr/share/clamav/clamd-wrapper which has all of the parts this is warning about.

To create this clamd instance I followed the instructions in the README of the clamav-server package. 

I debated creating a subpackage for the clamd instance, but decided against it. I guess it is possible that someone would want to install clamsmtp without a pre-setup clamd instance, however they can still do so with this setup. Granted installing this will pull in clamav-server and its dependancies. 

So that's basically what I'm wondering... should it be split in two?

Comment 2 Mamoru TASAKA 2010-01-20 16:36:19 UTC
As there is another earlier review request (and as I said
in bug 537587 comment 60), once closing this one.

Please ask morpheusv to incorporate your spec to his one.

*** This bug has been marked as a duplicate of bug 555059 ***

Comment 3 Nathanael Noblet 2010-06-22 21:40:07 UTC
re-opening...

Comment 4 Nathanael Noblet 2010-06-22 21:40:21 UTC
*** Bug 555059 has been marked as a duplicate of this bug. ***

Comment 5 Mamoru TASAKA 2010-06-24 19:17:28 UTC
Nathanael, would you upload the new srpm anyway?

Comment 6 Nathanael Noblet 2010-07-06 15:56:19 UTC
SRPM: http://www.gnat.ca/clamsmtp-1.10-0.1.fc12.src.rpm

Sorry the url was a bit off above...

Comment 7 Mamoru TASAKA 2010-07-07 18:45:35 UTC
For 0.1:

* Notes for clamd-clamsmtp service

  A Your spec file contains:
--------------------------------------------------------
    80      /sbin/service clamd.clamsmtp stop &>/dev/null || :
    81      /sbin/chkconfig --del clamd-clamsmtp
--------------------------------------------------------
    This is not consistent (clamd.clamsmtp vs clamd-clamsmtp).

  B # service clamd-clamsmtp status won't work
--------------------------------------------------------
[root@localhost ~]# service clamd-clamsmtp start 
Starting clamd.clamsmtp: LibClamAV Warning: **************************************************
LibClamAV Warning: ***  The virus database is older than 7 days!  ***
LibClamAV Warning: ***   Please update it as soon as possible.    ***
LibClamAV Warning: **************************************************
                                                           [  OK  ]
[root@localhost ~]# service clamd-clamsmtp status
clamd.clamsmtp dead but subsys locked
--------------------------------------------------------
    However:
--------------------------------------------------------
[root@localhost ~]# ps auwwx | grep [c]lamd
clamsmtp 12151  0.0 16.8 153680 126688 ?       Ssl  02:51   0:00 clamd.clamsmtp -c /etc/clamd.d/clamsmtp.conf --pid /var/run/clamd.clamsmtp/clamd.pid
--------------------------------------------------------
    Note that this ps output says that pid file is /var/run/clamd.clamsmtp/clamd.pid
    (also check the wrapper file /usr/share/clamav/clamd-wrapper), however
    on my system there is no /var/run/clamd.clamsmtp/ directory.
    /usr/share/clamav/clamd-wrapper:
--------------------------------------------------------
    29  CLAMD_PIDFILE=/var/run/clamd.${CLAMD_SERVICE}/clamd.pid
--------------------------------------------------------

  C logrotate config file won't work (as I said in bug 555059)
--------------------------------------------------------
[root@localhost ~]# ps auwwx | grep [c]lamd
clamsmtp 12151  1.0 17.6 154840 132868 ?       Ssl  02:51   0:07 clamd.clamsmtp -c /etc/clamd.d/clamsmtp.conf --pid /var/run/clamd.clamsmtp/clamd.pid
[root@localhost ~]# killall -HUP clamd.clamsmtp 
clamd.clamsmtp: no process found
--------------------------------------------------------
    The problem here is that the actual binary name exected here is not
    clamd.clamsmtp.
--------------------------------------------------------
[root@localhost ~]# pgrep -f clamd.clamsmtp 
12151
[root@localhost ~]# ls -lad /proc/$(pgrep -f clamd.clamsmtp)/exe
lrwxrwxrwx. 1 root root 0 Jul  8 03:07 /proc/12151/exe -> /usr/sbin/clamd
--------------------------------------------------------
    killall searches process by the actual binary name currently being executed
    (strace says killall reads /proc/XXXX/stat), so clamd.clamsmtp matches nothing.

    This should be like "pkill -SIGHUP -f clamd.clamsmtp", for example

  D. # service clamd-clamsmtp stop won't work
--------------------------------------------------------
[root@localhost ~]# service clamd-clamsmtp stop
Stopping clamd.clamsmtp:                                   [FAILED]
[root@localhost ~]# ps auwwx | grep [c]lamd
clamsmtp 12151  0.5 17.6 154840 132872 ?       Ssl  02:51   0:07 clamd.clamsmtp -c /etc/clamd.d/clamsmtp.conf --pid /var/run/clamd.clamsmtp/clamd.pid
--------------------------------------------------------

  ! Note
    The issues B. and D. seems because the directory /var/run/clamd.clamsmtp/
    does not exist.
    Or other solution would be to specify CLAMD_PIDFILE vairable in
    %{_sysconfdir}/sysconfig/clamd.clamsmtp
---------------------------------------------------------
[root@localhost ~]# install -d -m 0750 -o clamsmtp -g root  /var/run/clamd.clamsmtp
[root@localhost ~]# service clamd-clamsmtp start
Starting clamd.clamsmtp: LibClamAV Warning: **************************************************
LibClamAV Warning: ***  The virus database is older than 7 days!  ***
LibClamAV Warning: ***   Please update it as soon as possible.    ***
LibClamAV Warning: **************************************************
                                                           [  OK  ]
[root@localhost ~]# service clamd-clamsmtp status
clamd.clamsmtp (pid  12890) is running...
[root@localhost ~]# service clamd-clamsmtp stop  
Stopping clamd.clamsmtp:                                   [  OK  ]
[root@localhost ~]# service clamd-clamsmtp status
clamd.clamsmtp is stopped
---------------------------------------------------------

Comment 8 Mamoru TASAKA 2010-07-07 18:46:47 UTC
Other things:

* Requires
  - Some "Requires(postun)" is needed (postun calls /sbin/service)

* Macros / symlink
--------------------------------------------------------------
ln -s ../..%{_sbindir}/clamd $RPM_BUILD_ROOT/usr/sbin/clamd.clamsmtp
--------------------------------------------------------------
  - Note that %{_sbindir} = /usr/bin, i.e. clamd binary and clamd.clamsmtp
    symlink (to be created) are in the same directory. So the
    following is enough.
--------------------------------------------------------------
ln -sf clamd $RPM_BUILD_ROOT%{_sbindir}/clamd.clamsmtp
--------------------------------------------------------------
    Also please use macros for "basic" directories.

  - Use macros consistently. For example:
--------------------------------------------------------------
    57  mkdir -p $RPM_BUILD_ROOT%{_var}/lib/clamsmtp
    85  getent passwd clamsmtp >/dev/null || useradd -r -g mail -d /var/lib/clamsmtp -s ......
   105  %attr(755,clamsmtp,mail) %{_var}/lib/clamsmtp
--------------------------------------------------------------


By the way please begin release number with "1", and increment it
every time you modify your spec file (when version does not change)

Comment 9 Mamoru TASAKA 2010-07-17 15:45:31 UTC
ping?

Comment 10 Nathanael Noblet 2010-07-20 04:00:21 UTC
So I think everything you pointed out is fixed.

SRPM: http://www.gnat.ca/clamsmtp-1.10-0.2.fc13.src.rpm
SPEC: http://www.gnat.ca/clamsmtp.spec

Let me know if I missed anything.

Comment 11 Mamoru TASAKA 2010-07-25 17:03:04 UTC
Sorry for late.

Again please use <integer>%{?dist} for release number
unless the tarball is preversion or so.

* Requires(foo)
  - Please unify Requires(foo) to "/sbin/service" type or
    "initscripts" style, not use both, ref:
    https://fedoraproject.org/wiki/Packaging/SysVInitScript#InitscriptScriptlets

  - Some Requires(pre) is missing:
    https://fedoraproject.org/wiki/Packaging/UsersAndGroups

* Macros
  - Please use macros consistently.
    - If you want to use %{__rm} or %{__install}, please also use
      %{__make}, %{__mkdir_p}, %{__ln_s} or so. 

      Also both "%{__rm}" and rm are used.

* service name
  - Now rpmlint complains:
------------------------------------------------------------
clamsmtp.i686: E: init-script-name-with-dot /etc/rc.d/init.d/clamd.clamsmtp
------------------------------------------------------------
    Please check "$ rpmlint -I init-script-name-with-dot" for
    explanation.

Comment 12 Mamoru TASAKA 2010-08-05 15:53:29 UTC
ping?

Comment 13 Nathanael Noblet 2010-08-12 15:20:21 UTC
SRPM: http://www.gnat.ca/clamsmtp-1.10-1.fc13.src.rpm
SPEC: http://www.gnat.ca/clamsmtp.spec

Should all be fixed up. rpmlint complains a bit on the binary RPM about a few init script stuff that isn't true as the initscript is using the wrapper.

I added a small patch to fix a man page issue (I've emailed upstream...)

Comment 14 Mamoru TASAKA 2010-08-13 17:51:22 UTC
Okay.

----------------------------------------------------------
    This package (clamsmtp) is APPROVED by mtasaka
----------------------------------------------------------

Comment 15 Nathanael Noblet 2010-08-13 19:36:50 UTC
Package Change Request
======================
Package Name: clamsmtpd
New Branches: F12 F13 F14 EL5
Owners: gnat

Comment 16 Nathanael Noblet 2010-08-13 19:37:55 UTC
Package Change Request
======================
Package Name: clamsmtpd
New Branches: F12 F13 F14 EL5 EL6
Owners: gnat   

Sorry missed the EL6 branch...

Comment 17 Nathanael Noblet 2010-08-13 19:40:06 UTC
New Package SCM Request
=======================
Package Name:  clamsmtpd
Short Description: A daemon to virus scan mail using clamav
Owners: gnat
Branches: F12 F13 F14 EL5 EL6
InitialCC:

Comment 18 Jason Tibbitts 2010-08-14 14:47:41 UTC
The summary says "clamsmtp", the SCM request says "clamsmtpd".  Which did you actually want?  Please correct either the request or the bug summary so that they match.

Comment 19 Mamoru TASAKA 2010-08-14 15:28:00 UTC
Ah, Package Name should be clamsmtp
For now I rewrite git request (CC not needed for me)

New Package SCM Request
=======================
Package Name:  clamsmtp
Short Description: A daemon to virus scan mail using clamav
Owners: gnat
Branches: F12 F13 F14 EL5 EL6
InitialCC:

Comment 20 Kevin Fenzi 2010-08-14 20:28:23 UTC
Git done (by process-git-requests).

Comment 21 Mamoru TASAKA 2010-08-20 15:48:03 UTC
Closing.


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