Bug 220789 - Review Request: fail2ban - Ban IPs that make too many password failures
Review Request: fail2ban - Ban IPs that make too many password failures
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
: 210424 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-12-26 19:04 EST by Axel Thimm
Modified: 2009-01-28 19:26 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-31 13:54:16 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Axel Thimm 2006-12-26 19:04:36 EST
Spec URL: http://dl.atrpms.net/all/fail2ban.spec
SRPM URL: http://dl.atrpms.net/all/fail2ban-0.6.2-1.at.src.rpm
Description:
Fail2ban scans log files like /var/log/pwdfail or
/var/log/apache/error_log and bans IP that makes too many password
failures. It updates firewall rules to reject the IP address.
Comment 1 Axel Thimm 2006-12-26 19:09:35 EST
Expected rpmlint output:

E: fail2ban only-non-binary-in-usr-lib
W: fail2ban service-default-enabled /etc/rc.d/init.d/fail2ban

o service-default-enabled: The service may be enabled, but in absence of
  /etc/fail2ban.conf (which is the default) it will not start.
o only-non-binary-in-usr-lib: Unfortunately it installs its python bits under
  /usr/lib. I'm not sure why upstream has chosen so, it looks quite similar to how
  yum deals with its python files, too.
Comment 2 Mamoru TASAKA 2006-12-26 19:22:36 EST
I will review this, however, now I am preparing for going back
to my parents' home so it may be tomorrow.
Comment 3 Mamoru TASAKA 2006-12-26 19:24:59 EST
*** Bug 210424 has been marked as a duplicate of this bug. ***
Comment 4 Mamoru TASAKA 2006-12-28 11:24:49 EST
Well,

A. First for general packaging issue of this package:

E: fail2ban only-non-binary-in-usr-lib
!  Well, for this package moving all files in /usr/lib
   to %{_datadir} seems very easy and I recommend it
   (currently not a blocker, however would you contact with
    upstream?)

*  And... for this package the directory is /usr/lib,
   not %{_libdir}!!
   You can check this by setup.py (hard-coded)

W: fail2ban service-default-enabled /etc/rc.d/init.d/fail2ban
*  Umm.. I think this should be avoided.
   This warning is due to the line
---------------------------------------------------------------
# chkconfig: 345 91 9
---------------------------------------------------------------
   of /etc/rc.d/init.d/fail2ban . The description "345"
   means that fail2ban service is automatically enabled when
   installed on the level of 3-5 (man 8 chkconfig)

   And...
> The service may be enabled, but in absence of
> /etc/fail2ban.conf (which is the default) it will not start.
*  I think only the default behaviour of this script is
   unkind because fail2ban won't start but no error message
   is printed out.
   Current message is:
------------------------------------------------------
Starting fail2ban: 
------------------------------------------------------
   Some messages like
------------------------------------------------------
Starting fail2ban: configulation file not found
                                       [  FAILED  ]
------------------------------------------------------
   should be printed out. Also, the exit status of the
   failure should not be 0.

   Even I copyed /usr/share/doc/fail2ban/fail2ban.conf.iptables
   to /etc/fail2ban.conf, no message is printed out.
   Some messages which tells that starting daemon succeeded
   should be printed out.

Well, then...
B. From http://fedoraproject.org/wiki/Packaging/Guidelines :
! Licensing
  - Well, this package is licensed under GPL, however,
    GPL document is not included in source tarball. Currently
    this is not a blocker, however, please ask the upstream
    to include GPL document to source tarball.

! Filesystem Layout
  - Described above (not a blocker)
  - My opinion is fail2ban should be under %{_sbindir}.
  - Usually config files of initscripts should be under
    %{_sysconfdir}/sysconfig

