From 2974cac40cc7709909fcb11a510f164c88e7db46 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Wed, 19 Jun 2013 22:10:18 -0400 Subject: [PATCH 1/5] RF: log all logging output from fail2ban-client to stderr. Close #264 otherwise it 1. 'interferes' with meaninful output of the client 2. if ERROR is logged it better go to stderr and separating ERROR from other levels is not that transparent with python's logging --- ChangeLog | 3 ++- fail2ban-client | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index a6bc4e21..f2425d2f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -24,7 +24,8 @@ ver. 0.8.11 (2013/XX/XXX) - wanna-be-released sample logs Yaroslav Halchenko * fail2ban-regex -- refactored to provide more details (missing and - ignored lines, control over logging, etc) while maintaining look&feel. + ignored lines, control over logging, etc) while maintaining look&feel + * fail2ban-client -- log to standard error ver. 0.8.10 (2013/06/12) - wanna-be-secure ----------- diff --git a/fail2ban-client b/fail2ban-client index de8519f4..57235d79 100755 --- a/fail2ban-client +++ b/fail2ban-client @@ -336,13 +336,13 @@ class Fail2banClient: logSys.setLevel(logging.INFO) else: logSys.setLevel(logging.DEBUG) - # Add the default logging handler - stdout = logging.StreamHandler(sys.stdout) + # Add the default logging handler to dump to stderr + logout = logging.StreamHandler(sys.stderr) # set a format which is simpler for console use formatter = logging.Formatter('%(levelname)-6s %(message)s') # tell the handler to use this format - stdout.setFormatter(formatter) - logSys.addHandler(stdout) + logout.setFormatter(formatter) + logSys.addHandler(logout) # Set the configuration path self.__configurator.setBaseDir(self.__conf["conf"]) From b6be8b8243273bbe6549a1f1057e8633e767c314 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 21 Jun 2013 11:09:16 -0400 Subject: [PATCH 2/5] ENH/RF: remove __readJailConfig in favor of __readConfig + catch/error exceptions while reading the configuration --- fail2ban-client | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/fail2ban-client b/fail2ban-client index 57235d79..be48a613 100755 --- a/fail2ban-client +++ b/fail2ban-client @@ -229,7 +229,7 @@ class Fail2banClient: elif len(cmd) == 2 and cmd[0] == "reload": if self.__ping(): jail = cmd[1] - ret = self.__readJailConfig(jail) + ret = self.__readConfig(jail) # Do not continue if configuration is not 100% valid if not ret: return False @@ -394,19 +394,18 @@ class Fail2banClient: return False return self.__processCommand(args) - def __readConfig(self): + def __readConfig(self, jail=None): # Read the configuration - self.__configurator.readAll() - ret = self.__configurator.getOptions() - self.__configurator.convertToProtocol() - self.__stream = self.__configurator.getConfigStream() - return ret - - def __readJailConfig(self, jail): - self.__configurator.readAll() - ret = self.__configurator.getOptions(jail) - self.__configurator.convertToProtocol() - self.__stream = self.__configurator.getConfigStream() + # TODO: get away from stew of return codes and exception + # handling -- handle via exceptions + try: + self.__configurator.readAll() + ret = self.__configurator.getOptions(jail) + self.__configurator.convertToProtocol() + self.__stream = self.__configurator.getConfigStream() + except Exception, e: + logSys.error("Failed during configuration: %s" % e) + ret = False return ret #@staticmethod From 27947407bc7910f0f50972113218ebc73c4a22c7 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 21 Jun 2013 11:08:27 -0400 Subject: [PATCH 3/5] ENH: raise an exception if not a single file was found for the jail. Close #63 --- client/jailreader.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/jailreader.py b/client/jailreader.py index f8757e26..ef444125 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -107,12 +107,17 @@ class JailReader(ConfigReader): stream = [] for opt in self.__opts: if opt == "logpath": + found_files = 0 for path in self.__opts[opt].split("\n"): pathList = glob.glob(path) if len(pathList) == 0: - logSys.error("No file found for " + path) + logSys.error("No file(s) found for glob %s" % path) for p in pathList: + found_files += 1 stream.append(["set", self.__name, "addlogpath", p]) + if not found_files: + raise ValueError( + "Have not found any log file for %s jail" % self.__name) elif opt == "backend": backend = self.__opts[opt] elif opt == "maxretry": From 61f81c6b2f17576e970967a918997af55bc2b829 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 21 Jun 2013 11:12:27 -0400 Subject: [PATCH 4/5] Changelog entries with close statements for recent changes --- ChangeLog | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index f2425d2f..0dd7a073 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,7 +25,9 @@ ver. 0.8.11 (2013/XX/XXX) - wanna-be-released Yaroslav Halchenko * fail2ban-regex -- refactored to provide more details (missing and ignored lines, control over logging, etc) while maintaining look&feel - * fail2ban-client -- log to standard error + * fail2ban-client -- log to standard error. Closes gh-264 + * Fail to configure if not a single log file was found for an + enabled jail. Closes gh-63 ver. 0.8.10 (2013/06/12) - wanna-be-secure ----------- From 057f0ad1357273f027b944e046d66e486c2f197f Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 21 Jun 2013 12:44:37 -0400 Subject: [PATCH 5/5] ENH: allow_no_files option for jail's convert to allow testing of stock jail.conf --- client/jailreader.py | 13 +++++++++++-- client/jailsreader.py | 13 +++++++++++-- testcases/clientreadertestcase.py | 2 +- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/client/jailreader.py b/client/jailreader.py index ef444125..1a2be432 100644 --- a/client/jailreader.py +++ b/client/jailreader.py @@ -103,7 +103,16 @@ class JailReader(ConfigReader): logSys.warn("No actions were defined for %s" % self.__name) return True - def convert(self): + def convert(self, allow_no_files=False): + """Convert read before __opts to the commands stream + + Parameters + ---------- + allow_missing : bool + Either to allow log files to be missing entirely. Primarily is + used for testing + """ + stream = [] for opt in self.__opts: if opt == "logpath": @@ -115,7 +124,7 @@ class JailReader(ConfigReader): for p in pathList: found_files += 1 stream.append(["set", self.__name, "addlogpath", p]) - if not found_files: + if not (found_files or allow_no_files): raise ValueError( "Have not found any log file for %s jail" % self.__name) elif opt == "backend": diff --git a/client/jailsreader.py b/client/jailsreader.py index 098b525d..408a40bf 100644 --- a/client/jailsreader.py +++ b/client/jailsreader.py @@ -79,14 +79,23 @@ class JailsReader(ConfigReader): return False return True - def convert(self): + def convert(self, allow_no_files=False): + """Convert read before __opts and jails to the commands stream + + Parameters + ---------- + allow_missing : bool + Either to allow log files to be missing entirely. Primarily is + used for testing + """ + stream = list() for opt in self.__opts: if opt == "": stream.append([]) # Convert jails for jail in self.__jails: - stream.extend(jail.convert()) + stream.extend(jail.convert(allow_no_files=allow_no_files)) # Start jails for jail in self.__jails: stream.append(["start", jail.getName()]) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index a8c60bf8..948431b3 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -132,7 +132,7 @@ class JailsReaderTest(unittest.TestCase): jails = JailsReader(basedir='config', force_enable=True) # we are running tests from root project dir atm self.assertTrue(jails.read()) # opens fine self.assertTrue(jails.getOptions()) # reads fine - comm_commands = jails.convert() + comm_commands = jails.convert(allow_no_files=True) # by default we have lots of jails ;) self.assertTrue(len(comm_commands))