Use IPAdmin rather than raw python-ldap in migration.py and ipadiscovery.py

These used ipautil.get_ipa_basedn. Convert that to use the new wrappers.

Beef up the error handling in ipaldap to accomodate the errors we catch
in the server discovery.
Add a DatabaseTimeout exception to errors.py.

These were the last uses of ipautil.convert_ldap_error, remove that.

https://fedorahosted.org/freeipa/ticket/3487
https://fedorahosted.org/freeipa/ticket/3446

6 files changed, 119 insertions, 140 deletions

diff --git a/install/migration/migration.py b/install/migration/migration.py
index 81b15b021..27e23a59f 100644
--- a/install/migration/migration.py
+++ b/install/migration/migration.py
@@ -23,7 +23,6 @@ Password migration script
 import cgi
 import errno
 import glob
-import ldap
 import wsgiref
 
 from ipapython.ipa_log_manager import root_logger
@@ -33,19 +32,6 @@ from ipapython.ipaldap import IPAdmin
 from ipalib import errors
 
 
-def convert_exception(error):
-    """
-    Convert an LDAP exception into something more readable.
-    """
-    if not isinstance(error, ldap.TIMEOUT):
-        desc = error.args[0]['desc'].strip()
-        info = error.args[0].get('info', '').strip()
-    else:
-        desc = ''
-        info = ''
-
-    return '%s (%s)' % (desc, info)
-
 def wsgi_redirect(start_response, loc):
     start_response('302 Found', [('Location', loc)])
     return []
@@ -63,14 +49,14 @@ def get_base_dn(ldap_uri):
     Retrieve LDAP server base DN.
     """
     try:
-        conn = ldap.initialize(ldap_uri)
-        conn.simple_bind_s('', '')
+        conn = IPAdmin(ldap_uri=ldap_uri)
+        conn.do_simple_bind(DN(), '')
         base_dn = get_ipa_basedn(conn)
-    except ldap.LDAPError, e:
+    except Exception, e:
         root_logger.error('migration context search failed: %s' % e)
         return ''
     finally:
-        conn.unbind_s()
+        conn.unbind()
 
     return base_dn
 
@@ -82,14 +68,14 @@ def bind(ldap_uri, base_dn, username, password):
     bind_dn = DN(('uid', username), ('cn', 'users'), ('cn', 'accounts'), base_dn)
     try:
         conn = IPAdmin(ldap_uri=ldap_uri)
-        conn.do_simple_bind(str(bind_dn), password)
+        conn.do_simple_bind(bind_dn, password)
     except (errors.ACIError, errors.DatabaseError, errors.NotFound), e:
         root_logger.error(
             'migration invalid credentials for %s: %s' % (bind_dn, e))
         raise IOError(
             errno.EPERM, 'Invalid LDAP credentials for user %s' % username)
     except Exception, e:
-        root_logger.error('migration bind failed: %s' % convert_exception(e))
+        root_logger.error('migration bind failed: %s' % e)
         raise IOError(errno.EIO, 'Bind error')
     finally:
         conn.unbind()
diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 7fc6aae88..d62430be4 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -19,16 +19,16 @@
 
 import socket
 import os
-import copy
-from ipapython.ipa_log_manager import *
 import tempfile
 import ldap
 from ldap import LDAPError
+
+from ipapython.ipa_log_manager import root_logger
 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
+from ipalib import errors
+from ipapython import ipaldap
+from ipapython.ipautil import valid_ip, get_ipa_basedn, realm_to_suffix
 from ipapython.dn import DN
 
 CACERT = '/etc/ipa/ca.crt'
@@ -328,16 +328,29 @@ class IPADiscovery(object):
 
         #now verify the server is really an IPA server
         try:
-            ldap_url = "ldap://" + format_netloc(thost, 389)
-            root_logger.debug("Init LDAP connection with: %s", ldap_url)
-            lh = ldap.initialize(ldap_url)
+            root_logger.debug("Init LDAP connection to: %s", thost)
             if ca_cert_path:
-                ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, True)
-                ldap.set_option(ldap.OPT_X_TLS_CACERTFILE, ca_cert_path)
-                lh.set_option(ldap.OPT_X_TLS_DEMAND, True)
-                lh.start_tls_s()
-            lh.set_option(ldap.OPT_PROTOCOL_VERSION, 3)
-            lh.simple_bind_s("","")
+                lh = ipaldap.IPAdmin(thost, protocol='ldap',
+                                     cacert=ca_cert_path, start_tls=True,
+                                     demand_cert=True)
+            else:
+                lh = ipaldap.IPAdmin(thost, protocol='ldap')
+            try:
+                lh.do_simple_bind(DN(), '')
+            except errors.ACIError:
+                root_logger.debug("LDAP Error: Anonymous access not allowed")
+                return [NO_ACCESS_TO_LDAP]
+            except errors.DatabaseError, err:
+                root_logger.error("Error checking LDAP: %s" % err.strerror)
+                # We should only get UNWILLING_TO_PERFORM if the remote LDAP
+                # server has minssf > 0 and we have attempted a non-TLS conn.
+                if ca_cert_path is None:
+                    root_logger.debug(
+                        "Cannot connect to LDAP server. Check that minssf is "
+                        "not enabled")
+                    return [NO_TLS_LDAP]
+                else:
+                    return [UNKNOWN_ERROR]
 
             # get IPA base DN
             root_logger.debug("Search LDAP server for IPA base DN")
@@ -348,23 +361,23 @@ class IPADiscovery(object):
                 return [NOT_IPA_SERVER]
 
             self.basedn = basedn
-            self.basedn_source = 'From IPA server %s' % ldap_url
+            self.basedn_source = 'From IPA server %s' % lh.ldap_uri
 
             #search and return known realms
             root_logger.debug(
                 "Search for (objectClass=krbRealmContainer) in %s (sub)",
                 self.basedn)
-            lret = lh.search_s(str(DN(('cn', 'kerberos'), self.basedn)), ldap.SCOPE_SUBTREE, "(objectClass=krbRealmContainer)")
-            if not lret:
+            try:
+                lret = lh.get_entries(
+                    DN(('cn', 'kerberos'), self.basedn),
+                    lh.SCOPE_SUBTREE, "(objectClass=krbRealmContainer)")
+            except errors.NotFound:
                 #something very wrong
                 return [REALM_NOT_FOUND]
 
             for lres in lret:
-                root_logger.debug("Found: %s", lres[0])
-                for lattr in lres[1]:
-                    if lattr.lower() == "cn":
-                        lrealms.append(lres[1][lattr][0])
-
+                root_logger.debug("Found: %s", lres.dn)
+                lrealms.append(lres.single_value('cn'))
 
             if trealm:
                 for r in lrealms:
@@ -382,27 +395,21 @@ class IPADiscovery(object):
             #we shouldn't get here
             return [UNKNOWN_ERROR]
 
-        except LDAPError, err:
-            if isinstance(err, ldap.TIMEOUT):
-                root_logger.debug("LDAP Error: timeout")
-                return [NO_LDAP_SERVER]
-
-            if isinstance(err, ldap.SERVER_DOWN):
-                root_logger.debug("LDAP Error: server down")
-                return [NO_LDAP_SERVER]
-
-            if isinstance(err, ldap.INAPPROPRIATE_AUTH):
-                root_logger.debug("LDAP Error: Anonymous access not allowed")
-                return [NO_ACCESS_TO_LDAP]
-
-            # We should only get UNWILLING_TO_PERFORM if the remote LDAP server
-            # has minssf > 0 and we have attempted a non-TLS connection.
-            if ca_cert_path is None and isinstance(err, ldap.UNWILLING_TO_PERFORM):
-                root_logger.debug("LDAP server returned UNWILLING_TO_PERFORM. This likely means that minssf is enabled")
-                return [NO_TLS_LDAP]
+        except errors.DatabaseTimeout:
+            root_logger.error("LDAP Error: timeout")
+            return [NO_LDAP_SERVER]
+        except errors.NetworkError, err:
+            root_logger.debug("LDAP Error: %s" % err.strerror)
+            return [NO_LDAP_SERVER]
+        except errors.ACIError:
+            root_logger.debug("LDAP Error: Anonymous access not allowed")
+            return [NO_ACCESS_TO_LDAP]
+        except errors.DatabaseError, err:
+            root_logger.error("Error checking LDAP: %s" % err.strerror)
+            return [UNKNOWN_ERROR]
+        except Exception, err:
+            root_logger.error("Error checking LDAP: %s" % err)
 
-            root_logger.error("LDAP Error: %s: %s" %
-               (err.args[0]['desc'], err.args[0].get('info', '')))
             return [UNKNOWN_ERROR]
 
 
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 15a228ea9..658c8cbc2 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1481,6 +1481,22 @@ class NotAllowedOnNonLeaf(ExecutionError):
     format = _('Not allowed on non-leaf entry')
 
 
+class DatabaseTimeout(DatabaseError):
+    """
+    **4211** Raised when an LDAP call times out
+
+    For example:
+
+    >>> raise DatabaseTimeout()
+    Traceback (most recent call last):
+      ...
+    DatabaseTimeout: LDAP timeout
+    """
+
+    errno = 4211
+    format = _('LDAP timeout')
+
+
 class CertificateError(ExecutionError):
     """
     **4300** Base class for Certificate execution errors (*4300 - 4399*).
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 1403c9e80..10492d178 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -899,9 +899,7 @@ class LDAPClient(object):
             try:
                 yield
             except ldap.TIMEOUT:
-                desc = ''
-                info = ''
-                raise
+                raise errors.DatabaseTimeout()
             except ldap.LDAPError, e:
                 desc = e.args[0]['desc'].strip()
                 info = e.args[0].get('info', '').strip()
@@ -923,6 +921,8 @@ class LDAPClient(object):
             raise errors.ACIError(info=info)
         except ldap.INVALID_CREDENTIALS:
             raise errors.ACIError(info="%s %s" % (info, desc))
+        except ldap.INAPPROPRIATE_AUTH:
+            raise errors.ACIError(info="%s: %s" % (desc, info))
         except ldap.NO_SUCH_ATTRIBUTE:
             # this is raised when a 'delete' attribute isn't found.
             # it indicates the previous attribute was removed by another
@@ -946,16 +946,19 @@ class LDAPClient(object):
             raise errors.NotAllowedOnNonLeaf()
         except ldap.SERVER_DOWN:
             raise errors.NetworkError(uri=self.ldap_uri,
-                                      error=u'LDAP Server Down')
+                                      error=info)
         except ldap.LOCAL_ERROR:
             raise errors.ACIError(info=info)
         except ldap.SUCCESS:
             pass
+        except ldap.CONNECT_ERROR:
+            raise errors.DatabaseError(desc=desc, info=info)
         except ldap.LDAPError, e:
             if 'NOT_ALLOWED_TO_DELEGATE' in info:
                 raise errors.ACIError(
                     info="KDC returned NOT_ALLOWED_TO_DELEGATE")
-            self.log.info('Unhandled LDAPError: %s' % str(e))
+            self.log.debug(
+                'Unhandled LDAPError: %s: %s' % (type(e).__name__, str(e)))
             raise errors.DatabaseError(desc=desc, info=info)
 
     @property
@@ -1658,7 +1661,7 @@ class IPAdmin(LDAPClient):
     def __init__(self, host='', port=389, cacert=None, debug=None, ldapi=False,
                  realm=None, protocol=None, force_schema_updates=True,
                  start_tls=False, ldap_uri=None, no_schema=False,
-                 decode_attrs=True, sasl_nocanon=False):
+                 decode_attrs=True, sasl_nocanon=False, demand_cert=False):
         self.conn = None
         log_mgr.get_logger(self, True)
         if debug and debug.lower() == "on":
@@ -1678,15 +1681,21 @@ class IPAdmin(LDAPClient):
 
         LDAPClient.__init__(self, ldap_uri)
 
-        self.conn = IPASimpleLDAPObject(ldap_uri, force_schema_updates=True,
-                                        no_schema=no_schema,
-                                        decode_attrs=decode_attrs)
+        with self.error_handler():
+            self.conn = IPASimpleLDAPObject(ldap_uri,
+                                            force_schema_updates=True,
+                                            no_schema=no_schema,
+                                            decode_attrs=decode_attrs)
+
+            if demand_cert:
+                ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, True)
+                self.conn.set_option(ldap.OPT_X_TLS_DEMAND, True)
 
-        if sasl_nocanon:
-            self.conn.set_option(ldap.OPT_X_SASL_NOCANON, ldap.OPT_ON)
+            if sasl_nocanon:
+                self.conn.set_option(ldap.OPT_X_SASL_NOCANON, ldap.OPT_ON)
 
-        if start_tls:
-            self.conn.start_tls_s()
+            if start_tls:
+                self.conn.start_tls_s()
 
     def __str__(self):
         return self.host + ":" + str(self.port)
@@ -1700,18 +1709,16 @@ class IPAdmin(LDAPClient):
             wait_for_open_ports(host, int(port), timeout)
 
     def __bind_with_wait(self, bind_func, timeout, *args, **kwargs):
-        try:
-            bind_func(*args, **kwargs)
-        except (ldap.CONNECT_ERROR, ldap.SERVER_DOWN), e:
-            if not timeout or 'TLS' in e.args[0].get('info', ''):
-                # No connection to continue on if we have a TLS failure
-                # https://bugzilla.redhat.com/show_bug.cgi?id=784989
-                raise e
+        with self.error_handler():
             try:
+                bind_func(*args, **kwargs)
+            except (ldap.CONNECT_ERROR, ldap.SERVER_DOWN), e:
+                if not timeout or 'TLS' in e.args[0].get('info', ''):
+                    # No connection to continue on if we have a TLS failure
+                    # https://bugzilla.redhat.com/show_bug.cgi?id=784989
+                    raise
                 self.__wait_for_connection(timeout)
