From 77dc5a334c0e1ba79482a7799fefef27b6df413e Mon Sep 17 00:00:00 2001
From: sebres <serg.brester@sebres.de>
Date: Tue, 22 Nov 2016 13:19:09 +0100
Subject: [PATCH] really skips invalid jails (because of theirs wrong
 configuration) - server starts nevertheless, as long as one jail was
 successful configured; message about wrong jail configuration logged in
 client log (stdout, systemd journal etc.) and in server log as error

---
 fail2ban/client/jailreader.py  | 134 ++++++++++++++++++---------------
 fail2ban/client/jailsreader.py |  13 +++-
 fail2ban/server/transmitter.py |   3 +
 3 files changed, 86 insertions(+), 64 deletions(-)

diff --git a/fail2ban/client/jailreader.py b/fail2ban/client/jailreader.py
index 9d01a693..df6c6664 100644
--- a/fail2ban/client/jailreader.py
+++ b/fail2ban/client/jailreader.py
@@ -118,70 +118,78 @@ class JailReader(ConfigReader):
 		defsec = self._cfg.get_defaults()
 		defsec["fail2ban_version"] = version
 
-		# Read first options only needed for merge defaults ('known/...' from filter):
-		self.__opts = ConfigReader.getOptions(self, self.__name, opts1st, shouldExist=True)
-		if not self.__opts:
-			return False
-		
-		if self.isEnabled():
-			# Read filter
-			if self.__opts["filter"]:
-				filterName, filterOpt = JailReader.extractOptions(
-					self.__opts["filter"])
-				self.__filter = FilterReader(
-					filterName, self.__name, filterOpt, share_config=self.share_config, basedir=self.getBaseDir())
-				ret = self.__filter.read()
-				# merge options from filter as 'known/...':
-				self.__filter.getOptions(self.__opts)
-				ConfigReader.merge_section(self, self.__name, self.__filter.getCombined(), 'known/')
-				if not ret:
-					logSys.error("Unable to read the filter")
-					return False
-			else:
-				self.__filter = None
-				logSys.warning("No filter set for jail %s" % self.__name)
+		try:
 
-			# Read second all options (so variables like %(known/param) can be interpolated):
-			self.__opts = ConfigReader.getOptions(self, self.__name, opts)
+			# Read first options only needed for merge defaults ('known/...' from filter):
+			self.__opts = ConfigReader.getOptions(self, self.__name, opts1st, shouldExist=True)
 			if not self.__opts:
-				return False
-		
-			# cumulate filter options again (ignore given in jail):
-			if self.__filter:
-				self.__filter.getOptions(self.__opts)
-		
-			# Read action
-			for act in self.__opts["action"].split('\n'):
-				try:
-					if not act:			  # skip empty actions
-						continue
-					actName, actOpt = JailReader.extractOptions(act)
-					if actName.endswith(".py"):
-						self.__actions.append([
-							"set",
-							self.__name,
-							"addaction",
-							actOpt.pop("actname", os.path.splitext(actName)[0]),
-							os.path.join(
-								self.getBaseDir(), "action.d", actName),
-							json.dumps(actOpt),
-							])
-					else:
-						action = ActionReader(
-							actName, self.__name, actOpt,
-							share_config=self.share_config, basedir=self.getBaseDir())
-						ret = action.read()
-						if ret:
-							action.getOptions(self.__opts)
-							self.__actions.append(action)
+				raise ValueError("Init jail options failed")
+			
+			if self.isEnabled():
+				# Read filter
+				if self.__opts["filter"]:
+					filterName, filterOpt = JailReader.extractOptions(
+						self.__opts["filter"])
+					self.__filter = FilterReader(
+						filterName, self.__name, filterOpt, share_config=self.share_config, basedir=self.getBaseDir())
+					ret = self.__filter.read()
+					# merge options from filter as 'known/...':
+					self.__filter.getOptions(self.__opts)
+					ConfigReader.merge_section(self, self.__name, self.__filter.getCombined(), 'known/')
+					if not ret:
+						raise ValueError("Unable to read the filter %r" % filterName)
+				else:
+					self.__filter = None
+					logSys.warning("No filter set for jail %s" % self.__name)
+
+				# Read second all options (so variables like %(known/param) can be interpolated):
+				self.__opts = ConfigReader.getOptions(self, self.__name, opts)
+				if not self.__opts:
+					raise ValueError("Read jail options failed")
+			
+				# cumulate filter options again (ignore given in jail):
+				if self.__filter:
+					self.__filter.getOptions(self.__opts)
+			
+				# Read action
+				for act in self.__opts["action"].split('\n'):
+					try:
+						if not act:			  # skip empty actions
+							continue
+						actName, actOpt = JailReader.extractOptions(act)
+						if actName.endswith(".py"):
+							self.__actions.append([
+								"set",
+								self.__name,
+								"addaction",
+								actOpt.pop("actname", os.path.splitext(actName)[0]),
+								os.path.join(
+									self.getBaseDir(), "action.d", actName),
+								json.dumps(actOpt),
+								])
 						else:
