better and scalable solution for gh-867 (and gh-868), using only name convention like %(known/failregex)s to add custom expressions, so no interface changes in jail.conf are necessary (for example see test-known-interp in test cases);

ChangeLog

@@ -19,10 +19,10 @@ ver. 0.9.2 (2014/XX/XXX) - wanna-be-released
 
 
 - New Features:
-   - new interpolation feature for config readers - `%(known/parameter)s`.
-     (means last known option with name `parameter`).
-   - new options for jail introduced addfailregex/addignoreregex: extends regex 
-     specified in filter (opposite to failregex/ignoreregex that overwrites it)
+   - New interpolation feature for config readers - `%(known/parameter)s`.
+     (means last known option with name `parameter`). This interpolation makes
+     possible to extend a stock filter or jail regexp in .local file
+     (opposite to simply set failregex/ignoreregex that overwrites it),
      see gh-867.
 
 - Enhancements:

fail2ban/client/configparserinc.py

@@ -249,3 +249,12 @@ after = 1.conf
 			return SafeConfigParser.read(self, fileNamesFull, encoding='utf-8')
 		else:
 			return SafeConfigParser.read(self, fileNamesFull)
+
+	def merge_section(self, section, options, pref='known/'):
+		alls = self.get_sections()
+		sk = {}
+		for k, v in options.iteritems():
+			if pref == '' or not k.startswith(pref):
+				sk[pref+k] = v
+		alls[section].update(sk)
+

fail2ban/client/configreader.py

@@ -116,6 +116,10 @@ class ConfigReader():
 			return self._cfg.has_section(sec)
 		return False
 
+	def merge_section(self, *args, **kwargs):
+		if self._cfg is not None:
+			return self._cfg.merge_section(*args, **kwargs)
+
 	def options(self, *args):
 		if self._cfg is not None:
 			return self._cfg.options(*args)

fail2ban/client/filterreader.py

@@ -47,14 +47,20 @@ class FilterReader(DefinitionInitConfigReader):
 	def getFile(self):
 		return self.__file
 
-	def convert(self):
-		stream = list()
+	def getCombined(self):
 		combinedopts = dict(list(self._opts.items()) + list(self._initOpts.items()))
 		if not len(combinedopts):
-			return stream;
+			return {};
 		opts = CommandAction.substituteRecursiveTags(combinedopts)
 		if not opts:
 			raise ValueError('recursive tag definitions unable to be resolved')
+		return opts;
+	
+	def convert(self):
+		stream = list()
+		opts = self.getCombined()
+		if not len(opts):
+			return stream;
 		for opt, value in opts.iteritems():
 			if opt == "failregex":
 				for regex in value.split('\n'):

fail2ban/client/jailreader.py

@@ -87,6 +87,8 @@ class JailReader(ConfigReader):
 		return pathList
 
 	def getOptions(self):
+		opts1st = [["bool", "enabled", False],
+				["string", "filter", ""]]
 		opts = [["bool", "enabled", False],
 				["string", "logpath", None],
 				["string", "logencoding", None],
@@ -97,13 +99,13 @@ class JailReader(ConfigReader):
 				["string", "usedns", None],
 				["string", "failregex", None],
 				["string", "ignoreregex", None],
-				["string", "addfailregex", None],
-				["string", "addignoreregex", None],
 				["string", "ignorecommand", None],
 				["string", "ignoreip", None],
 				["string", "filter", ""],
 				["string", "action", ""]]
-		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)
 		if not self.__opts:
 			return False
 		
@@ -115,15 +117,25 @@ class JailReader(ConfigReader):
 				self.__filter = FilterReader(
 					filterName, self.__name, filterOpt, share_config=self.share_config, basedir=self.getBaseDir())
 				ret = self.__filter.read()
-				if ret:
+				# merge options from filter as 'known/...':
 				self.__filter.getOptions(self.__opts)
-				else:
+				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)
 
+			# Read second all options (so variables like %(known/param) can be interpolated):
+			self.__opts = ConfigReader.getOptions(self, self.__name, opts)
+			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:
@@ -203,14 +215,14 @@ class JailReader(ConfigReader):
 				stream.append(["set", self.__name, "bantime", self.__opts[opt]])
 			elif opt == "usedns":
 				stream.append(["set", self.__name, "usedns", self.__opts[opt]])
