Merge branch 'master' of git://github.com/fail2ban/fail2ban

* 'master' of git://github.com/fail2ban/fail2ban:
  BF: log error only if there were missed config files that couldn't be read
  DOC: missing cinfo tags are ok. Log error for self referencing definitions
  DOC: s/defination/definition/g learn to spell
  ENH: remove stats of config files and use results of SafeConfigParserWithIncludes.read to facilitate meaningful error messages
  DOC: ChangeLog for recursive tag substition
  ENH: allow recursive tag substitution in action files.

ChangeLog

@@ -94,6 +94,8 @@ Borreli, blotus:
    * [c8c7b0b,23bbc60] Better logging of log file read errors.
    * [3665e6d] Added code coverage to development process.
    * [41b9f7b,32d10e9] More complete ssh filter rules to match openssh source.
+   * [1d9abd1] Action files can have tags in definition that refer to other
+     tags.
   Pascal Borreli
    * [a2b29b4] Fixed lots of typos in config files and documentation.
   hamilton5

client/configreader.py

@@ -52,9 +52,9 @@ class ConfigReader(SafeConfigParserWithIncludes):
 		return self._basedir
 	
 	def read(self, filename):
-		if not (os.path.exists(self._basedir) and os.access(self._basedir, os.R_OK | os.X_OK)):
+		if not os.path.exists(self._basedir):
-			raise ValueError("Base configuration directory %s either does not exist "
+			raise ValueError("Base configuration directory %s does not exist "
-							 "or is not accessible" % self._basedir)
+							  % self._basedir)
 		basename = os.path.join(self._basedir, filename)
 		logSys.debug("Reading configs for %s under %s "  % (basename, self._basedir))
 		config_files = [ basename + ".conf",
@@ -65,27 +65,20 @@ class ConfigReader(SafeConfigParserWithIncludes):
 
 		# possible further customizations under a .conf.d directory
 		config_dir = basename + '.d'
-		if os.path.exists(config_dir):
-			if os.path.isdir(config_dir) and os.access(config_dir, os.X_OK | os.R_OK):
-				# files must carry .conf suffix as well
 		config_files += sorted(glob.glob('%s/*.conf' % config_dir))
-			else:
-				logSys.warn("%s exists but not a directory or not accessible"
-							 % config_dir)
-
-		# check if files are accessible, warn if any is not accessible
-		# and remove it from the list
-		config_files_accessible = []
-		for f in config_files:
-			if os.access(f, os.R_OK):
-				config_files_accessible.append(f)
-			else:
-				logSys.warn("%s exists but not accessible - skipping" % f)
 
-		if len(config_files_accessible):
+		if len(config_files):
 			# at least one config exists and accessible
-			SafeConfigParserWithIncludes.read(self, config_files_accessible)
+			logSys.debug("Reading config files: " + ', '.join(config_files))
+			config_files_read = SafeConfigParserWithIncludes.read(self, config_files)
+			missed = [ cf for cf in config_files if cf not in config_files_read ]
+			if missed:
+				logSys.error("Could not read config files: " + ', '.join(missed))
+			if config_files_read:
 				return True
+			logSys.error("Found no accessible config files for %r under %s" %
+						 ( filename, self.getBaseDir() ))
+			return False
 		else:
 			logSys.error("Found no accessible config files for %r " % filename
 						 + (["under %s" % self.getBaseDir(),

server/action.py

@@ -22,7 +22,7 @@ __copyright__ = "Copyright (c) 2004 Cyril Jaquier, 2011-2012 Yaroslav Halchenko"
 __license__ = "GPL"
 
 import logging, os
-import threading
+import threading, re
 #from subprocess import call
 
 # Gets the instance of the logger.
@@ -137,6 +137,10 @@ class Action:
 	# @return True if the command succeeded
 	
 	def execActionStart(self):
+		if self.__cInfo:
+			if not Action.substituteRecursiveTags(self.__cInfo):
+				logSys.error("Cinfo/definitions contain self referencing definitions and cannot be resolved")
+				return False
 		startCmd = Action.replaceTag(self.__actionStart, self.__cInfo)
 		return Action.executeCmd(startCmd)
 	
@@ -236,6 +240,38 @@ class Action:
 		stopCmd = Action.replaceTag(self.__actionStop, self.__cInfo)
 		return Action.executeCmd(stopCmd)
 
+	##
+	# Sort out tag definitions within other tags
+	#
+	# so:		becomes:
+	# a = 3		a = 3
+	# b = <a>_3	b = 3_3
+	# @param	tags, a dictionary
+	# @returns	tags altered or False if there is a recursive definition
+	#@staticmethod
+	def substituteRecursiveTags(tags):
+		t = re.compile(r'<([^ >]+)>')
+		for tag, value in tags.iteritems():
+			value = str(value)
+			m = t.search(value)
+			while m:
+				if m.group(1) == tag:
+					# recursive definitions are bad
+					return False
+				else:
+					if tags.has_key(m.group(1)):
+						value = value[0:m.start()] + tags[m.group(1)] + value[m.end():]
+						m = t.search(value, m.start())
+					else:
+						# Missing tags are ok so we just continue on searching.
+						# cInfo can contain aInfo elements like <HOST> and valid shell
+						# constructs like <STDIN>.
+						m = t.search(value, m.start() + 1)
+			tags[tag] = value
+		return tags
+	substituteRecursiveTags = staticmethod(substituteRecursiveTags)
+
+	#@staticmethod
 	def escapeTag(tag):
 		for c in '\\#&;`|*?~<>^()[]{}$\n\'"':
 			if c in tag:

testcases/actiontestcase.py

@@ -61,6 +61,27 @@ class ExecuteAction(unittest.TestCase):
 	def _is_logged(self, s):
 		return s in self._log.getvalue()
 
+	def testSubstituteRecursiveTags(self):
+		aInfo = {
+			'HOST': "192.0.2.0",
+			'ABC': "123 <HOST>",
+			'xyz': "890 <ABC>",
+		}
+		# Recursion is bad
+		self.assertFalse(Action.substituteRecursiveTags({'A': '<A>'}))
+		self.assertFalse(Action.substituteRecursiveTags({'A': '<B>', 'B': '<A>'}))
+		self.assertFalse(Action.substituteRecursiveTags({'A': '<B>', 'B': '<C>', 'C': '<A>'}))
+		# missing tags are ok
+		self.assertEquals(Action.substituteRecursiveTags({'A': '<C>'}), {'A': '<C>'})
+		self.assertEquals(Action.substituteRecursiveTags({'A': '<C> <D> <X>','X':'fun'}), {'A': '<C> <D> fun', 'X':'fun'})
+		self.assertEquals(Action.substituteRecursiveTags({'A': '<C> <B>', 'B': 'cool'}), {'A': '<C> cool', 'B': 'cool'})
+		# rest is just cool
+		self.assertEquals(Action.substituteRecursiveTags(aInfo),
+								{ 'HOST': "192.0.2.0",
+									'ABC': '123 192.0.2.0',
+									'xyz': '890 123 192.0.2.0',
+								})
+
 	def testReplaceTag(self):
 		aInfo = {
 			'HOST': "192.0.2.0",