* Scriptlets requirements
  ( http://fedoraproject.org/wiki/Packaging/ScriptletSnippets )
  - For /sbin/chkconfig and etc
    Please write Requires(post): /sbin/chkconfig and others
  - condrestart scriptlet on %postun stage is needed

* File and Directory Ownership
  - My opinion is that this package should own /var/log/fail2ban
    as a ghost file.

C. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
  = Okay, except for written in A and B.
Comment 5 Axel Thimm 2006-12-28 11:53:03 EST
Thanks for the first pass. I agree on the /usr/lib stuff. I'll fix it and
contact upstream. I also agree with most other points and am disappointed for
forgetting /sbin/chkconfig :/

I'll attack all points and come back with "-2"
Comment 6 Axel Thimm 2006-12-29 07:22:52 EST
Spec URL: http://dl.atrpms.net/all/fail2ban.spec
SRPM URL: http://dl.atrpms.net/all/fail2ban-0.6.2-2.at.src.rpm

* Fri Dec 29 2006 Axel Thimm <Axel.Thimm@ATrpms.net> - 0.6.2-2
- Move /usr/lib/fail2ban to %%{_datadir}/fail2ban.
- Don't default chkconfig to enabled.
- Add dependencies on service/chkconfig.
- Use example iptables/ssh config as default config.
Comment 7 Mamoru TASAKA 2006-12-30 03:10:28 EST
Well:

* Would you explain why you think that condrestart treatment of the
  service on %postun stage is unneeded?
* I still does not like the appearance of the start/exit of fai2ban service.
  Currently:
--------------------------------------------
[root@localhost ~]# service fail2ban start
Starting fail2ban: 
[root@localhost ~]# service fail2ban stop 
Stopping fail2ban: 
--------------------------------------------
  This should be like:
--------------------------------------------
[root@localhost ~]# service sshd start
Starting sshd:                              [ OK ]
[root@localhost ~]# service sshd stop 
Stopping sshd:                              [ OK ]
--------------------------------------------
* And.. 
--------------------------------------------
[ "${NETWORKING}" = "no" ] && exit 0
[ -f /etc/fail2ban.conf ] || exit 0
---------------------------------------------
  should be "exit 1" or something else: exit code 0 is
  wrong IMO. Also some messages which tells why starting
  fail2ban failed should be printed out.

* Still I think (strongly) that /usr/bin/fail2ban should 
  be moved under
  /usr/sbin because this is a sysadmin tool
  ... and /etc/fail2ban.conf should be /etc/sysconfig/fail2ban .

* And I think this package should own /var/log/fail2ban
Comment 8 Axel Thimm 2006-12-30 05:33:39 EST
> * Would you explain why you think that condrestart treatment of the
>   service on %postun stage is unneeded?

Yes, I consider fail2ban in this respect to be as fragile as for example the
iptables or httpd services: I don't want to automate therestart, the sysadmin
should do that manually and watch for side effects.

> [ "${NETWORKING}" = "no" ] && exit 0

This is the typical snipplet used throught all FC packages:

$ grep -l '\[ "${NETWORKING}" = "no" \] && exit 0' /etc/init.d/* | tr '\n' ' '
/etc/init.d/bgpd /etc/init.d/btseed /etc/init.d/bttrack /etc/init.d/dhcdbd
/etc/init.d/fail2ban /etc/init.d/gkrellmd /etc/init.d/innd /etc/init.d/netfs
/etc/init.d/network /etc/init.d/nfs /etc/init.d/nfslock /etc/init.d/ospfd
/etc/init.d/postgresql /etc/init.d/ripd /etc/init.d/rpcgssd
/etc/init.d/rpcidmapd /etc/init.d/rpcsvcgssd /etc/init.d/sendmail /etc/init.d/zebra

> [ -f /etc/fail2ban.conf ] || exit 0

Same here

$ grep -l '\[ -f .* \] || exit 0' /etc/init.d/* | tr '\n' ' '
/etc/init.d/acpid /etc/init.d/anacron /etc/init.d/bgpd /etc/init.d/bootparamd
/etc/init.d/capi /etc/init.d/clamav /etc/init.d/cpuspeed /etc/init.d/dhcp6r
/etc/init.d/dhcp6s /etc/init.d/dhcpd /etc/init.d/dhcrelay /etc/init.d/dund
/etc/init.d/exim /etc/init.d/fail2ban /etc/init.d/gkrellmd /etc/init.d/hidd
/etc/init.d/hsqldb /etc/init.d/innd /etc/init.d/irda /etc/init.d/irqbalance
/etc/init.d/mdmonitor /etc/init.d/mdmpd /etc/init.d/netdump /etc/init.d/netfs
/etc/init.d/nscd /etc/init.d/ospf6d /etc/init.d/ospfd /etc/init.d/pand
/etc/init.d/portmap /etc/init.d/radiusd /etc/init.d/radvd
/etc/init.d/restorecond /etc/init.d/rgmanager /etc/init.d/rhnsd /etc/init.d/ripd
/etc/init.d/ripngd /etc/init.d/sendmail /etc/init.d/spamassassin
/etc/init.d/squid /etc/init.d/syslog /etc/init.d/winbind /etc/init.d/yppasswdd
/etc/init.d/ypserv /etc/init.d/ypxfrd /etc/init.d/zaptel /etc/init.d/zebra

> ---------------------------------------------
>   should be "exit 1" or something else: exit code 0 is
>   wrong IMO. Also some messages which tells why starting
>   fail2ban failed should be printed out.

Well, it is obviously a Fedora convention not to do so. Whether it is right or
wrong is a different thing, but fail2ban has to blend in properly so the above
are correct. Anything else would have to be discussed with the FPC.

> * Still I think (strongly) that /usr/bin/fail2ban should 
>   be moved under
>   /usr/sbin because this is a sysadmin tool

You can use fail2ban as a user, too.

>   ... and /etc/fail2ban.conf should be /etc/sysconfig/fail2ban .

No, that's wrong, /etc/sysconfig carries config files for the init files
themselves (e.g. what arguments to use for calling a daemon), everything else is
defined by the application, e.g. check httpd, ntpd and so on.

> * And I think this package should own /var/log/fail2ban

Again no other packages caters for its logfile ownership, having fail2ban behave
differently is wrong. But I 100% with you on defining a general solution, just
not through a package submission. You're welcome to raise the issues at
fedora-packaging instead.
Comment 9 Mamoru TASAKA 2006-12-30 13:35:13 EST
Well:

A. From http://fedoraproject.org/wiki/Packaging/Guidelines
! Licensing
  - Please ask upstream to include to src tarball a copy of GPL
    (not a blocker)

= Scriptlets requirements
(In reply to comment #8)
> > * Would you explain why you think that condrestart treatment of the
> >   service on %postun stage is unneeded?
> 
> Yes, I consider fail2ban in this respect to be as 
> fragile as for example the
> iptables or httpd services: I don't want to automate therestart

  However, I found on your spec file (0.6.2-2)
------------------------------------------
%post
/sbin/chkconfig --add %{name}
/sbin/service %{name} condrestart > /dev/null 2>&1 <- THIS LINE
------------------------------------------
  Perhaps you may want to remove the line (I don't object to
  it according to your opinition).
  Note: leaving the line needs "Requires(post): /sbin/service"

B. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
   = okay, except for the issues on A.

------------------------------------------------------------------
   This package (fail2ban) is APPROVED by me.
-------------------------------------------------------------------

NOTE: the issue under discussion on fedora-maintainers about
      "very short APPROVED comments" was actually caused by
      my review.......
Comment 10 Axel Thimm 2006-12-30 14:13:56 EST
Upstream has been notified on both /usr/lib and license issue

> /sbin/service %{name} condrestart > /dev/null 2>&1 <- THIS LINE

Yes, that needs to go away. I added it, then regretted it and forgot to remove
it. :/

> This package (fail2ban) is APPROVED by me.

Thanks, I'll remove the line and import 0.6.2-3
Comment 11 Adam Miller 2009-01-28 15:29:13 EST
Package Change Request
======================
Package Name: fail2ban
New Branches: EL-4 EL-5
Owners: maxamillion

Fedora owner has been contacted and expressed no interest in EPEL, I would like to maintain the EPEL package for EL-4 and EL-5

Email citing upstream packagers response to supporting EPEL:


> According to
>
> https://admin.fedoraproject.org/pkgdb/packages/name/fail2ban#Fedora%20EPEL5
> , fail2ban is available.  A yum search fail2ban doesn't show it, however,
> and http://download.fedora.redhat.com/pub/epel/5/i386/repoview/F.group.html
> doesn't show it.  Am I missing something, or is there still a step before it
> goes fully available?  I don't even see it in the testing branch.

This was a bad entry, I am not supporting EPEL. You can get a fail2ban
working on RHEL5/CentOS5 at ATrpms or DAG.
--
Axel.Thimm at ATrpms.net
Comment 12 Kevin Fenzi 2009-01-28 19:26:20 EST
cvs done.

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