BF+TST: Fix handling of spaces and + char for journalmatch

Previous fix attempted shlex split which whilst worked for reading from
config file, failed when using fail2ban-client, as the input is already
effectively shelx split by the executing shell.

FilterSystemd journal match methods now handle list structures which
should be shlex split when reading from config file, and simply pass all
the relevant arguments from the shell when using fail2ban-client

fail2ban/client/beautifier.py

@@ -118,7 +118,7 @@ class Beautifier:
 					msg = "No journal match filter set"
 				else:
 					msg = "Current match filter:\n"
-					msg += ' + '.join(response)
+					msg += ' + '.join(" ".join(res) for res in response)
 			elif inC[2] in ("ignoreip", "addignoreip", "delignoreip"):
 				if len(response) == 0:
 					msg = "No IP address/network is ignored"

fail2ban/client/filterreader.py

@@ -24,7 +24,7 @@ __author__ = "Cyril Jaquier"
 __copyright__ = "Copyright (c) 2004 Cyril Jaquier"
 __license__ = "GPL"
 
-import logging, os
+import logging, os, shlex
 from configreader import ConfigReader, DefinitionInitConfigReader
 
 # Gets the instance of the logger.
@@ -59,6 +59,8 @@ class FilterReader(DefinitionInitConfigReader):
 			# Do not send a command if the match is empty.
 			if self._initOpts.get("journalmatch", '') != '':
 				for match in self._initOpts["journalmatch"].split("\n"):
-					stream.append(["set", self._jailName, "addjournalmatch", match])
+					stream.append(
+						["set", self._jailName, "addjournalmatch"] +
+                        shlex.split(match))
 		return stream
 		

fail2ban/server/filtersystemd.py

@@ -23,7 +23,7 @@ __author__ = "Cyril Jaquier, Lee Clemens, Yaroslav Halchenko, Steven Hiscocks"
 __copyright__ = "Copyright (c) 2004 Cyril Jaquier, 2011-2012 Lee Clemens, 2012 Yaroslav Halchenko, 2013 Steven Hiscocks"
 __license__ = "GPL"
 
-import logging, datetime, shlex
+import logging, datetime
 from distutils.version import LooseVersion
 
 from systemd import journal
@@ -60,31 +60,45 @@ class FilterSystemd(JournalFilter): # pragma: systemd no cover
 		self.__matches = []
 		logSys.debug("Created FilterSystemd")
 
+
+	##
+	# Add a journal match filters from list structure
+	#
+	# @param matches list structure with journal matches
+
+	def _addJournalMatches(self, matches):
+		if self.__matches:
+			self.__journal.add_disjunction() # Add OR
+		newMatches = []
+		for match in matches:
+			newMatches.append([])
+			for match_element in match:
+				self.__journal.add_match(match_element)
+				newMatches[-1].append(match_element)
+			self.__journal.add_disjunction()
+		self.__matches.extend(newMatches)
+
 	##
 	# Add a journal match filter
 	#
-	# @param match journalctl syntax matches
+	# @param match journalctl syntax matches in list structure
 
 	def addJournalMatch(self, match):
-		if self.__matches:
-			self.__journal.add_disjunction() # Add OR
 		newMatches = [[]]
+		for match_element in match:
+			if match_element == "+":
+				newMatches.append([])
+			else:
+				newMatches[-1].append(match_element)
 		try:
-			for match_element in shlex.split(match):
-				if match_element == "+":
-					self.__journal.add_disjunction()
-					newMatches.append([])
-				else:
-					self.__journal.add_match(match_element)
-					newMatches[-1].append(match_element)
+			self._addJournalMatches(newMatches)
 		except ValueError:
-			logSys.error("Error adding journal match for: %s", match)
+			logSys.error(
+				"Error adding journal match for: %r", " ".join(match))
 			self.resetJournalMatches()
 			raise
 		else:
-			self.__matches.extend(
-				" ".join(newMatch) for newMatch in newMatches)
-			logSys.debug("Adding journal match for: %s", match)
+			logSys.info("Added journal match for: %r", " ".join(match))
 	##
 	# Reset a journal match filter called on removal or failure
 	#
@@ -95,8 +109,13 @@ class FilterSystemd(JournalFilter): # pragma: systemd no cover
 		logSys.debug("Flushed all journal matches")
 		match_copy = self.__matches[:]
 		self.__matches = []
-		for match in match_copy:
-			self.addJournalMatch(match)
+		try:
+			self._addJournalMatches(match_copy)
+		except ValueError:
+			logSys.error("Error restoring journal matches")
+			raise
+		else:
+			logSys.debug("Journal matches restored")
 
 	##
 	# Delete a journal match filter
