From 6c788a32eee6b1fdc98cf024d6ab7cc160535aa5 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 25 Feb 2015 11:54:46 +0100 Subject: [PATCH 1/3] BF: binding parameter error (unsupported type) by writing json with invalid encoded lines into sqlite database (gh-973); especially python < 3.0; try to prevent occurring such errors in the future; --- ChangeLog | 2 ++ fail2ban/server/database.py | 38 ++++++++++++++++++++++-------- fail2ban/server/filter.py | 4 ++-- fail2ban/tests/databasetestcase.py | 11 +++++++++ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index bb688ad1..d8d0eb48 100644 --- a/ChangeLog +++ b/ChangeLog @@ -38,6 +38,8 @@ ver. 0.9.2 (2014/XX/XXX) - wanna-be-released system authentication issues * fail2ban-regex reads filter file(s) completely, incl. '.local' file etc. (gh-954) * firewallcmd-* actions: split output into separate lines for grepping (gh-908) + * binding parameter error (unsupported type) by writing json with invalid encoded + lines into sqlite database (gh-973), thanks to kot for issue reporting; - New Features: - New filters: diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index bbf7adad..dd48ceb3 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -37,17 +37,35 @@ from ..helpers import getLogger logSys = getLogger(__name__) if sys.version_info >= (3,): - sqlite3.register_adapter( - dict, - lambda x: json.dumps(x, ensure_ascii=False).encode( - locale.getpreferredencoding(), 'replace')) - sqlite3.register_converter( - "JSON", - lambda x: json.loads(x.decode( - locale.getpreferredencoding(), 'replace'))) + def _json_dumps_safe(x): + try: + x = json.dumps(x, ensure_ascii=False).encode( + locale.getpreferredencoding(), 'replace') + except Exception, e: # pragma: no cover + logSys.error('json dumps failed: %s', e) + x = '{}' + return x else: - sqlite3.register_adapter(dict, json.dumps) - sqlite3.register_converter("JSON", json.loads) + def _json_dumps_safe(x): + try: + x = json.dumps(x, ensure_ascii=False).decode( + locale.getpreferredencoding(), 'replace') + except Exception, e: # pragma: no cover + logSys.error('json dumps failed: %s', e) + x = '{}' + return x + +def _json_loads_safe(x): + try: + x = json.loads(x.decode( + locale.getpreferredencoding(), 'replace')) + except Exception, e: # pragma: no cover + logSys.error('json loads failed: %s', e) + x = {} + return x + +sqlite3.register_adapter(dict, _json_dumps_safe) +sqlite3.register_converter("JSON", _json_loads_safe) def commitandrollback(f): @wraps(f) diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py index 22866841..cf0c0e2b 100644 --- a/fail2ban/server/filter.py +++ b/fail2ban/server/filter.py @@ -802,8 +802,8 @@ class FileContainer: " encoding) for this jail. Continuing" " to process line ignoring invalid characters: %r" % (self.getFileName(), self.getEncoding(), line)) - if sys.version_info >= (3,): # In python3, must be decoded - line = line.decode(self.getEncoding(), 'ignore') + # decode with replacing error chars: + line = line.decode(self.getEncoding(), 'replace') return line def close(self): diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index aef7443f..9d750601 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -177,6 +177,17 @@ class DatabaseTest(LogCaptureTestCase): self.assertTrue( isinstance(self.db.getBans(jail=self.jail)[0], FailTicket)) + def testAddBanInvalidEncoded(self): + if Fail2BanDb is None: # pragma: no cover + return + self.testAddJail() + ticket = FailTicket("127.0.0.1", 0, {'m': ['... user "\xd1\xe2\xe5\xf2\xe0" ...'], 'a': 1}) + self.db.addBan(self.jail, ticket) + + self.assertEqual(len(self.db.getBans(jail=self.jail)), 1) + self.assertTrue( + isinstance(self.db.getBans(jail=self.jail)[0], FailTicket)) + def testDelBan(self): self.testAddBan() ticket = self.db.getBans(jail=self.jail)[0] From 2bfe22aa66a3f3bbd2046fa10b92cf05be882a30 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 25 Feb 2015 12:38:09 +0100 Subject: [PATCH 2/3] makes test case more precise; --- ChangeLog | 4 ++-- fail2ban/tests/databasetestcase.py | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index d8d0eb48..02488bee 100644 --- a/ChangeLog +++ b/ChangeLog @@ -38,8 +38,8 @@ ver. 0.9.2 (2014/XX/XXX) - wanna-be-released system authentication issues * fail2ban-regex reads filter file(s) completely, incl. '.local' file etc. (gh-954) * firewallcmd-* actions: split output into separate lines for grepping (gh-908) - * binding parameter error (unsupported type) by writing json with invalid encoded - lines into sqlite database (gh-973), thanks to kot for issue reporting; + * Guard unicode encode/decode issues while storing records in the database. + Fixes "binding parameter error (unsupported type)" (gh-973), thanks to kot for reporting - New Features: - New filters: diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index 9d750601..b56414ae 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -181,12 +181,16 @@ class DatabaseTest(LogCaptureTestCase): if Fail2BanDb is None: # pragma: no cover return self.testAddJail() - ticket = FailTicket("127.0.0.1", 0, {'m': ['... user "\xd1\xe2\xe5\xf2\xe0" ...'], 'a': 1}) + ticket = FailTicket("127.0.0.1", 0, ['... user "\xd1\xe2\xe5\xf2\xe0" ...']) self.db.addBan(self.jail, ticket) self.assertEqual(len(self.db.getBans(jail=self.jail)), 1) + readticket = self.db.getBans(jail=self.jail)[0] + ## python 2 or 3 : self.assertTrue( - isinstance(self.db.getBans(jail=self.jail)[0], FailTicket)) + readticket == FailTicket("127.0.0.1", 0, [u'... user "\ufffd\ufffd\ufffd\ufffd\ufffd" ...']) + or readticket == ticket + ) def testDelBan(self): self.testAddBan() From 5ab30c88c29043bf738400670fa466e38e2b5e78 Mon Sep 17 00:00:00 2001 From: sebres Date: Wed, 25 Feb 2015 18:16:06 +0100 Subject: [PATCH 3/3] more stable handling of json dump/load different encoded strings for older python versions; extended test cases (more precise, python version insensitive, etc.) --- fail2ban/server/database.py | 42 ++++++++++++++++++++---------- fail2ban/tests/databasetestcase.py | 28 +++++++++++++++----- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/fail2ban/server/database.py b/fail2ban/server/database.py index dd48ceb3..4e38b6ed 100644 --- a/fail2ban/server/database.py +++ b/fail2ban/server/database.py @@ -45,24 +45,38 @@ if sys.version_info >= (3,): logSys.error('json dumps failed: %s', e) x = '{}' return x + def _json_loads_safe(x): + try: + x = json.loads(x.decode(locale.getpreferredencoding(), 'replace')) + except Exception, e: # pragma: no cover + logSys.error('json loads failed: %s', e) + x = {} + return x else: + def _normalize(x): + if isinstance(x, dict): + return dict((_normalize(k), _normalize(v)) for k, v in x.iteritems()) + elif isinstance(x, list): + return [_normalize(element) for element in x] + elif isinstance(x, unicode): + return x.encode(locale.getpreferredencoding()) + else: + return x def _json_dumps_safe(x): try: - x = json.dumps(x, ensure_ascii=False).decode( + x = json.dumps(_normalize(x), ensure_ascii=False).decode( locale.getpreferredencoding(), 'replace') except Exception, e: # pragma: no cover logSys.error('json dumps failed: %s', e) x = '{}' return x - -def _json_loads_safe(x): - try: - x = json.loads(x.decode( - locale.getpreferredencoding(), 'replace')) - except Exception, e: # pragma: no cover - logSys.error('json loads failed: %s', e) - x = {} - return x + def _json_loads_safe(x): + try: + x = _normalize(json.loads(x.decode(locale.getpreferredencoding(), 'replace'))) + except Exception, e: # pragma: no cover + logSys.error('json loads failed: %s', e) + x = {} + return x sqlite3.register_adapter(dict, _json_dumps_safe) sqlite3.register_converter("JSON", _json_loads_safe) @@ -449,8 +463,8 @@ class Fail2BanDb(object): tickets = [] for ip, timeofban, data in self._getBans(**kwargs): #TODO: Implement data parts once arbitrary match keys completed - tickets.append(FailTicket(ip, timeofban, data['matches'])) - tickets[-1].setAttempt(data['failures']) + tickets.append(FailTicket(ip, timeofban, data.get('matches'))) + tickets[-1].setAttempt(data.get('failures', 1)) return tickets def getBansMerged(self, ip=None, jail=None, bantime=None): @@ -502,8 +516,8 @@ class Fail2BanDb(object): prev_banip = banip matches = [] failures = 0 - matches.extend(data['matches']) - failures += data['failures'] + matches.extend(data.get('matches', [])) + failures += data.get('failures', 1) prev_timeofban = timeofban ticket = FailTicket(banip, prev_timeofban, matches) ticket.setAttempt(failures) diff --git a/fail2ban/tests/databasetestcase.py b/fail2ban/tests/databasetestcase.py index b56414ae..9665e322 100644 --- a/fail2ban/tests/databasetestcase.py +++ b/fail2ban/tests/databasetestcase.py @@ -181,15 +181,31 @@ class DatabaseTest(LogCaptureTestCase): if Fail2BanDb is None: # pragma: no cover return self.testAddJail() - ticket = FailTicket("127.0.0.1", 0, ['... user "\xd1\xe2\xe5\xf2\xe0" ...']) - self.db.addBan(self.jail, ticket) + # invalid + valid, invalid + valid unicode, invalid + valid dual converted (like in filter:readline by fallback) ... + tickets = [ + FailTicket("127.0.0.1", 0, ['user "\xd1\xe2\xe5\xf2\xe0"', 'user "\xc3\xa4\xc3\xb6\xc3\xbc\xc3\x9f"']), + FailTicket("127.0.0.2", 0, ['user "\xd1\xe2\xe5\xf2\xe0"', u'user "\xc3\xa4\xc3\xb6\xc3\xbc\xc3\x9f"']), + FailTicket("127.0.0.3", 0, ['user "\xd1\xe2\xe5\xf2\xe0"', b'user "\xc3\xa4\xc3\xb6\xc3\xbc\xc3\x9f"'.decode('utf-8', 'replace')]) + ] + self.db.addBan(self.jail, tickets[0]) + self.db.addBan(self.jail, tickets[1]) + self.db.addBan(self.jail, tickets[2]) - self.assertEqual(len(self.db.getBans(jail=self.jail)), 1) - readticket = self.db.getBans(jail=self.jail)[0] + readtickets = self.db.getBans(jail=self.jail) + self.assertEqual(len(readtickets), 3) ## python 2 or 3 : + invstr = u'user "\ufffd\ufffd\ufffd\ufffd\ufffd"'.encode('utf-8', 'replace') self.assertTrue( - readticket == FailTicket("127.0.0.1", 0, [u'... user "\ufffd\ufffd\ufffd\ufffd\ufffd" ...']) - or readticket == ticket + readtickets[0] == FailTicket("127.0.0.1", 0, [invstr, 'user "\xc3\xa4\xc3\xb6\xc3\xbc\xc3\x9f"']) + or readtickets[0] == tickets[0] + ) + self.assertTrue( + readtickets[1] == FailTicket("127.0.0.2", 0, [invstr, u'user "\xc3\xa4\xc3\xb6\xc3\xbc\xc3\x9f"'.encode('utf-8', 'replace')]) + or readtickets[1] == tickets[1] + ) + self.assertTrue( + readtickets[2] == FailTicket("127.0.0.3", 0, [invstr, 'user "\xc3\xa4\xc3\xb6\xc3\xbc\xc3\x9f"']) + or readtickets[2] == tickets[2] ) def testDelBan(self):