[Bug 220789] New: Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

Summary: Review Request: fail2ban - Ban IPs that make too many
                    password failures
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]

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.

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

------- Additional Comments From [e-mail]  2006-12-26 19:09 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.

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
OtherBugsDependingO|163776                      |163778
              nThis|                            |

------- Additional Comments From [e-mail]  2006-12-26 19:22 EST -------
I will review this, however, now I am preparing for going back
to my parents' home so it may be tomorrow.

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

------- Additional Comments From [e-mail]  2006-12-26 19:24 EST -------
*** Bug 210424 has been marked as a duplicate of this bug. ***

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

------- Additional Comments From [e-mail]  2006-12-28 11:24 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.

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

------- Additional Comments From [e-mail]  2006-12-28 11:53 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"

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

------- Additional Comments From [e-mail]  2006-12-29 07:22 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  - 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.

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

------- Additional Comments From [e-mail]  2006-12-30 03:10 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

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

------- Additional Comments From [e-mail]  2006-12-30 05:33 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.

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|163778                      |163779
              nThis|                            |

------- Additional Comments From [e-mail]  2006-12-30 13:35 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.......

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

------- Additional Comments From [e-mail]  2006-12-30 14:13 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

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fail2ban - Ban IPs that make too many password failures

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=220789

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=220789

Adam Miller  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
               Flag|                            |fedora-cvs?

--- Comment #11 from Adam Miller   2009-01-28 15:29:13 EDT ---
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

[Bug 220789] Review Request: fail2ban - Ban IPs that make too many password failures

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=220789

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #12 from Kevin Fenzi   2009-01-28 19:26:20 EDT ---
cvs done.