From bba7a6c5cfa24c15c639ad2e28adfef45662b85f Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 17 Apr 2018 18:59:24 +0200 Subject: [PATCH 1/5] amend to (gh-2067) / b34ae5999e0d8ee1af8939527305c13152844b3d: fix parameter in config (dynamic parameters stating with '_' are protected and don't allowed in command-actions); the interpolation of hostsdeny is test-covered now; closes gh-2114. --- config/action.d/hostsdeny.conf | 8 +++---- fail2ban/tests/servertestcase.py | 38 ++++++++++++++++++-------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/config/action.d/hostsdeny.conf b/config/action.d/hostsdeny.conf index 4277fed8..2a93c82b 100644 --- a/config/action.d/hostsdeny.conf +++ b/config/action.d/hostsdeny.conf @@ -31,7 +31,7 @@ actioncheck = # Tags: See jail.conf(5) man page # Values: CMD # -actionban = printf %%b ": <_ip_value>\n" >> +actionban = printf %%b ": \n" >> # Option: actionunban # Notes.: command executed when unbanning an IP. Take care that the @@ -39,7 +39,7 @@ actionban = printf %%b ": <_ip_value>\n" >> # Tags: See jail.conf(5) man page # Values: CMD # -actionunban = IP=$(echo "<_ip_value>" | sed 's/[][\.]/\\\0/g') && sed -i "/^: $IP$/d" +actionunban = IP=$(echo "" | sed 's/[][\.]/\\\0/g') && sed -i "/^: $IP$/d" [Init] @@ -56,7 +56,7 @@ file = /etc/hosts.deny daemon_list = ALL # internal variable IP (to differentiate the IPv4 and IPv6 syntax, where it is enclosed in brackets): -_ip_value = +ip_value = [Init?family=inet6] -_ip_value = [] +ip_value = [] diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index c207311d..c9719bf7 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -1186,6 +1186,22 @@ class ServerConfigReaderTests(LogCaptureTestCase): # 'start', 'stop' - should be found (logged) on action start/stop, # etc. testJailsActions = ( + # hostsdeny -- + ('j-hostsdeny', 'hostsdeny[name=%(__name__)s, actionstop="rm ", file="/tmp/fail2ban.dummy"]', { + 'ip4': ('family: inet4',), 'ip6': ('family: inet6',), + 'ip4-ban': ( + r'''`printf %b "ALL: 192.0.2.1\n" >> /tmp/fail2ban.dummy`''', + ), + 'ip4-unban': ( + r'''`IP=$(echo "192.0.2.1" | sed 's/[][\.]/\\\0/g') && sed -i "/^ALL: $IP$/d" /tmp/fail2ban.dummy`''', + ), + 'ip6-ban': ( + r'''`printf %b "ALL: [2001:db8::]\n" >> /tmp/fail2ban.dummy`''', + ), + 'ip6-unban': ( + r'''`IP=$(echo "[2001:db8::]" | sed 's/[][\.]/\\\0/g') && sed -i "/^ALL: $IP$/d" /tmp/fail2ban.dummy`''', + ), + }), # dummy -- ('j-dummy', 'dummy[name=%(__name__)s, init="==", target="/tmp/fail2ban.dummy"]', { 'ip4': ('family: inet4',), 'ip6': ('family: inet6',), @@ -1198,8 +1214,6 @@ class ServerConfigReaderTests(LogCaptureTestCase): 'stop': ( '`echo "[j-dummy] dummy /tmp/fail2ban.dummy -- stopped"`', ), - 'ip4-check': (), - 'ip6-check': (), 'ip4-ban': ( '`echo "[j-dummy] dummy /tmp/fail2ban.dummy -- banned 192.0.2.1 (family: inet4)"`', ), @@ -1324,8 +1338,6 @@ class ServerConfigReaderTests(LogCaptureTestCase): "`ipset flush f2b-j-w-iptables-ipset6`", "`ipset destroy f2b-j-w-iptables-ipset6`", ), - 'ip4-check': (), - 'ip6-check': (), 'ip4-ban': ( r"`ipset add f2b-j-w-iptables-ipset 192.0.2.1 timeout 600 -exist`", ), @@ -1362,8 +1374,6 @@ class ServerConfigReaderTests(LogCaptureTestCase): "`ipset flush f2b-j-w-iptables-ipset-ap6`", "`ipset destroy f2b-j-w-iptables-ipset-ap6`", ), - 'ip4-check': (), - 'ip6-check': (), 'ip4-ban': ( r"`ipset add f2b-j-w-iptables-ipset-ap 192.0.2.1 timeout 600 -exist`", ), @@ -1671,8 +1681,6 @@ class ServerConfigReaderTests(LogCaptureTestCase): "`ipset flush f2b-j-w-fwcmd-ipset6`", "`ipset destroy f2b-j-w-fwcmd-ipset6`", ), - 'ip4-check': (), - 'ip6-check': (), 'ip4-ban': ( r"`ipset add f2b-j-w-fwcmd-ipset 192.0.2.1 timeout 600 -exist`", ), @@ -1709,8 +1717,6 @@ class ServerConfigReaderTests(LogCaptureTestCase): "`ipset flush f2b-j-w-fwcmd-ipset-ap6`", "`ipset destroy f2b-j-w-fwcmd-ipset-ap6`", ), - 'ip4-check': (), - 'ip6-check': (), 'ip4-ban': ( r"`ipset add f2b-j-w-fwcmd-ipset-ap 192.0.2.1 timeout 600 -exist`", ), @@ -1762,7 +1768,7 @@ class ServerConfigReaderTests(LogCaptureTestCase): action.start() if tests.get('start'): self.assertLogged(*tests['start'], all=True) - else: + elif tests.get('ip4-start') and tests.get('ip6-start'): self.assertNotLogged(*tests['ip4-start']+tests['ip6-start'], all=True) ainfo = { 'ip4': _actions.Actions.ActionInfo(tickets['ip4'], jails[jail]), @@ -1773,24 +1779,24 @@ class ServerConfigReaderTests(LogCaptureTestCase): action.ban(ainfo['ip4']) if tests.get('ip4-start'): self.assertLogged(*tests['ip4-start'], all=True) if tests.get('ip6-start'): self.assertNotLogged(*tests['ip6-start'], all=True) - self.assertLogged(*tests['ip4-check']+tests['ip4-ban'], all=True) + self.assertLogged(*tests.get('ip4-check',())+tests['ip4-ban'], all=True) self.assertNotLogged(*tests['ip6'], all=True) # test unban ip4 : self.pruneLog('# === unban ipv4 ===') action.unban(ainfo['ip4']) - self.assertLogged(*tests['ip4-check']+tests['ip4-unban'], all=True) + self.assertLogged(*tests.get('ip4-check',())+tests['ip4-unban'], all=True) self.assertNotLogged(*tests['ip6'], all=True) # test ban ip6 : self.pruneLog('# === ban ipv6 ===') action.ban(ainfo['ip6']) if tests.get('ip6-start'): self.assertLogged(*tests['ip6-start'], all=True) if tests.get('ip4-start'): self.assertNotLogged(*tests['ip4-start'], all=True) - self.assertLogged(*tests['ip6-check']+tests['ip6-ban'], all=True) + self.assertLogged(*tests.get('ip6-check',())+tests['ip6-ban'], all=True) self.assertNotLogged(*tests['ip4'], all=True) # test unban ip6 : self.pruneLog('# === unban ipv6 ===') action.unban(ainfo['ip6']) - self.assertLogged(*tests['ip6-check']+tests['ip6-unban'], all=True) + self.assertLogged(*tests.get('ip6-check',())+tests['ip6-unban'], all=True) self.assertNotLogged(*tests['ip4'], all=True) # test flush for actions should supported this: if tests.get('flush'): @@ -1800,7 +1806,7 @@ class ServerConfigReaderTests(LogCaptureTestCase): # test stop : self.pruneLog('# === stop ===') action.stop() - self.assertLogged(*tests['stop'], all=True) + if tests.get('stop'): self.assertLogged(*tests['stop'], all=True) def _executeMailCmd(self, realCmd, timeout=60): # replace pipe to mail with pipe to cat: From 9f3a80a21a12dc1f6559f37f0269528d8163892c Mon Sep 17 00:00:00 2001 From: "Sergey G. Brester" Date: Tue, 17 Apr 2018 19:08:48 +0200 Subject: [PATCH 2/5] Update ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index a7d21383..57b6c60a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -35,6 +35,8 @@ ver. 0.10.4-dev-1 (20??/??/??) - development edition ----------- ### Fixes +* `action.d/hostsdeny.conf`: fix parameter in config (dynamic parameters stating with '_' are protected + and don't allowed in command-actions), see gh-2114; ### New Features From 4099897be009c19921873927875c773c0759a2d1 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 17 Apr 2018 19:10:31 +0200 Subject: [PATCH 3/5] test dummy action first --- fail2ban/tests/servertestcase.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index c9719bf7..59dfef50 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -1186,22 +1186,6 @@ class ServerConfigReaderTests(LogCaptureTestCase): # 'start', 'stop' - should be found (logged) on action start/stop, # etc. testJailsActions = ( - # hostsdeny -- - ('j-hostsdeny', 'hostsdeny[name=%(__name__)s, actionstop="rm ", file="/tmp/fail2ban.dummy"]', { - 'ip4': ('family: inet4',), 'ip6': ('family: inet6',), - 'ip4-ban': ( - r'''`printf %b "ALL: 192.0.2.1\n" >> /tmp/fail2ban.dummy`''', - ), - 'ip4-unban': ( - r'''`IP=$(echo "192.0.2.1" | sed 's/[][\.]/\\\0/g') && sed -i "/^ALL: $IP$/d" /tmp/fail2ban.dummy`''', - ), - 'ip6-ban': ( - r'''`printf %b "ALL: [2001:db8::]\n" >> /tmp/fail2ban.dummy`''', - ), - 'ip6-unban': ( - r'''`IP=$(echo "[2001:db8::]" | sed 's/[][\.]/\\\0/g') && sed -i "/^ALL: $IP$/d" /tmp/fail2ban.dummy`''', - ), - }), # dummy -- ('j-dummy', 'dummy[name=%(__name__)s, init="==", target="/tmp/fail2ban.dummy"]', { 'ip4': ('family: inet4',), 'ip6': ('family: inet6',), @@ -1227,6 +1211,22 @@ class ServerConfigReaderTests(LogCaptureTestCase): '`echo "[j-dummy] dummy /tmp/fail2ban.dummy -- unbanned 2001:db8:: (family: inet6)"`', ), }), + # hostsdeny -- + ('j-hostsdeny', 'hostsdeny[name=%(__name__)s, actionstop="rm ", file="/tmp/fail2ban.dummy"]', { + 'ip4': ('family: inet4',), 'ip6': ('family: inet6',), + 'ip4-ban': ( + r'''`printf %b "ALL: 192.0.2.1\n" >> /tmp/fail2ban.dummy`''', + ), + 'ip4-unban': ( + r'''`IP=$(echo "192.0.2.1" | sed 's/[][\.]/\\\0/g') && sed -i "/^ALL: $IP$/d" /tmp/fail2ban.dummy`''', + ), + 'ip6-ban': ( + r'''`printf %b "ALL: [2001:db8::]\n" >> /tmp/fail2ban.dummy`''', + ), + 'ip6-unban': ( + r'''`IP=$(echo "[2001:db8::]" | sed 's/[][\.]/\\\0/g') && sed -i "/^ALL: $IP$/d" /tmp/fail2ban.dummy`''', + ), + }), # iptables-multiport -- ('j-w-iptables-mp', 'iptables-multiport[name=%(__name__)s, bantime="10m", port="http,https", protocol="tcp", chain=""]', { 'ip4': ('`iptables ', 'icmp-port-unreachable'), 'ip6': ('`ip6tables ', 'icmp6-port-unreachable'), From a208b11796e07fe86aba4397028a0de36e2638ca Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 25 Apr 2018 19:06:03 +0200 Subject: [PATCH 4/5] added log message if ignored by `ignoreself` rule (similar to both other rules `ignoreip` and `ignorecommand`), and test covered now; --- fail2ban/server/filter.py | 1 + fail2ban/tests/filtertestcase.py | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py index f86d9613..bac1ba53 100644 --- a/fail2ban/server/filter.py +++ b/fail2ban/server/filter.py @@ -496,6 +496,7 @@ class Filter(JailThread): # check own IPs should be ignored and 'ip' is self IP: if self.__ignoreSelf and ip in DNSUtils.getSelfIPs(): + self.logIgnoreIp(ip, log_ignore, ignore_source="ignoreself rule") return True for net in self.__ignoreIpList: diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index 253ae111..bf1bf84d 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -336,17 +336,21 @@ class IgnoreIP(LogCaptureTestCase): ipList = ("127.0.0.1",) # test ignoreSelf is false: for ip in ipList: - self.assertFalse(self.filter.inIgnoreIPList(ip)) + self.assertFalse(self.filter.inIgnoreIPList(ip, log_ignore=True)) + self.assertNotLogged("[%s] Ignore %s by %s" % (self.jail.name, ip, "ignoreself rule")) # test ignoreSelf with true: self.filter.ignoreSelf = True + self.pruneLog() for ip in ipList: - self.assertTrue(self.filter.inIgnoreIPList(ip)) + self.assertTrue(self.filter.inIgnoreIPList(ip, log_ignore=True)) + self.assertLogged("[%s] Ignore %s by %s" % (self.jail.name, ip, "ignoreself rule")) def testIgnoreIPOK(self): ipList = "127.0.0.1", "192.168.0.1", "255.255.255.255", "99.99.99.99" for ip in ipList: self.filter.addIgnoreIP(ip) - self.assertTrue(self.filter.inIgnoreIPList(ip)) + self.assertTrue(self.filter.inIgnoreIPList(ip, log_ignore=True)) + self.assertLogged("[%s] Ignore %s by %s" % (self.jail.name, ip, "ip")) def testIgnoreIPNOK(self): ipList = "", "999.999.999.999", "abcdef.abcdef", "192.168.0." From e01981cc72052ff14bd62f4d06931493e519ef88 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 25 Apr 2018 19:13:56 +0200 Subject: [PATCH 5/5] tweak performance a bit (filter.py: inIgnoreIPList: changed default of parameter `log_ignore=True`, mostly used against filter-calls) --- fail2ban/server/filter.py | 6 +++--- fail2ban/tests/filtertestcase.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py index bac1ba53..9ffc9177 100644 --- a/fail2ban/server/filter.py +++ b/fail2ban/server/filter.py @@ -418,7 +418,7 @@ class Filter(JailThread): def addBannedIP(self, ip): if not isinstance(ip, IPAddr): ip = IPAddr(ip) - if self.inIgnoreIPList(ip): + if self.inIgnoreIPList(ip, log_ignore=False): logSys.warning('Requested to manually ban an ignored IP %s. User knows best. Proceeding to ban it.', ip) unixTime = MyTime.time() @@ -490,7 +490,7 @@ class Filter(JailThread): # @param ip IP address object # @return True if IP address is in ignore list - def inIgnoreIPList(self, ip, log_ignore=False): + def inIgnoreIPList(self, ip, log_ignore=True): if not isinstance(ip, IPAddr): ip = IPAddr(ip) @@ -549,7 +549,7 @@ class Filter(JailThread): fail = element[3] logSys.debug("Processing line with time:%s and ip:%s", unixTime, ip) - if self.inIgnoreIPList(ip, log_ignore=True): + if self.inIgnoreIPList(ip): continue logSys.info( "[%s] Found %s - %s", self.jailName, ip, datetime.datetime.fromtimestamp(unixTime).strftime("%Y-%m-%d %H:%M:%S") diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index bf1bf84d..2c1e42be 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -336,20 +336,20 @@ class IgnoreIP(LogCaptureTestCase): ipList = ("127.0.0.1",) # test ignoreSelf is false: for ip in ipList: - self.assertFalse(self.filter.inIgnoreIPList(ip, log_ignore=True)) + self.assertFalse(self.filter.inIgnoreIPList(ip)) self.assertNotLogged("[%s] Ignore %s by %s" % (self.jail.name, ip, "ignoreself rule")) # test ignoreSelf with true: self.filter.ignoreSelf = True self.pruneLog() for ip in ipList: - self.assertTrue(self.filter.inIgnoreIPList(ip, log_ignore=True)) + self.assertTrue(self.filter.inIgnoreIPList(ip)) self.assertLogged("[%s] Ignore %s by %s" % (self.jail.name, ip, "ignoreself rule")) def testIgnoreIPOK(self): ipList = "127.0.0.1", "192.168.0.1", "255.255.255.255", "99.99.99.99" for ip in ipList: self.filter.addIgnoreIP(ip) - self.assertTrue(self.filter.inIgnoreIPList(ip, log_ignore=True)) + self.assertTrue(self.filter.inIgnoreIPList(ip)) self.assertLogged("[%s] Ignore %s by %s" % (self.jail.name, ip, "ip")) def testIgnoreIPNOK(self):