Bug 214980 - blacklistd and sshd incorrect counting of failed login attempts
Summary: blacklistd and sshd incorrect counting of failed login attempts
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 11.0-STABLE
Hardware: amd64 Any
: --- Affects Only Me
Assignee: Kurt Lidl
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-01 14:10 UTC by azhegalov
Modified: 2017-02-27 04:10 UTC (History)
2 users (show)

See Also:


Attachments
some output from ssh, blacklistd and blacklistctl (7.37 KB, text/plain)
2016-12-01 14:10 UTC, azhegalov
no flags Details
blacklist_helper diff (2.99 KB, patch)
2017-02-22 00:01 UTC, Kurt Lidl
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description azhegalov 2016-12-01 14:10:00 UTC
Created attachment 177576 [details]
some output from ssh, blacklistd and blacklistctl

Every one failed ssh login attempt generates several counts in blacklistd.db.

After two attempts

ssh -b 10.10.0.1 test@192.168.4.75
Password for test@192.168.4.75:
Password for test@192.168.4.75:

i got:
blacklistctl dump -a
        address/ma:port id      nfail   last access
      10.10.0.1/32:22   OK      6/5     2016/12/01 16:55:48


And /usr/libexec/blacklistd-helper script does not check ipfw rule existence before adding it. It generates excess rules like:

ipfw show
02022     27     2244 deny tcp from table(port22) to any dst-port 22
02022      0        0 deny tcp from table(port22) to any dst-port 22 <-----
02022      0        0 deny tcp from table(port22) to any dst-port 22 <-----
02022      0        0 deny tcp from table(port22) to any dst-port 22 <-----
65535 799979 77763414 allow ip from any to any
Comment 1 Kurt Lidl freebsd_committer freebsd_triage 2016-12-01 15:58:35 UTC
The relevant bit of the current blacklist-helper script:

        ipfw)
                # use $ipfw_offset+$port for rule number
                rule=$(($ipfw_offset + $6))
                tname="port$6"
                /sbin/ipfw table $tname create type addr 2>/dev/null
                /sbin/ipfw -q table $tname add "$addr/$mask"
                /sbin/ipfw -q add $rule drop $3 from "table("$tname")" to \
                    any dst-port $6 && echo OK
                ;;

I think that the problem is the '-q' on the last line is forcing "quiet"
behaviour, and (I missed this when adapting the code from 'pf'), it no
longer errors out on duplicate rules with the same number.

There's two different ways to address this that come to mind immediately:

1) Check to see if the rule exists before attempting to add it, and if
   it already exists, don't add it a second time.
2) Attempt to add the rule without -q, but with stderr redirected to /dev/null.
   I don't particularly like this, as it means that otherwise legitimate
   errors cannot be handled.

I'll take a stab at implementing #1 in the next few days.  (My dev machines
run 'pf'.)
Comment 2 azhegalov 2016-12-02 06:50:38 UTC
Kurt,
 can you try to reproduce my problem with incorrect counting of failed login? I checked ftpd and result is ok. So I have problem only with sshd.
Comment 3 Kurt Lidl freebsd_committer freebsd_triage 2017-02-19 23:39:59 UTC
I believe the incorrect counting for sshd has been resolved as of r313965.
(I had forgotten that you noted the incorrect counting as well as the ipfw
issue in this report, I had just mentally put this report into just an
ipfw issue).

There's a related issue in blacklistd itself, that I've fixed and sent
upstream.  I just need to pull in the latest upstream changes and merge
them.

Finally, there's the issue of the duplicate rules being inserted when
using ipfw.  I have a fix coded up, but haven't installed it all into
a virtual machine to test it.
Comment 4 Kurt Lidl freebsd_committer freebsd_triage 2017-02-22 00:01:56 UTC
Created attachment 180208 [details]
blacklist_helper diff
Comment 5 Kurt Lidl freebsd_committer freebsd_triage 2017-02-22 00:03:22 UTC
I just added the diff to the blacklist-helper script.

If you get a chance, please try it out and let me know if it
solves the duplicate rules problem.  I think it should, and
it seems to work properly in my testing.
Comment 6 commit-hook freebsd_committer freebsd_triage 2017-02-22 21:51:18 UTC
A commit references this bug:

Author: lidl
Date: Wed Feb 22 21:50:37 UTC 2017
New revision: 314111
URL: https://svnweb.freebsd.org/changeset/base/314111

Log:
  Improve ipfw rule creation for blacklist-helper script

  When blocking an address, the blacklist-helper script
  needs to do the following things for the ipfw packet
  filter:

   - create a table to hold the addresses to be blocked,
     so lookups can be done quickly, and place the address
     to be blocked in that table
   - create rule that does the lookup in the table and
     blocks the packet

  The ipfw system allows multiple rules to be inserted for
  a given rule number.  There only needs to be one rule
  to do the lookup per port.  Modify the script to probe
  for the existence of the rule before attempting to create
  it, so only one rule is inserted, rather than one rule per
  blocked address.

  PR:		214980
  Reported by:	azhegalov (at) gmail.com
  Reviewed by:	emaste
  MFC after:	3 days
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D9681

Changes:
  head/contrib/blacklist/libexec/blacklistd-helper
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-02-27 04:06:40 UTC
A commit references this bug:

Author: lidl
Date: Mon Feb 27 04:05:35 UTC 2017
New revision: 314324
URL: https://svnweb.freebsd.org/changeset/base/314324

Log:
  MFC r314111: Improve ipfw rule creation for blacklist-helper script

  When blocking an address, the blacklist-helper script
  needs to do the following things for the ipfw packet
  filter:

   - create a table to hold the addresses to be blocked,
     so lookups can be done quickly, and place the address
     to be blocked in that table
   - create rule that does the lookup in the table and
     blocks the packet

  The ipfw system allows multiple rules to be inserted for
  a given rule number.  There only needs to be one rule
  to do the lookup per port.  Modify the script to probe
  for the existence of the rule before attempting to create
  it, so only one rule is inserted, rather than one rule per
  blocked address.

  PR:		214980
  Reported by:	azhegalov (at) gmail.com
  Reviewed by:	emaste
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D9681

Changes:
  stable/11/contrib/blacklist/libexec/blacklistd-helper
Comment 8 Kurt Lidl freebsd_committer freebsd_triage 2017-02-27 04:10:21 UTC
The believe the most recent two MFCs to stable/11 that I have
performed (r314324, r314325) should address the last of the
issues identified in this bug.