From 1f0959735f9828a09439f17f1468dcd3dfb914db Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Fri, 18 Mar 2016 09:49:41 +0100 Subject: [PATCH] differentiate between limit types when LDAP search exceeds configured limits When LDAP search fails on exceeded limits, we should raise an specific exception for the type of limit raised (size, time, administrative) so that the consumer can distinguish between e.g. searches returning too many entries and those timing out. https://fedorahosted.org/freeipa/ticket/5677 Reviewed-By: Petr Spacek --- install/tools/ipa-httpd-kdcproxy | 6 +-- install/tools/ipactl | 6 +-- ipalib/errors.py | 28 +++++++++++++ ipalib/plugins/automount.py | 6 +-- ipalib/plugins/baseldap.py | 14 +++---- ipapython/ipaldap.py | 68 +++++++++++++++++++++++--------- ipaserver/plugins/ldap2.py | 5 ++- 7 files changed, 92 insertions(+), 41 deletions(-) diff --git a/install/tools/ipa-httpd-kdcproxy b/install/tools/ipa-httpd-kdcproxy index 5e67f61a6..c14b3a721 100755 --- a/install/tools/ipa-httpd-kdcproxy +++ b/install/tools/ipa-httpd-kdcproxy @@ -97,10 +97,8 @@ class KDCProxyConfig(object): def _find_entry(self, dn, attrs, filter, scope=IPAdmin.SCOPE_BASE): """Find an LDAP entry, handles NotFound and Limit""" try: - entries, truncated = self.con.find_entries( - filter, attrs, dn, scope, time_limit=self.time_limit) - if truncated: - raise errors.LimitsExceeded() + entries = self.con.get_entries( + dn, scope, filter, attrs, time_limit=self.time_limit) except errors.NotFound: self.log.debug('Entry not found: %s', dn) return None diff --git a/install/tools/ipactl b/install/tools/ipactl index fb1e890ea..b41b10c8a 100755 --- a/install/tools/ipactl +++ b/install/tools/ipactl @@ -160,14 +160,12 @@ def get_config(dirsrv): wait_for_open_ports(host, [int(port)], timeout=api.env.startup_timeout) con = IPAdmin(ldap_uri=api.env.ldap_uri) con.do_external_bind() - res, truncated = con.find_entries( + res = con.get_entries( + base, filter=srcfilter, attrs_list=attrs, - base_dn=base, scope=con.SCOPE_SUBTREE, time_limit=10) - if truncated: - raise errors.LimitsExceeded() except errors.NetworkError: # LSB status code 3: program is not running raise IpactlError("Failed to get list of services to probe status:\n" + diff --git a/ipalib/errors.py b/ipalib/errors.py index 52b770027..2507e13dc 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -1612,6 +1612,34 @@ class TaskTimeout(DatabaseError): format = _("%(task)s LDAP task timeout, Task DN: '%(task_dn)s'") +class TimeLimitExceeded(LimitsExceeded): + """ + **4214** Raised when time limit for the operation is exceeded. + """ + + errno = 4214 + format = _('Configured time limit exceeded') + + +class SizeLimitExceeded(LimitsExceeded): + """ + **4215** Raised when size limit for the operation is exceeded. + """ + + errno = 4215 + format = _('Configured size limit exceeded') + + +class AdminLimitExceeded(LimitsExceeded): + """ + **4216** Raised when server limit imposed by administrative authority was + exceeded + """ + + errno = 4216 + format = _('Configured administrative server limit exceeded') + + class CertificateError(ExecutionError): """ **4300** Base class for Certificate execution errors (*4300 - 4399*). diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py index 7dc00224e..c0a55d5da 100644 --- a/ipalib/plugins/automount.py +++ b/ipalib/plugins/automount.py @@ -803,12 +803,10 @@ class automountkey(LDAPObject): ('cn', parent_keys[0]), self.container_dn, api.env.basedn) attrs_list = ['*'] - entries, truncated = ldap.find_entries( - sfilter, attrs_list, basedn, ldap.SCOPE_ONELEVEL) + entries = ldap.get_entries( + basedn, ldap.SCOPE_ONELEVEL, sfilter, attrs_list) if len(entries) > 1: raise errors.NotFound(reason=_('More than one entry with key %(key)s found, use --info to select specific entry.') % dict(key=pkey)) - if truncated: - raise errors.LimitsExceeded() dn = entries[0].dn return dn diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index ffc0008a6..0eae45dbd 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -684,14 +684,12 @@ class LDAPObject(Object): filter = self.backend.combine_filters( ('(member=*)', mo_filter), self.backend.MATCH_ALL) try: - result, truncated = self.backend.find_entries( - base_dn=self.api.env.basedn, + result = self.backend.get_entries( + self.api.env.basedn, filter=filter, attrs_list=['member'], size_limit=-1, # paged search will get everything anyway paged_search=True) - if truncated: - raise errors.LimitsExceeded() except errors.NotFound: result = [] @@ -709,12 +707,10 @@ class LDAPObject(Object): filter = self.backend.make_filter( {'member': dn, 'memberuser': dn, 'memberhost': dn}) try: - result, truncated = self.backend.find_entries( - base_dn=self.api.env.basedn, + result = self.backend.get_entries( + self.api.env.basedn, filter=filter, attrs_list=['']) - if truncated: - raise errors.LimitsExceeded() except errors.NotFound: result = [] @@ -2105,7 +2101,7 @@ class LDAPSearch(BaseLDAPCommand, crud.Search): result = dict( result=entries, count=len(entries), - truncated=truncated, + truncated=bool(truncated), ) if truncated: diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 2965ba4a5..405f1ee2f 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -60,6 +60,11 @@ AUTOBIND_AUTO = 1 AUTOBIND_ENABLED = 2 AUTOBIND_DISABLED = 3 +TRUNCATED_SIZE_LIMIT = object() +TRUNCATED_TIME_LIMIT = object() +TRUNCATED_ADMIN_LIMIT = object() + + def unicode_from_utf8(val): ''' val is a UTF-8 encoded string, return a unicode object. @@ -971,11 +976,11 @@ class LDAPClient(object): except ldap.OBJECT_CLASS_VIOLATION: raise errors.ObjectclassViolation(info=info) except ldap.ADMINLIMIT_EXCEEDED: - raise errors.LimitsExceeded() + raise errors.AdminLimitExceeded() except ldap.SIZELIMIT_EXCEEDED: - raise errors.LimitsExceeded() + raise errors.SizeLimitExceeded() except ldap.TIMELIMIT_EXCEEDED: - raise errors.LimitsExceeded() + raise errors.TimeLimitExceeded() except ldap.NOT_ALLOWED_ON_RDN: raise errors.NotAllowedOnRDN(attr=info) except ldap.FILTER_ERROR: @@ -1003,6 +1008,20 @@ class LDAPClient(object): 'Unhandled LDAPError: %s: %s' % (type(e).__name__, str(e))) raise errors.DatabaseError(desc=desc, info=info) + @staticmethod + def handle_truncated_result(truncated): + if not truncated: + return + + if truncated is TRUNCATED_ADMIN_LIMIT: + raise errors.AdminLimitExceeded() + elif truncated is TRUNCATED_SIZE_LIMIT: + raise errors.SizeLimitExceeded() + elif truncated is TRUNCATED_TIME_LIMIT: + raise errors.TimeLimitExceeded() + else: + raise errors.LimitsExceeded() + @property def schema(self): """schema associated with this LDAP server""" @@ -1249,7 +1268,7 @@ class LDAPClient(object): return self.combine_filters(flts, rules) def get_entries(self, base_dn, scope=ldap.SCOPE_SUBTREE, filter=None, - attrs_list=None): + attrs_list=None, **kwargs): """Return a list of matching entries. :raises: errors.LimitsExceeded if the list is truncated by the server @@ -1260,13 +1279,21 @@ class LDAPClient(object): :param scope: search scope, see LDAP docs (default ldap2.SCOPE_SUBTREE) :param filter: LDAP filter to apply :param attrs_list: ist of attributes to return, all if None (default) - - Use the find_entries method for more options. + :param kwargs: additional keyword arguments. See find_entries method + for their description. """ entries, truncated = self.find_entries( base_dn=base_dn, scope=scope, filter=filter, attrs_list=attrs_list) - if truncated: - raise errors.LimitsExceeded() + try: + self.handle_truncated_result(truncated) + except errors.LimitsExceeded as e: + self.log.error( + "{} while getting entries (base DN: {}, filter: {})".format( + e, base_dn, filter + ) + ) + raise + return entries def find_entries(self, filter=None, attrs_list=None, base_dn=None, @@ -1357,6 +1384,15 @@ class LDAPClient(object): break else: cookie = '' + except ldap.ADMINLIMIT_EXCEEDED: + truncated = TRUNCATED_ADMIN_LIMIT + break + except ldap.SIZELIMIT_EXCEEDED: + truncated = TRUNCATED_SIZE_LIMIT + break + except ldap.TIMELIMIT_EXCEEDED: + truncated = TRUNCATED_TIME_LIMIT + break except ldap.LDAPError as e: # If paged search is in progress, try to cancel it if paged_search and cookie: @@ -1402,15 +1438,13 @@ class LDAPClient(object): search_kw = {attr: value, 'objectClass': object_class} filter = self.make_filter(search_kw, rules=self.MATCH_ALL) - (entries, truncated) = self.find_entries(filter, attrs_list, base_dn) + entries = self.get_entries( + base_dn, filter=filter, attrs_list=attrs_list) if len(entries) > 1: raise errors.SingleMatchExpected(found=len(entries)) - else: - if truncated: - raise errors.LimitsExceeded() - else: - return entries[0] + + return entries[0] def get_entry(self, dn, attrs_list=None, time_limit=None, size_limit=None): @@ -1423,13 +1457,11 @@ class LDAPClient(object): assert isinstance(dn, DN) - (entries, truncated) = self.find_entries( - None, attrs_list, dn, self.SCOPE_BASE, time_limit=time_limit, + entries = self.get_entries( + dn, self.SCOPE_BASE, None, attrs_list, time_limit=time_limit, size_limit=size_limit ) - if truncated: - raise errors.LimitsExceeded() return entries[0] def add_entry(self, entry): diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index a1f1e1982..d50ae1698 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -230,12 +230,13 @@ class ldap2(CrudBackend, LDAPClient): # Not in our context yet pass try: + # use find_entries here lest we hit an infinite recursion when + # ldap2.get_entries tries to determine default time/size limits (entries, truncated) = self.find_entries( None, attrs_list, base_dn=dn, scope=self.SCOPE_BASE, time_limit=2, size_limit=10 ) - if truncated: - raise errors.LimitsExceeded() + self.handle_truncated_result(truncated) config_entry = entries[0] except errors.NotFound: config_entry = self.make_entry(dn)