@@ -104,12 +123,12 @@ class FilterSystemd(JournalFilter): # pragma: systemd no cover
 	# @param match journalctl syntax matches
 
 	def delJournalMatch(self, match):
-		match = " ".join(shlex.split(match))
 		if match in self.__matches:
 			del self.__matches[self.__matches.index(match)]
 			self.resetJournalMatches()
 		else:
 			raise ValueError("Match not found")
+		logSys.info("Removed journal match for: %r" % " ".join(match))
 
 	##
 	# Get current journal match filter

fail2ban/server/transmitter.py

@@ -145,11 +145,11 @@ class Transmitter:
 			self.__server.setLogEncoding(name, value)
 			return self.__server.getLogEncoding(name)
 		elif command[1] == "addjournalmatch": # pragma: systemd no cover
-			value = ' '.join(command[2:])
+			value = command[2:]
 			self.__server.addJournalMatch(name, value)
 			return self.__server.getJournalMatch(name)
 		elif command[1] == "deljournalmatch": # pragma: systemd no cover
-			value = ' '.join(command[2:])
+			value = command[2:]
 			self.__server.delJournalMatch(name, value)
 			return self.__server.getJournalMatch(name)
 		elif command[1] == "addfailregex":

fail2ban/tests/clientreadertestcase.py

@@ -197,7 +197,12 @@ class FilterReaderTest(unittest.TestCase):
 			"+$<SKIPLINES>^.+ module for .* from <HOST>\\s*$"],
 			['set', 'testcase01', 'addignoreregex', 
 			"^.+ john from host 192.168.1.1\\s*$"],
-			['set', 'testcase01', 'maxlines', "1"]]
+			['set', 'testcase01', 'addjournalmatch',
+				"_COMM=sshd", "+", "_SYSTEMD_UNIT=sshd.service", "_UID=0"],
+			['set', 'testcase01', 'addjournalmatch',
+				"FIELD= with spaces ", "+", "AFIELD= with + char and spaces"],
+			['set', 'testcase01', 'maxlines', "1"], # Last for overide test
+		]
 		filterReader = FilterReader("testcase01", "testcase01", {})
 		filterReader.setBaseDir(TEST_FILES_DIR)
 		filterReader.read()

fail2ban/tests/files/filter.d/testcase01.conf

@@ -36,3 +36,10 @@ ignoreregex = ^.+ john from host 192.168.1.1\s*$
 [Init]
 # "maxlines" is number of log lines to buffer for multi-line regex searches
 maxlines = 1
+
+# Option:  journalmatch
+# Notes.:  systemd journalctl style match filter for journal based backends
+# Values:  TEXT
+#
+journalmatch = _COMM=sshd + _SYSTEMD_UNIT=sshd.service _UID=0
+               "FIELD= with spaces " + AFIELD=" with + char and spaces"

fail2ban/tests/filtertestcase.py

@@ -622,14 +622,14 @@ def get_monitor_failures_journal_testcase(Filter_): # pragma: systemd no cover
 			import uuid
 			self.test_uuid = str(uuid.uuid4())
 			self.name = "monitorjournalfailures-%s" % self.test_uuid
-			self.filter.addJournalMatch(
-				"SYSLOG_IDENTIFIER=fail2ban-testcases "
-				"TEST_FIELD=1 "
-				"TEST_UUID=%s" % str(self.test_uuid))
-			self.filter.addJournalMatch(
-				"SYSLOG_IDENTIFIER=fail2ban-testcases "
-				"TEST_FIELD=2 "
-				"TEST_UUID=%s" % self.test_uuid)
+			self.filter.addJournalMatch([
+				"SYSLOG_IDENTIFIER=fail2ban-testcases",
+				"TEST_FIELD=1",
+				"TEST_UUID=%s" % self.test_uuid])
+			self.filter.addJournalMatch([
+				"SYSLOG_IDENTIFIER=fail2ban-testcases",
+				"TEST_FIELD=2",
+				"TEST_UUID=%s" % self.test_uuid])
 			self.journal_fields = {
 				'TEST_FIELD': "1", 'TEST_UUID': self.test_uuid}
 			self.filter.setActive(True)
@@ -708,10 +708,10 @@ def get_monitor_failures_journal_testcase(Filter_): # pragma: systemd no cover
 			self.assert_correct_ban("193.168.0.128", 3)
 
 			# and now remove the JournalMatch