-			elif opt in ("failregex", "addfailregex"):
+			elif opt == "failregex":
 				for regex in self.__opts[opt].split('\n'):
 					# Do not send a command if the rule is empty.
 					if regex != '':
 						stream.append(["set", self.__name, "addfailregex", regex])
 			elif opt == "ignorecommand":
 				stream.append(["set", self.__name, "ignorecommand", self.__opts[opt]])
-			elif opt in ("ignoreregex", "addignoreregex"):
+			elif opt == "ignoreregex":
 				for regex in self.__opts[opt].split('\n'):
 					# Do not send a command if the rule is empty.
 					if regex != '':

fail2ban/tests/clientreadertestcase.py

@@ -155,12 +155,16 @@ c = d ;in line comment
 
 class JailReaderTest(LogCaptureTestCase):
 
+	def __init__(self, *args, **kwargs):
+		super(JailReaderTest, self).__init__(*args, **kwargs)
+		self.__share_cfg = {}
+
 	def testIncorrectJail(self):
-		jail = JailReader('XXXABSENTXXX', basedir=CONFIG_DIR)
+		jail = JailReader('XXXABSENTXXX', basedir=CONFIG_DIR, share_config = self.__share_cfg)
 		self.assertRaises(ValueError, jail.read)
 		
 	def testJailActionEmpty(self):
-		jail = JailReader('emptyaction', basedir=IMPERFECT_CONFIG)
+		jail = JailReader('emptyaction', basedir=IMPERFECT_CONFIG, share_config = self.__share_cfg)
 		self.assertTrue(jail.read())
 		self.assertTrue(jail.getOptions())
 		self.assertTrue(jail.isEnabled())
@@ -168,7 +172,7 @@ class JailReaderTest(LogCaptureTestCase):
 		self.assertTrue(self._is_logged('No actions were defined for emptyaction'))
 
 	def testJailActionFilterMissing(self):
-		jail = JailReader('missingbitsjail', basedir=IMPERFECT_CONFIG)
+		jail = JailReader('missingbitsjail', basedir=IMPERFECT_CONFIG, share_config = self.__share_cfg)
 		self.assertTrue(jail.read())
 		self.assertFalse(jail.getOptions())
 		self.assertTrue(jail.isEnabled())
@@ -176,7 +180,7 @@ class JailReaderTest(LogCaptureTestCase):
 		self.assertTrue(self._is_logged('Unable to read the filter'))
 
 	def TODOtestJailActionBrokenDef(self):
-		jail = JailReader('brokenactiondef', basedir=IMPERFECT_CONFIG)
+		jail = JailReader('brokenactiondef', basedir=IMPERFECT_CONFIG, share_config = self.__share_cfg)
 		self.assertTrue(jail.read())
 		self.assertFalse(jail.getOptions())
 		self.assertTrue(jail.isEnabled())
@@ -187,7 +191,7 @@ class JailReaderTest(LogCaptureTestCase):
 
 	if STOCK:
 		def testStockSSHJail(self):
-			jail = JailReader('sshd', basedir=CONFIG_DIR) # we are running tests from root project dir atm
+			jail = JailReader('sshd', basedir=CONFIG_DIR, share_config = self.__share_cfg) # we are running tests from root project dir atm
 			self.assertTrue(jail.read())
 			self.assertTrue(jail.getOptions())
 			self.assertFalse(jail.isEnabled())
@@ -411,13 +415,17 @@ class JailsReaderTestCache(LogCaptureTestCase):
 
 class JailsReaderTest(LogCaptureTestCase):
 
+	def __init__(self, *args, **kwargs):
+		super(JailsReaderTest, self).__init__(*args, **kwargs)
+		self.__share_cfg = {}
+
 	def testProvidingBadBasedir(self):
 		if not os.path.exists('/XXX'):
 			reader = JailsReader(basedir='/XXX')
 			self.assertRaises(ValueError, reader.read)
 
 	def testReadTestJailConf(self):
-		jails = JailsReader(basedir=IMPERFECT_CONFIG)
+		jails = JailsReader(basedir=IMPERFECT_CONFIG, share_config=self.__share_cfg)
 		self.assertTrue(jails.read())
 		self.assertFalse(jails.getOptions())
 		self.assertRaises(ValueError, jails.convert)
