Merge pull request #265 from yarikoptic/master

Improve fail2ban-client errors logging/handling -- fail if no log file could be read, dump logs to stderr

ChangeLog

@@ -24,7 +24,10 @@ 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. 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
 -----------

client/jailreader.py

@@ -103,16 +103,30 @@ 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":
+				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 or allow_no_files):
+					raise ValueError(
+						"Have not found any log file for %s jail" % self.__name)
 			elif opt == "backend":
 				backend = self.__opts[opt]
 			elif opt == "maxretry":

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()])

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
@@ -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"])
@@ -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

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))