-			self.filter.delJournalMatch(
-				"SYSLOG_IDENTIFIER=fail2ban-testcases "
-				"TEST_FIELD=1 "
-				"TEST_UUID=%s" % str(self.test_uuid))
+			self.filter.delJournalMatch([
+				"SYSLOG_IDENTIFIER=fail2ban-testcases",
+				"TEST_FIELD=1",
+				"TEST_UUID=%s" % self.test_uuid])
 
 			_copy_lines_to_journal(
 				self.test_file, self.journal_fields, n=5, skip=5)
@@ -719,10 +719,10 @@ def get_monitor_failures_journal_testcase(Filter_): # pragma: systemd no cover
 			self.assertTrue(self.isEmpty(2))
 
 			# but then if we add it back again
-			self.filter.addJournalMatch(
-				"SYSLOG_IDENTIFIER=fail2ban-testcases "
-				"TEST_FIELD=1 "
-				"TEST_UUID=%s" % str(self.test_uuid))
+			self.filter.addJournalMatch([
+				"SYSLOG_IDENTIFIER=fail2ban-testcases",
+				"TEST_FIELD=1",
+				"TEST_UUID=%s" % self.test_uuid])
 			self.assert_correct_ban("193.168.0.128", 4)
 			_copy_lines_to_journal(
 				self.test_file, self.journal_fields, n=6, skip=10)

fail2ban/tests/servertestcase.py

@@ -506,68 +506,58 @@ class Transmitter(TransmitterBase):
 		def testJournalMatch(self):
 			jailName = "TestJail2"
 			self.server.addJail(jailName, "systemd")
-			self.jailAddDelTest(
-				"journalmatch",
-				[
-					"_SYSTEMD_UNIT=sshd.service",
-					"TEST_FIELD1=ABC TEST_FIELD2=123",
-					"_HOSTNAME=example.com",
-				],
-				jailName
-			)
 			values = [
-				'"FIELD=Test + Value+ \\\"Test+Value=\'Test"',
-				'FIELD="Test + Value+ \\\"Test+Value=\'Test"',
+				"_SYSTEMD_UNIT=sshd.service",
+				"TEST_FIELD1=ABC",
+				"_HOSTNAME=example.com",
 			]
-
-			# Test shell like escaping for spaces
-			for value in values:
+			for n, value in enumerate(values):
 				self.assertEqual(
 					self.transm.proceed(
 						["set", jailName, "addjournalmatch", value]),
-					(0, ["FIELD=Test + Value+ \"Test+Value='Test"]))
+					(0, [[val] for val in values[:n+1]]))
+			for n, value in enumerate(values):
 				self.assertEqual(
 					self.transm.proceed(
 						["set", jailName, "deljournalmatch", value]),
-					(0, []))
+					(0, [[val] for val in values[n+1:]]))
 
 			# Try duplicates
 			value = "_COMM=sshd"
 			self.assertEqual(
 				self.transm.proceed(
 					["set", jailName, "addjournalmatch", value]),
-				(0, [value]))
+				(0, [[value]]))
 			# Duplicates are accepted, as automatically OR'd, and journalctl
 			# also accepts them without issue.
 			self.assertEqual(
 				self.transm.proceed(
 					["set", jailName, "addjournalmatch", value]),
-				(0, [value, value]))
+				(0, [[value], [value]]))
 			# Remove first instance
 			self.assertEqual(
 				self.transm.proceed(
 					["set", jailName, "deljournalmatch", value]),
-				(0, [value]))
+				(0, [[value]]))
 			# Remove second instance
 			self.assertEqual(
 				self.transm.proceed(
 					["set", jailName, "deljournalmatch", value]),
 				(0, []))
 
-			# Test splitting of OR'd values
-			value1, value2 = "_COMM=sshd", "_SYSTEMD_UNIT=sshd.service"
+			value = [
+				"_COMM=sshd", "+", "_SYSTEMD_UNIT=sshd.service", "_UID=0"]
 			self.assertEqual(
 				self.transm.proceed(
-					["set", jailName, "addjournalmatch",
-					" + ".join([value1, value2])]),
-				(0, [value1, value2]))
+					["set", jailName, "addjournalmatch"] + value),
+				(0, [["_COMM=sshd"], ["_SYSTEMD_UNIT=sshd.service", "_UID=0"]]))
 			self.assertEqual(
 				self.transm.proceed(
-					["set", jailName, "deljournalmatch", value1]),
-				(0, [value2]))
+					["set", jailName, "deljournalmatch"] + value[:1]),
+				(0, [["_SYSTEMD_UNIT=sshd.service", "_UID=0"]]))
 			self.assertEqual(
 				self.transm.proceed(
-					["set", jailName, "deljournalmatch", value2]),
+					["set", jailName, "deljournalmatch"] + value[2:]),
 				(0, []))
 
 			# Invalid match