From 786722814682a7279314505ad2a1602e52e997c8 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 16 Aug 2017 10:45:37 +0200 Subject: [PATCH 1/6] closes part of gh-1865: fixed "Retrieving own IPs of localhost failed: inet_pton() argument 2 must be string, not int" some python-versions resp. host configurations causes returning of integer (instead of ip-string) --- fail2ban/server/ipdns.py | 8 ++++++-- fail2ban/tests/filtertestcase.py | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/fail2ban/server/ipdns.py b/fail2ban/server/ipdns.py index 6ef36888..7110d974 100644 --- a/fail2ban/server/ipdns.py +++ b/fail2ban/server/ipdns.py @@ -69,10 +69,14 @@ class DNSUtils: for fam, ipfam in ((socket.AF_INET, IPAddr.FAM_IPv4), (socket.AF_INET6, IPAddr.FAM_IPv6)): try: for result in socket.getaddrinfo(dns, None, fam, 0, socket.IPPROTO_TCP): - ip = IPAddr(result[4][0], ipfam) + # if getaddrinfo returns something unexpected: + if len(result) < 4 or not len(result[4]): continue + # get ip from `(2, 1, 6, '', ('127.0.0.1', 0))`,be sure we've an ip-string + # (some python-versions resp. host configurations causes returning of integer there): + ip = IPAddr(str(result[4][0]), ipfam) if ip.isValid: ips.append(ip) - except socket.error as e: + except Exception as e: saveerr = e if not ips and saveerr: logSys.warning("Unable to find a corresponding IP address for %s: %s", dns, saveerr) diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index cb0edb06..fe90f2ee 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -1852,6 +1852,10 @@ class DNSUtilsNetworkTests(unittest.TestCase): self.assertTrue(IPAddr("93.184.216.34").isInNet(ips)) self.assertTrue(IPAddr("2606:2800:220:1:248:1893:25c8:1946").isInNet(ips)) + def testIPAddr_wrongDNS_IP(self): + DNSUtils.dnsToIp('`this`.dns-is-wrong.`wrong-nic`-dummy') + DNSUtils.ipToName('*') + def testIPAddr_Cached(self): ips = [DNSUtils.dnsToIp('example.com'), DNSUtils.dnsToIp('example.com')] for ip1, ip2 in zip(ips, ips): From e5169d8f8421b3736c4610ad84c3953596b6d4b3 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 16 Aug 2017 11:37:22 +0200 Subject: [PATCH 2/6] pyinotify: be sure possible IOError/OSError by remove monitor (log-rotate? normally not raises) are handled properly. --- fail2ban/server/filterpyinotify.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fail2ban/server/filterpyinotify.py b/fail2ban/server/filterpyinotify.py index 41b8be7c..10f51847 100644 --- a/fail2ban/server/filterpyinotify.py +++ b/fail2ban/server/filterpyinotify.py @@ -231,6 +231,9 @@ class FilterPyinotify(FileFilter): return True except KeyError: # pragma: no cover pass + # EnvironmentError is parent of IOError, OSError, etc. + except EnvironmentError as e: # pragma: no cover (normally unreached) + logSys.error("Remove file monitor for %s causes %s", path, e) return False def _addDirWatcher(self, path_dir): @@ -249,6 +252,9 @@ class FilterPyinotify(FileFilter): self.__monitor.rm_watch(wdInt) except KeyError: # pragma: no cover pass + # EnvironmentError is parent of IOError, OSError, etc. + except EnvironmentError as e: # pragma: no cover (normally unreached) + logSys.error("Remove monitor for the parent directory %s causes %s", path_dir, e) logSys.debug("Removed monitor for the parent directory %s", path_dir) ## From 08646bc3399f62d6a7eed87579f72614fd3e9355 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 16 Aug 2017 13:14:42 +0200 Subject: [PATCH 3/6] Always supply jail name as name parameter (if not specified explicit in the action parameters `action[name=...]`). Avoid usage of the same chains (etc.) if someone use `action` instead of `banaction` jail parameter. --- fail2ban/client/actionreader.py | 3 +++ fail2ban/tests/clientreadertestcase.py | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/fail2ban/client/actionreader.py b/fail2ban/client/actionreader.py index ace0b898..559705bc 100644 --- a/fail2ban/client/actionreader.py +++ b/fail2ban/client/actionreader.py @@ -54,6 +54,9 @@ class ActionReader(DefinitionInitConfigReader): if actname is None: actname = file_ initOpts["actname"] = actname + # always supply jail name as name parameter if not specified in options: + if initOpts.get("name") is None: + initOpts["name"] = jailName self._name = actname DefinitionInitConfigReader.__init__( self, file_, jailName, initOpts, **kwargs) diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index c3a10c36..f9c5bf4c 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -33,7 +33,7 @@ from ..client import configparserinc from ..client.jailreader import JailReader from ..client.filterreader import FilterReader from ..client.jailsreader import JailsReader -from ..client.actionreader import ActionReader +from ..client.actionreader import ActionReader, CommandAction from ..client.configurator import Configurator from ..server.mytime import MyTime from ..version import version @@ -571,7 +571,8 @@ class JailsReaderTest(LogCaptureTestCase): ['set', 'brokenaction', 'addaction', 'brokenaction'], ['multi-set', 'brokenaction', 'action', 'brokenaction', [ ['actionban', 'hit with big stick '], - ['actname', 'brokenaction'] + ['actname', 'brokenaction'], + ['name', 'brokenaction'] ]], ['add', 'parse_to_end_of_jail.conf', 'auto'], ['set', 'parse_to_end_of_jail.conf', 'addfailregex', ''], @@ -612,6 +613,16 @@ class JailsReaderTest(LogCaptureTestCase): # all must have some actionban defined self.assertTrue(actionReader._opts.get('actionban', '').strip(), msg="Action file %r is lacking actionban" % actionConfig) + # test name of jail is set in options (also if not supplied within parameters): + opts = actionReader.getCombined( + ignore=CommandAction._escapedTags | set(('timeout', 'bantime'))) + self.assertEqual(opts.get('name'), 'TEST', + msg="Action file %r does not contains jail-name 'f2b-TEST'" % actionConfig) + # and the name is substituted (test several actions surely contains name-interpolation): + if actionName in ('pf', 'iptables-allports', 'iptables-multiport'): + #print('****', actionName, opts.get('actionstart', '')) + self.assertIn('f2b-TEST', opts.get('actionstart', ''), + msg="Action file %r: interpolation of actionstart does not contains jail-name 'f2b-TEST'" % actionConfig) self.assertIn('Init', actionReader.sections(), msg="Action file %r is lacking [Init] section" % actionConfig) From 19e59fff3edecde4e7be18f8b1f1023603981457 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 16 Aug 2017 15:38:44 +0200 Subject: [PATCH 4/6] ChangeLog: added incompatibility list (compared to v.0.9) --- ChangeLog | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/ChangeLog b/ChangeLog index d84ba34d..abcb0bb2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,31 @@ Fail2Ban: Changelog =================== +Incompatibility list (compared to v.0.9): +----------- + +* Filter (or `failregex`) internal capture-groups: + + - If you've your own `failregex` or custom filters using conditional match `(?P=host)`, you should + rewrite the regex like in example below resp. using `(?:(?P=ip4)|(?P=ip6)` instead of `(?P=host)` + (or `(?:(?P=ip4)|(?P=ip6)|(?P=dns))` corresponding your `usedns` and `raw` settings). + + Of course you can always your own capture-group (like below `_cond_ip_`) to do this. + ``` + testln="1500000000 failure from 192.0.2.1: bad host 192.0.2.1" + fail2ban-regex "$testln" "^\s*failure from (?P<_cond_ip_>): bad host (?P=_cond_ip_)$" + ``` + - New internal groups (currently reserved for internal usage): + `ip4`, `ip6`, `dns`, `fid`, `fport`, additionally `user` and another captures in lower case if + mapping from tag `` used in failregex (e. g. `user` by ``). + +* v.0.10 uses more precise date template handling, that can be theoretically incompatible to some + user configurations resp. `datepattern`. + +* Since v0.10 fail2ban supports the matching of the IPv6 addresses, but not all ban actions are + IPv6-capable now. + + ver. 0.10.1-dev-1 (2016/??/??) - development edition ----------- From f6ccede2f183f44f947e9ae2030a1aed1f73ba4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20K=C3=B6ppe?= Date: Wed, 16 Aug 2017 11:33:45 +0200 Subject: [PATCH 5/6] Update pf.conf fixing #1863 Fix #1863 Introduce own PF anchors for fail2ban rules. --- config/action.d/pf.conf | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/config/action.d/pf.conf b/config/action.d/pf.conf index deb38c09..02fbcd8d 100644 --- a/config/action.d/pf.conf +++ b/config/action.d/pf.conf @@ -15,8 +15,8 @@ # # we don't enable PF automatically; to enable run pfctl -e # or add `pf_enable="YES"` to /etc/rc.conf (tested on FreeBSD) -actionstart = echo "table <-> persist counters" | pfctl -f- - echo "block proto from <-> to " | pfctl -f- +actionstart = echo "table <-> persist counters" | pfctl -a f2b/ -f- + echo "block proto from <-> to " | pfctl -a f2b/ -f- # Option: start_on_demand - to start action on demand # Example: `action=pf[actionstart_on_demand=true]` @@ -27,16 +27,16 @@ actionstart_on_demand = false # Values: CMD # # we only disable PF rules we've installed prior -actionstop = pfctl -sr 2>/dev/null | grep -v - | pfctl -f- - pfctl -t - -T flush - pfctl -t - -T kill +actionstop = pfctl -a f2b/ -sr 2>/dev/null | grep -v - | pfctl -a f2b/ -f- + pfctl -a f2b/ -t - -T flush + pfctl -a f2b/ -t - -T kill # Option: actioncheck # Notes.: command executed once before each actionban command # Values: CMD # -actioncheck = pfctl -sr | grep -q - +actioncheck = pfctl -a f2b/ -sr | grep -q - # Option: actionban @@ -47,7 +47,7 @@ actioncheck = pfctl -sr | grep -q - #