From 697da99f8f579a565de0947f2c9b402d5b5a47a6 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 2 Dec 2014 00:56:20 +0100 Subject: [PATCH] code review and few new test cases --- fail2ban/server/actions.py | 14 ++++---------- fail2ban/server/datetemplate.py | 8 ++++---- fail2ban/server/jail.py | 1 - fail2ban/server/mytime.py | 8 ++++---- fail2ban/server/observer.py | 3 --- fail2ban/tests/misctestcase.py | 18 ++++++++++++++++++ fail2ban/tests/observertestcase.py | 19 +++++++++++-------- fail2ban/tests/utils.py | 1 + 8 files changed, 42 insertions(+), 30 deletions(-) diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index f4a833ca..6daef31c 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -25,7 +25,7 @@ __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" import time, logging -import os, datetime +import os import sys if sys.version_info >= (3, 3): import importlib.machinery @@ -322,21 +322,16 @@ class Actions(JailThread, Mapping): if btime != -1: bendtime = aInfo["time"] + btime - logtime = (datetime.timedelta(seconds=int(btime)), - datetime.datetime.fromtimestamp(bendtime).strftime("%Y-%m-%d %H:%M:%S")) # check ban is not too old : if bendtime < MyTime.time(): - logSys.info('[%s] Ignore %s, expired bantime - %s', self._jail.name, ip, logtime[1]) + logSys.info('[%s] Ignore %s, expired bantime', self._jail.name, ip) return False - else: - logtime = ('permanent', 'infinite') if self.__banManager.addBanTicket(bTicket): # report ticket to observer, to check time should be increased and hereafter observer writes ban to database (asynchronous) if Observers.Main is not None and not bTicket.getRestored(): Observers.Main.add('banFound', bTicket, self._jail, btime) - logSys.notice("[%s] %sBan %s (%s # %s -> %s)", self._jail.name, ('' if not bTicket.getRestored() else 'Restore '), - aInfo["ip"], (bTicket.getBanCount() if bTicket.getRestored() else '_'), *logtime) + logSys.notice("[%s] %sBan %s", self._jail.name, ('' if not bTicket.getRestored() else 'Restore '), aInfo["ip"]) # do actions : for name, action in self._actions.iteritems(): try: @@ -349,8 +344,7 @@ class Actions(JailThread, Mapping): exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) return True else: - logSys.notice("[%s] %s already banned (%d # %s -> %s)" % ((self._jail.name, - aInfo["ip"], bTicket.getBanCount()) + logtime)) + logSys.notice("[%s] %s already banned" % (self._jail.name, aInfo["ip"])) return False def __checkUnBan(self): diff --git a/fail2ban/server/datetemplate.py b/fail2ban/server/datetemplate.py index 013a8105..c0346f9f 100644 --- a/fail2ban/server/datetemplate.py +++ b/fail2ban/server/datetemplate.py @@ -98,7 +98,7 @@ class DateTemplate(object): return dateMatch @abstractmethod - def getDate(self, line, dateMatch = None): + def getDate(self, line, dateMatch=None): """Abstract method, which should return the date for a log line This should return the date for a log line, typically taking the @@ -134,7 +134,7 @@ class DateEpoch(DateTemplate): DateTemplate.__init__(self) self.regex = "(?:^|(?P(?<=^\[))|(?P(?<=audit\()))\d{10}(?:\.\d{3,6})?(?(selinux)(?=:\d+\))(?(square)(?=\])))" - def getDate(self, line, dateMatch = None): + def getDate(self, line, dateMatch=None): """Method to return the date for a log line. Parameters @@ -212,7 +212,7 @@ class DatePatternRegex(DateTemplate): def name(self, value): raise NotImplementedError("Name derived from pattern") - def getDate(self, line, dateMatch = None): + def getDate(self, line, dateMatch=None): """Method to return the date for a log line. This uses a custom version of strptime, using the named groups @@ -253,7 +253,7 @@ class DateTai64n(DateTemplate): # yoh: we should not add an additional front anchor self.setRegex("@[0-9a-f]{24}", wordBegin=False) - def getDate(self, line, dateMatch = None): + def getDate(self, line, dateMatch=None): """Method to return the date for a log line. Parameters diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index 0de5f783..a1a9acf3 100644 --- a/fail2ban/server/jail.py +++ b/fail2ban/server/jail.py @@ -275,7 +275,6 @@ class Jail: self.putFailTicket(ticket) except Exception as e: # pragma: no cover logSys.error('%s', e, exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) - #logSys.error('%s', e, exc_info=True) def start(self): """Start the jail, by starting filter and actions threads. diff --git a/fail2ban/server/mytime.py b/fail2ban/server/mytime.py index f675bcf4..43ea4a29 100644 --- a/fail2ban/server/mytime.py +++ b/fail2ban/server/mytime.py @@ -108,9 +108,9 @@ class MyTime: 1year-6mo = 15778800 6 months = 15778800 warn: month is not 30 days, it is a year in seconds / 12, the leap years will be respected also: - >>>> float(Test.str2seconds("1month")) / 60 / 60 / 24 + >>>> float(str2seconds("1month")) / 60 / 60 / 24 30.4375 - >>>> float(Test.str2seconds("1year")) / 60 / 60 / 24 + >>>> float(str2seconds("1year")) / 60 / 60 / 24 365.25 @returns number (calculated seconds from expression "val") @@ -121,9 +121,9 @@ class MyTime: val = re.sub(r"(?i)(?<=[a-z])(\d)", r" \1", val) # replace abbreviation with expression: for rexp, rpl in ( - (r"days?|da|dd?", 24*60*60), (r"week?|wee?|ww?", 7*24*60*60), (r"months?|mon?", (365*3+366)*24*60*60/4/12), + (r"days?|da|dd?", 24*60*60), (r"weeks?|wee?|ww?", 7*24*60*60), (r"months?|mon?", (365*3+366)*24*60*60/4/12), (r"years?|yea?|yy?", (365*3+366)*24*60*60/4), - (r"seconds?|sec?|ss?", 1), (r"minutes?|min?|mm?", 60), (r"hours?|ho|hh?", 60*60), + (r"seconds?|sec?|ss?", 1), (r"minutes?|min?|mm?", 60), (r"hours?|hou?|hh?", 60*60), ): val = re.sub(r"(?i)(?<=[\d\s])(%s)\b" % rexp, "*"+str(rpl), val) val = re.sub(r"(\d)\s+(\d)", r"\1+\2", val); diff --git a/fail2ban/server/observer.py b/fail2ban/server/observer.py index 0b97e732..bc8dbca8 100644 --- a/fail2ban/server/observer.py +++ b/fail2ban/server/observer.py @@ -389,7 +389,6 @@ class ObserverThread(JailThread): except Exception as e: logSys.error('%s', e, exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) - #logSys.error('%s', e, exc_info=True) class BanTimeIncr: @@ -438,7 +437,6 @@ class ObserverThread(JailThread): break except Exception as e: logSys.error('%s', e, exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) - #logSys.error('%s', e, exc_info=True) return banTime def banFound(self, ticket, jail, btime): @@ -480,7 +478,6 @@ class ObserverThread(JailThread): jail.database.addBan(jail, ticket) except Exception as e: logSys.error('%s', e, exc_info=logSys.getEffectiveLevel()<=logging.DEBUG) - #logSys.error('%s', e, exc_info=True) # Global observer initial created in server (could be later rewriten via singleton) class _Observers: diff --git a/fail2ban/tests/misctestcase.py b/fail2ban/tests/misctestcase.py index 64b6b65e..2a57b071 100644 --- a/fail2ban/tests/misctestcase.py +++ b/fail2ban/tests/misctestcase.py @@ -34,6 +34,7 @@ from StringIO import StringIO from ..helpers import formatExceptionInfo, mbasename, TraceBack, FormatterWithTraceBack, getLogger from ..server.datetemplate import DatePatternRegex +from ..server.mytime import MyTime class HelpersTest(unittest.TestCase): @@ -211,3 +212,20 @@ class CustomDateFormatsTest(unittest.TestCase): self.assertEqual( date, datetime.datetime(2007, 1, 25, 16, 0)) + +class MyTimeTest(unittest.TestCase): + + def testStr2Seconds(self): + # several formats / write styles: + str2sec = MyTime.str2seconds + self.assertEqual(str2sec('1y6mo30w15d12h35m25s'), 66821725) + self.assertEqual(str2sec('2yy 3mo 4ww 10dd 5hh 30mm 20ss'), 74307620) + self.assertEqual(str2sec('2 years 3 months 4 weeks 10 days 5 hours 30 minutes 20 seconds'), 74307620) + self.assertEqual(str2sec('1 year + 1 month - 1 week + 1 day'), 33669000) + self.assertEqual(str2sec('2 * 0.5 yea + 1*1 mon - 3*1/3 wee + 2/2 day - (2*12 hou 3*20 min 80 sec) '), 33578920.0) + self.assertEqual(str2sec('2*.5y+1*1mo-3*1/3w+2/2d-(2*12h3*20m80s) '), 33578920.0) + self.assertEqual(str2sec('1ye -2mo -3we -4da -5ho -6mi -7se'), 24119633) + # month and year in days : + self.assertEqual(float(str2sec("1 month")) / 60 / 60 / 24, 30.4375) + self.assertEqual(float(str2sec("1 year")) / 60 / 60 / 24, 365.25) + diff --git a/fail2ban/tests/observertestcase.py b/fail2ban/tests/observertestcase.py index bc484f07..a858d641 100644 --- a/fail2ban/tests/observertestcase.py +++ b/fail2ban/tests/observertestcase.py @@ -83,7 +83,7 @@ class BanTimeIncr(LogCaptureTestCase): arr = arr[0:multcnt-1] + ([arr[multcnt-2]] * (11-multcnt)) self.assertEqual( [a.calcBanTime(600, i) for i in xrange(1, 11)], - arr + arr ) a.setBanTimeExtra('maxtime', '1d') # change factor : @@ -377,20 +377,20 @@ class BanTimeIncrDB(unittest.TestCase): self.assertEqual(len(restored_tickets), 2) self.assertEqual(restored_tickets[0].getIP(), ip) - # purge remove 1st ip + # purge remove 1st ip self.db._purgeAge = -48*60*60 self.db.purge() restored_tickets = self.db.getCurrentBans(fromtime=stime) self.assertEqual(len(restored_tickets), 1) self.assertEqual(restored_tickets[0].getIP(), ip+'1') - # this should purge all bans, bips and logs - nothing should be found now + # this should purge all bans, bips and logs - nothing should be found now self.db._purgeAge = -240*60*60 self.db.purge() restored_tickets = self.db.getCurrentBans(fromtime=stime) self.assertEqual(restored_tickets, []) - # two separate jails : + # two separate jails : jail1 = DummyJail(backend='polling') jail1.database = self.db self.db.addJail(jail1) @@ -418,14 +418,14 @@ class BanTimeIncrDB(unittest.TestCase): str(restored_tickets[0]), 'FailTicket: ip=%s time=%s bantime=%s bancount=2 #attempts=0 matches=[]' % (ip, stime-6000, 12000) ) - # get last ban values for this ip separately for each jail: + # get last ban values for this ip separately for each jail: for row in self.db.getBan(ip, jail1): self.assertEqual(row, (1, stime, 6000)) break for row in self.db.getBan(ip, jail2): self.assertEqual(row, (2, stime-6000, 12000)) break - # get max values for this ip (over all jails): + # get max values for this ip (over all jails): for row in self.db.getBan(ip, overalljails=True): self.assertEqual(row, (3, stime, 18000)) break @@ -477,7 +477,7 @@ class BanTimeIncrDB(unittest.TestCase): obs.add('failureFound', failManager, jail, ticket) obs.wait_empty(5) self.assertEqual(ticket.getBanCount(), 0) - # check still not ban : + # check still not ban : self.assertTrue(not jail.getFailTicket()) # add manually 4th times banned (added to bips - make ip bad): ticket.setBanCount(4) @@ -493,11 +493,14 @@ class BanTimeIncrDB(unittest.TestCase): obs.add('failureFound', failManager, self.jail, ticket) obs.wait_empty(5) # wait until ticket transfered from failmanager into jail: + to = int(MyTime.time())+30 while True: ticket2 = jail.getFailTicket() if ticket2: break time.sleep(0.1) + if MyTime.time() > to: # pragma: no cover + raise RuntimeError('unexpected timeout: wait 30 seconds instead of few ms.') # check ticket and failure count: self.assertFalse(not ticket2) self.assertEqual(ticket2.getAttempt(), failManager.getMaxRetry()) @@ -509,7 +512,7 @@ class BanTimeIncrDB(unittest.TestCase): self.assertEqual(ticket2.getBanTime(), 160) self.assertEqual(ticket2.getBanCount(), 5) - # check prolonged in database also : + # check prolonged in database also : restored_tickets = self.db.getCurrentBans(jail=jail, fromtime=stime) self.assertEqual(len(restored_tickets), 1) self.assertEqual(restored_tickets[0].getBanTime(), 160) diff --git a/fail2ban/tests/utils.py b/fail2ban/tests/utils.py index 5da9c694..95bf312e 100644 --- a/fail2ban/tests/utils.py +++ b/fail2ban/tests/utils.py @@ -121,6 +121,7 @@ def gatherTests(regexps=None, no_network=False): tests.addTest(unittest.makeSuite(misctestcase.SetupTest)) tests.addTest(unittest.makeSuite(misctestcase.TestsUtilsTest)) tests.addTest(unittest.makeSuite(misctestcase.CustomDateFormatsTest)) + tests.addTest(unittest.makeSuite(misctestcase.MyTimeTest)) # Database tests.addTest(unittest.makeSuite(databasetestcase.DatabaseTest)) # Observer