From 00e289e11b7bc5b3219df953249ab19ec034ed4b Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Wed, 15 May 2013 00:19:22 +0100 Subject: [PATCH] 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 | 2 +- fail2ban/client/filterreader.py | 6 +- fail2ban/server/filtersystemd.py | 55 +++++++++++++------ fail2ban/server/transmitter.py | 4 +- fail2ban/tests/clientreadertestcase.py | 7 ++- fail2ban/tests/files/filter.d/testcase01.conf | 7 +++ fail2ban/tests/filtertestcase.py | 32 +++++------ fail2ban/tests/servertestcase.py | 44 ++++++--------- 8 files changed, 90 insertions(+), 67 deletions(-) diff --git a/fail2ban/client/beautifier.py b/fail2ban/client/beautifier.py index ea03b7b8..0c5e31fb 100644 --- a/fail2ban/client/beautifier.py +++ b/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" diff --git a/fail2ban/client/filterreader.py b/fail2ban/client/filterreader.py index 5c58046b..e5fe8f4f 100644 --- a/fail2ban/client/filterreader.py +++ b/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 diff --git a/fail2ban/server/filtersystemd.py b/fail2ban/server/filtersystemd.py index b4f7e7db..838805a5 100644 --- a/fail2ban/server/filtersystemd.py +++ b/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 diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 8bacdd0a..39c74452 100644 --- a/fail2ban/server/transmitter.py +++ b/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": diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 5e35eabc..6fd9f451 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -197,7 +197,12 @@ class FilterReaderTest(unittest.TestCase): "+$^.+ module for .* from \\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() diff --git a/fail2ban/tests/files/filter.d/testcase01.conf b/fail2ban/tests/files/filter.d/testcase01.conf index c549572d..8bc4261d 100644 --- a/fail2ban/tests/files/filter.d/testcase01.conf +++ b/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" diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index c2f43fb4..7b68e16c 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/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) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 953cf318..19a070c4 100644 --- a/fail2ban/tests/servertestcase.py +++ b/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