From 7a463eb3f7280e8db04e0e6dbd308d9c35c34823 Mon Sep 17 00:00:00 2001 From: "Sergey G. Brester" Date: Wed, 3 Apr 2019 23:58:23 +0200 Subject: [PATCH 01/10] closes gh-2395: safe conversion of `SYSLOG_PID` or `_PID` (if journal entry contains a string instead of numeric) --- fail2ban/server/filtersystemd.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fail2ban/server/filtersystemd.py b/fail2ban/server/filtersystemd.py index 73db618e..15c90548 100644 --- a/fail2ban/server/filtersystemd.py +++ b/fail2ban/server/filtersystemd.py @@ -204,7 +204,14 @@ class FilterSystemd(JournalFilter): # pragma: systemd no cover if not v: v = logentry.get('_PID') if v: - logelements[-1] += ("[%i]" % v) + try: # [integer] (if already numeric): + v = "[%i]" % v + except TypeError: + try: # as [integer] (try to convert to int): + v = "[%i]" % int(v, 0) + except (TypeError, ValueError): # fallback - [string] as it is + v = "[%s]" % v + logelements[-1] += v logelements[-1] += ":" if logelements[-1] == "kernel:": if '_SOURCE_MONOTONIC_TIMESTAMP' in logentry: From 5df78ad11f96e0daf11882cafd3b52a9998e23b8 Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 19:37:42 +0200 Subject: [PATCH 02/10] fix corner cases by maxEntries = 0 (no matches should be saved), test cases extended to cover it + code review --- fail2ban/server/database.py | 14 +++++++++++--- fail2ban/server/failmanager.py | 11 +++++++---- fail2ban/server/ticket.py | 8 +++++++- fail2ban/tests/databasetestcase.py | 8 +++++++- fail2ban/tests/failmanagertestcase.py | 8 ++++++++ 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 90a48d8d..7b457fc9 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -541,8 +541,13 @@ class Fail2BanDb(object): #TODO: Implement data parts once arbitrary match keys completed data = ticket.getData() matches = data.get('matches') - if matches and len(matches) > self.maxEntries: - data['matches'] = matches[-self.maxEntries:] + if self.maxEntries: + if matches and len(matches) > self.maxEntries: + data = data.copy() + data['matches'] = matches[-self.maxEntries:] + elif matches: + data = data.copy() + del data['matches'] cur.execute( "INSERT INTO bans(jail, ip, timeofban, data) VALUES(?, ?, ?, ?)", (jail.name, ip, int(round(ticket.getTime())), data)) @@ -702,6 +707,8 @@ class Fail2BanDb(object): queryArgs.append(fromtime - forbantime) if ip is None: query += " GROUP BY ip ORDER BY ip, timeofban DESC" + else: + query += " ORDER BY timeofban DESC LIMIT 1" cur = self._db.cursor() return cur.execute(query, queryArgs) @@ -718,9 +725,10 @@ class Fail2BanDb(object): # logSys.debug('restore ticket %r, %r, %r', banip, timeofban, data) ticket = FailTicket(banip, timeofban, data=data) # logSys.debug('restored ticket: %r', ticket) + if ip is not None: return ticket tickets.append(ticket) - return tickets if ip is None else ticket + return tickets @commitandrollback def purge(self, cur): diff --git a/fail2ban/server/failmanager.py b/fail2ban/server/failmanager.py index 06967bae..bd1f3e73 100644 --- a/fail2ban/server/failmanager.py +++ b/fail2ban/server/failmanager.py @@ -87,7 +87,7 @@ class FailManager: attempt = 1 else: # will be incremented / extended (be sure we have at least +1 attempt): - matches = ticket.getMatches() + matches = ticket.getMatches() if self.maxEntries else None attempt = ticket.getAttempt() if attempt <= 0: attempt += 1 @@ -98,9 +98,12 @@ class FailManager: fData.setRetry(0) fData.inc(matches, attempt, count) # truncate to maxEntries: - matches = fData.getMatches() - if len(matches) > self.maxEntries: - fData.setMatches(matches[-self.maxEntries:]) + if self.maxEntries: + matches = fData.getMatches() + if len(matches) > self.maxEntries: + fData.setMatches(matches[-self.maxEntries:]) + else: + fData.setMatches(None) except KeyError: # if already FailTicket - add it direct, otherwise create (using copy all ticket data): if isinstance(ticket, FailTicket): diff --git a/fail2ban/server/ticket.py b/fail2ban/server/ticket.py index a1d67583..5a750d02 100644 --- a/fail2ban/server/ticket.py +++ b/fail2ban/server/ticket.py @@ -135,7 +135,13 @@ class Ticket(object): return self._data['failures'] def setMatches(self, matches): - self._data['matches'] = matches or [] + if matches: + self._data['matches'] = matches + else: + try: + del self._data['matches'] + except KeyError: + pass def getMatches(self): return [(line if not isinstance(line, (list, tuple)) else "".join(line)) \ diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index cb8beecc..8865cf45 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -361,7 +361,6 @@ class DatabaseTest(LogCaptureTestCase): 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) @@ -372,6 +371,13 @@ class DatabaseTest(LogCaptureTestCase): self.assertEqual(ticket.getAttempt(), len(failures)) self.assertEqual(len(ticket.getMatches()), maxEntries) self.assertEqual(ticket.getMatches(), matches2find[-maxEntries:]) + # should retrieve 0 matches by last ban: + self.db.maxEntries = 0; + self.db.addBan(self.jail, ticket) + ticket = self.db.getCurrentBans(self.jail, "127.0.0.1", fromtime=MyTime.time()-100) + self.assertTrue(ticket is not None) + self.assertEqual(ticket.getAttempt(), len(failures)) + self.assertEqual(len(ticket.getMatches()), 0) def testGetBansMerged(self): self.testAddJail() diff --git a/fail2ban/tests/failmanagertestcase.py b/fail2ban/tests/failmanagertestcase.py index 18f2c545..7e4451eb 100644 --- a/fail2ban/tests/failmanagertestcase.py +++ b/fail2ban/tests/failmanagertestcase.py @@ -110,6 +110,14 @@ class AddFailure(unittest.TestCase): self.assertEqual(ticket.getAttempt(), 2 * len(failures) + 1) self.assertEqual(len(ticket.getMatches()), maxEntries) self.assertEqual(ticket.getMatches(), failures[len(failures) - maxEntries:]) + # no matches by maxEntries == 0 : + self.__failManager.maxEntries = 0 + self.__failManager.addFailure(ticket) + manFailList = self.__failManager._FailManager__failList + ticket = manFailList["127.0.0.1"] + self.assertEqual(len(ticket.getMatches()), 0) + # test set matches None to None: + ticket.setMatches(None) def testFailManagerMaxTime(self): self._addDefItems() From 1083788e705adecca7f25967e4f76aa80def579e Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 19:50:48 +0200 Subject: [PATCH 03/10] small amend (rename maxEntries -> maxMatches for consistency reasons) --- fail2ban/server/database.py | 10 +++++----- fail2ban/server/failmanager.py | 12 ++++++------ fail2ban/tests/databasetestcase.py | 20 ++++++++++---------- fail2ban/tests/failmanagertestcase.py | 22 +++++++++++----------- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 7b457fc9..5eb7d8d6 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -177,7 +177,7 @@ class Fail2BanDb(object): def __init__(self, filename, purgeAge=24*60*60): - self.maxEntries = 50 + self.maxMatches = 10 self._lock = RLock() self._dbFilename = filename self._purgeAge = purgeAge @@ -541,10 +541,10 @@ class Fail2BanDb(object): #TODO: Implement data parts once arbitrary match keys completed data = ticket.getData() matches = data.get('matches') - if self.maxEntries: - if matches and len(matches) > self.maxEntries: + if self.maxMatches: + if matches and len(matches) > self.maxMatches: data = data.copy() - data['matches'] = matches[-self.maxEntries:] + data['matches'] = matches[-self.maxMatches:] elif matches: data = data.copy() del data['matches'] @@ -672,7 +672,7 @@ class Fail2BanDb(object): tickdata = {} m = data.get('matches', []) # pre-insert "maxadd" enries (because tickets are ordered desc by time) - maxadd = self.maxEntries - len(matches) + maxadd = self.maxMatches - len(matches) if maxadd > 0: if len(m) <= maxadd: matches = m + matches diff --git a/fail2ban/server/failmanager.py b/fail2ban/server/failmanager.py index bd1f3e73..80a6414a 100644 --- a/fail2ban/server/failmanager.py +++ b/fail2ban/server/failmanager.py @@ -43,7 +43,7 @@ class FailManager: self.__maxRetry = 3 self.__maxTime = 600 self.__failTotal = 0 - self.maxEntries = 50 + self.maxMatches = 50 self.__bgSvc = BgService() def setFailTotal(self, value): @@ -87,7 +87,7 @@ class FailManager: attempt = 1 else: # will be incremented / extended (be sure we have at least +1 attempt): - matches = ticket.getMatches() if self.maxEntries else None + matches = ticket.getMatches() if self.maxMatches else None attempt = ticket.getAttempt() if attempt <= 0: attempt += 1 @@ -97,11 +97,11 @@ class FailManager: fData.setLastReset(unixTime) fData.setRetry(0) fData.inc(matches, attempt, count) - # truncate to maxEntries: - if self.maxEntries: + # truncate to maxMatches: + if self.maxMatches: matches = fData.getMatches() - if len(matches) > self.maxEntries: - fData.setMatches(matches[-self.maxEntries:]) + if len(matches) > self.maxMatches: + fData.setMatches(matches[-self.maxMatches:]) else: fData.setMatches(None) except KeyError: diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 8865cf45..f03ec18d 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -331,9 +331,9 @@ class DatabaseTest(LogCaptureTestCase): # be returned self.assertEqual(len(self.db.getBans(jail=self.jail,bantime=-1)), 2) - def testGetBansMerged_MaxEntries(self): + def testGetBansMerged_MaxMatches(self): self.testAddJail() - maxEntries = 2 + maxMatches = 2 failures = [ {"matches": ["abc\n"], "user": set(['test'])}, {"matches": ["123\n"], "user": set(['test'])}, @@ -349,12 +349,12 @@ class DatabaseTest(LogCaptureTestCase): ticket.setAttempt(1) self.db.addBan(self.jail, ticket) # should retrieve 2 matches only, but count of all attempts: - self.db.maxEntries = maxEntries; + self.db.maxMatches = maxMatches; 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(), matches2find[-maxEntries:]) + self.assertEqual(len(ticket.getMatches()), maxMatches) + self.assertEqual(ticket.getMatches(), matches2find[-maxMatches:]) # add more failures at once: ticket = FailTicket("127.0.0.1", MyTime.time() - 10, matches2find, data={"user": set(['test', 'root'])}) @@ -363,16 +363,16 @@ class DatabaseTest(LogCaptureTestCase): # should retrieve 2 matches only, but count of all attempts: 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(), matches2find[-maxEntries:]) + self.assertEqual(len(ticket.getMatches()), maxMatches) + self.assertEqual(ticket.getMatches(), matches2find[-maxMatches:]) # also using getCurrentBans: ticket = self.db.getCurrentBans(self.jail, "127.0.0.1", fromtime=MyTime.time()-100) self.assertTrue(ticket is not None) self.assertEqual(ticket.getAttempt(), len(failures)) - self.assertEqual(len(ticket.getMatches()), maxEntries) - self.assertEqual(ticket.getMatches(), matches2find[-maxEntries:]) + self.assertEqual(len(ticket.getMatches()), maxMatches) + self.assertEqual(ticket.getMatches(), matches2find[-maxMatches:]) # should retrieve 0 matches by last ban: - self.db.maxEntries = 0; + self.db.maxMatches = 0; self.db.addBan(self.jail, ticket) ticket = self.db.getCurrentBans(self.jail, "127.0.0.1", fromtime=MyTime.time()-100) self.assertTrue(ticket is not None) diff --git a/fail2ban/tests/failmanagertestcase.py b/fail2ban/tests/failmanagertestcase.py index 7e4451eb..c3592407 100644 --- a/fail2ban/tests/failmanagertestcase.py +++ b/fail2ban/tests/failmanagertestcase.py @@ -69,9 +69,9 @@ class AddFailure(unittest.TestCase): self.assertEqual(self.__failManager.getFailTotal(), 0) self.__failManager.setFailTotal(13) - def testFailManagerAdd_MaxEntries(self): - maxEntries = 2 - self.__failManager.maxEntries = maxEntries + def testFailManagerAdd_MaxMatches(self): + maxMatches = 2 + self.__failManager.maxMatches = maxMatches failures = ["abc\n", "123\n", "ABC\n", "1234\n"] # add failures sequential: i = 80 @@ -86,8 +86,8 @@ class AddFailure(unittest.TestCase): 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:]) + self.assertEqual(len(ticket.getMatches()), maxMatches) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxMatches:]) # add more failures at once: ticket = FailTicket("127.0.0.1", 1000002000 - 10, failures) ticket.setAttempt(len(failures)) @@ -98,8 +98,8 @@ class AddFailure(unittest.TestCase): 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:]) + self.assertEqual(len(ticket.getMatches()), maxMatches) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxMatches:]) # add self ticket again: self.__failManager.addFailure(ticket) # @@ -108,10 +108,10 @@ class AddFailure(unittest.TestCase): 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:]) - # no matches by maxEntries == 0 : - self.__failManager.maxEntries = 0 + self.assertEqual(len(ticket.getMatches()), maxMatches) + self.assertEqual(ticket.getMatches(), failures[len(failures) - maxMatches:]) + # no matches by maxMatches == 0 : + self.__failManager.maxMatches = 0 self.__failManager.addFailure(ticket) manFailList = self.__failManager._FailManager__failList ticket = manFailList["127.0.0.1"] From 0386df0042d68f8b2ffb34643b7aed9342e23762 Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 20:31:39 +0200 Subject: [PATCH 04/10] introduced new options: `dbmaxmatches` (fail2ban.conf) and `maxmatches` (jail.conf); setting `maxmatches` and `dbmaxmatches` to 0 saves memory usage and database size (closes gh-2118). --- config/fail2ban.conf | 6 ++++++ config/jail.conf | 3 +++ fail2ban/client/fail2banreader.py | 3 ++- fail2ban/client/jailreader.py | 1 + fail2ban/server/server.py | 6 ++++++ fail2ban/server/transmitter.py | 22 ++++++++++++++++++++++ fail2ban/tests/clientreadertestcase.py | 18 ++++++++++-------- fail2ban/tests/fail2banclienttestcase.py | 1 + fail2ban/tests/servertestcase.py | 6 ++++++ 9 files changed, 57 insertions(+), 9 deletions(-) diff --git a/config/fail2ban.conf b/config/fail2ban.conf index 08e8fcf3..cf1cf5ae 100644 --- a/config/fail2ban.conf +++ b/config/fail2ban.conf @@ -68,6 +68,12 @@ dbfile = /var/lib/fail2ban/fail2ban.sqlite3 # Values: [ SECONDS ] Default: 86400 (24hours) dbpurgeage = 1d +# Options: dbmaxmatches +# Notes.: Number of matches stored in database per ticket (resolvable via +# tags / in actions) +# Values: [ INT ] Default: 10 +dbmaxmatches = 10 + [Thread] # Options: stacksize diff --git a/config/jail.conf b/config/jail.conf index e61731c6..32b6647e 100644 --- a/config/jail.conf +++ b/config/jail.conf @@ -69,6 +69,9 @@ findtime = 10m # "maxretry" is the number of failures before a host get banned. maxretry = 5 +# "maxmatches" is the number of matches stored in ticket (resolvable via tag in actions). +maxmatches = %(maxretry)s + # "backend" specifies the backend used to get files modification. # Available options are "pyinotify", "gamin", "polling", "systemd" and "auto". # This option can be overridden in each jail as well. diff --git a/fail2ban/client/fail2banreader.py b/fail2ban/client/fail2banreader.py index 5760e0ba..3270b767 100644 --- a/fail2ban/client/fail2banreader.py +++ b/fail2ban/client/fail2banreader.py @@ -54,6 +54,7 @@ class Fail2banReader(ConfigReader): ["string", "logtarget", "STDERR"], ["string", "syslogsocket", "auto"], ["string", "dbfile", "/var/lib/fail2ban/fail2ban.sqlite3"], + ["int", "dbmaxmatches", None], ["string", "dbpurgeage", "1d"]] self.__opts = ConfigReader.getOptions(self, "Definition", opts) if updateMainOpt: @@ -73,7 +74,7 @@ class Fail2banReader(ConfigReader): # Also dbfile should be set before all other database options. # So adding order indices into items, to be stripped after sorting, upon return order = {"thread":0, "syslogsocket":11, "loglevel":12, "logtarget":13, - "dbfile":50, "dbpurgeage":51} + "dbfile":50, "dbmaxmatches":51, "dbpurgeage":51} stream = list() for opt in self.__opts: if opt in order: diff --git a/fail2ban/client/jailreader.py b/fail2ban/client/jailreader.py index 4f9d08d3..ba76e706 100644 --- a/fail2ban/client/jailreader.py +++ b/fail2ban/client/jailreader.py @@ -93,6 +93,7 @@ class JailReader(ConfigReader): opts = [["bool", "enabled", False], ["string", "backend", "auto"], ["int", "maxretry", None], + ["int", "maxmatches", None], ["string", "findtime", None], ["string", "bantime", None], ["string", "usedns", None], # be sure usedns is before all regex(s) in stream diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index 33edf315..35f966e8 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -443,6 +443,12 @@ class Server: def getUseDns(self, name): return self.__jails[name].filter.getUseDns() + def setMaxMatches(self, name, value): + self.__jails[name].filter.failManager.maxMatches = value + + def getMaxMatches(self, name): + return self.__jails[name].filter.failManager.maxMatches + def setMaxRetry(self, name, value): self.__jails[name].filter.setMaxRetry(value) diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 9d1ddbc9..0faecb85 100644 --- a/fail2ban/server/transmitter.py +++ b/fail2ban/server/transmitter.py @@ -183,6 +183,15 @@ class Transmitter: else: if self.__quiet: return return db.filename + elif name == "dbmaxmatches": + db = self.__server.getDatabase() + if db is None: + logSys.msg("dbmaxmatches setting was not in effect since no db yet") + return None + else: + db.maxMatches = int(command[1]) + if self.__quiet: return + return db.maxMatches elif name == "dbpurgeage": db = self.__server.getDatabase() if db is None: @@ -310,6 +319,11 @@ class Transmitter: self.__server.setLogTimeZone(name, value) if self.__quiet: return return self.__server.getLogTimeZone(name) + elif command[1] == "maxmatches": + value = command[2] + self.__server.setMaxMatches(name, int(value)) + if self.__quiet: return + return self.__server.getMaxMatches(name) elif command[1] == "maxretry": value = command[2] self.__server.setMaxRetry(name, int(value)) @@ -398,6 +412,12 @@ class Transmitter: return None else: return db.filename + elif name == "dbmaxmatches": + db = self.__server.getDatabase() + if db is None: + return None + else: + return db.maxMatches elif name == "dbpurgeage": db = self.__server.getDatabase() if db is None: @@ -433,6 +453,8 @@ class Transmitter: return self.__server.getDatePattern(name) elif command[1] == "logtimezone": return self.__server.getLogTimeZone(name) + elif command[1] == "maxmatches": + return self.__server.getMaxMatches(name) elif command[1] == "maxretry": return self.__server.getMaxRetry(name) elif command[1] == "maxlines": diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 29629a53..8320370b 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -881,18 +881,20 @@ class JailsReaderTest(LogCaptureTestCase): self.assertTrue( find_set('syslogsocket') < find_set('loglevel') < find_set('logtarget') ) - # then dbfile should be before dbpurgeage + # then dbfile should be before dbmaxmatches and dbpurgeage self.assertTrue(find_set('dbpurgeage') > find_set('dbfile')) + self.assertTrue(find_set('dbmaxmatches') > find_set('dbfile')) # and there is logging information left to be passed into the # server - self.assertSortedEqual(commands, - [['set', 'dbfile', - '/var/lib/fail2ban/fail2ban.sqlite3'], - ['set', 'dbpurgeage', '1d'], - ['set', 'loglevel', "INFO"], - ['set', 'logtarget', '/var/log/fail2ban.log'], - ['set', 'syslogsocket', 'auto']]) + self.assertSortedEqual(commands,[ + ['set', 'syslogsocket', 'auto'], + ['set', 'loglevel', "INFO"], + ['set', 'logtarget', '/var/log/fail2ban.log'], + ['set', 'dbfile', '/var/lib/fail2ban/fail2ban.sqlite3'], + ['set', 'dbmaxmatches', 10], + ['set', 'dbpurgeage', '1d'], + ]) # and if we force change configurator's fail2ban's baseDir # there should be an error message (test visually ;) -- diff --git a/fail2ban/tests/fail2banclienttestcase.py b/fail2ban/tests/fail2banclienttestcase.py index c532143e..12b092a8 100644 --- a/fail2ban/tests/fail2banclienttestcase.py +++ b/fail2ban/tests/fail2banclienttestcase.py @@ -179,6 +179,7 @@ def _start_params(tmp, use_stock=False, use_stock_cfg=None, "pidfile = " + pjoin(tmp, "f2b.pid"), "backend = polling", "dbfile = " + db, + "dbmaxmatches = 100", "dbpurgeage = 1d", "", ) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index aba5dd7f..b44674f2 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -374,6 +374,12 @@ class Transmitter(TransmitterBase): self.assertLogged("Ban 192.0.2.2", wait=True) self.assertNotLogged("Ban 192.0.2.1") + def testJailMaxMatches(self): + self.setGetTest("maxmatches", "5", 5, jail=self.jailName) + self.setGetTest("maxmatches", "2", 2, jail=self.jailName) + self.setGetTest("maxmatches", "-2", -2, jail=self.jailName) + self.setGetTestNOK("maxmatches", "Duck", jail=self.jailName) + def testJailMaxRetry(self): self.setGetTest("maxretry", "5", 5, jail=self.jailName) self.setGetTest("maxretry", "2", 2, jail=self.jailName) From 25f1aa334e75d398ef7eeaac94e64bbe01e41cab Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 20:53:11 +0200 Subject: [PATCH 05/10] fail2ban.conf: move default settings into DEFAULT section (to be more similar to jail.conf, Definition section overwrites the options, so it is backwards compatible) --- config/fail2ban.conf | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/config/fail2ban.conf b/config/fail2ban.conf index cf1cf5ae..ba0e9204 100644 --- a/config/fail2ban.conf +++ b/config/fail2ban.conf @@ -5,11 +5,11 @@ # Changes: in most of the cases you should not modify this # file, but provide customizations in fail2ban.local file, e.g.: # -# [Definition] +# [DEFAULT] # loglevel = DEBUG # -[Definition] +[DEFAULT] # Option: loglevel # Notes.: Set the log level output. @@ -74,6 +74,9 @@ dbpurgeage = 1d # Values: [ INT ] Default: 10 dbmaxmatches = 10 +[Definition] + + [Thread] # Options: stacksize From 852cb0362c2b74526a4c8db5ce7d11e0d23457db Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 21:17:38 +0200 Subject: [PATCH 06/10] fix restoring of tickets from database if `maxmatches` of jail smaller as `dbmaxmatches` (so read fewer matches in memory): --- fail2ban/server/database.py | 11 ++++++++++- fail2ban/server/jail.py | 3 ++- fail2ban/tests/databasetestcase.py | 10 +++++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 5eb7d8d6..f584d74f 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -712,7 +712,7 @@ class Fail2BanDb(object): cur = self._db.cursor() return cur.execute(query, queryArgs) - def getCurrentBans(self, jail = None, ip = None, forbantime=None, fromtime=None): + def getCurrentBans(self, jail = None, ip = None, forbantime=None, fromtime=None, maxmatches=None): tickets = [] ticket = None @@ -724,6 +724,15 @@ class Fail2BanDb(object): for banip, timeofban, data in results: # logSys.debug('restore ticket %r, %r, %r', banip, timeofban, data) ticket = FailTicket(banip, timeofban, data=data) + # filter matches if expected (current count > as maxmatches specified): + if maxmatches is None: + maxmatches = self.maxMatches + if maxmatches: + matches = ticket.getMatches() + if matches and len(matches) > maxmatches: + ticket.setMatches(matches[-maxmatches:]) + else: + ticket.setMatches(None) # logSys.debug('restored ticket: %r', ticket) if ip is not None: return ticket tickets.append(ticket) diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index eb990c47..1deea49b 100644 --- a/fail2ban/server/jail.py +++ b/fail2ban/server/jail.py @@ -213,7 +213,8 @@ class Jail(object): try: if self.database is not None: forbantime = self.actions.getBanTime() - for ticket in self.database.getCurrentBans(jail=self, forbantime=forbantime): + for ticket in self.database.getCurrentBans(jail=self, + forbantime=forbantime, maxmatches=self.filter.failManager.maxMatches): #logSys.debug('restored ticket: %s', ticket) if not self.filter.inIgnoreIPList(ticket.getIP(), log_ignore=True): # mark ticked was restored from database - does not put it again into db: diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index f03ec18d..8bb28dfe 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -371,7 +371,15 @@ class DatabaseTest(LogCaptureTestCase): self.assertEqual(ticket.getAttempt(), len(failures)) self.assertEqual(len(ticket.getMatches()), maxMatches) self.assertEqual(ticket.getMatches(), matches2find[-maxMatches:]) - # should retrieve 0 matches by last ban: + # maxmatches of jail < dbmaxmatches (so read 1 match and 0 matches): + ticket = self.db.getCurrentBans(self.jail, "127.0.0.1", fromtime=MyTime.time()-100, + maxmatches=1) + self.assertEqual(len(ticket.getMatches()), 1) + self.assertEqual(ticket.getMatches(), failures[3]['matches']) + ticket = self.db.getCurrentBans(self.jail, "127.0.0.1", fromtime=MyTime.time()-100, + maxmatches=0) + self.assertEqual(len(ticket.getMatches()), 0) + # dbmaxmatches = 0, should retrieve 0 matches by last ban: self.db.maxMatches = 0; self.db.addBan(self.jail, ticket) ticket = self.db.getCurrentBans(self.jail, "127.0.0.1", fromtime=MyTime.time()-100) From 4629e4320f85c41c0e3d0d0aea3e4c57f932ef3e Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 21:48:58 +0200 Subject: [PATCH 07/10] coverage and code review --- fail2ban/server/transmitter.py | 4 ++-- fail2ban/tests/servertestcase.py | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 0faecb85..de80f624 100644 --- a/fail2ban/server/transmitter.py +++ b/fail2ban/server/transmitter.py @@ -186,7 +186,7 @@ class Transmitter: elif name == "dbmaxmatches": db = self.__server.getDatabase() if db is None: - logSys.msg("dbmaxmatches setting was not in effect since no db yet") + logSys.log(logging.MSG, "dbmaxmatches setting was not in effect since no db yet") return None else: db.maxMatches = int(command[1]) @@ -195,7 +195,7 @@ class Transmitter: elif name == "dbpurgeage": db = self.__server.getDatabase() if db is None: - logSys.warning("dbpurgeage setting was not in effect since no db yet") + logSys.log(logging.MSG, "dbpurgeage setting was not in effect since no db yet") return None else: db.purgeage = command[1] diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index b44674f2..c1c5cc10 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -197,6 +197,8 @@ class Transmitter(TransmitterBase): self.setGetTest("dbfile", tmpFilename) # the same file name (again no jails / not changed): self.setGetTest("dbfile", tmpFilename) + self.setGetTest("dbmaxmatches", "100", 100) + self.setGetTestNOK("dbmaxmatches", "LIZARD") self.setGetTest("dbpurgeage", "600", 600) self.setGetTestNOK("dbpurgeage", "LIZARD") # the same file name (again with jails / not changed): @@ -211,6 +213,12 @@ class Transmitter(TransmitterBase): self.assertEqual(self.transm.proceed( ["get", "dbfile"]), (0, None)) + self.assertEqual(self.transm.proceed( + ["set", "dbmaxmatches", "100"]), + (0, None)) + self.assertEqual(self.transm.proceed( + ["get", "dbmaxmatches"]), + (0, None)) self.assertEqual(self.transm.proceed( ["set", "dbpurgeage", "500"]), (0, None)) From 46fc4c461588118b0c617de0528f095ccf7d9e4f Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 22:14:34 +0200 Subject: [PATCH 08/10] protocol and documentation --- MANIFEST | 3 +++ fail2ban/protocol.py | 4 ++++ man/fail2ban-client.1 | 18 ++++++++++++++++++ man/jail.conf.5 | 8 ++++++++ 4 files changed, 33 insertions(+) diff --git a/MANIFEST b/MANIFEST index cb393096..892f33b7 100644 --- a/MANIFEST +++ b/MANIFEST @@ -145,6 +145,7 @@ config/filter.d/sshd.conf config/filter.d/stunnel.conf config/filter.d/suhosin.conf config/filter.d/tine20.conf +config/filter.d/traefik-auth.conf config/filter.d/uwimap-auth.conf config/filter.d/vsftpd.conf config/filter.d/webmin-auth.conf @@ -333,9 +334,11 @@ fail2ban/tests/files/logs/solid-pop3d fail2ban/tests/files/logs/squid fail2ban/tests/files/logs/squirrelmail fail2ban/tests/files/logs/sshd +fail2ban/tests/files/logs/sshd-journal fail2ban/tests/files/logs/stunnel fail2ban/tests/files/logs/suhosin fail2ban/tests/files/logs/tine20 +fail2ban/tests/files/logs/traefik-auth fail2ban/tests/files/logs/uwimap-auth fail2ban/tests/files/logs/vsftpd fail2ban/tests/files/logs/webmin-auth diff --git a/fail2ban/protocol.py b/fail2ban/protocol.py index de9665f8..92b0dcc0 100644 --- a/fail2ban/protocol.py +++ b/fail2ban/protocol.py @@ -72,6 +72,8 @@ protocol = [ ['', "DATABASE", ""], ["set dbfile ", "set the location of fail2ban persistent datastore. Set to \"None\" to disable"], ["get dbfile", "get the location of fail2ban persistent datastore"], +["set dbmaxmatches ", "sets the max number of matches stored in database per ticket"], +["get dbmaxmatches", "gets the max number of matches stored in database per ticket"], ["set dbpurgeage ", "sets the max age in that history of bans will be kept"], ["get dbpurgeage", "gets the max age in seconds that history of bans will be kept"], ['', "JAIL CONTROL", ""], @@ -103,6 +105,7 @@ protocol = [ ["set banip ... ", "manually Ban for "], ["set unbanip [--report-absent] ... ", "manually Unban in "], ["set maxretry ", "sets the number of failures before banning the host for "], +["set maxmatches ", "sets the max number of matches stored in memory per ticket in "], ["set maxlines ", "sets the number of to buffer for regex search for "], ["set addaction [ ]", "adds a new action named for . Optionally for a Python based action, a and can be specified, else will be a Command Action"], ["set delaction ", "removes the action from "], @@ -130,6 +133,7 @@ protocol = [ ["get datepattern", "gets the patern used to match date/times for "], ["get usedns", "gets the usedns setting for "], ["get maxretry", "gets the number of failures allowed for "], +["get maxmatches", "gets the max number of matches stored in memory per ticket in "], ["get maxlines", "gets the number of lines to buffer for "], ["get actions", "gets a list of actions for "], ["", "COMMAND ACTION INFORMATION",""], diff --git a/man/fail2ban-client.1 b/man/fail2ban-client.1 index 22035f85..6c1ff7ef 100644 --- a/man/fail2ban-client.1 +++ b/man/fail2ban-client.1 @@ -168,6 +168,14 @@ persistent datastore. Set to get the location of fail2ban persistent datastore .TP +\fBset dbmaxmatches \fR +sets the max number of matches +stored in database per ticket +.TP +\fBget dbmaxmatches\fR +gets the max number of matches +stored in database per ticket +.TP \fBset dbpurgeage \fR sets the max age in that history of bans will be kept @@ -286,6 +294,11 @@ sets the number of failures before banning the host for .TP +\fBset maxmatches \fR +sets the max number of matches +stored in memory per ticket in + +.TP \fBset maxlines \fR sets the number of to buffer for regex search for @@ -393,6 +406,11 @@ gets the usedns setting for gets the number of failures allowed for .TP +\fBget maxmatches\fR +gets the max number of matches +stored in memory per ticket in + +.TP \fBget maxlines\fR gets the number of lines to buffer for diff --git a/man/jail.conf.5 b/man/jail.conf.5 index 0f3a4aa7..4f5c48e0 100644 --- a/man/jail.conf.5 +++ b/man/jail.conf.5 @@ -156,6 +156,11 @@ Database filename. Default: /var/lib/fail2ban/fail2ban.sqlite3 .br This defines where the persistent data for fail2ban is stored. This persistent data allows bans to be reinstated and continue reading log files from the last read position when fail2ban is restarted. A value of \fINone\fR disables this feature. .TP +.B dbmaxmatches +Max number of matches stored in database per ticket. Default: 10 +.br +This option sets the max number of matched log-lines could be stored per ticket in the database. This also affects values resolvable via tags \fB\fR and \fB\fR in actions. +.TP .B dbpurgeage Database purge age in seconds. Default: 86400 (24hours) .br @@ -276,6 +281,9 @@ regex (Python \fBreg\fRular \fBex\fRpression) to be added to the filter's failre .TP .B ignoreregex regex which, if the log line matches, would cause Fail2Ban not consider that line. This line will be ignored even if it matches a failregex of the jail or any of its filters. +.TP +.B maxmatches +max number of matched log-lines the jail would hold in memory per ticket. By default it is the same value as \fBmaxretry\fR of jail (or default). This option also affects values resolvable via tag \fB\fR in actions. .SS Backends Available options are listed below. From 3e5b8fdc6ae18c4b51bd89f6b2eca564f08125c9 Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 18 Apr 2019 22:44:14 +0200 Subject: [PATCH 09/10] fixes coverage of dbmaxmatches = 0 --- fail2ban/tests/databasetestcase.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 8bb28dfe..af9298a2 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -380,6 +380,7 @@ class DatabaseTest(LogCaptureTestCase): maxmatches=0) self.assertEqual(len(ticket.getMatches()), 0) # dbmaxmatches = 0, should retrieve 0 matches by last ban: + ticket.setMatches(["1","2","3"]) self.db.maxMatches = 0; self.db.addBan(self.jail, ticket) ticket = self.db.getCurrentBans(self.jail, "127.0.0.1", fromtime=MyTime.time()-100) From 7d6db7391edd27ea5bf90bd7da9b02f87b08c4a3 Mon Sep 17 00:00:00 2001 From: "Sergey G. Brester" Date: Fri, 19 Apr 2019 12:50:35 +0200 Subject: [PATCH 10/10] Update ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 4d9e8c6e..2f449c8e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -70,6 +70,8 @@ filter = flt[logtype=short] * `filter.d/traefik-auth.conf`: used to ban hosts, that were failed through traefik ### Enhancements +* introduced new options: `dbmaxmatches` (fail2ban.conf) and `maxmatches` (jail.conf) to contol + how many matches per ticket fail2ban can hold in memory and store in database (gh-2402, gh-2118); * fail2ban.conf: introduced new section `[Thread]` and option `stacksize` to configure default size of the stack for threads running in fail2ban (gh-2356), it could be set in `fail2ban.local` to avoid runtime error "can't start new thread" (see gh-969);