@@ -425,22 +433,11 @@ class JailsReaderTest(LogCaptureTestCase):
 		self.maxDiff = None
 		self.assertEqual(sorted(comm_commands),
 			sorted([['add', 'emptyaction', 'auto'],
-			 ['add', 'test1addfailregex', 'auto'], 
-			 ['set', 'test1addfailregex', 'addfailregex', 'failure 1 <HOST>'], 
-			 ['set', 'test1addfailregex', 'addfailregex', 'failure 2 <HOST>'], 
-			 ['set', 'test1addfailregex', 'addignoreregex', 'ignore 1 <HOST>'], 
-			 ['set', 'test1addfailregex', 'addignoreregex', 'ignore 2 <HOST>'], 
-			 ['set', 'test1addfailregex', 'addfailregex', '<IP>'],
-			 ['start', 'test1addfailregex'],
-			 ['add', 'test2failregex', 'auto'], 
-			 ['set', 'test2failregex', 'addfailregex', 'failure 1 <IP>'], 
-			 ['set', 'test2failregex', 'addfailregex', 'failure 2 <IP>'],
-			 ['start', 'test2failregex'],
-			 ['add', 'test3known-interp', 'auto'],
-			 ['set', 'test3known-interp', 'addfailregex', 'failure test 1 (filter.d/test.conf) <HOST>'],
-			 ['set', 'test3known-interp', 'addfailregex', 'failure test 2 (filter.d/test.local) <HOST>'],
-			 ['set', 'test3known-interp', 'addfailregex', 'failure test 3 (jail.local) <HOST>'],
-			 ['start', 'test3known-interp'],
+			 ['add', 'test-known-interp', 'auto'],
+			 ['set', 'test-known-interp', 'addfailregex', 'failure test 1 (filter.d/test.conf) <HOST>'],
+			 ['set', 'test-known-interp', 'addfailregex', 'failure test 2 (filter.d/test.local) <HOST>'],
+			 ['set', 'test-known-interp', 'addfailregex', 'failure test 3 (jail.local) <HOST>'],
+			 ['start', 'test-known-interp'],
 			 ['add', 'missinglogfiles', 'auto'],
 			 ['set', 'missinglogfiles', 'addfailregex', '<IP>'],
 			 ['add', 'brokenaction', 'auto'],
@@ -463,7 +460,7 @@ class JailsReaderTest(LogCaptureTestCase):
 
 	if STOCK:
 		def testReadStockJailConf(self):
-			jails = JailsReader(basedir=CONFIG_DIR) # we are running tests from root project dir atm
+			jails = JailsReader(basedir=CONFIG_DIR, share_config=self.__share_cfg) # 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()
@@ -524,7 +521,7 @@ class JailsReaderTest(LogCaptureTestCase):
 
 		# Verify that all filters found under config/ have a jail
 		def testReadStockJailFilterComplete(self):
-			jails = JailsReader(basedir=CONFIG_DIR, force_enable=True)
+			jails = JailsReader(basedir=CONFIG_DIR, force_enable=True, share_config=self.__share_cfg)
 			self.assertTrue(jails.read())             # opens fine
 			self.assertTrue(jails.getOptions())       # reads fine
 			# grab all filter names
@@ -541,7 +538,7 @@ class JailsReaderTest(LogCaptureTestCase):
 		def testReadStockJailConfForceEnabled(self):
 			# more of a smoke test to make sure that no obvious surprises
 			# on users' systems when enabling shipped jails
-			jails = JailsReader(basedir=CONFIG_DIR, force_enable=True) # we are running tests from root project dir atm
+			jails = JailsReader(basedir=CONFIG_DIR, force_enable=True, share_config=self.__share_cfg) # 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(allow_no_files=True)
@@ -636,7 +633,7 @@ action = testaction1[actname=test1]
 filter = testfilter1
 """)
 		jailfd.close()
-		jails = JailsReader(basedir=basedir)
+		jails = JailsReader(basedir=basedir, share_config=self.__share_cfg)
 		self.assertTrue(jails.read())
 		self.assertTrue(jails.getOptions())
 		comm_commands = jails.convert(allow_no_files=True)

fail2ban/tests/config/jail.conf

@@ -13,24 +13,11 @@ failregex = <IP>
 ignoreregex =
 ignoreip =
 
-[test1addfailregex]
-enabled = true
-filter = simple
-addfailregex = failure 1 <HOST>
-               failure 2 <HOST>
-addignoreregex = ignore 1 <HOST>
-                 ignore 2 <HOST>
-
-[test2failregex]
-enabled = true
-filter = simple
-failregex = failure 1 <IP>
-            failure 2 <IP>
-
-[test3known-interp]
+[test-known-interp]
 enabled = true
 filter = test
-addfailregex = failure test 3 (jail.local) <HOST>
+failregex = %(known/failregex)s
+            failure test 3 (jail.local) <HOST>
 
 [missinglogfiles]
 enabled = true