Replace DNS client based on acutil with python-dns

IPA client and server tool set used authconfig acutil module to
for client DNS operations. This is not optimal DNS interface for
several reasons:
- does not provide native Python object oriented interface
  but but rather C-like interface based on functions and
  structures which is not easy to use and extend
- acutil is not meant to be used by third parties besides
  authconfig and thus can break without notice

Replace the acutil with python-dns package which has a feature rich
interface for dealing with all different aspects of DNS including
DNSSEC. The main target of this patch is to replace all uses of
acutil DNS library with a use python-dns. In most cases, even
though the larger parts of the code are changed, the actual
functionality is changed only in the following cases:
- redundant DNS checks were removed from verify_fqdn function
  in installutils to make the whole DNS check simpler and
  less error-prone. Logging was improves for the remaining
  checks
- improved logging for ipa-client-install DNS discovery

https://fedorahosted.org/freeipa/ticket/2730
https://fedorahosted.org/freeipa/ticket/1837

3 files changed, 85 insertions, 95 deletions

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 67279b3ed..6854581d2 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -25,6 +25,7 @@ try:
     import os
     import time
     import socket
+
     from ipapython.ipa_log_manager import *
     import tempfile
     import getpass
@@ -35,7 +36,6 @@ try:
     from ipapython.ipautil import run, user_input, CalledProcessError, file_exists, realm_to_suffix
     import ipapython.services as ipaservices
     from ipapython import ipautil
-    from ipapython import dnsclient
     from ipapython import sysrestore
     from ipapython import version
     from ipapython import certmonger
@@ -996,18 +996,10 @@ def update_dns(server, hostname):
 
 def client_dns(server, hostname, dns_updates=False):
 
-    dns_ok = False
+    dns_ok = ipautil.is_host_resolvable(hostname)
 
-    # Check if the client has an A record registered in its name.
-    rs = dnsclient.query(hostname+".", dnsclient.DNS_C_IN, dnsclient.DNS_T_A)
-    if len([ rec for rec in rs if rec.dns_type is not dnsclient.DNS_T_SOA ]) > 0:
-        dns_ok = True
-    else:
-        rs = dnsclient.query(hostname+".", dnsclient.DNS_C_IN, dnsclient.DNS_T_AAAA)
-        if len([ rec for rec in rs if rec.dns_type is not dnsclient.DNS_T_SOA ]) > 0:
-            dns_ok = True
-        else:
-            print "Warning: Hostname (%s) not found in DNS" % hostname
+    if not dns_ok:
+        print "Warning: Hostname (%s) not found in DNS" % hostname
 
     if dns_updates or not dns_ok:
         update_dns(server, hostname)
@@ -1243,15 +1235,15 @@ def install(options, env, fstore, statestore):
             # We assume that NTP servers are discoverable through SRV records in the DNS
             # If that fails, we try to sync directly with IPA server, assuming it runs NTP
             print 'Synchronizing time with KDC...'
-            ntp_servers = ipautil.parse_items(ds.ipadnssearchntp(cli_domain))
+            ntp_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp', None, break_on_first=False)
             synced_ntp = False
-            if len(ntp_servers) > 0:
+            if ntp_servers:
                 for s in ntp_servers:
-                   synced_ntp = ipaclient.ntpconf.synconce_ntp(s)
+                   synced_ntp = ipaclient.ntpconf.synconce_ntp(s, debug=True)
                    if synced_ntp:
                        break
             if not synced_ntp:
-                synced_ntp = ipaclient.ntpconf.synconce_ntp(cli_server)
+                synced_ntp = ipaclient.ntpconf.synconce_ntp(cli_server, debug=True)
             if not synced_ntp:
                 print "Unable to sync time with IPA NTP server, assuming the time is in sync."
             (krb_fd, krb_name) = tempfile.mkstemp()
diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 86bef28b2..281daf42a 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -20,12 +20,14 @@
 import socket
 import os
 from ipapython.ipa_log_manager import *
-import ipapython.dnsclient
 import tempfile
 import ldap
 from ldap import LDAPError
+from dns import resolver, rdatatype
+from dns.exception import DNSException
+
 from ipapython.ipautil import run, CalledProcessError, valid_ip, get_ipa_basedn, \
-                              realm_to_suffix, format_netloc, parse_items
+                              realm_to_suffix, format_netloc
 
 
 NOT_FQDN = -1
