[Git][NTPsec/ntpsec][master] Revert incorrect solution to DNS lookup problem

Ian Bruene gitlab at mg.gitlab.com
Mon Jan 1 23:12:20 UTC 2018


Ian Bruene pushed to branch master at NTPsec / ntpsec


Commits:
9ba9a887 by Ian Bruene at 2018-01-01T17:11:00-06:00
Revert incorrect solution to DNS lookup problem

This reverts commits:
8c9f64298f78079cbe58345bc88de699295e61d8
9795f701fc82fb78698a962dddcbd025c933fcca
b4f2101a0594d455c63791137d457c604fe10c8c

- - - - -


4 changed files:

- ntpclients/ntpq.py
- ntpclients/ntpsnmpd.py
- pylib/util.py
- tests/pylib/test_util.py


Changes:

=====================================
ntpclients/ntpq.py
=====================================
--- a/ntpclients/ntpq.py
+++ b/ntpclients/ntpq.py
@@ -520,9 +520,8 @@ usage: timeout [ msec ]
                     if self.showhostnames:
                         if self.debug:
                             self.say("DNS lookup begins...")
-                            self.say(ntp.util.__file__)
-                        value = ntp.util.timed_canonicalize_dns(
-                            value, family=self.ai_family, log=self.say)
+                        value = ntp.util.canonicalize_dns(
+                            value, family=self.ai_family)
                         if self.debug:
                             self.say("DNS lookup complete.")
                     self.say("%s  %s\n" % (legend, value))


=====================================
ntpclients/ntpsnmpd.py
=====================================
--- a/ntpclients/ntpsnmpd.py
+++ b/ntpclients/ntpsnmpd.py
@@ -423,7 +423,7 @@ class DataSource:  # This will be broken up in future to be less NTP-specific
         data = self.safeReadvar(0, ["peeradr"])
         if data is None:
             return ax.Varbind(ax.VALUE_NULL, oid)
-        data = ntp.util.timed_canonicalize_dns(data["peeradr"])
+        data = ntp.util.canonicalize_dns(data["peeradr"])
         return ax.Varbind(ax.VALUE_OCTET_STR, oid, data)
 
     def cbr_statusActiveOffset(self, oid):
@@ -626,7 +626,7 @@ class DataSource:  # This will be broken up in future to be less NTP-specific
             if pdata is None:
                 return ax.Varbind(ax.VALUE_NULL, oid)
             peername = pdata[associd]["srcadr"][1]
-            peername = ntp.util.timed_canonicalize_dns(peername)
+            peername = ntp.util.canonicalize_dns(peername)
             return ax.Varbind(ax.VALUE_OCTET_STR, oid, peername)
         return self.dynamicCallbackSkeleton(handler)
 


=====================================
pylib/util.py
=====================================
--- a/pylib/util.py
+++ b/pylib/util.py
@@ -10,7 +10,6 @@ import os
 import re
 import shutil
 import socket
-import signal
 import sys
 import time
 import ntp.ntpc
@@ -554,38 +553,11 @@ class Cache:
 canonicalization_cache = Cache()
 
 
-import subprocess
-
-# Hack to avoid occasional multi-second long delays when doing a DNS
-# lookup. Delays of that length will cause the SNMP master agent to drop
-# the connection. Uaccceptable.
-# Unfortunately there is no good way to timeout a function in Python, so
-# we are left with one of the ugly options that happened to work.
-def timed_canonicalize_dns(inhost, family=socket.AF_UNSPEC, ttl=1.0):
+def canonicalize_dns(inhost, family=socket.AF_UNSPEC):
+    "Canonicalize a hostname or numeric IP address."
     resname = canonicalization_cache.get(inhost)
     if resname is not None:
         return resname
-    if "'" in inhost:  # Invalid IP address that will break the function.
-        return inhost  # Potentially hostile.
-    cmd = "import ntp.util; print(ntp.util.canonicalize_dns('%s', %s))"
-    cmd = cmd % (str(inhost), str(family))
-    p = subprocess.Popen(["python", "-c", cmd],
-                         stdout=subprocess.PIPE,
-                         stderr=subprocess.PIPE,
-                         stdin=subprocess.PIPE)
-    starttime = time.time()
-    while p.poll() is None:  # subprocess not yet returned
-        timediff = time.time() - starttime
-        if timediff > ttl:  # timed out; bail
-            p.terminate()
-            return inhost
-        time.sleep(.01)  # don't waste cycles with spinning
-    result = p.communicate()[0].strip("\n")
-    canonicalization_cache.set(inhost, result)
-    return result
-
-def canonicalize_dns(inhost, family=socket.AF_UNSPEC):
-    "Canonicalize a hostname or numeric IP address."
     # Catch garbaged hostnames in corrupted Mode 6 responses
     m = re.match("([:.[\]]|\w)*", inhost)
     if not m:
@@ -605,6 +577,7 @@ def canonicalize_dns(inhost, family=socket.AF_UNSPEC):
         # Fall back to the hostname.
         canonicalized = canonname or hostname
         result = canonicalized.lower() + portsuffix
+    canonicalization_cache.set(inhost, result)
     return result
 
 
@@ -1085,7 +1058,7 @@ class PeerSummary:
                 try:
                     if self.debug:
                         self.logfp.write("DNS lookup begins...\n")
-                    clock_name = timed_canonicalize_dns(srcadr)
+                    clock_name = canonicalize_dns(srcadr)
                     if self.debug:
                         self.logfp.write("DNS lookup ends.\n")
                 except TypeError:


=====================================
tests/pylib/test_util.py
=====================================
--- a/tests/pylib/test_util.py
+++ b/tests/pylib/test_util.py
@@ -510,9 +510,16 @@ class TestPylibUtilMethods(unittest.TestCase):
         f = ntp.util.canonicalize_dns
 
         fakesockmod = jigs.SocketModuleJig()
+        mycache = ntp.util.Cache()
+        mycache.set("foo", "bar")
         try:
+            cachetemp = ntp.util.canonicalization_cache
+            ntp.util.canonicalization_cache = mycache
             sockettemp = ntp.util.socket
             ntp.util.socket = fakesockmod
+            # Test cache hit
+            self.assertEqual(f("foo"), "bar")
+            self.assertEqual(fakesockmod.gai_calls, [])
             # Test addrinfo fail
             fakesockmod.__init__()
             fakesockmod.gai_error_count = 1
@@ -531,6 +538,7 @@ class TestPylibUtilMethods(unittest.TestCase):
             self.assertEqual(f("bar:42"), "san.hastur.invalid:42")
             # Test nameinfo fail, no canonname
             fakesockmod.__init__()
+            mycache.__init__()
             fakesockmod.gni_error_count = 1
             fakesockmod.gni_returns = [("www.Hastur.invalid", 42)]
             fakesockmod.gai_returns = [(("family", "socktype", "proto",
@@ -538,11 +546,13 @@ class TestPylibUtilMethods(unittest.TestCase):
             self.assertEqual(f("bar:42"), "bar:42")
             # Test success
             fakesockmod.__init__()
+            mycache.__init__()
             fakesockmod.gni_returns = [("www.Hastur.invalid", 42)]
             fakesockmod.gai_returns = [(("family", "socktype", "proto",
                                          None, "42.23.%$.(#"),)]
             self.assertEqual(f("bar:42"), "www.hastur.invalid:42")
         finally:
+            ntp.util.canonicalization_cache = cachetemp
             ntp.util.socket = sockettemp
 
     def test_termsize(self):
@@ -1113,8 +1123,8 @@ class TestPeerSummary(unittest.TestCase):
         try:
             timetemp = ntp.util.time
             ntp.util.time = faketimemod
-            cdnstemp = ntp.util.timed_canonicalize_dns
-            ntp.util.timed_canonicalize_dns = cdns_jig
+            cdnstemp = ntp.util.canonicalize_dns
+            ntp.util.canonicalize_dns = cdns_jig
             # Test, no units, hmode=BCLIENTX, peers
             cdns_jig_returns = ["clock_canon"]
             faketimemod.time_returns = [0xA0000000]
@@ -1220,7 +1230,7 @@ class TestPeerSummary(unittest.TestCase):
                              "   32  764 1.2346ms 2.7183ms 3.1416ms\n")
         finally:
             ntp.util.time = timetemp
-            ntp.util.timed_canonicalize_dns = cdnstemp
+            ntp.util.canonicalize_dns = cdnstemp
 
     def test_intervals(self):
         cls = self.target("peers", 4, True, False)



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/9ba9a887546c71c730f4a10de369cab34f332727

---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/9ba9a887546c71c730f4a10de369cab34f332727
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20180101/2ae07222/attachment.html>


More information about the vc mailing list