Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 220789 - Review Request: fail2ban - Ban IPs that make too many password failures
Summary: Review Request: fail2ban - Ban IPs that make too many password failures
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
: 210424 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-12-27 00:04 UTC by Axel Thimm
Modified: 2009-01-29 00:26 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-12-31 18:54:16 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Axel Thimm 2006-12-27 00:04:36 UTC
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-27 00:09:35 UTC
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-27 00:22:36 UTC
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-27 00:24:59 UTC
*** Bug 210424 has been marked as a duplicate of this bug. ***

Comment 4 Mamoru TASAKA 2006-12-28 16:24:49 UTC
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 16:53:03 UTC
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 12:22:52 UTC
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> - 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 08:10:28 UTC
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 10:33:39 UTC
> * 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 18:35:13 UTC
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 19:13:56 UTC
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 20:29:13 UTC
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-29 00:26:20 UTC
cvs done.


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