-            except:
-                raise e
-            bind_func(*args, **kwargs)
+                bind_func(*args, **kwargs)
 
     def do_simple_bind(self, binddn=DN(('cn', 'directory manager')), bindpw="",
                        timeout=DEFAULT_TIMEOUT):
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index c0ac3a1f7..3d174ed02 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -34,7 +34,6 @@ import stat
 import shutil
 import urllib2
 import socket
-import ldap
 import struct
 from types import *
 import re
@@ -829,30 +828,31 @@ def get_ipa_basedn(conn):
 
     None is returned if the suffix is not found
 
-    :param conn: Bound LDAP connection that will be used for searching
+    :param conn: Bound LDAPClient that will be used for searching
     """
-    entries = conn.search_ext_s(
-        '', scope=ldap.SCOPE_BASE, attrlist=['defaultnamingcontext', 'namingcontexts']
-    )
+    entry = conn.get_entry(
+        DN(), attrs_list=['defaultnamingcontext', 'namingcontexts'])
 
-    contexts = entries[0][1]['namingcontexts']
-    if entries[0][1].get('defaultnamingcontext'):
+    # FIXME: import ipalib here to prevent import loops
+    from ipalib import errors
+
+    contexts = entry['namingcontexts']
+    if 'defaultnamingcontext' in entry:
         # If there is a defaultNamingContext examine that one first
-        default = entries[0][1]['defaultnamingcontext'][0]
+        default = entry.single_value('defaultnamingcontext')
         if default in contexts:
             contexts.remove(default)
-        contexts.insert(0, entries[0][1]['defaultnamingcontext'][0])
+        contexts.insert(0, default)
     for context in contexts:
         root_logger.debug("Check if naming context '%s' is for IPA" % context)
         try:
-            entry = conn.search_s(context, ldap.SCOPE_BASE, "(info=IPA*)")
-        except ldap.NO_SUCH_OBJECT:
-            root_logger.debug("LDAP server did not return info attribute to check for IPA version")
-            continue
-        if len(entry) == 0:
-            root_logger.debug("Info attribute with IPA server version not found")
+            [entry] = conn.get_entries(
+                DN(context), conn.SCOPE_BASE, "(info=IPA*)")
+        except errors.NotFound:
+            root_logger.debug("LDAP server did not return info attribute to "
+                              "check for IPA version")
             continue
-        info = entry[0][1]['info'][0].lower()
+        info = entry.single_value('info').lower()
         if info != IPA_BASEDN_INFO:
             root_logger.debug("Detected IPA server version (%s) did not match the client (%s)" \
                 % (info, IPA_BASEDN_INFO))
@@ -1174,39 +1174,3 @@ def restore_hostname(statestore):
             run(['/bin/hostname', old_hostname])
         except CalledProcessError, e:
             print >>sys.stderr, "Failed to set this machine hostname back to %s: %s" % (old_hostname, str(e))
-
-def convert_ldap_error(exc):
-    """
-    Make LDAP exceptions prettier.
-
-    Some LDAP exceptions have a dict with descriptive information, if
-    this exception has a dict extract useful information from it and
-    format it into something usable and return that. If the LDAP
-    exception does not have an information dict then return the name
-    of the LDAP exception.
-
-    If the exception is not an LDAP exception then convert the
-    exception to a string and return that instead.
-    """
-    if isinstance(exc, ldap.LDAPError):
-        name = exc.__class__.__name__
-
-        if len(exc.args):
-            d = exc.args[0]
-            if isinstance(d, dict):
-                desc = d.get('desc', '').strip()
-                info = d.get('info', '').strip()
-                if desc and info:
-                    return '%s %s' % (desc, info)
-                elif desc:
-                    return desc
-                elif info:
-                    return info
-                else:
-                    return name
-            else:
-                return name
-        else:
-            return name
-    else:
-        return str(exc)
diff --git a/tests/test_ipaserver/test_ldap.py b/tests/test_ipaserver/test_ldap.py
index 6ac0c2243..21363f2ef 100644
--- a/tests/test_ipaserver/test_ldap.py
+++ b/tests/test_ipaserver/test_ldap.py
@@ -140,9 +140,8 @@ class test_ldap(object):
         self.conn = ldap2(shared_instance=False, ldap_uri=ldapuri)
         try:
             self.conn.connect(autobind=True)
-        except errors.DatabaseError, e:
-            if e.desc == 'Inappropriate authentication':
-                raise nose.SkipTest("Only executed as root")
+        except errors.ACIError:
+            raise nose.SkipTest("Only executed as root")
         (dn, entry_attrs) = self.conn.get_entry(self.dn, ['usercertificate'])
         cert = entry_attrs.get('usercertificate')
         cert = cert[0]