From d3442742716e8253ca592831ae61b51305571fb1 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 31 May 2016 21:55:26 +0200 Subject: [PATCH 1/5] separated host match group and tags for ip4, ip6, dns, fid (failure-id): - better recognition for usage of textToIp, expected or raw value should be used; - separated failure id vs. host (if found use `fid` instead of `host` resp. `ip`); - additional optional groups may be used in tags replacement by executing actions; --- fail2ban/server/failmanager.py | 26 ++++----- fail2ban/server/failregex.py | 95 +++++++++++++++++++++++------- fail2ban/server/filter.py | 50 +++++++++++++--- fail2ban/server/ipdns.py | 96 ++++++++++++++++--------------- fail2ban/server/ticket.py | 10 +++- fail2ban/tests/samplestestcase.py | 2 +- fail2ban/tests/servertestcase.py | 50 ++++++++++++++++ 7 files changed, 237 insertions(+), 92 deletions(-) diff --git a/fail2ban/server/failmanager.py b/fail2ban/server/failmanager.py index b342b280..ee4b049d 100644 --- a/fail2ban/server/failmanager.py +++ b/fail2ban/server/failmanager.py @@ -78,9 +78,9 @@ class FailManager: def addFailure(self, ticket, count=1): attempts = 1 with self.__lock: - ip = ticket.getIP() + fid = ticket.getID() try: - fData = self.__failList[ip] + fData = self.__failList[fid] # if the same object - the same matches but +1 attempt: if fData is ticket: matches = None @@ -109,7 +109,7 @@ class FailManager: fData = FailTicket(ticket=ticket) if count > ticket.getAttempt(): fData.setRetry(count) - self.__failList[ip] = fData + self.__failList[fid] = fData attempts = fData.getRetry() self.__failTotal += 1 @@ -132,7 +132,7 @@ class FailManager: def cleanup(self, time): with self.__lock: - todelete = [ip for ip,item in self.__failList.iteritems() \ + todelete = [fid for fid,item in self.__failList.iteritems() \ if item.getLastTime() + self.__maxTime <= time] if len(todelete) == len(self.__failList): # remove all: @@ -142,27 +142,27 @@ class FailManager: return if len(todelete) / 2.0 <= len(self.__failList) / 3.0: # few as 2/3 should be removed - remove particular items: - for ip in todelete: - del self.__failList[ip] + for fid in todelete: + del self.__failList[fid] else: # create new dictionary without items to be deleted: - self.__failList = dict((ip,item) for ip,item in self.__failList.iteritems() \ + self.__failList = dict((fid,item) for fid,item in self.__failList.iteritems() \ if item.getLastTime() + self.__maxTime > time) self.__bgSvc.service() - def delFailure(self, ip): + def delFailure(self, fid): with self.__lock: try: - del self.__failList[ip] + del self.__failList[fid] except KeyError: pass - def toBan(self, ip=None): + def toBan(self, fid=None): with self.__lock: - for ip in ([ip] if ip != None and ip in self.__failList else self.__failList): - data = self.__failList[ip] + for fid in ([fid] if fid != None and fid in self.__failList else self.__failList): + data = self.__failList[fid] if data.getRetry() >= self.__maxRetry: - del self.__failList[ip] + del self.__failList[fid] return data self.__bgSvc.service() raise FailManagerEmpty diff --git a/fail2ban/server/failregex.py b/fail2ban/server/failregex.py index 6076acc3..d343dbe0 100644 --- a/fail2ban/server/failregex.py +++ b/fail2ban/server/failregex.py @@ -62,18 +62,39 @@ class Regex: def __str__(self): return "%s(%r)" % (self.__class__.__name__, self._regex) + ## + # Replaces "", "", "", "" with default regular expression for host + # + # (see gh-1374 for the discussion about other candidates) + # @return the replaced regular expression as string + @staticmethod def _resolveHostTag(regex): - # Replace "" with default regular expression for host: - # Other candidates (see gh-1374 for the discussion about): - # differentiate: r"""(?:(?:::f{4,6}:)?(?P(?:\d{1,3}\.){3}\d{1,3})|\[?(?P(?:[0-9a-fA-F]{1,4}::?|::){1,7}(?:[0-9a-fA-F]{1,4}|(?<=:):))\]?|(?P[\w\-.^_]*\w))""" - # expected many changes in filter, failregex, etc... - # simple: r"""(?:::f{4,6}:)?(?P[\w\-.^_:]*\w)""" - # not good enough, if not precise expressions around , because for example will match '1.2.3.4:23930' as ip-address; - # Todo: move this functionality to filter reader, as default replacement, - # make it configurable (via jail/filter configs) - return regex.replace("", - r"""(?:::f{4,6}:)?(?P(?:\d{1,3}\.){3}\d{1,3}|\[?(?:[0-9a-fA-F]{1,4}::?|::){1,7}(?:[0-9a-fA-F]{1,4}\]?|(?<=:):)|[\w\-.^_]*\w)""") + # 3 groups instead of - separated ipv4, ipv6 and host + regex = regex.replace("", + r"""(?:(?:::f{4,6}:)?(?P(?:\d{1,3}\.){3}\d{1,3})|\[?(?P(?:[0-9a-fA-F]{1,4}::?|::){1,7}(?:[0-9a-fA-F]{1,4}|(?<=:):))\]?|(?P[\w\-.^_]*\w))""") + # separated ipv4: + r = r"""(?:::f{4,6}:)?(?P(?:\d{1,3}\.){3}\d{1,3})""" + regex = regex.replace("", r); # self closed + regex = regex.replace("", r); # closed + # separated ipv6: + r = r"""(?P(?:[0-9a-fA-F]{1,4}::?|::){1,7}(?:[0-9a-fA-F]{1,4}?|(?<=:):))""" + regex = regex.replace("", r); # self closed + regex = regex.replace("", r); # closed + # separated dns: + r = r"""(?P[\w\-.^_]*\w)""" + regex = regex.replace("", r); # self closed + regex = regex.replace("", r); # closed + # default failure-id as no space tag: + regex = regex.replace("", r"""(?P\S+)"""); # closed + # default failure port, like 80 or http : + regex = regex.replace("", r"""(?P\w+)"""); # closed + # default failure groups (begin / end tag) for customizable expressions: + for o,r in (('IP4', 'ip4'), ('IP6', 'ip6'), ('DNS', 'dns'), ('ID', 'fid'), ('PORT', 'fport')): + regex = regex.replace("" % o, "(?P<%s>" % r); # open tag + regex = regex.replace("" % o, ")"); # close tag + + return regex ## # Gets the regular expression. @@ -207,6 +228,13 @@ class RegexException(Exception): pass +## +# Groups used as failure identifier. +# +# The order of this tuple is important while searching for failure-id +# +FAILURE_ID_GROPS = ("fid", "ip4", "ip6", "dns") + ## # Regular expression class. # @@ -220,25 +248,48 @@ class FailRegex(Regex): # Creates a new object. This method can throw RegexException in order to # avoid construction of invalid object. # @param value the regular expression - + def __init__(self, regex): # Initializes the parent. Regex.__init__(self, regex) - # Check for group "host" - if "host" not in self._regexObj.groupindex: - raise RegexException("No 'host' group in '%s'" % self._regex) + # Check for group "dns", "ip4", "ip6", "fid" + if not [grp for grp in FAILURE_ID_GROPS if grp in self._regexObj.groupindex]: + raise RegexException("No failure-id group in '%s'" % self._regex) ## - # Returns the matched host. + # Returns all matched groups. # - # This corresponds to the pattern matched by the named group "host". - # @return the matched host + + def getGroups(self): + return self._matchCache.groupdict() + + ## + # Returns the matched failure id. + # + # This corresponds to the pattern matched by the named group from given groups. + # @return the matched failure-id - def getHost(self): - host = self._matchCache.group("host") - if host is None: + def getFailID(self, groups=FAILURE_ID_GROPS): + fid = None + for grp in groups: + try: + fid = self._matchCache.group(grp) + except IndexError: + continue + if fid is not None: + break + if fid is None: # Gets a few information. s = self._matchCache.string r = self._matchCache.re - raise RegexException("No 'host' found in '%s' using '%s'" % (s, r)) - return str(host) + raise RegexException("No group found in '%s' using '%s'" % (s, r)) + return str(fid) + + ## + # Returns the matched host. + # + # This corresponds to the pattern matched by the named group "ip4", "ip6" or "dns". + # @return the matched host + + def getHost(self): + return self.getFailID(("ip4", "ip6", "dns")) diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py index c1cd2d09..d0d8f74e 100644 --- a/fail2ban/server/filter.py +++ b/fail2ban/server/filter.py @@ -418,6 +418,9 @@ class Filter(JailThread): ip = element[1] unixTime = element[2] lines = element[3] + fail = {} + if len(element) > 4: + fail = element[4] logSys.debug("Processing line with time:%s and ip:%s", unixTime, ip) if unixTime < MyTime.time() - self.getFindTime(): @@ -429,7 +432,7 @@ class Filter(JailThread): logSys.info( "[%s] Found %s - %s", self.jail.name, ip, datetime.datetime.fromtimestamp(unixTime).strftime("%Y-%m-%d %H:%M:%S") ) - tick = FailTicket(ip, unixTime, lines) + tick = FailTicket(ip, unixTime, lines, data=fail) self.failManager.addFailure(tick) ## @@ -457,7 +460,12 @@ class Filter(JailThread): checkAllRegex=False): failList = list() - # Checks if we must ignore this line. + cidr = IPAddr.CIDR_UNSPEC + if self.__useDns == "raw": + returnRawHost = True + cidr = IPAddr.CIDR_RAW + + # Checks if we mut ignore this line. if self.ignoreLine([tupleLine[::2]]) is not None: # The ignoreregex matched. Return. logSys.log(7, "Matched ignoreregex and was \"%s\" ignored", @@ -518,19 +526,45 @@ class Filter(JailThread): % ("\n".join(failRegex.getMatchedLines()), timeText)) else: self.__lineBuffer = failRegex.getUnmatchedTupleLines() + # retrieve failure-id, host, etc from failure match: + raw = returnRawHost try: - host = failRegex.getHost() - if returnRawHost or self.__useDns == "raw": - failList.append([failRegexIndex, IPAddr(host), date, - failRegex.getMatchedLines()]) + fail = failRegex.getGroups() + # failure-id: + fid = fail.get('fid') + # ip-address or host: + host = fail.get('ip4') + if host is not None: + raw = True + else: + host = fail.get('ip6') + if host is not None: + raw = True + else: + host = fail.get('dns') + if host is None: + # if no failure-id also (obscure case, wrong regex), throw error inside getFailID: + if fid is None: + fid = failRegex.getFailID() + host = fid + cidr = IPAddr.CIDR_RAW + # if raw - add single ip or failure-id, + # otherwise expand host to multiple ips using dns (or ignore it if not valid): + if raw: + ip = IPAddr(host, cidr) + # check host equal failure-id, if not - failure with complex id: + if fid is not None and fid != host: + ip = IPAddr(fid, IPAddr.CIDR_RAW) + failList.append([failRegexIndex, ip, date, + failRegex.getMatchedLines(), fail]) if not checkAllRegex: break else: ips = DNSUtils.textToIp(host, self.__useDns) if ips: for ip in ips: - failList.append([failRegexIndex, ip, - date, failRegex.getMatchedLines()]) + failList.append([failRegexIndex, ip, date, + failRegex.getMatchedLines(), fail]) if not checkAllRegex: break except RegexException, e: # pragma: no cover - unsure if reachable diff --git a/fail2ban/server/ipdns.py b/fail2ban/server/ipdns.py index 8109c21a..61c1ba68 100644 --- a/fail2ban/server/ipdns.py +++ b/fail2ban/server/ipdns.py @@ -138,18 +138,21 @@ class IPAddr(object): # todo: make configurable the expired time and max count of cache entries: CACHE_OBJ = Utils.Cache(maxCount=1000, maxTime=5*60) - def __new__(cls, ipstr, cidr=-1): + CIDR_RAW = -2 + CIDR_UNSPEC = -1 + + def __new__(cls, ipstr, cidr=CIDR_UNSPEC): # check already cached as IPAddr args = (ipstr, cidr) ip = IPAddr.CACHE_OBJ.get(args) if ip is not None: return ip # wrap mask to cidr (correct plen): - if cidr == -1: + if cidr == IPAddr.CIDR_UNSPEC: ipstr, cidr = IPAddr.__wrap_ipstr(ipstr) args = (ipstr, cidr) # check cache again: - if cidr != -1: + if cidr != IPAddr.CIDR_UNSPEC: ip = IPAddr.CACHE_OBJ.get(args) if ip is not None: return ip @@ -166,7 +169,7 @@ class IPAddr(object): ipstr = ipstr[1:-1] # test mask: if "/" not in ipstr: - return ipstr, -1 + return ipstr, IPAddr.CIDR_UNSPEC s = ipstr.split('/', 1) # IP address without CIDR mask if len(s) > 2: @@ -176,7 +179,7 @@ class IPAddr(object): s[1] = long(s[1]) return s - def __init(self, ipstr, cidr=-1): + def __init(self, ipstr, cidr=CIDR_UNSPEC): """ initialize IP object by converting IP address string to binary to integer """ @@ -184,49 +187,48 @@ class IPAddr(object): self._addr = 0 self._plen = 0 self._maskplen = None - self._raw = "" + # always save raw value (normally used if really raw or not valid only): + self._raw = ipstr + # if not raw - recognize family, set addr, etc.: + if cidr != IPAddr.CIDR_RAW: + for family in [socket.AF_INET, socket.AF_INET6]: + try: + binary = socket.inet_pton(family, ipstr) + self._family = family + break + except socket.error: + continue - for family in [socket.AF_INET, socket.AF_INET6]: - try: - binary = socket.inet_pton(family, ipstr) - self._family = family - break - except socket.error: - continue - - if self._family == socket.AF_INET: - # convert host to network byte order - self._addr, = struct.unpack("!L", binary) - self._plen = 32 - - # mask out host portion if prefix length is supplied - if cidr is not None and cidr >= 0: - mask = ~(0xFFFFFFFFL >> cidr) - self._addr &= mask - self._plen = cidr - - elif self._family == socket.AF_INET6: - # convert host to network byte order - hi, lo = struct.unpack("!QQ", binary) - self._addr = (hi << 64) | lo - self._plen = 128 - - # mask out host portion if prefix length is supplied - if cidr is not None and cidr >= 0: - mask = ~(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFL >> cidr) - self._addr &= mask - self._plen = cidr - - # if IPv6 address is a IPv4-compatible, make instance a IPv4 - elif self.isInNet(IPAddr.IP6_4COMPAT): - self._addr = lo & 0xFFFFFFFFL - self._family = socket.AF_INET + if self._family == socket.AF_INET: + # convert host to network byte order + self._addr, = struct.unpack("!L", binary) self._plen = 32 + + # mask out host portion if prefix length is supplied + if cidr is not None and cidr >= 0: + mask = ~(0xFFFFFFFFL >> cidr) + self._addr &= mask + self._plen = cidr + + elif self._family == socket.AF_INET6: + # convert host to network byte order + hi, lo = struct.unpack("!QQ", binary) + self._addr = (hi << 64) | lo + self._plen = 128 + + # mask out host portion if prefix length is supplied + if cidr is not None and cidr >= 0: + mask = ~(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFL >> cidr) + self._addr &= mask + self._plen = cidr + + # if IPv6 address is a IPv4-compatible, make instance a IPv4 + elif self.isInNet(IPAddr.IP6_4COMPAT): + self._addr = lo & 0xFFFFFFFFL + self._family = socket.AF_INET + self._plen = 32 else: - # string couldn't be converted neither to a IPv4 nor - # to a IPv6 address - retain raw input for later use - # (e.g. DNS resolution) - self._raw = ipstr + self._family = IPAddr.CIDR_RAW def __repr__(self): return self.ntoa @@ -270,6 +272,8 @@ class IPAddr(object): return self._family != socket.AF_UNSPEC def __eq__(self, other): + if self._family == IPAddr.CIDR_RAW and not isinstance(other, IPAddr): + return self._raw == other if not isinstance(other, IPAddr): if other is None: return False other = IPAddr(other) @@ -285,6 +289,8 @@ class IPAddr(object): return not (self == other) def __lt__(self, other): + if self._family == IPAddr.CIDR_RAW and not isinstance(other, IPAddr): + return self._raw < other if not isinstance(other, IPAddr): if other is None: return False other = IPAddr(other) diff --git a/fail2ban/server/ticket.py b/fail2ban/server/ticket.py index 130de4f2..65ed83c3 100644 --- a/fail2ban/server/ticket.py +++ b/fail2ban/server/ticket.py @@ -36,7 +36,7 @@ logSys = getLogger(__name__) class Ticket: - def __init__(self, ip=None, time=None, matches=None, ticket=None): + def __init__(self, ip=None, time=None, matches=None, data={}, ticket=None): """Ticket constructor @param ip the IP address @@ -50,6 +50,7 @@ class Ticket: self._banTime = None; self._time = time if time is not None else MyTime.time() self._data = {'matches': [], 'failures': 0} + self._data.update(data) if ticket: # ticket available - copy whole information from ticket: self.__dict__.update(i for i in ticket.__dict__.iteritems() if i[0] in self.__dict__) @@ -78,6 +79,9 @@ class Ticket: value = IPAddr(value) self.__ip = value + def getID(self): + return self._data.get('fid', self.__ip) + def getIP(self): return self.__ip @@ -164,12 +168,12 @@ class Ticket: class FailTicket(Ticket): - def __init__(self, ip=None, time=None, matches=None, ticket=None): + def __init__(self, ip=None, time=None, matches=None, data={}, ticket=None): # this class variables: self.__retry = 0 self.__lastReset = None # create/copy using default ticket constructor: - Ticket.__init__(self, ip, time, matches, ticket) + Ticket.__init__(self, ip, time, matches, data, ticket) # init: if ticket is None: self.__lastReset = time if time is not None else self.getTime() diff --git a/fail2ban/tests/samplestestcase.py b/fail2ban/tests/samplestestcase.py index 326d09f7..7af12f13 100644 --- a/fail2ban/tests/samplestestcase.py +++ b/fail2ban/tests/samplestestcase.py @@ -127,7 +127,7 @@ def testSampleRegexsFactory(name, basedir): (map(lambda x: x[0], ret),logFile.filename(), logFile.filelineno())) # Verify timestamp and host as expected - failregex, host, fail2banTime, lines = ret[0] + failregex, host, fail2banTime, lines, fail = ret[0] self.assertEqual(host, faildata.get("host", None)) t = faildata.get("time", None) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 7557a501..9eb88c21 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -933,6 +933,14 @@ class RegexTests(unittest.TestCase): def testHost(self): self.assertRaises(RegexException, FailRegex, '') + self.assertRaises(RegexException, FailRegex, '^test no group$') + self.assertTrue(FailRegex('^test group$')) + self.assertTrue(FailRegex('^test group$')) + self.assertTrue(FailRegex('^test group$')) + self.assertTrue(FailRegex('^test group$')) + self.assertTrue(FailRegex('^test id group: ip:port = (?::)?$')) + self.assertTrue(FailRegex('^test id group: user:\([^\)]+\)$')) + self.assertTrue(FailRegex('^test id group: anything = $')) # Testing obscure case when host group might be missing in the matched pattern, # e.g. if we made it optional. fr = FailRegex('%%?') @@ -940,6 +948,30 @@ class RegexTests(unittest.TestCase): fr.search([('%%',"","")]) self.assertTrue(fr.hasMatched()) self.assertRaises(RegexException, fr.getHost) + # The same as above but using separated IPv4/IPv6 expressions + fr = FailRegex('%%inet(?:=|inet6=)?') + self.assertFalse(fr.hasMatched()) + fr.search([('%%inet=test',"","")]) + self.assertTrue(fr.hasMatched()) + self.assertRaises(RegexException, fr.getHost) + # Success case: using separated IPv4/IPv6 expressions (no HOST) + fr = FailRegex('%%(?:inet(?:=|6=)?|dns=?)') + self.assertFalse(fr.hasMatched()) + fr.search([('%%inet=192.0.2.1',"","")]) + self.assertTrue(fr.hasMatched()) + self.assertEqual(fr.getHost(), '192.0.2.1') + fr.search([('%%inet6=2001:DB8::',"","")]) + self.assertTrue(fr.hasMatched()) + self.assertEqual(fr.getHost(), '2001:DB8::') + fr.search([('%%dns=example.com',"","")]) + self.assertTrue(fr.hasMatched()) + self.assertEqual(fr.getHost(), 'example.com') + # Success case: using user as failure-id + fr = FailRegex('^test id group: user:\([^\)]+\)$') + self.assertFalse(fr.hasMatched()) + fr.search([('test id group: user:(test login name)',"","")]) + self.assertTrue(fr.hasMatched()) + self.assertEqual(fr.getFailID(), 'test login name') class _BadThread(JailThread): @@ -998,6 +1030,24 @@ class ServerConfigReaderTests(LogCaptureTestCase): self.assertTrue(IPAddr('192.0.2.1').isIPv4) self.assertTrue(IPAddr('2001:DB8::').isIPv6) + def test_IPAddr_Raw(self): + # raw string: + r = IPAddr('xxx', IPAddr.CIDR_RAW) + self.assertFalse(r.isIPv4) + self.assertFalse(r.isIPv6) + self.assertTrue(r.isValid) + self.assertEqual(r, 'xxx') + self.assertEqual('xxx', str(r)) + self.assertNotEqual(r, IPAddr('xxx')) + # raw (not IP, for example host:port as string): + r = IPAddr('1:2', IPAddr.CIDR_RAW) + self.assertFalse(r.isIPv4) + self.assertFalse(r.isIPv6) + self.assertTrue(r.isValid) + self.assertEqual(r, '1:2') + self.assertEqual('1:2', str(r)) + self.assertNotEqual(r, IPAddr('1:2')) + def _testExecActions(self, server): jails = server._Server__jails for jail in jails: From e40a8c8ae832504107d2ebdd69903e516c89ca7f Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 10 Jun 2016 11:22:14 +0200 Subject: [PATCH 2/5] small code review --- fail2ban/server/filter.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py index d0d8f74e..318d8e49 100644 --- a/fail2ban/server/filter.py +++ b/fail2ban/server/filter.py @@ -533,21 +533,17 @@ class Filter(JailThread): # failure-id: fid = fail.get('fid') # ip-address or host: - host = fail.get('ip4') + host = fail.get('ip4') or fail.get('ip6') if host is not None: raw = True else: - host = fail.get('ip6') - if host is not None: - raw = True - else: - host = fail.get('dns') - if host is None: - # if no failure-id also (obscure case, wrong regex), throw error inside getFailID: - if fid is None: - fid = failRegex.getFailID() - host = fid - cidr = IPAddr.CIDR_RAW + host = fail.get('dns') + if host is None: + # if no failure-id also (obscure case, wrong regex), throw error inside getFailID: + if fid is None: + fid = failRegex.getFailID() + host = fid + cidr = IPAddr.CIDR_RAW # if raw - add single ip or failure-id, # otherwise expand host to multiple ips using dns (or ignore it if not valid): if raw: From 8893473d82f3732cfec6c7e302e1b94ff6b62b99 Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 10 Jun 2016 11:47:24 +0200 Subject: [PATCH 3/5] pypy fix: KeyError instead of IndexError by missing group --- fail2ban/server/failregex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fail2ban/server/failregex.py b/fail2ban/server/failregex.py index d343dbe0..12b8986a 100644 --- a/fail2ban/server/failregex.py +++ b/fail2ban/server/failregex.py @@ -274,7 +274,7 @@ class FailRegex(Regex): for grp in groups: try: fid = self._matchCache.group(grp) - except IndexError: + except (IndexError, KeyError): continue if fid is not None: break From e39126f6301fdf86348f43283b802dc28ad2a76f Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 10 Jun 2016 13:15:46 +0200 Subject: [PATCH 4/5] badip timeout option introduced, set to 30 seconds in our test cases --- config/action.d/badips.py | 11 +++++++---- fail2ban/tests/action_d/test_badips.py | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/config/action.d/badips.py b/config/action.d/badips.py index 025289ca..4bc879a1 100644 --- a/config/action.d/badips.py +++ b/config/action.d/badips.py @@ -80,14 +80,17 @@ class BadIPsAction(ActionBase): If invalid `category`, `score`, `banaction` or `updateperiod`. """ + TIMEOUT = 10 _badips = "http://www.badips.com" def _Request(self, url, **argv): return Request(url, headers={'User-Agent': self.agent}, **argv) def __init__(self, jail, name, category, score=3, age="24h", key=None, - banaction=None, bancategory=None, bankey=None, updateperiod=900, agent="Fail2Ban"): + banaction=None, bancategory=None, bankey=None, updateperiod=900, agent="Fail2Ban", + timeout=TIMEOUT): super(BadIPsAction, self).__init__(jail, name) + self.timeout = timeout self.agent = agent self.category = category self.score = score @@ -119,7 +122,7 @@ class BadIPsAction(ActionBase): """ try: response = urlopen( - self._Request("/".join([self._badips, "get", "categories"])), None, 3) + self._Request("/".join([self._badips, "get", "categories"])), timeout=self.timeout) except HTTPError as response: messages = json.loads(response.read().decode('utf-8')) self._logSys.error( @@ -173,7 +176,7 @@ class BadIPsAction(ActionBase): urlencode({'age': age})]) if key: url = "&".join([url, urlencode({'key': key})]) - response = urlopen(self._Request(url)) + response = urlopen(self._Request(url), timeout=self.timeout) except HTTPError as response: messages = json.loads(response.read().decode('utf-8')) self._logSys.error( @@ -358,7 +361,7 @@ class BadIPsAction(ActionBase): url = "/".join([self._badips, "add", self.category, aInfo['ip']]) if self.key: url = "?".join([url, urlencode({'key': self.key})]) - response = urlopen(self._Request(url)) + response = urlopen(self._Request(url), timeout=self.timeout) except HTTPError as response: messages = json.loads(response.read().decode('utf-8')) self._logSys.error( diff --git a/fail2ban/tests/action_d/test_badips.py b/fail2ban/tests/action_d/test_badips.py index 74594420..97dabada 100644 --- a/fail2ban/tests/action_d/test_badips.py +++ b/fail2ban/tests/action_d/test_badips.py @@ -39,6 +39,7 @@ if sys.version_info >= (2,7): self.jail.actions.add("badips", pythonModule, initOpts={ 'category': "ssh", 'banaction': "test", + 'timeout': (3 if unittest.F2B.fast else 30), }) self.action = self.jail.actions["badips"] From 2efcf3c17be9414a8fdbb9f748b38a1b260bd7be Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 10 Jun 2016 13:17:18 +0200 Subject: [PATCH 5/5] increase of test coverage (+ test_IPAddr moved to one place), small bugs fixed --- fail2ban/server/ipdns.py | 7 +-- fail2ban/tests/filtertestcase.py | 84 ++++++++++++++++++++++++++++++++ fail2ban/tests/servertestcase.py | 22 --------- 3 files changed, 86 insertions(+), 27 deletions(-) diff --git a/fail2ban/server/ipdns.py b/fail2ban/server/ipdns.py index 61c1ba68..cca63535 100644 --- a/fail2ban/server/ipdns.py +++ b/fail2ban/server/ipdns.py @@ -85,10 +85,7 @@ class DNSUtils: return v # retrieve name try: - if not isinstance(ip, IPAddr): - v = socket.gethostbyaddr(ip)[0] - else: - v = socket.gethostbyaddr(ip.ntoa)[0] + v = socket.gethostbyaddr(ip)[0] except socket.error, e: logSys.debug("Unable to find a name for the IP %s: %s", ip, e) v = None @@ -359,7 +356,7 @@ class IPAddr(object): if not suffix: suffix = "in-addr.arpa." elif self.isIPv6: - exploded_ip = self.hexdump() + exploded_ip = self.hexdump if not suffix: suffix = "ip6.arpa." else: diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index 4f5dbb55..dd3468ed 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -1353,6 +1353,42 @@ class DNSUtilsNetworkTests(unittest.TestCase): """Call before every test case.""" unittest.F2B.SkipIfNoNetwork() + def test_IPAddr(self): + self.assertTrue(IPAddr('192.0.2.1').isIPv4) + self.assertTrue(IPAddr('2001:DB8::').isIPv6) + + def test_IPAddr_Raw(self): + # raw string: + r = IPAddr('xxx', IPAddr.CIDR_RAW) + self.assertFalse(r.isIPv4) + self.assertFalse(r.isIPv6) + self.assertTrue(r.isValid) + self.assertEqual(r, 'xxx') + self.assertEqual('xxx', str(r)) + self.assertNotEqual(r, IPAddr('xxx')) + # raw (not IP, for example host:port as string): + r = IPAddr('1:2', IPAddr.CIDR_RAW) + self.assertFalse(r.isIPv4) + self.assertFalse(r.isIPv6) + self.assertTrue(r.isValid) + self.assertEqual(r, '1:2') + self.assertEqual('1:2', str(r)) + self.assertNotEqual(r, IPAddr('1:2')) + # raw vs ip4 (raw is not an ip): + r = IPAddr('93.184.0.1', IPAddr.CIDR_RAW) + ip4 = IPAddr('93.184.0.1') + self.assertNotEqual(ip4, r) + self.assertNotEqual(r, ip4) + self.assertTrue(r < ip4) + self.assertTrue(r < ip4) + # raw vs ip6 (raw is not an ip): + r = IPAddr('1::2', IPAddr.CIDR_RAW) + ip6 = IPAddr('1::2') + self.assertNotEqual(ip6, r) + self.assertNotEqual(r, ip6) + self.assertTrue(r < ip6) + self.assertTrue(r < ip6) + def testUseDns(self): res = DNSUtils.textToIp('www.example.com', 'no') self.assertEqual(res, []) @@ -1377,14 +1413,25 @@ class DNSUtilsNetworkTests(unittest.TestCase): self.assertEqual(sorted(res), ['93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946']) else: self.assertEqual(res, []) + # pure ips: + for s in ('93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946'): + ips = DNSUtils.textToIp(s, 'yes') + self.assertEqual(ips, [s]) + self.assertTrue(isinstance(ips[0], IPAddr)) def testIpToName(self): unittest.F2B.SkipIfNoNetwork() res = DNSUtils.ipToName('8.8.4.4') self.assertEqual(res, 'google-public-dns-b.google.com') + # same as above, but with IPAddr: + res = DNSUtils.ipToName(IPAddr('8.8.4.4')) + self.assertEqual(res, 'google-public-dns-b.google.com') # invalid ip (TEST-NET-1 according to RFC 5737) res = DNSUtils.ipToName('192.0.2.0') self.assertEqual(res, None) + # invalid ip: + res = DNSUtils.ipToName('192.0.2.888') + self.assertEqual(res, None) def testAddr2bin(self): res = IPAddr('10.0.0.0') @@ -1398,11 +1445,44 @@ class DNSUtilsNetworkTests(unittest.TestCase): res = IPAddr('10.0.0.1', cidr=31L) self.assertEqual(res.addr, 167772160L) + self.assertEqual(IPAddr('10.0.0.0').hexdump, '0a000000') + self.assertEqual(IPAddr('1::2').hexdump, '00010000000000000000000000000002') + self.assertEqual(IPAddr('xxx').hexdump, '') + + self.assertEqual(IPAddr('192.0.2.0').getPTR(), '0.2.0.192.in-addr.arpa.') + self.assertEqual(IPAddr('192.0.2.1').getPTR(), '1.2.0.192.in-addr.arpa.') + self.assertEqual(IPAddr('2606:2800:220:1:248:1893:25c8:1946').getPTR(), + '6.4.9.1.8.c.5.2.3.9.8.1.8.4.2.0.1.0.0.0.0.2.2.0.0.0.8.2.6.0.6.2.ip6.arpa.') + def testIPAddr_Equal6(self): self.assertEqual( IPAddr('2606:2800:220:1:248:1893::'), IPAddr('2606:2800:220:1:248:1893:0:0') ) + # special case IPv6 in brackets: + self.assertEqual( + IPAddr('[2606:2800:220:1:248:1893::]'), + IPAddr('2606:2800:220:1:248:1893:0:0') + ) + + def testIPAddr_InInet(self): + ip4net = IPAddr('93.184.0.1/24') + ip6net = IPAddr('2606:2800:220:1:248:1893:25c8:0/120') + # ip4: + self.assertTrue(IPAddr('93.184.0.1').isInNet(ip4net)) + self.assertTrue(IPAddr('93.184.0.255').isInNet(ip4net)) + self.assertFalse(IPAddr('93.184.1.0').isInNet(ip4net)) + self.assertFalse(IPAddr('93.184.0.1').isInNet(ip6net)) + # ip6: + self.assertTrue(IPAddr('2606:2800:220:1:248:1893:25c8:1').isInNet(ip6net)) + self.assertTrue(IPAddr('2606:2800:220:1:248:1893:25c8:ff').isInNet(ip6net)) + self.assertFalse(IPAddr('2606:2800:220:1:248:1893:25c8:100').isInNet(ip6net)) + self.assertFalse(IPAddr('2606:2800:220:1:248:1893:25c8:100').isInNet(ip4net)) + # raw not in net: + self.assertFalse(IPAddr('93.184.0.1', IPAddr.CIDR_RAW).isInNet(ip4net)) + self.assertFalse(IPAddr('2606:2800:220:1:248:1893:25c8:1', IPAddr.CIDR_RAW).isInNet(ip6net)) + # invalid not in net: + self.assertFalse(IPAddr('xxx').isInNet(ip4net)) def testIPAddr_Compare(self): ip4 = [ @@ -1473,6 +1553,10 @@ class DNSUtilsNetworkTests(unittest.TestCase): self.assertEqual(str(IPAddr('2606:28ff:220:1:248:1893:25c8::/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff')), '2606:28ff:220:1:248:1893:25c8:0') + def testIPAddr_CIDR_Wrong(self): + # too many plen representations: + self.assertRaises(ValueError, IPAddr, '2606:28ff:220:1:248:1893:25c8::/ffff::/::1') + def testIPAddr_CIDR_Repr(self): self.assertEqual(["127.0.0.0/8", "::/32", "2001:db8::/32"], [IPAddr("127.0.0.0", 8), IPAddr("::1", 32), IPAddr("2001:db8::", 32)] diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 9eb88c21..48b0b255 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -1026,28 +1026,6 @@ class ServerConfigReaderTests(LogCaptureTestCase): logSys.debug(l) return True - def test_IPAddr(self): - self.assertTrue(IPAddr('192.0.2.1').isIPv4) - self.assertTrue(IPAddr('2001:DB8::').isIPv6) - - def test_IPAddr_Raw(self): - # raw string: - r = IPAddr('xxx', IPAddr.CIDR_RAW) - self.assertFalse(r.isIPv4) - self.assertFalse(r.isIPv6) - self.assertTrue(r.isValid) - self.assertEqual(r, 'xxx') - self.assertEqual('xxx', str(r)) - self.assertNotEqual(r, IPAddr('xxx')) - # raw (not IP, for example host:port as string): - r = IPAddr('1:2', IPAddr.CIDR_RAW) - self.assertFalse(r.isIPv4) - self.assertFalse(r.isIPv6) - self.assertTrue(r.isValid) - self.assertEqual(r, '1:2') - self.assertEqual('1:2', str(r)) - self.assertNotEqual(r, IPAddr('1:2')) - def _testExecActions(self, server): jails = server._Server__jails for jail in jails: