From e065941ac52c2566595d9dfa29f07e5fa419cf1c Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 22 Dec 2015 21:35:37 +0100 Subject: [PATCH] use "maxEntries" (currently 50 as default) as range for max number of the last matches/failures, fail2ban will hold per IP in the list of failures in failmanager resp. in the database; prevents out of memory situation if many IP's makes extremely many failures (or very large files since last fail2ban run); closes gh-1277 todo: parameter `maxentries` should be configurable (jail.conf resp. fail2ban.conf); todo: adjust ban-time-incr branch by merge (table "bips"). --- fail2ban/server/database.py | 12 +++++- fail2ban/server/failmanager.py | 16 +++++-- fail2ban/tests/databasetestcase.py | 31 ++++++++++++++ fail2ban/tests/failmanagertestcase.py | 62 ++++++++++++++++++++++++--- 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 3b419ed3..b7fd4d47 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -163,6 +163,7 @@ class Fail2BanDb(object): def __init__(self, filename, purgeAge=24*60*60): + self.maxEntries = 50 try: self._lock = RLock() self._db = sqlite3.connect( @@ -454,7 +455,7 @@ class Fail2BanDb(object): if ip is not None: query += " AND ip=?" queryArgs.append(ip) - query += " ORDER BY ip, timeofban" + query += " ORDER BY ip, timeofban desc" return cur.execute(query, queryArgs) @@ -535,7 +536,14 @@ class Fail2BanDb(object): matches = [] failures = 0 tickdata = {} - matches.extend(data.get('matches', ())) + m = data.get('matches', []) + # pre-insert "maxadd" enries (because tickets are ordered desc by time) + maxadd = self.maxEntries - len(matches) + if maxadd > 0: + if len(m) <= maxadd: + matches = m + matches + else: + matches = m[-maxadd:] + matches failures += data.get('failures', 1) tickdata.update(data.get('data', {})) prev_timeofban = timeofban diff --git a/fail2ban/server/failmanager.py b/fail2ban/server/failmanager.py index 45f3a393..ae97b36a 100644 --- a/fail2ban/server/failmanager.py +++ b/fail2ban/server/failmanager.py @@ -43,6 +43,7 @@ class FailManager: self.__maxRetry = 3 self.__maxTime = 600 self.__failTotal = 0 + self.maxEntries = 50 self.__bgSvc = BgService() def setFailTotal(self, value): @@ -71,17 +72,26 @@ class FailManager: ip = ticket.getIP() try: fData = self.__failList[ip] - # if the same object: + # if the same object - the same matches but +1 attempt: if fData is ticket: matches = None + attempt = 1 else: + # will be incremented / extended (be sure we have at least +1 attempt): matches = ticket.getMatches() + attempt = ticket.getAttempt() + if attempt <= 0: + attempt += 1 unixTime = ticket.getTime() + fData.setLastTime(unixTime) if fData.getLastReset() < unixTime - self.__maxTime: fData.setLastReset(unixTime) fData.setRetry(0) - fData.inc(matches, 1, count) - fData.setLastTime(unixTime) + fData.inc(matches, attempt, count) + # truncate to maxEntries: + matches = fData.getMatches() + if len(matches) > self.maxEntries: + fData.setMatches(matches[-self.maxEntries:]) except KeyError: # if already FailTicket - add it direct, otherwise create (using copy all ticket data): if isinstance(ticket, FailTicket): diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 6b85af56..5d83710f 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -251,6 +251,37 @@ class DatabaseTest(LogCaptureTestCase): # be returned self.assertEqual(len(self.db.getBans(jail=self.jail,bantime=-1)), 2) + def testGetBansMerged_MaxEntries(self): + if Fail2BanDb is None: # pragma: no cover + return + self.testAddJail() + maxEntries = 2 + failures = ["abc\n", "123\n", "ABC\n", "1234\n"] + # add failures sequential: + i = 80 + for f in failures: + i -= 10 + ticket = FailTicket("127.0.0.1", MyTime.time() - i, [f]) + ticket.setAttempt(1) + self.db.addBan(self.jail, ticket) + # should retrieve 2 matches only, but count of all attempts: + self.db.maxEntries = maxEntries; + ticket = self.db.getBansMerged("127.0.0.1") + self.assertEqual(ticket.getIP(), "127.0.0.1") + self.assertEqual(ticket.getAttempt(), len(failures)) + self.assertEqual(len(ticket.getMatches()), maxEntries) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxEntries:]) + # add more failures at once: + ticket = FailTicket("127.0.0.1", MyTime.time() - 10, failures) + ticket.setAttempt(len(failures)) + self.db.addBan(self.jail, ticket) + # should retrieve 2 matches only, but count of all attempts: + self.db.maxEntries = maxEntries; + ticket = self.db.getBansMerged("127.0.0.1") + self.assertEqual(ticket.getAttempt(), 2 * len(failures)) + self.assertEqual(len(ticket.getMatches()), maxEntries) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxEntries:]) + def testGetBansMerged(self): if Fail2BanDb is None: # pragma: no cover return diff --git a/fail2ban/tests/failmanagertestcase.py b/fail2ban/tests/failmanagertestcase.py index 78f7b509..3cac0540 100644 --- a/fail2ban/tests/failmanagertestcase.py +++ b/fail2ban/tests/failmanagertestcase.py @@ -35,6 +35,13 @@ class AddFailure(unittest.TestCase): def setUp(self): """Call before every test case.""" + self.__items = None + self.__failManager = FailManager() + + def tearDown(self): + """Call after every test case.""" + + def _addDefItems(self): self.__items = [[u'193.168.0.128', 1167605999.0], [u'193.168.0.128', 1167605999.0], [u'193.168.0.128', 1167605999.0], @@ -48,44 +55,87 @@ class AddFailure(unittest.TestCase): ['100.100.10.10', 1000001000.0], ['100.100.10.10', 1000001500.0], ['100.100.10.10', 1000002000.0]] - - self.__failManager = FailManager() for i in self.__items: self.__failManager.addFailure(FailTicket(i[0], i[1])) - def tearDown(self): - """Call after every test case.""" - def testFailManagerAdd(self): + self._addDefItems() self.assertEqual(self.__failManager.size(), 3) self.assertEqual(self.__failManager.getFailTotal(), 13) self.__failManager.setFailTotal(0) self.assertEqual(self.__failManager.getFailTotal(), 0) self.__failManager.setFailTotal(13) + def testFailManagerAdd_MaxEntries(self): + maxEntries = 2 + self.__failManager.maxEntries = maxEntries + failures = ["abc\n", "123\n", "ABC\n", "1234\n"] + # add failures sequential: + i = 80 + for f in failures: + i -= 10 + ticket = FailTicket("127.0.0.1", 1000002000 - i, [f]) + ticket.setAttempt(1) + self.__failManager.addFailure(ticket) + # + manFailList = self.__failManager._FailManager__failList + self.assertEqual(len(manFailList), 1) + ticket = manFailList["127.0.0.1"] + # should retrieve 2 matches only, but count of all attempts (4): + self.assertEqual(ticket.getAttempt(), len(failures)) + self.assertEqual(len(ticket.getMatches()), maxEntries) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxEntries:]) + # add more failures at once: + ticket = FailTicket("127.0.0.1", 1000002000 - 10, failures) + ticket.setAttempt(len(failures)) + self.__failManager.addFailure(ticket) + # + manFailList = self.__failManager._FailManager__failList + self.assertEqual(len(manFailList), 1) + ticket = manFailList["127.0.0.1"] + # should retrieve 2 matches only, but count of all attempts (8): + self.assertEqual(ticket.getAttempt(), 2 * len(failures)) + self.assertEqual(len(ticket.getMatches()), maxEntries) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxEntries:]) + # add self ticket again: + self.__failManager.addFailure(ticket) + # + manFailList = self.__failManager._FailManager__failList + self.assertEqual(len(manFailList), 1) + ticket = manFailList["127.0.0.1"] + # same matches, but +1 attempt (9) + self.assertEqual(ticket.getAttempt(), 2 * len(failures) + 1) + self.assertEqual(len(ticket.getMatches()), maxEntries) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxEntries:]) + def testFailManagerMaxTime(self): + self._addDefItems() self.assertEqual(self.__failManager.getMaxTime(), 600) self.__failManager.setMaxTime(13) self.assertEqual(self.__failManager.getMaxTime(), 13) self.__failManager.setMaxTime(600) def testDel(self): + self._addDefItems() self.__failManager.delFailure('193.168.0.128') self.__failManager.delFailure('111.111.1.111') self.assertEqual(self.__failManager.size(), 2) def testCleanupOK(self): + self._addDefItems() timestamp = 1167606999.0 self.__failManager.cleanup(timestamp) self.assertEqual(self.__failManager.size(), 0) def testCleanupNOK(self): + self._addDefItems() timestamp = 1167605990.0 self.__failManager.cleanup(timestamp) self.assertEqual(self.__failManager.size(), 2) def testbanOK(self): + self._addDefItems() self.__failManager.setMaxRetry(5) #ticket = FailTicket('193.168.0.128', None) ticket = self.__failManager.toBan() @@ -112,10 +162,12 @@ class AddFailure(unittest.TestCase): 'FailTicket: ip=193.168.0.128 time=1000002000.0 #attempts=5 matches=[]') def testbanNOK(self): + self._addDefItems() self.__failManager.setMaxRetry(10) self.assertRaises(FailManagerEmpty, self.__failManager.toBan) def testWindow(self): + self._addDefItems() ticket = self.__failManager.toBan() self.assertNotEqual(ticket.getIP(), "100.100.10.10") ticket = self.__failManager.toBan()