-							raise AttributeError("Unable to read action")
-				except Exception as e:
-					logSys.error("Error in action definition " + act)
-					logSys.debug("Caught exception: %s" % (e,))
-					return False
-			if not len(self.__actions):
-				logSys.warning("No actions were defined for %s" % self.__name)
+							action = ActionReader(
+								actName, self.__name, actOpt,
+								share_config=self.share_config, basedir=self.getBaseDir())
+							ret = action.read()
+							if ret:
+								action.getOptions(self.__opts)
+								self.__actions.append(action)
+							else:
+								raise AttributeError("Unable to read action")
+					except Exception as e:
+						logSys.debug("Caught exception: %s" % (e,))
+						raise ValueError("Error in action definition %r" % e)
+				if not len(self.__actions):
+					logSys.warning("No actions were defined for %s" % self.__name)
+			
+		except ValueError as e:
+			e = str(e)
+			logSys.error(e)
+			if not self.__opts:
+				self.__opts = dict()
+			self.__opts['config-error'] = e
+			return False
 		return True
 	
 	def convert(self, allow_no_files=False):
@@ -195,6 +203,10 @@ class JailReader(ConfigReader):
 		 """
 
 		stream = []
+		e = self.__opts.get('config-error')
+		if e:
+			stream.extend([['config-error', "Jail '%s' skipped, because of wrong configuration: %s" % (self.__name, e)]])
+			return stream
 		for opt, value in self.__opts.iteritems():
 			if opt == "logpath" and	\
 					not self.__opts.get('backend', None).startswith("systemd"):
diff --git a/fail2ban/client/jailsreader.py b/fail2ban/client/jailsreader.py
index 09725ec9..ec7baca7 100644
--- a/fail2ban/client/jailsreader.py
+++ b/fail2ban/client/jailsreader.py
@@ -66,7 +66,7 @@ class JailsReader(ConfigReader):
 			sections = [ section ]
 
 		# Get the options of all jails.
-		parse_status = True
+		parse_status = None
 		for sec in sections:
 			if sec == 'INCLUDES':
 				continue
@@ -77,11 +77,17 @@ class JailsReader(ConfigReader):
 			ret = jail.getOptions()
 			if ret:
 				if jail.isEnabled():
+					# at least one jail was successful:
+					parse_status = True
 					# We only add enabled jails
 					self.__jails.append(jail)
 			else:
 				logSys.error("Errors in jail %r. Skipping..." % sec)
-				parse_status = False
+				self.__jails.append(jail)
+				if parse_status is None:
+					parse_status = False
+		if parse_status is None:
+			parse_status = True
 		return parse_status
 
 	def convert(self, allow_no_files=False):
@@ -103,7 +109,8 @@ class JailsReader(ConfigReader):
 			stream.extend(jail.convert(allow_no_files=allow_no_files))
 		# Start jails
 		for jail in self.__jails:
-			stream.append(["start", jail.getName()])
+			if not jail.options.get('config-error'):
+				stream.append(["start", jail.getName()])
 
 		return stream
 
diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py
index 2f5be043..ae1017b9 100644
--- a/fail2ban/server/transmitter.py
+++ b/fail2ban/server/transmitter.py
@@ -131,6 +131,9 @@ class Transmitter:
 			return self.status(command[1:])
 		elif command[0] == "version":
 			return version.version
+		elif command[0] == "config-error":
+			logSys.error(command[1])
+			return None
 		raise Exception("Invalid command")
 	
 	def __commandSet(self, command, multiple=False):