@@ -93,11 +95,12 @@ class IPADiscovery:
         isn't found.
         """
         server = None
+        root_logger.debug("Start searching for LDAP SRV record in %s and"
+                          " its sub-domains", domain)
         while not server:
-            root_logger.debug("[ipadnssearchldap("+domain+")]")
-            server = self.ipadnssearchldap(domain)
+            server = self.ipadns_search_srv(domain, '_ldap._tcp', 389)
             if server:
-                return (server, domain)
+                return (server[0], domain)
             else:
                 p = domain.find(".")
                 if p == -1: #no ldap server found and last component of the domain already tested
@@ -148,11 +151,13 @@ class IPADiscovery:
                 if not self.domain: #no ldap server found
                     return NO_LDAP_SERVER
             else:
-                root_logger.debug("[ipadnssearchldap]")
-                self.server = self.ipadnssearchldap(domain)
-                if self.server:
+                root_logger.debug("Search for LDAP SRV record in %s", domain)
+                server = self.ipadns_search_srv(domain, '_ldap._tcp', 389)
+                if server:
+                    self.server = server[0]
                     self.domain = domain
                 else:
+                    self.server = None
                     return NO_LDAP_SERVER
 
         else: #server forced on us, this means DNS doesn't work :/
@@ -172,19 +177,16 @@ class IPADiscovery:
         root_logger.debug("[ipacheckldap]")
         # We may have received multiple servers corresponding to the domain
         # Iterate through all of those to check if it is IPA LDAP server
-        servers = parse_items(self.server)
         ldapret = [NOT_IPA_SERVER]
         ldapaccess = True
-        for server in servers:
+        if self.server:
             # check ldap now
-            ldapret = self.ipacheckldap(server, self.realm)
+            ldapret = self.ipacheckldap(self.server, self.realm)
 
             if ldapret[0] == 0:
                 self.server = ldapret[1]
                 self.realm = ldapret[2]
-                break
-
-            if ldapret[0] == NO_ACCESS_TO_LDAP:
+            elif ldapret[0] == NO_ACCESS_TO_LDAP:
                 ldapaccess = False
 
         # If one of LDAP servers checked rejects access (may be anonymous
@@ -310,46 +312,43 @@ class IPADiscovery:
             os.rmdir(temp_ca_dir)
 
 
-    def ipadnssearchldap(self, tdomain):
-        servers = ""
-        rserver = ""
+    def ipadns_search_srv(self, domain, srv_record_name, default_port,
+                          break_on_first=True):
+        """
+        Search for SRV records in given domain. When no record is found,
+        en empty string is returned
+
+        :param domain: Search domain name
+        :param srv_record_name: SRV record name, e.g. "_ldap._tcp"
+        :param default_port: When default_port is not None, it is being
+                    checked with the port in SRV record and if they don't
+                    match, the port from SRV record is appended to
+                    found hostname in this format: "hostname:port"
+        :param break_on_first: break on the first find and return just one
+                    entry
+        """
+        servers = []
 
-        qname = "_ldap._tcp."+tdomain
-        # terminate the name
-        if not qname.endswith("."):
-            qname += "."
-        results = ipapython.dnsclient.query(qname, ipapython.dnsclient.DNS_C_IN, ipapython.dnsclient.DNS_T_SRV)
+        qname = '%s.%s' % (srv_record_name, domain)
 
-        for result in results:
-            if result.dns_type == ipapython.dnsclient.DNS_T_SRV:
-                rserver = result.rdata.server.rstrip(".")
-                if result.rdata.port and result.rdata.port != 389:
-                    rserver += ":" + str(result.rdata.port)
-                if servers:
-                    servers += "," + rserver
-                else:
-                    servers = rserver
-                break
+        root_logger.debug("Search DNS for SRV record of %s", qname)
 
-        return servers
-
-    def ipadnssearchntp(self, tdomain):
-        servers = ""
-        rserver = ""
-
-        qname = "_ntp._udp."+tdomain
-        # terminate the name
-        if not qname.endswith("."):
-            qname += "."
-        results = ipapython.dnsclient.query(qname, ipapython.dnsclient.DNS_C_IN, ipapython.dnsclient.DNS_T_SRV)
-
-        for result in results:
-            if result.dns_type == ipapython.dnsclient.DNS_T_SRV:
-                rserver = result.rdata.server.rstrip(".")
-                if servers:
-                    servers += "," + rserver
-                else:
-                    servers = rserver
+        try:
+            answers = resolver.query(qname, rdatatype.SRV)
+        except DNSException, e:
+            root_logger.debug("DNS record not found: %s", e.__class__.__name__)
+            answers = []
+
+        for answer in answers:
+            root_logger.debug("DNS record found: %s", answer)
+            server = str(answer.target).rstrip(".")
+            if not server:
+                root_logger.debug("Cannot parse the hostname from SRV record: %s", answer)
+                continue
+            if default_port is not None and answer.port != default_port:
+                server = "%s:%s" % (server, str(answer.port))
+            servers.append(server)
+            if break_on_first:
                 break
 
         return servers
@@ -359,35 +358,32 @@ class IPADiscovery:
         kdc = None
         # now, check for a Kerberos realm the local host or domain is in
         qname = "_kerberos." + tdomain
-        # terminate the name
-        if not qname.endswith("."):
-            qname += "."
-        results = ipapython.dnsclient.query(qname, ipapython.dnsclient.DNS_C_IN, ipapython.dnsclient.DNS_T_TXT)
-
-        for result in results:
-            if result.dns_type == ipapython.dnsclient.DNS_T_TXT:
-                realm = result.rdata.data
+
+        root_logger.debug("Search DNS for TXT record of %s", qname)
+
+        try:
+            answers = resolver.query(qname, rdatatype.TXT)
+        except DNSException, e:
+            root_logger.debug("DNS record not found: %s", e.__class__.__name__)
+            answers = []
+
+        for answer in answers:
+            root_logger.debug("DNS record found: %s", answer)
+            if answer.strings:
+                realm = answer.strings[0]
                 if realm:
                     break
 
         if realm:
             # now fetch server information for the realm
-            qname = "_kerberos._udp." + realm.lower()
-            # terminate the name
-            if not qname.endswith("."):
-                qname += "."
-            results = ipapython.dnsclient.query(qname, ipapython.dnsclient.DNS_C_IN, ipapython.dnsclient.DNS_T_SRV)
-            for result in results:
-                if result.dns_type == ipapython.dnsclient.DNS_T_SRV:
-                    qname = result.rdata.server.rstrip(".")
-                    if result.rdata.port and result.rdata.port != 88:
-                        qname += ":" + str(result.rdata.port)
-                    if kdc:
-                        kdc += "," + qname
-                    else:
-                        kdc = qname
+            domain = realm.lower()
+
+            kdc = self.ipadns_search_srv(domain, '_kerberos._udp', 88,
+                    break_on_first=False)
 
             if not kdc:
                 root_logger.debug("SRV record for KDC not found! Realm: %s, SRV record: %s" % (realm, qname))
+                kdc = None
+            kdc = ','.join(kdc)
 
         return [realm, kdc]
diff --git a/ipa-client/ipaclient/ntpconf.py b/ipa-client/ipaclient/ntpconf.py
index e71692f40..aa9261cb2 100644
--- a/ipa-client/ipaclient/ntpconf.py
+++ b/ipa-client/ipaclient/ntpconf.py
@@ -133,7 +133,7 @@ def config_ntp(server_fqdn, fstore = None, sysstore = None):
     # Restart ntpd
     ipaservices.knownservices.ntpd.restart()
 
-def synconce_ntp(server_fqdn):
+def synconce_ntp(server_fqdn, debug=False):
     """
     Syncs time with specified server using ntpdate.
     Primarily designed to be used before Kerberos setup
@@ -142,15 +142,17 @@ def synconce_ntp(server_fqdn):
     Returns True if sync was successful
     """
     ntpdate="/usr/sbin/ntpdate"
-    result = False
     if os.path.exists(ntpdate):
         # retry several times -- logic follows /etc/init.d/ntpdate
         # implementation
+        cmd = [ntpdate, "-U", "ntp", "-s", "-b"]
+        if debug:
+            cmd.append('-d')
+        cmd.append(server_fqdn)
         for retry in range(0,3):
             try:
-                ipautil.run([ntpdate, "-U", "ntp", "-s", "-b", server_fqdn])
-                result = True
-                break
+                ipautil.run(cmd)
+                return True
             except:
                 pass
-    return result
+    return False