From c2d2e79b0d48bf66b04c3772c2419f30a4b1f9db Mon Sep 17 00:00:00 2001
From: sebres <serg.brester@sebres.de>
Date: Fri, 30 Sep 2016 17:01:06 +0200
Subject: [PATCH] ExtendedCymruInfo: better availability check (code review and
 timeout's); max sleep time check of too long sleep increased to 1 second
 (typo fix)

---
 fail2ban/server/banmanager.py        | 60 +++++++++++++++++-----------
 fail2ban/tests/banmanagertestcase.py | 10 ++++-
 fail2ban/tests/utils.py              |  2 +-
 3 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/fail2ban/server/banmanager.py b/fail2ban/server/banmanager.py
index 10578945..054ae4bb 100644
--- a/fail2ban/server/banmanager.py
+++ b/fail2ban/server/banmanager.py
@@ -28,7 +28,7 @@ from threading import Lock
 
 from .ticket import BanTicket
 from .mytime import MyTime
-from ..helpers import getLogger
+from ..helpers import getLogger, logging
 
 # Gets the instance of the logger.
 logSys = getLogger(__name__)
@@ -132,17 +132,23 @@ class BanManager:
 	#
 	# @return {"asn": [], "country": [], "rir": []} dict for self.__banList IPs
 
-	def getBanListExtendedCymruInfo(self):
+	def getBanListExtendedCymruInfo(self, timeout=10):
 		return_dict = {"asn": [], "country": [], "rir": []}
-		try:
-			import dns.exception
-			import dns.resolver
-		except ImportError:
-			logSys.error("dnspython package is required but could not be imported")
-			return_dict["asn"].append("error")
-			return_dict["country"].append("error")
-			return_dict["rir"].append("error")
-			return return_dict
+		if not hasattr(self, 'dnsResolver'):
+			try:
+				import dns.exception
+				import dns.resolver
+				resolver = dns.resolver.Resolver()
+				resolver.lifetime = timeout
+				resolver.timeout = timeout / 2
+				self.dnsResolver = resolver
+			except ImportError: # pragma: no cover
+				logSys.error("dnspython package is required but could not be imported")
+				return_dict["error"] = repr(e)
+				return_dict["asn"].append("error")
+				return_dict["country"].append("error")
+				return_dict["rir"].append("error")
+				return return_dict
 		# get ips in lock:
 		with self.__lock:
 			banIPs = [banData.getIP() for banData in self.__banList.values()]
@@ -155,7 +161,10 @@ class BanManager:
 					else "origin6.asn.cymru.com"
 				)
 				try:
-					answers = dns.resolver.query(question, "TXT")
+					resolver = self.dnsResolver
+					answers = resolver.query(question, "TXT")
+					if not answers:
+						raise ValueError("No data retrieved")
 					for rdata in answers:
 						asn, net, country, rir, changed =\
 							[answer.strip("'\" ") for answer in rdata.to_text().split("|")]
@@ -169,20 +178,23 @@ class BanManager:
 					return_dict["asn"].append("nxdomain")
 					return_dict["country"].append("nxdomain")
 					return_dict["rir"].append("nxdomain")
-				except dns.exception.DNSException as dnse:
-					logSys.error("Unhandled DNSException querying Cymru for %s TXT" % question)
-					logSys.exception(dnse)
-					return_dict["error"] = dnse
+				except (dns.exception.DNSException, dns.resolver.NoNameservers, dns.exception.Timeout) as dnse: # pragma: no cover
+					logSys.error("DNSException %r querying Cymru for %s TXT", dnse, question)
+					if logSys.level <= logging.DEBUG:
+						logSys.exception(dnse)
+					return_dict["error"] = repr(dnse)
 					break
-				except Exception as e:
-					logSys.error("Unhandled Exception querying Cymru for %s TXT" % question)
-					logSys.exception(e)
-					return_dict["error"] = e
+				except Exception as e: # pragma: no cover
+					logSys.error("Unhandled Exception %r querying Cymru for %s TXT", e, question)
+					if logSys.level <= logging.DEBUG:
+						logSys.exception(e)
+					return_dict["error"] = repr(e)
 					break
-		except Exception as e:
-			logSys.error("Failure looking up extended Cymru info")
-			logSys.exception(e)
-			return_dict["error"] = e
+		except Exception as e: # pragma: no cover
+			logSys.error("Failure looking up extended Cymru info: %s", e)
+			if logSys.level <= logging.DEBUG:
+				logSys.exception(e)
+			return_dict["error"] = repr(e)
 		return return_dict
 
 	##
diff --git a/fail2ban/tests/banmanagertestcase.py b/fail2ban/tests/banmanagertestcase.py
index d557f16e..4d964425 100644
--- a/fail2ban/tests/banmanagertestcase.py
+++ b/fail2ban/tests/banmanagertestcase.py
@@ -147,9 +147,17 @@ class StatusExtendedCymruInfo(unittest.TestCase):
 		"""Call after every test case."""
 		pass
 
+	available = True, None
+
 	def _getBanListExtendedCymruInfo(self):
-		cymru_info = self.__banManager.getBanListExtendedCymruInfo()
+		tc = StatusExtendedCymruInfo
+		if tc.available[0]:
+			cymru_info = self.__banManager.getBanListExtendedCymruInfo(
+				timeout=(2 if unittest.F2B.fast else 20))
+		else:
+			cymru_info = tc.available[1]
 		if cymru_info.get("error"): # pragma: no cover - availability
+			tc.available = False, cymru_info
 			raise unittest.SkipTest('Skip test because service is not available: %s' % cymru_info["error"])
 		return cymru_info
 
diff --git a/fail2ban/tests/utils.py b/fail2ban/tests/utils.py
index 0f5ea9bd..f60dfd1f 100644
--- a/fail2ban/tests/utils.py
+++ b/fail2ban/tests/utils.py
@@ -248,7 +248,7 @@ def initTests(opts):
 		# sleep intervals are large - use replacement for sleep to check time to sleep:
 		_org_sleep = time.sleep
 		def _new_sleep(v):
-			if v > min(1, Utils.DEFAULT_SLEEP_TIME): # pragma: no cover
+			if v > max(1, Utils.DEFAULT_SLEEP_TIME): # pragma: no cover
 				raise ValueError('[BAD-CODE] To long sleep interval: %s, try to use conditional Utils.wait_for instead' % v)
 			_org_sleep(min(v, Utils.DEFAULT_SLEEP_TIME))
 		time.sleep = _new_sleep