From df8d700d17789053a7439db4fc93d1d2d0f1b3f2 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 23 Feb 2014 17:35:36 +0000 Subject: [PATCH 1/2] RF: Refactor Jail and JailThread Includes: - documentation to new format and use of properties - change isActive->is_active as former no longer documented for python3, and later introduction and documented in python2.6 - status formatter in beautifier somewhat more automatically formatted; no changes are required for additional status elements - JailThread now set to active within `start` method, complimenting `stop` method --- config/action.d/smtp.py | 2 +- fail2ban/client/beautifier.py | 35 +++--- fail2ban/server/actions.py | 51 ++++----- fail2ban/server/database.py | 18 +-- fail2ban/server/filter.py | 24 ++-- fail2ban/server/filtergamin.py | 9 +- fail2ban/server/filterpoll.py | 15 ++- fail2ban/server/filterpyinotify.py | 3 +- fail2ban/server/filtersystemd.py | 21 ++-- fail2ban/server/jail.py | 158 +++++++++++++++++---------- fail2ban/server/jailthread.py | 116 ++++++-------------- fail2ban/server/server.py | 15 +-- fail2ban/tests/action_d/test_smtp.py | 6 +- fail2ban/tests/actionstestcase.py | 4 +- fail2ban/tests/databasetestcase.py | 4 +- fail2ban/tests/dummyjail.py | 15 +-- fail2ban/tests/filtertestcase.py | 10 +- fail2ban/tests/samplestestcase.py | 2 +- fail2ban/tests/servertestcase.py | 17 ++- 19 files changed, 238 insertions(+), 287 deletions(-) diff --git a/config/action.d/smtp.py b/config/action.d/smtp.py index 6830d535..933e27a0 100644 --- a/config/action.d/smtp.py +++ b/config/action.d/smtp.py @@ -121,7 +121,7 @@ class SMTPAction(ActionBase): self.matches = matches self.message_values = CallingMap( - jailname = self._jail.getName(), # Doesn't change + jailname = self._jail.name, hostname = socket.gethostname, bantime = self._jail.actions.getBanTime, ) diff --git a/fail2ban/client/beautifier.py b/fail2ban/client/beautifier.py index 724f068f..68816b06 100644 --- a/fail2ban/client/beautifier.py +++ b/fail2ban/client/beautifier.py @@ -67,28 +67,23 @@ class Beautifier: msg = "logs: " + response elif inC[0:1] == ['status']: if len(inC) > 1: - # Create IP list - ipList = "" - for ip in response[1][1][2][1]: - ipList += ip + " " - # Creates file list. - fileList = "" - for f in response[0][1][2][1]: - fileList += f + " " # Display information - msg = "Status for the jail: " + inC[1] + "\n" - msg = msg + "|- " + response[0][0] + "\n" - msg = msg + "| |- " + response[0][1][2][0] + ":\t" + fileList + "\n" - msg = msg + "| |- " + response[0][1][0][0] + ":\t" + `response[0][1][0][1]` + "\n" - msg = msg + "| `- " + response[0][1][1][0] + ":\t" + `response[0][1][1][1]` + "\n" - msg = msg + "`- " + response[1][0] + "\n" - msg = msg + " |- " + response[1][1][0][0] + ":\t" + `response[1][1][0][1]` + "\n" - msg = msg + " | `- " + response[1][1][2][0] + ":\t" + ipList + "\n" - msg = msg + " `- " + response[1][1][1][0] + ":\t" + `response[1][1][1][1]` + msg = ["Status for the jail: %s" % inC[1]] + for n, res1 in enumerate(response): + prefix1 = "`-" if n == len(response) - 1 else "|-" + msg.append("%s %s" % (prefix1, res1[0])) + prefix1 = " " if n == len(response) - 1 else "| " + for m, res2 in enumerate(res1[1]): + prefix2 = prefix1 + ("`-" if m == len(res1[1]) - 1 else "|-") + val = " ".join(res2[1]) if isinstance(res2[1], list) else res2[1] + msg.append("%s %s:\t%s" % (prefix2, res2[0], val)) else: - msg = "Status\n" - msg = msg + "|- " + response[0][0] + ":\t" + `response[0][1]` + "\n" - msg = msg + "`- " + response[1][0] + ":\t\t" + response[1][1] + msg = ["Status"] + for n, res1 in enumerate(response): + prefix1 = "`-" if n == len(response) - 1 else "|-" + val = " ".join(res1[1]) if isinstance(res1[1], list) else res1[1] + msg.append("%s %s:\t%s" % (prefix1, res1[0], val)) + msg = "\n".join(msg) elif inC[1] == "logtarget": msg = "Current logging target is:\n" msg = msg + "`- " + response diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index 52624f29..dba72680 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -192,30 +192,29 @@ class Actions(JailThread, Mapping): bool True when the thread exits nicely. """ - self.setActive(True) for name, action in self._actions.iteritems(): try: action.start() except Exception as e: logSys.error("Failed to start jail '%s' action '%s': %s", - self._jail.getName(), name, e) - while self._isActive(): - if not self.getIdle(): - #logSys.debug(self._jail.getName() + ": action") + self._jail.name, name, e) + while self.active: + if not self.idle: + #logSys.debug(self._jail.name + ": action") ret = self.__checkBan() if not ret: self.__checkUnBan() - time.sleep(self.getSleepTime()) + time.sleep(self.sleeptime) else: - time.sleep(self.getSleepTime()) + time.sleep(self.sleeptime) self.__flushBan() for name, action in self._actions.iteritems(): try: action.stop() except Exception as e: logSys.error("Failed to stop jail '%s' action '%s': %s", - self._jail.getName(), name, e) - logSys.debug(self._jail.getName() + ": action terminated") + self._jail.name, name, e) + logSys.debug(self._jail.name + ": action terminated") return True def __checkBan(self): @@ -237,31 +236,31 @@ class Actions(JailThread, Mapping): aInfo["failures"] = bTicket.getAttempt() aInfo["time"] = bTicket.getTime() aInfo["matches"] = "\n".join(bTicket.getMatches()) - if self._jail.getDatabase() is not None: + if self._jail.database is not None: aInfo["ipmatches"] = lambda: "\n".join( - self._jail.getDatabase().getBansMerged( + self._jail.database.getBansMerged( ip=bTicket.getIP()).getMatches()) aInfo["ipjailmatches"] = lambda: "\n".join( - self._jail.getDatabase().getBansMerged( + self._jail.database.getBansMerged( ip=bTicket.getIP(), jail=self._jail).getMatches()) aInfo["ipfailures"] = lambda: "\n".join( - self._jail.getDatabase().getBansMerged( + self._jail.database.getBansMerged( ip=bTicket.getIP()).getAttempt()) aInfo["ipjailfailures"] = lambda: "\n".join( - self._jail.getDatabase().getBansMerged( + self._jail.database.getBansMerged( ip=bTicket.getIP(), jail=self._jail).getAttempt()) if self.__banManager.addBanTicket(bTicket): - logSys.warning("[%s] Ban %s" % (self._jail.getName(), aInfo["ip"])) + logSys.warning("[%s] Ban %s" % (self._jail.name, aInfo["ip"])) for name, action in self._actions.iteritems(): try: action.ban(aInfo) except Exception as e: logSys.error( "Failed to execute ban jail '%s' action '%s': %s", - self._jail.getName(), name, e) + self._jail.name, name, e) return True else: - logSys.info("[%s] %s already banned" % (self._jail.getName(), + logSys.info("[%s] %s already banned" % (self._jail.name, aInfo["ip"])) return False @@ -298,28 +297,20 @@ class Actions(JailThread, Mapping): aInfo["failures"] = ticket.getAttempt() aInfo["time"] = ticket.getTime() aInfo["matches"] = "".join(ticket.getMatches()) - logSys.warning("[%s] Unban %s" % (self._jail.getName(), aInfo["ip"])) + logSys.warning("[%s] Unban %s" % (self._jail.name, aInfo["ip"])) for name, action in self._actions.iteritems(): try: action.unban(aInfo) except Exception as e: logSys.error( "Failed to execute unban jail '%s' action '%s': %s", - self._jail.getName(), name, e) + self._jail.name, name, e) + @property def status(self): - """Get the status of the filter. - - Get some informations about the filter state such as the total - number of failures. - - Returns - ------- - list - List of tuple pairs, each containing a description and value - for general status information. + """Status of active bans, and total ban counts. """ ret = [("Currently banned", self.__banManager.size()), ("Total banned", self.__banManager.getBanTotal()), - ("IP list", self.__banManager.getBanList())] + ("Banned IP list", self.__banManager.getBanList())] return ret diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 82bc86ba..35835faf 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -184,10 +184,10 @@ class Fail2BanDb(object): def addJail(self, cur, jail): cur.execute( "INSERT OR REPLACE INTO jails(name, enabled) VALUES(?, 1)", - (jail.getName(),)) + (jail.name,)) def delJail(self, jail): - return self.delJailName(jail.getName()) + return self.delJailName(jail.name) @commitandrollback def delJailName(self, cur, name): @@ -211,7 +211,7 @@ class Fail2BanDb(object): cur.execute( "SELECT firstlinemd5, lastfilepos FROM logs " "WHERE jail=? AND path=?", - (jail.getName(), container.getFileName())) + (jail.name, container.getFileName())) try: firstLineMD5, lastLinePos = cur.fetchone() except TypeError: @@ -220,7 +220,7 @@ class Fail2BanDb(object): cur.execute( "INSERT OR REPLACE INTO logs(jail, path, firstlinemd5, lastfilepos) " "VALUES(?, ?, ?, ?)", - (jail.getName(), container.getFileName(), + (jail.name, container.getFileName(), container.getHash(), container.getPos())) if container.getHash() != firstLineMD5: lastLinePos = None @@ -232,7 +232,7 @@ class Fail2BanDb(object): queryArgs = [] if jail is not None: query += " WHERE jail=?" - queryArgs.append(jail.getName()) + queryArgs.append(jail.name) cur.execute(query, queryArgs) return set(row[0] for row in cur.fetchmany()) @@ -245,7 +245,7 @@ class Fail2BanDb(object): "UPDATE logs SET firstlinemd5=?, lastfilepos=? " "WHERE jail=? AND path=?", (container.getHash(), container.getPos(), - jail.getName(), container.getFileName())) + jail.name, container.getFileName())) @commitandrollback def addBan(self, cur, jail, ticket): @@ -253,7 +253,7 @@ class Fail2BanDb(object): #TODO: Implement data parts once arbitrary match keys completed cur.execute( "INSERT INTO bans(jail, ip, timeofban, data) VALUES(?, ?, ?, ?)", - (jail.getName(), ticket.getIP(), ticket.getTime(), + (jail.name, ticket.getIP(), ticket.getTime(), {"matches": ticket.getMatches(), "failures": ticket.getAttempt()})) @@ -264,7 +264,7 @@ class Fail2BanDb(object): if jail is not None: query += " AND jail=?" - queryArgs.append(jail.getName()) + queryArgs.append(jail.name) if bantime is not None: query += " AND timeofban > ?" queryArgs.append(MyTime.time() - bantime) @@ -284,7 +284,7 @@ class Fail2BanDb(object): return tickets def getBansMerged(self, ip, jail=None, **kwargs): - cacheKey = ip if jail is None else "%s|%s" % (ip, jail.getName()) + cacheKey = ip if jail is None else "%s|%s" % (ip, jail.name) if cacheKey in self._bansMergedCache: return self._bansMergedCache[cacheKey] matches = [] diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py index 67d91538..76ea3f68 100644 --- a/fail2ban/server/filter.py +++ b/fail2ban/server/filter.py @@ -530,15 +530,10 @@ class Filter(JailThread): logSys.error(e) return failList - - ## - # Get the status of the filter. - # - # Get some informations about the filter state such as the total - # number of failures. - # @return a list with tuple - + @property def status(self): + """Status of failures detected by filter. + """ ret = [("Currently failed", self.failManager.size()), ("Total failed", self.failManager.getFailTotal())] return ret @@ -562,7 +557,7 @@ class FileFilter(Filter): logSys.error(path + " already exists") else: container = FileContainer(path, self.getLogEncoding(), tail) - db = self.jail.getDatabase() + db = self.jail.database if db is not None: lastpos = db.addLog(self.jail, container) if lastpos and not tail: @@ -586,7 +581,7 @@ class FileFilter(Filter): for log in self.__logPath: if log.getFileName() == path: self.__logPath.remove(log) - db = self.jail.getDatabase() + db = self.jail.database if db is not None: db.updateLog(self.jail, log) logSys.info("Removed logfile = %s" % path) @@ -682,18 +677,21 @@ class FileFilter(Filter): # might occur leading at least to tests failures. while has_content: line = container.readline() - if not line or not self._isActive(): + if not line or not self.active: # The jail reached the bottom or has been stopped break self.processLineAndAdd(line) container.close() - db = self.jail.getDatabase() + db = self.jail.database if db is not None: db.updateLog(self.jail, container) return True + @property def status(self): - ret = Filter.status(self) + """Status of Filter plus files being monitored. + """ + ret = super(FileFilter, self).status path = [m.getFileName() for m in self.getLogPath()] ret.append(("File list", path)) return ret diff --git a/fail2ban/server/filtergamin.py b/fail2ban/server/filtergamin.py index 6cc9c45b..898c8dfc 100644 --- a/fail2ban/server/filtergamin.py +++ b/fail2ban/server/filtergamin.py @@ -109,16 +109,15 @@ class FilterGamin(FileFilter): # @return True when the thread exits nicely def run(self): - self.setActive(True) # Gamin needs a loop to collect and dispatch events - while self._isActive(): - if not self.getIdle(): + while self.active: + if not self.idle: # We cannot block here because we want to be able to # exit. if self.monitor.event_pending(): self.monitor.handle_events() - time.sleep(self.getSleepTime()) - logSys.debug(self.jail.getName() + ": filter terminated") + time.sleep(self.sleeptime) + logSys.debug(self.jail.name + ": filter terminated") return True diff --git a/fail2ban/server/filterpoll.py b/fail2ban/server/filterpoll.py index d2f553b2..27af97b4 100644 --- a/fail2ban/server/filterpoll.py +++ b/fail2ban/server/filterpoll.py @@ -82,12 +82,11 @@ class FilterPoll(FileFilter): # @return True when the thread exits nicely def run(self): - self.setActive(True) - while self._isActive(): + while self.active: if logSys.getEffectiveLevel() <= 6: logSys.log(6, "Woke up idle=%s with %d files monitored", - self.getIdle(), len(self.getLogPath())) - if not self.getIdle(): + self.idle, len(self.getLogPath())) + if not self.idle: # Get file modification for container in self.getLogPath(): filename = container.getFileName() @@ -104,11 +103,11 @@ class FilterPoll(FileFilter): self.failManager.cleanup(MyTime.time()) self.dateDetector.sortTemplate() self.__modified = False - time.sleep(self.getSleepTime()) + time.sleep(self.sleeptime) else: - time.sleep(self.getSleepTime()) + time.sleep(self.sleeptime) logSys.debug( - (self.jail is not None and self.jail.getName() or "jailless") + + (self.jail is not None and self.jail.name or "jailless") + " filter terminated") return True @@ -143,7 +142,7 @@ class FilterPoll(FileFilter): if self.__file404Cnt[filename] > 2: logSys.warning("Too many errors. Setting the jail idle") if self.jail is not None: - self.jail.setIdle(True) + self.jail.idle = True else: logSys.warning("No jail is assigned to %s" % self) self.__file404Cnt[filename] = 0 diff --git a/fail2ban/server/filterpyinotify.py b/fail2ban/server/filterpyinotify.py index ab57290f..36d7fadc 100644 --- a/fail2ban/server/filterpyinotify.py +++ b/fail2ban/server/filterpyinotify.py @@ -168,11 +168,10 @@ class FilterPyinotify(FileFilter): # loop is necessary def run(self): - self.setActive(True) self.__notifier = pyinotify.ThreadedNotifier(self.__monitor, ProcessPyinotify(self)) self.__notifier.start() - logSys.debug("pyinotifier started for %s.", self.jail.getName()) + logSys.debug("pyinotifier started for %s.", self.jail.name) # TODO: verify that there is nothing really to be done for # idle jails return True diff --git a/fail2ban/server/filtersystemd.py b/fail2ban/server/filtersystemd.py index d39ba280..13a4902b 100644 --- a/fail2ban/server/filtersystemd.py +++ b/fail2ban/server/filtersystemd.py @@ -211,7 +211,6 @@ class FilterSystemd(JournalFilter): # pragma: systemd no cover # handover to FailManager def run(self): - self.setActive(True) # Seek to now - findtime in journal start_time = datetime.datetime.now() - \ @@ -224,9 +223,9 @@ class FilterSystemd(JournalFilter): # pragma: systemd no cover except OSError: pass # Reading failure, so safe to ignore - while self._isActive(): - if not self.getIdle(): - while self._isActive(): + while self.active: + if not self.idle: + while self.active: try: logentry = self.__journal.get_next() except OSError: @@ -247,20 +246,14 @@ class FilterSystemd(JournalFilter): # pragma: systemd no cover except FailManagerEmpty: self.failManager.cleanup(MyTime.time()) self.__modified = False - self.__journal.wait(self.getSleepTime()) - logSys.debug((self.jail is not None and self.jail.getName() + self.__journal.wait(self.sleeptime) + logSys.debug((self.jail is not None and self.jail.name or "jailless") +" filter terminated") return True - ## - # Get the status of the filter. - # - # Get some informations about the filter state such as the total - # number of failures. - # @return a list with tuple - + @property def status(self): - ret = JournalFilter.status(self) + ret = super(FilterSystemd, self).status ret.append(("Journal matches", [" + ".join(" ".join(match) for match in self.__matches)])) return ret diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index bdd8e89a..c32e120d 100644 --- a/fail2ban/server/jail.py +++ b/fail2ban/server/jail.py @@ -31,6 +31,12 @@ from .actions import Actions logSys = logging.getLogger(__name__) class Jail: + """Fail2Ban jail, which manages a filter and associated actions. + + The class handles the initialisation of a filter, and actions. It's + role is then to act as an interface between the filter and actions, + passing bans detected by the filter, for the actions to then act upon. + """ #Known backends. Each backend should have corresponding __initBackend method # yoh: stored in a list instead of a tuple since only @@ -38,15 +44,32 @@ class Jail: _BACKENDS = ['pyinotify', 'gamin', 'polling', 'systemd'] def __init__(self, name, backend = "auto", db=None): + """Initialise a jail, by initalises filter and actions. + + Parameters + ---------- + name : str + Name assigned to the jail. + backend : str + Backend to be used for filter. "auto" will attempt to pick + the most preferred backend method. Default: "auto" + db : Fail2BanDb + Fail2Ban persistent database instance. Default: `None` + """ self.__db = db - self.setName(name) + # 26 based on iptable chain name limit of 30 less len('f2b-') + if len(name) >= 26: + logSys.warning("Jail name %r might be too long and some commands " + "might not function correctly. Please shorten" + % name) + self.__name = name self.__queue = Queue.Queue() self.__filter = None - logSys.info("Creating new jail '%s'" % self.__name) + logSys.info("Creating new jail '%s'" % self.name) self._setBackend(backend) def __repr__(self): - return "%s(%r)" % (self.__class__.__name__, self.__name) + return "%s(%r)" % (self.__class__.__name__, self.name) def _setBackend(self, backend): backend = backend.lower() # to assure consistent matching @@ -78,51 +101,49 @@ class Jail: "Backend %r failed to initialize due to %s" % (b, e)) # log error since runtime error message isn't printed, INVALID COMMAND logSys.error( - "Failed to initialize any backend for Jail %r" % self.__name) + "Failed to initialize any backend for Jail %r" % self.name) raise RuntimeError( - "Failed to initialize any backend for Jail %r" % self.__name) + "Failed to initialize any backend for Jail %r" % self.name) def _initPolling(self): - logSys.info("Jail '%s' uses poller" % self.__name) + logSys.info("Jail '%s' uses poller" % self.name) from filterpoll import FilterPoll self.__filter = FilterPoll(self) def _initGamin(self): # Try to import gamin import gamin - logSys.info("Jail '%s' uses Gamin" % self.__name) + logSys.info("Jail '%s' uses Gamin" % self.name) from filtergamin import FilterGamin self.__filter = FilterGamin(self) def _initPyinotify(self): # Try to import pyinotify import pyinotify - logSys.info("Jail '%s' uses pyinotify" % self.__name) + logSys.info("Jail '%s' uses pyinotify" % self.name) from filterpyinotify import FilterPyinotify self.__filter = FilterPyinotify(self) def _initSystemd(self): # pragma: systemd no cover # Try to import systemd import systemd - logSys.info("Jail '%s' uses systemd" % self.__name) + logSys.info("Jail '%s' uses systemd" % self.name) from filtersystemd import FilterSystemd self.__filter = FilterSystemd(self) - - def setName(self, name): - # 26 based on iptable chain name limit of 30 less len('f2b-') - if len(name) >= 26: - logSys.warning("Jail name %r might be too long and some commands " - "might not function correctly. Please shorten" - % name) - self.__name = name - - def getName(self): + + @property + def name(self): + """Name of jail. + """ return self.__name - - def getDatabase(self): + + @property + def database(self): + """The database used to store persistent data for the jail. + """ return self.__db - + @property def filter(self): """The filter which the jail is using to monitor log files. @@ -134,50 +155,71 @@ class Jail: """Actions object used to manage actions for jail. """ return self.__actions - + + @property + def idle(self): + """A boolean indicating whether jail is idle. + """ + return self.filter.idle or self.actions.idle + + @idle.setter + def idle(self, value): + self.filter.idle = value + self.actions.idle = value + + @property + def status(self): + """The status of the jail. + """ + return [ + ("Filter", self.filter.status), + ("Actions", self.actions.status), + ] + def putFailTicket(self, ticket): + """Add a fail ticket to the jail. + + Used by filter to add a failure for banning. + """ self.__queue.put(ticket) - if self.__db is not None: - self.__db.addBan(self, ticket) - + if self.database is not None: + self.database.addBan(self, ticket) + def getFailTicket(self): + """Get a fail ticket from the jail. + + Used by actions to get a failure for banning. + """ try: return self.__queue.get(False) except Queue.Empty: return False - + def start(self): - self.__filter.start() - self.__actions.start() + """Start the jail, by starting filter and actions threads. + + Once stated, also queries the persistent database to reinstate + any valid bans. + """ + self.filter.start() + self.actions.start() # Restore any previous valid bans from the database - if self.__db is not None: - for ticket in self.__db.getBans( - jail=self, bantime=self.__actions.getBanTime()): + if self.database is not None: + for ticket in self.database.getBans( + jail=self, bantime=self.actions.getBanTime()): self.__queue.put(ticket) - logSys.info("Jail '%s' started" % self.__name) - + logSys.info("Jail '%s' started" % self.name) + def stop(self): - self.__filter.stop() - self.__actions.stop() - self.__filter.join() - self.__actions.join() - logSys.info("Jail '%s' stopped" % self.__name) - - def isAlive(self): - isAlive0 = self.__filter.isAlive() - isAlive1 = self.__actions.isAlive() - return isAlive0 or isAlive1 - - def setIdle(self, value): - self.__filter.setIdle(value) - self.__actions.setIdle(value) - - def getIdle(self): - return self.__filter.getIdle() or self.__actions.getIdle() - - def getStatus(self): - fStatus = self.__filter.status() - aStatus = self.__actions.status() - ret = [("filter", fStatus), - ("action", aStatus)] - return ret + """Stop the jail, by stopping filter and actions threads. + """ + self.filter.stop() + self.actions.stop() + self.filter.join() + self.actions.join() + logSys.info("Jail '%s' stopped" % self.name) + + def is_alive(self): + """Check jail "is_alive" by checking filter and actions threads. + """ + return self.filter.is_alive() or self.actions.is_alive() diff --git a/fail2ban/server/jailthread.py b/fail2ban/server/jailthread.py index 4901f2e0..b444a13d 100644 --- a/fail2ban/server/jailthread.py +++ b/fail2ban/server/jailthread.py @@ -25,94 +25,42 @@ __copyright__ = "Copyright (c) 2004 Cyril Jaquier" __license__ = "GPL" from threading import Thread -import logging - -# Gets the instance of the logger. -logSys = logging.getLogger(__name__) +from abc import abstractproperty, abstractmethod class JailThread(Thread): - - ## - # Constructor. - # - # Initialize the filter object with default values. - # @param jail the jail object - + """Abstract class for threading elements in Fail2Ban. + """ + def __init__(self): - Thread.__init__(self) + """Initialise a JailThread instance. + """ + super(JailThread, self).__init__() ## Control the state of the thread. - self.__isRunning = False + self.active = False ## Control the idle state of the thread. - self.__isIdle = False + self.idle = False ## The time the thread sleeps in the loop. - self.__sleepTime = 1 - - ## - # Set the time that the thread sleeps. - # - # This value could also be called "polling time". A value of 1 is a - # good one. This unit is "second" - # @param value the polling time (second) - - def setSleepTime(self, value): - self.__sleepTime = value - logSys.info("Set sleeptime %s" % value) - - ## - # Get the time that the thread sleeps. - # - # @return the polling time - - def getSleepTime(self): - return self.__sleepTime - - ## - # Set the idle flag. - # - # This flag stops the check of the log file. - # @param value boolean value - - def setIdle(self, value): - self.__isIdle = value - - ## - # Get the idle state. - # - # @return the idle state - - def getIdle(self): - return self.__isIdle - - ## - # Stop the thread. - # - # Stop the exection of the thread and quit. - - def stop(self): - self.__isRunning = False - - ## - # Set the isRunning flag. - # - # @param value True if the thread is running - - def setActive(self, value): - self.__isRunning = value - - ## - # Check if the thread is active. - # - # Check if the filter thread is running. - # @return True if the thread is running - - def _isActive(self): - return self.__isRunning - - ## - # Get the status of the thread - # - # Get some informations about the thread. This is an abstract method. - # @return a list with tuple - - def status(self): + self.sleeptime = 1 + + @abstractproperty + def status(self): # pragma: no cover - abstract + """Abstract - Should provide status information. + """ + pass + + def start(self): + """Sets active flag and starts thread. + """ + self.active = True + super(JailThread, self).start() + + def stop(self): + """Sets `active` property to False, to flag run method to return. + """ + self.active = False + + @abstractmethod + def run(self): # pragma: no cover - absract + """Abstract - Called when thread starts, thread stops when returns. + """ pass diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index 3d2b54f7..3710e4e8 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -132,7 +132,7 @@ class Server: def startJail(self, name): try: self.__lock.acquire() - if not self.isAlive(name): + if not self.__jails[name].is_alive(): self.__jails[name].start() finally: self.__lock.release() @@ -141,7 +141,7 @@ class Server: logSys.debug("Stopping jail %s" % name) try: self.__lock.acquire() - if self.isAlive(name): + if self.__jails[name].is_alive(): self.__jails[name].stop() self.delJail(name) finally: @@ -155,16 +155,13 @@ class Server: self.stopJail(jail) finally: self.__lock.release() - - def isAlive(self, name): - return self.__jails[name].isAlive() - + def setIdleJail(self, name, value): - self.__jails[name].setIdle(value) + self.__jails[name].idle = value return True def getIdleJail(self, name): - return self.__jails[name].getIdle() + return self.__jails[name].idle # Filter def addIgnoreIP(self, name, ip): @@ -316,7 +313,7 @@ class Server: self.__lock.release() def statusJail(self, name): - return self.__jails[name].getStatus() + return self.__jails[name].status # Logging diff --git a/fail2ban/tests/action_d/test_smtp.py b/fail2ban/tests/action_d/test_smtp.py index 33fd9f46..7cd4edf3 100644 --- a/fail2ban/tests/action_d/test_smtp.py +++ b/fail2ban/tests/action_d/test_smtp.py @@ -77,7 +77,7 @@ class SMTPActionTest(unittest.TestCase): self.assertEqual(self.smtpd.mailfrom, "fail2ban") self.assertEqual(self.smtpd.rcpttos, ["root"]) self.assertTrue( - "Subject: [Fail2Ban] %s: started" % self.jail.getName() + "Subject: [Fail2Ban] %s: started" % self.jail.name in self.smtpd.data) def testStop(self): @@ -86,7 +86,7 @@ class SMTPActionTest(unittest.TestCase): self.assertEqual(self.smtpd.rcpttos, ["root"]) self.assertTrue( "Subject: [Fail2Ban] %s: stopped" % - self.jail.getName() in self.smtpd.data) + self.jail.name in self.smtpd.data) def testBan(self): aInfo = { @@ -102,7 +102,7 @@ class SMTPActionTest(unittest.TestCase): self.assertEqual(self.smtpd.rcpttos, ["root"]) self.assertTrue( "Subject: [Fail2Ban] %s: banned %s" % - (self.jail.getName(), aInfo['ip']) in self.smtpd.data) + (self.jail.name, aInfo['ip']) in self.smtpd.data) self.assertTrue( "%i attempts" % aInfo['failures'] in self.smtpd.data) diff --git a/fail2ban/tests/actionstestcase.py b/fail2ban/tests/actionstestcase.py index ccc0263d..74c3a326 100644 --- a/fail2ban/tests/actionstestcase.py +++ b/fail2ban/tests/actionstestcase.py @@ -84,8 +84,8 @@ class ExecuteActions(LogCaptureTestCase): self.__actions.stop() self.__actions.join() - self.assertEqual(self.__actions.status(),[("Currently banned", 0 ), - ("Total banned", 0 ), ("IP list", [] )]) + self.assertEqual(self.__actions.status,[("Currently banned", 0 ), + ("Total banned", 0 ), ("Banned IP list", [] )]) def testAddActionPython(self): diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index de2e3377..837b9cbe 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -61,7 +61,7 @@ class DatabaseTest(unittest.TestCase): self.db = Fail2BanDb(self.dbFilename) # and check jail of same name still present self.assertTrue( - self.jail.getName() in self.db.getJailNames(), + self.jail.name in self.db.getJailNames(), "Jail not retained in Db after disconnect reconnect.") def testUpdateDb(self): @@ -80,7 +80,7 @@ class DatabaseTest(unittest.TestCase): self.jail = DummyJail() self.db.addJail(self.jail) self.assertTrue( - self.jail.getName() in self.db.getJailNames(), + self.jail.name in self.db.getJailNames(), "Jail not added to database") def testAddLog(self): diff --git a/fail2ban/tests/dummyjail.py b/fail2ban/tests/dummyjail.py index 129940bd..77ab785e 100644 --- a/fail2ban/tests/dummyjail.py +++ b/fail2ban/tests/dummyjail.py @@ -32,6 +32,8 @@ class DummyJail(object): def __init__(self): self.lock = Lock() self.queue = [] + self.idle = False + self.database = None self.actions = Actions(self) def __len__(self): @@ -58,15 +60,6 @@ class DummyJail(object): finally: self.lock.release() - def setIdle(self, value): - pass - - def getIdle(self): - pass - - def getName(self): + @property + def name(self): return "DummyJail #%s with %d tickets" % (id(self), len(self)) - - def getDatabase(self): - return None - diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index 40ac6c13..b0b98cab 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -325,7 +325,7 @@ class LogFileMonitor(LogCaptureTestCase): self.file = open(self.name, 'a') self.filter = FilterPoll(DummyJail()) self.filter.addLogPath(self.name) - self.filter.setActive(True) + self.filter.active = True self.filter.addFailRegex("(?:(?:Authentication failure|Failed [-/\w+]+) for(?: [iI](?:llegal|nvalid) user)?|[Ii](?:llegal|nvalid) user|ROOT LOGIN REFUSED) .*(?: from|FROM) ") def tearDown(self): @@ -466,7 +466,7 @@ def get_monitor_failures_testcase(Filter_): self.jail = DummyJail() self.filter = Filter_(self.jail) self.filter.addLogPath(self.name) - self.filter.setActive(True) + self.filter.active = True self.filter.addFailRegex("(?:(?:Authentication failure|Failed [-/\w+]+) for(?: [iI](?:llegal|nvalid) user)?|[Ii](?:llegal|nvalid) user|ROOT LOGIN REFUSED) .*(?: from|FROM) ") self.filter.start() # If filter is polling it would sleep a bit to guarantee that @@ -687,7 +687,7 @@ def get_monitor_failures_journal_testcase(Filter_): # pragma: systemd no cover "TEST_UUID=%s" % self.test_uuid]) self.journal_fields = { 'TEST_FIELD': "1", 'TEST_UUID': self.test_uuid} - self.filter.setActive(True) + self.filter.active = True self.filter.addFailRegex("(?:(?:Authentication failure|Failed [-/\w+]+) for(?: [iI](?:llegal|nvalid) user)?|[Ii](?:llegal|nvalid) user|ROOT LOGIN REFUSED) .*(?: from|FROM) ") self.filter.start() @@ -804,7 +804,7 @@ class GetFailures(unittest.TestCase): setUpMyTime() self.jail = DummyJail() self.filter = FileFilter(self.jail) - self.filter.setActive(True) + self.filter.active = True # TODO Test this #self.filter.setTimeRegex("\S{3}\s{1,2}\d{1,2} \d{2}:\d{2}:\d{2}") #self.filter.setTimePattern("%b %d %H:%M:%S") @@ -895,7 +895,7 @@ class GetFailures(unittest.TestCase): ('warn', output_yes)): jail = DummyJail() filter_ = FileFilter(jail, useDns=useDns) - filter_.setActive(True) + filter_.active = True filter_.failManager.setMaxRetry(1) # we might have just few failures filter_.addLogPath(GetFailures.FILENAME_USEDNS) diff --git a/fail2ban/tests/samplestestcase.py b/fail2ban/tests/samplestestcase.py index b185da19..3529fcc2 100644 --- a/fail2ban/tests/samplestestcase.py +++ b/fail2ban/tests/samplestestcase.py @@ -45,7 +45,7 @@ class FilterSamplesRegex(unittest.TestCase): def setUp(self): """Call before every test case.""" self.filter = Filter(None) - self.filter.setActive(True) + self.filter.active = True setUpMyTime() diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 0d1c52d1..c325a337 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -227,8 +227,7 @@ class Transmitter(TransmitterBase): time.sleep(1) self.assertEqual( self.transm.proceed(["stop", self.jailName]), (0, None)) - self.assertRaises( - UnknownJailException, self.server.isAlive, self.jailName) + self.assertTrue(self.jailName not in self.server._Server__jails) def testStartStopAllJail(self): self.server.addJail("TestJail2", "auto") @@ -242,10 +241,8 @@ class Transmitter(TransmitterBase): time.sleep(0.1) self.assertEqual(self.transm.proceed(["stop", "all"]), (0, None)) time.sleep(1) - self.assertRaises( - UnknownJailException, self.server.isAlive, self.jailName) - self.assertRaises( - UnknownJailException, self.server.isAlive, "TestJail2") + self.assertTrue(self.jailName not in self.server._Server__jails) + self.assertTrue("TestJail2" not in self.server._Server__jails) def testJailIdle(self): self.assertEqual( @@ -482,15 +479,15 @@ class Transmitter(TransmitterBase): self.assertEqual(self.transm.proceed(["status", self.jailName]), (0, [ - ('filter', [ + ('Filter', [ ('Currently failed', 0), ('Total failed', 0), ('File list', [])] ), - ('action', [ + ('Actions', [ ('Currently banned', 0), ('Total banned', 0), - ('IP list', [])] + ('Banned IP list', [])] ) ] ) @@ -774,7 +771,7 @@ class JailTests(unittest.TestCase): # Just a smoke test for now longname = "veryveryverylongname" jail = Jail(longname) - self.assertEqual(jail.getName(), longname) + self.assertEqual(jail.name, longname) class RegexTests(unittest.TestCase): From edd0bf7d46d09aff9977c8925963491b1d66036a Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 23 Feb 2014 18:33:58 +0000 Subject: [PATCH 2/2] ENH+DOC: Update Fail2Ban database doc strings and use properties --- fail2ban/server/database.py | 170 +++++++++++++++++++++++++++-- fail2ban/server/server.py | 6 +- fail2ban/server/transmitter.py | 10 +- fail2ban/tests/databasetestcase.py | 4 +- 4 files changed, 169 insertions(+), 21 deletions(-) diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index 35835faf..13a19548 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -58,6 +58,11 @@ def commitandrollback(f): return wrapper class Fail2BanDb(object): + """Fail2Ban database for storing persistent data. + + This allows after Fail2Ban is restarted to reinstated bans and + to continue monitoring logs from the same point. + """ __version__ = 2 # Note all _TABLE_* strings must end in ';' for py26 compatibility _TABLE_fail2banDb = "CREATE TABLE fail2banDb(version INTEGER);" @@ -93,6 +98,27 @@ class Fail2BanDb(object): "CREATE INDEX bans_ip ON bans(ip);" \ def __init__(self, filename, purgeAge=24*60*60): + """Initialise the database by connecting/creating SQLite3 file. + + This will either create a new Fail2Ban database, connect to an + existing, and if applicable upgrade the schema in the process. + + Parameters + ---------- + filename : str + File name for SQLite3 database, which will be created if + doesn't already exist. + purgeAge : int + Purge age in seconds, used to remove old bans from + database during purge. + + Raises + ------ + sqlite3.OperationalError + Error connecting/creating a SQLite3 database. + RuntimeError + If exisiting database fails to update to new schema. + """ try: self._lock = Lock() self._db = sqlite3.connect( @@ -130,21 +156,30 @@ class Fail2BanDb(object): logSys.error( "Database update failed to acheive version '%i'" ": updated from '%i' to '%i'", Fail2BanDb.__version__, version, newversion) - raise Exception('Failed to fully update') + raise RuntimeError('Failed to fully update') finally: cur.close() - def getFilename(self): + @property + def filename(self): + """File name of SQLite3 database file. + """ return self._dbFilename - def getPurgeAge(self): + @property + def purgeage(self): + """Purge age in seconds. + """ return self._purgeAge - def setPurgeAge(self, value): + @purgeage.setter + def purgeage(self, value): self._purgeAge = int(value) @commitandrollback def createDb(self, cur): + """Creates a new database, called during initialisation. + """ # Version info cur.executescript(Fail2BanDb._TABLE_fail2banDb) cur.execute("INSERT INTO fail2banDb(version) VALUES(?)", @@ -161,8 +196,13 @@ class Fail2BanDb(object): @commitandrollback def updateDb(self, cur, version): - self.dbBackupFilename = self._dbFilename + '.' + time.strftime('%Y%m%d-%H%M%S', MyTime.gmtime()) - shutil.copyfile(self._dbFilename, self.dbBackupFilename) + """Update an existing database, called during initialisation. + + A timestamped backup is also created prior to attempting the update. + """ + self._dbBackupFilename = self.filename + '.' + time.strftime('%Y%m%d-%H%M%S', MyTime.gmtime()) + shutil.copyfile(self.filename, self._dbBackupFilename) + logSys.info("Database backup created: %s", self._dbBackupFilename) if version > Fail2BanDb.__version__: raise NotImplementedError( "Attempt to travel to future version of database ...how did you get here??") @@ -182,31 +222,68 @@ class Fail2BanDb(object): @commitandrollback def addJail(self, cur, jail): + """Adds a jail to the database. + + Parameters + ---------- + jail : Jail + Jail to be added to the database. + """ cur.execute( "INSERT OR REPLACE INTO jails(name, enabled) VALUES(?, 1)", (jail.name,)) - def delJail(self, jail): - return self.delJailName(jail.name) - @commitandrollback - def delJailName(self, cur, name): + def delJail(self, cur, jail): + """Deletes a jail from the database. + + Parameters + ---------- + jail : Jail + Jail to be removed from the database. + """ # Will be deleted by purge as appropriate cur.execute( - "UPDATE jails SET enabled=0 WHERE name=?", (name, )) + "UPDATE jails SET enabled=0 WHERE name=?", (jail.name, )) @commitandrollback def delAllJails(self, cur): + """Deletes all jails from the database. + """ # Will be deleted by purge as appropriate cur.execute("UPDATE jails SET enabled=0") @commitandrollback def getJailNames(self, cur): + """Get name of jails in database. + + Currently only used for testing purposes. + + Returns + ------- + set + Set of jail names. + """ cur.execute("SELECT name FROM jails") return set(row[0] for row in cur.fetchmany()) @commitandrollback def addLog(self, cur, jail, container): + """Adds a log to the database. + + Parameters + ---------- + jail : Jail + Jail that log is being monitored by. + container : FileContainer + File container of the log file being added. + + Returns + ------- + int + If log was already present in database, value of last position + in the log file; else `None` + """ lastLinePos = None cur.execute( "SELECT firstlinemd5, lastfilepos FROM logs " @@ -228,6 +305,20 @@ class Fail2BanDb(object): @commitandrollback def getLogPaths(self, cur, jail=None): + """Gets all the log paths from the database. + + Currently only for testing purposes. + + Parameters + ---------- + jail : Jail + If specified, will only reutrn logs belonging to the jail. + + Returns + ------- + set + Set of log paths. + """ query = "SELECT path FROM logs" queryArgs = [] if jail is not None: @@ -238,6 +329,15 @@ class Fail2BanDb(object): @commitandrollback def updateLog(self, cur, *args, **kwargs): + """Updates hash and last position in log file. + + Parameters + ---------- + jail : Jail + Jail of which the log file belongs to. + container : FileContainer + File container of the log file being updated. + """ self._updateLog(cur, *args, **kwargs) def _updateLog(self, cur, jail, container): @@ -249,6 +349,15 @@ class Fail2BanDb(object): @commitandrollback def addBan(self, cur, jail, ticket): + """Add a ban to the database. + + Parameters + ---------- + jail : Jail + Jail in which the ban has occured. + ticket : BanTicket + Ticket of the ban to be added. + """ self._bansMergedCache = {} #TODO: Implement data parts once arbitrary match keys completed cur.execute( @@ -276,6 +385,23 @@ class Fail2BanDb(object): return cur.execute(query, queryArgs) def getBans(self, **kwargs): + """Get bans from the database. + + Parameters + ---------- + jail : Jail + Jail that the ban belongs to. Default `None`; all jails. + bantime : int + Ban time in seconds, such that bans returned would still be + valid now. Default `None`; no limit. + ip : str + IP Address to filter bans by. Default `None`; all IPs. + + Returns + ------- + list + List of `Ticket`s for bans stored in database. + """ tickets = [] for ip, timeofban, data in self._getBans(**kwargs): #TODO: Implement data parts once arbitrary match keys completed @@ -284,6 +410,26 @@ class Fail2BanDb(object): return tickets def getBansMerged(self, ip, jail=None, **kwargs): + """Get bans from the database, merged into single ticket. + + This is the same as `getBans`, but bans merged into single + ticket. + + Parameters + ---------- + jail : Jail + Jail that the ban belongs to. Default `None`; all jails. + bantime : int + Ban time in seconds, such that bans returned would still be + valid now. Default `None`; no limit. + ip : str + IP Address to filter bans by. Default `None`; all IPs. + + Returns + ------- + Ticket + Single ticket representing bans stored in database. + """ cacheKey = ip if jail is None else "%s|%s" % (ip, jail.name) if cacheKey in self._bansMergedCache: return self._bansMergedCache[cacheKey] @@ -300,6 +446,8 @@ class Fail2BanDb(object): @commitandrollback def purge(self, cur): + """Purge old bans, jails and log files from database. + """ self._bansMergedCache = {} cur.execute( "DELETE FROM bans WHERE timeofban < ?", diff --git a/fail2ban/server/server.py b/fail2ban/server/server.py index 3710e4e8..8d0fe66a 100644 --- a/fail2ban/server/server.py +++ b/fail2ban/server/server.py @@ -125,10 +125,10 @@ class Server: self.__db.addJail(self.__jails[name]) def delJail(self, name): - del self.__jails[name] if self.__db is not None: - self.__db.delJailName(name) - + self.__db.delJail(self.__jails[name]) + del self.__jails[name] + def startJail(self, name): try: self.__lock.acquire() diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 0a955883..48405f58 100644 --- a/fail2ban/server/transmitter.py +++ b/fail2ban/server/transmitter.py @@ -123,14 +123,14 @@ class Transmitter: if db is None: return None else: - return db.getFilename() + return db.filename elif name == "dbpurgeage": db = self.__server.getDatabase() if db is None: return None else: - db.setPurgeAge(command[1]) - return db.getPurgeAge() + db.purgeage = command[1] + return db.purgeage # Jail elif command[1] == "idle": if command[2] == "on": @@ -265,13 +265,13 @@ class Transmitter: if db is None: return None else: - return db.getFilename() + return db.filename elif name == "dbpurgeage": db = self.__server.getDatabase() if db is None: return None else: - return db.getPurgeAge() + return db.purgeage # Filter elif command[1] == "logpath": return self.__server.getLogPath(name) diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 837b9cbe..83a0ba5b 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -47,7 +47,7 @@ class DatabaseTest(unittest.TestCase): os.remove(self.dbFilename) def testGetFilename(self): - self.assertEqual(self.dbFilename, self.db.getFilename()) + self.assertEqual(self.dbFilename, self.db.filename) def testCreateInvalidPath(self): self.assertRaises( @@ -74,7 +74,7 @@ class DatabaseTest(unittest.TestCase): self.assertEqual(self.db.updateDb(Fail2BanDb.__version__), Fail2BanDb.__version__) self.assertRaises(NotImplementedError, self.db.updateDb, Fail2BanDb.__version__ + 1) - os.remove(self.db.dbBackupFilename) + os.remove(self.db._dbBackupFilename) def testAddJail(self): self.jail = DummyJail()