diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index 23d426c725..6b5fd66fa2 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -27,6 +27,7 @@ from oslo_log import log from oslo_utils import reflection import six from six.moves import map, zip +from six import PY2 from keystone.common import driver_hints from keystone import exception @@ -152,8 +153,9 @@ def convert_ldap_result(ldap_result): are lists of strings. OpenStack wants to use Python types of its choosing. Strings will - be unicode, truth values boolean, whole numbers int's, etc. DN's will - also be decoded from UTF-8 to unicode. + be unicode, truth values boolean, whole numbers int's, etc. DN's are + represented as text in python-ldap by default for Python 3 and when + bytes_mode=False for Python 2, and therefore do not require decoding. :param ldap_result: LDAP search result :returns: list of 2-tuples containing (dn, attrs) where dn is unicode @@ -175,8 +177,7 @@ def convert_ldap_result(ldap_result): ldap_attrs[kind] = [val2py(x) for x in values] except UnicodeDecodeError: LOG.debug('Unable to decode value for attribute %s', kind) - - py_result.append((utf8_decode(dn), ldap_attrs)) + py_result.append((dn, ldap_attrs)) if at_least_one_referral: LOG.debug('Referrals were returned and ignored. Enable referral ' 'chasing in keystone.conf via [ldap] chase_referrals') @@ -299,9 +300,9 @@ def is_dn_equal(dn1, dn2): """ if not isinstance(dn1, list): - dn1 = ldap.dn.str2dn(utf8_encode(dn1)) + dn1 = ldap.dn.str2dn(dn1) if not isinstance(dn2, list): - dn2 = ldap.dn.str2dn(utf8_encode(dn2)) + dn2 = ldap.dn.str2dn(dn2) if len(dn1) != len(dn2): return False @@ -320,9 +321,9 @@ def dn_startswith(descendant_dn, dn): """ if not isinstance(descendant_dn, list): - descendant_dn = ldap.dn.str2dn(utf8_encode(descendant_dn)) + descendant_dn = ldap.dn.str2dn(descendant_dn) if not isinstance(dn, list): - dn = ldap.dn.str2dn(utf8_encode(dn)) + dn = ldap.dn.str2dn(dn) if len(descendant_dn) <= len(dn): return False @@ -345,6 +346,11 @@ class LDAPHandler(object): * unicode strings are encoded in UTF-8 + Note, in python-ldap some fields (DNs, RDNs, attribute names, queries) + are represented as text (str on Python 3, unicode on Python 2 when + bytes_mode=False). For more details see: + http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode + In addition to handling type conversions at the API boundary we have the requirement to support more than one LDAP API provider. Currently we have: @@ -482,7 +488,14 @@ class LDAPHandler(object): class PythonLDAPHandler(LDAPHandler): """LDAPHandler implementation which calls the python-ldap API. - Note, the python-ldap API requires all string values to be UTF-8 encoded. + Note, the python-ldap API requires all string attribute values to be UTF-8 + encoded. + + Note, in python-ldap some fields (DNs, RDNs, attribute names, queries) + are represented as text (str on Python 3, unicode on Python 2 when + bytes_mode=False). For more details see: + http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode + The KeystoneLDAPHandler enforces this prior to invoking the methods in this class. @@ -503,7 +516,14 @@ class PythonLDAPHandler(LDAPHandler): debug_level=debug_level, timeout=conn_timeout) - self.conn = ldap.initialize(url) + if PY2: + # NOTE: Once https://github.com/python-ldap/python-ldap/issues/249 + # is released, we can pass bytes_strictness='warn' as a parameter + # to ldap.initialize instead of setting it after ldap.initialize. + self.conn = ldap.initialize(url, bytes_mode=False) + self.conn.bytes_strictness = 'warn' + else: + self.conn = ldap.initialize(url) self.conn.protocol_version = ldap.VERSION3 if alias_dereferencing is not None: @@ -653,9 +673,14 @@ class PooledLDAPHandler(LDAPHandler): If 'use_auth_pool' is not enabled, then connection pooling is not used for those LDAP operations. - Note, the python-ldap API requires all string values to be UTF-8 - encoded. The KeystoneLDAPHandler enforces this prior to invoking - the methods in this class. + Note, the python-ldap API requires all string attribute values to be UTF-8 + encoded. The KeystoneLDAPHandler enforces this prior to invoking the + methods in this class. + + Note, in python-ldap some fields (DNs, RDNs, attribute names, queries) + are represented as text (str on Python 3, unicode on Python 2 when + bytes_mode=False). For more details see: + http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode """ @@ -822,11 +847,16 @@ class KeystoneLDAPHandler(LDAPHandler): """Convert data types and perform logging. This LDAP interface wraps the python-ldap based interfaces. The - python-ldap interfaces require string values encoded in UTF-8. The - OpenStack logging framework at the time of this writing is not - capable of accepting strings encoded in UTF-8, the log functions - will throw decoding errors if a non-ascii character appears in a - string. + python-ldap interfaces require string values encoded in UTF-8 with + the exception of [1]. The OpenStack logging framework at the time + of this writing is not capable of accepting strings encoded in + UTF-8, the log functions will throw decoding errors if a non-ascii + character appears in a string. + + [1] In python-ldap, some fields (DNs, RDNs, attribute names, + queries) are represented as text (str on Python 3, unicode on + Python 2 when bytes_mode=False). For more details see: + http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode Prior to the call Python data types are converted to a string representation as required by the LDAP APIs. @@ -885,9 +915,7 @@ class KeystoneLDAPHandler(LDAPHandler): def simple_bind_s(self, who='', cred='', serverctrls=None, clientctrls=None): LOG.debug('LDAP bind: who=%s', who) - who_utf8 = utf8_encode(who) - cred_utf8 = utf8_encode(cred) - return self.conn.simple_bind_s(who_utf8, cred_utf8, + return self.conn.simple_bind_s(who, cred, serverctrls=serverctrls, clientctrls=clientctrls) @@ -904,10 +932,9 @@ class KeystoneLDAPHandler(LDAPHandler): for kind, values in ldap_attrs] LOG.debug('LDAP add: dn=%s attrs=%s', dn, logging_attrs) - dn_utf8 = utf8_encode(dn) ldap_attrs_utf8 = [(kind, [utf8_encode(x) for x in safe_iter(values)]) for kind, values in ldap_attrs] - return self.conn.add_s(dn_utf8, ldap_attrs_utf8) + return self.conn.add_s(dn, ldap_attrs_utf8) def search_s(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0): @@ -924,15 +951,12 @@ class KeystoneLDAPHandler(LDAPHandler): ldap_result = self._paged_search_s(base, scope, filterstr, attrlist) else: - base_utf8 = utf8_encode(base) - filterstr_utf8 = utf8_encode(filterstr) if attrlist is None: attrlist_utf8 = None else: attrlist_utf8 = list(map(utf8_encode, attrlist)) try: - ldap_result = self.conn.search_s(base_utf8, scope, - filterstr_utf8, + ldap_result = self.conn.search_s(base, scope, filterstr, attrlist_utf8, attrsonly) except ldap.SIZELIMIT_EXCEEDED: raise exception.LDAPSizeLimitExceeded() @@ -977,16 +1001,14 @@ class KeystoneLDAPHandler(LDAPHandler): cookie='') page_ctrl_oid = ldap.controls.SimplePagedResultsControl.controlType - base_utf8 = utf8_encode(base) - filterstr_utf8 = utf8_encode(filterstr) if attrlist is None: attrlist_utf8 = None else: attrlist = [attr for attr in attrlist if attr is not None] attrlist_utf8 = list(map(utf8_encode, attrlist)) - msgid = self.conn.search_ext(base_utf8, + msgid = self.conn.search_ext(base, scope, - filterstr_utf8, + filterstr, attrlist_utf8, serverctrls=[lc]) # Endless loop request pages on ldap server until it has no data @@ -1008,9 +1030,9 @@ class KeystoneLDAPHandler(LDAPHandler): if cookie: # There is more data still on the server # so we request another page - msgid = self.conn.search_ext(base_utf8, + msgid = self.conn.search_ext(base, scope, - filterstr_utf8, + filterstr, attrlist_utf8, serverctrls=[lc]) else: @@ -1051,12 +1073,11 @@ class KeystoneLDAPHandler(LDAPHandler): LOG.debug('LDAP modify: dn=%s modlist=%s', dn, logging_modlist) - dn_utf8 = utf8_encode(dn) ldap_modlist_utf8 = [ (op, kind, (None if values is None else [utf8_encode(x) for x in safe_iter(values)])) for op, kind, values in ldap_modlist] - return self.conn.modify_s(dn_utf8, ldap_modlist_utf8) + return self.conn.modify_s(dn, ldap_modlist_utf8) def __exit__(self, exc_type, exc_val, exc_tb): """Exit runtime context, unbind LDAP.""" @@ -1283,7 +1304,7 @@ class BaseLdap(object): @staticmethod def _dn_to_id(dn): - return utf8_decode(ldap.dn.str2dn(utf8_encode(dn))[0][0][1]) + return ldap.dn.str2dn(dn)[0][0][1] def _ldap_res_to_model(self, res): # LDAP attribute names may be returned in a different case than @@ -1769,10 +1790,10 @@ class EnabledEmuMixIn(BaseLdap): naming_attr = (naming_attr_name, [naming_attr_value]) else: # Extract the attribute name and value from the configured DN. - naming_dn = ldap.dn.str2dn(utf8_encode(self.enabled_emulation_dn)) + naming_dn = ldap.dn.str2dn(self.enabled_emulation_dn) naming_rdn = naming_dn[0][0] - naming_attr = (utf8_decode(naming_rdn[0]), - utf8_decode(naming_rdn[1])) + naming_attr = (naming_rdn[0], + naming_rdn[1]) self.enabled_emulation_naming_attr = naming_attr def _get_enabled(self, object_id, conn): diff --git a/keystone/tests/unit/fakeldap.py b/keystone/tests/unit/fakeldap.py index f0bbb59ee7..32067d8188 100644 --- a/keystone/tests/unit/fakeldap.py +++ b/keystone/tests/unit/fakeldap.py @@ -65,7 +65,7 @@ def _internal_attr(attr_name, value_or_values): return 'CN=Doe\\2C John,OU=Users,CN=example,CN=com' try: - dn = ldap.dn.str2dn(common.utf8_encode(dn)) + dn = ldap.dn.str2dn(dn) except ldap.DECODING_ERROR: # NOTE(amakarov): In case of IDs instead of DNs in group members # they must be handled as regular values. @@ -74,10 +74,9 @@ def _internal_attr(attr_name, value_or_values): norm = [] for part in dn: name, val, i = part[0] - name = common.utf8_decode(name) name = name.upper() norm.append([(name, val, i)]) - return common.utf8_decode(ldap.dn.dn2str(norm)) + return ldap.dn.dn2str(norm) if attr_name in ('member', 'roleOccupant'): attr_fn = normalize_dn @@ -216,8 +215,8 @@ PendingRequests = {} class FakeLdap(common.LDAPHandler): """Emulate the python-ldap API. - The python-ldap API requires all strings to be UTF-8 encoded. This - is assured by the caller of this interface + The python-ldap API requires all strings to be UTF-8 encoded with the + exception of [1]. This is assured by the caller of this interface (i.e. KeystoneLDAPHandler). However, internally this emulation MUST process and store strings @@ -228,6 +227,11 @@ class FakeLdap(common.LDAPHandler): emulation, and encodes them back to UTF-8 when returning values from the emulation. + [1] Some fields (DNs, RDNs, attribute names, queries) are represented + as text in python-ldap for Python 3, and for Python 2 when + bytes_mode=False. For more details see: + http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode + """ __prefix = 'ldap:' @@ -278,19 +282,14 @@ class FakeLdap(common.LDAPHandler): self.pool_conn_lifetime = pool_conn_lifetime self.conn_timeout = conn_timeout - def dn(self, dn): - return common.utf8_decode(dn) - def _dn_to_id_attr(self, dn): - return common.utf8_decode( - ldap.dn.str2dn(common.utf8_encode(dn))[0][0][0]) + return ldap.dn.str2dn(dn)[0][0][0] def _dn_to_id_value(self, dn): - return common.utf8_decode( - ldap.dn.str2dn(common.utf8_encode(dn))[0][0][1]) + return ldap.dn.str2dn(dn)[0][0][1] def key(self, dn): - return '%s%s' % (self.__prefix, self.dn(dn)) + return '%s%s' % (self.__prefix, dn) def simple_bind_s(self, who='', cred='', serverctrls=None, clientctrls=None): @@ -298,27 +297,23 @@ class FakeLdap(common.LDAPHandler): if server_fail: raise ldap.SERVER_DOWN whos = ['cn=Admin', CONF.ldap.user] - if (common.utf8_decode(who) in whos and - common.utf8_decode(cred) in ['password', CONF.ldap.password]): + if (who in whos and cred in ['password', CONF.ldap.password]): return attrs = self.db.get(self.key(who)) if not attrs: - LOG.debug('who=%s not found, binding anonymously', - common.utf8_decode(who)) + LOG.debug('who=%s not found, binding anonymously', who) db_password = '' if attrs: try: db_password = attrs['userPassword'][0] except (KeyError, IndexError): - LOG.debug('bind fail: password for who=%s not found', - common.utf8_decode(who)) + LOG.debug('bind fail: password for who=%s not found', who) raise ldap.INAPPROPRIATE_AUTH - if cred != common.utf8_encode(db_password): - LOG.debug('bind fail: password for who=%s does not match', - common.utf8_decode(who)) + if cred != db_password: + LOG.debug('bind fail: password for who=%s does not match', who) raise ldap.INVALID_CREDENTIALS def unbind_s(self): @@ -352,10 +347,9 @@ class FakeLdap(common.LDAPHandler): raise ldap.NAMING_VIOLATION key = self.key(dn) LOG.debug('add item: dn=%(dn)s, attrs=%(attrs)s', { - 'dn': common.utf8_decode(dn), 'attrs': modlist}) + 'dn': dn, 'attrs': modlist}) if key in self.db: - LOG.debug('add item failed: dn=%s is already in store.', - common.utf8_decode(dn)) + LOG.debug('add item failed: dn=%s is already in store.', dn) raise ldap.ALREADY_EXISTS(dn) self.db[key] = {k: _internal_attr(k, v) for k, v in modlist} @@ -369,7 +363,7 @@ class FakeLdap(common.LDAPHandler): return [k for k, v in self.db.items() if re.match('%s.*,%s' % ( re.escape(self.__prefix), - re.escape(self.dn(dn))), k)] + re.escape(dn)), k)] def delete_ext_s(self, dn, serverctrls, clientctrls=None): """Remove the ldap object at specified dn.""" @@ -378,11 +372,10 @@ class FakeLdap(common.LDAPHandler): try: key = self.key(dn) - LOG.debug('FakeLdap delete item: dn=%s', common.utf8_decode(dn)) + LOG.debug('FakeLdap delete item: dn=%s', dn) del self.db[key] except KeyError: - LOG.debug('delete item failed: dn=%s not found.', - common.utf8_decode(dn)) + LOG.debug('delete item failed: dn=%s not found.', dn) raise ldap.NO_SUCH_OBJECT self.db.sync() @@ -398,12 +391,11 @@ class FakeLdap(common.LDAPHandler): key = self.key(dn) LOG.debug('modify item: dn=%(dn)s attrs=%(attrs)s', { - 'dn': common.utf8_decode(dn), 'attrs': modlist}) + 'dn': dn, 'attrs': modlist}) try: entry = self.db[key] except KeyError: - LOG.debug('modify item failed: dn=%s not found.', - common.utf8_decode(dn)) + LOG.debug('modify item failed: dn=%s not found.', dn) raise ldap.NO_SUCH_OBJECT for cmd, k, v in modlist: @@ -483,19 +475,19 @@ class FakeLdap(common.LDAPHandler): for k, v in self.db.items() if re.match('%s.*,%s' % (re.escape(self.__prefix), - re.escape(self.dn(base))), k)] + re.escape(base)), k)] results.extend(extraresults) elif scope == ldap.SCOPE_ONELEVEL: def get_entries(): - base_dn = ldap.dn.str2dn(common.utf8_encode(base)) + base_dn = ldap.dn.str2dn(base) base_len = len(base_dn) for k, v in self.db.items(): if not k.startswith(self.__prefix): continue k_dn_str = k[len(self.__prefix):] - k_dn = ldap.dn.str2dn(common.utf8_encode(k_dn_str)) + k_dn = ldap.dn.str2dn(k_dn_str) if len(k_dn) != base_len + 1: continue if k_dn[-base_len:] != base_dn: @@ -511,13 +503,11 @@ class FakeLdap(common.LDAPHandler): objects = [] for dn, attrs in results: # filter the objects by filterstr - id_attr, id_val, _ = ldap.dn.str2dn(common.utf8_encode(dn))[0][0] - id_attr = common.utf8_decode(id_attr) - id_val = common.utf8_decode(id_val) + id_attr, id_val, _ = ldap.dn.str2dn(dn)[0][0] match_attrs = attrs.copy() match_attrs[id_attr] = [id_val] attrs_checked = set() - if not filterstr or _match_query(common.utf8_decode(filterstr), + if not filterstr or _match_query(filterstr, match_attrs, attrs_checked): if (filterstr and @@ -650,8 +640,7 @@ class FakeLdapNoSubtreeDelete(FakeLdap): raise ldap.NOT_ALLOWED_ON_NONLEAF except KeyError: - LOG.debug('delete item failed: dn=%s not found.', - common.utf8_decode(dn)) + LOG.debug('delete item failed: dn=%s not found.', dn) raise ldap.NO_SUCH_OBJECT super(FakeLdapNoSubtreeDelete, self).delete_ext_s(dn, serverctrls, diff --git a/keystone/tests/unit/test_backend_ldap_pool.py b/keystone/tests/unit/test_backend_ldap_pool.py index 1754942fb3..34fcded3d2 100644 --- a/keystone/tests/unit/test_backend_ldap_pool.py +++ b/keystone/tests/unit/test_backend_ldap_pool.py @@ -231,15 +231,3 @@ class LDAPIdentity(LdapPoolCommonTestMixin, config_files = super(LDAPIdentity, self).config_files() config_files.append(unit.dirs.tests_conf('backend_ldap_pool.conf')) return config_files - - @mock.patch.object(common_ldap, 'utf8_encode') - def test_utf8_encoded_is_used_in_pool(self, mocked_method): - def side_effect(arg): - return arg - mocked_method.side_effect = side_effect - # invalidate the cache to get utf8_encode function called. - PROVIDERS.identity_api.get_user.invalidate(PROVIDERS.identity_api, - self.user_foo['id']) - PROVIDERS.identity_api.get_user(self.user_foo['id']) - mocked_method.assert_any_call(CONF.ldap.user) - mocked_method.assert_any_call(CONF.ldap.password) diff --git a/keystone/tests/unit/test_ldap_livetest.py b/keystone/tests/unit/test_ldap_livetest.py index 34eb81d5f6..2be31f0215 100644 --- a/keystone/tests/unit/test_ldap_livetest.py +++ b/keystone/tests/unit/test_ldap_livetest.py @@ -16,6 +16,7 @@ import subprocess import ldap.modlist from six.moves import range +from six import PY2 from keystone.common import provider_api import keystone.conf @@ -30,7 +31,14 @@ PROVIDERS = provider_api.ProviderAPIs def create_object(dn, attrs): - conn = ldap.initialize(CONF.ldap.url) + if PY2: + # NOTE: Once https://github.com/python-ldap/python-ldap/issues/249 + # is released, we can pass bytes_strictness='warn' as a parameter to + # ldap.initialize instead of setting it after ldap.initialize. + conn = ldap.initialize(CONF.ldap.url, bytes_mode=False) + conn.bytes_strictness = 'warn' + else: + conn = ldap.initialize(CONF.ldap.url) conn.simple_bind_s(CONF.ldap.user, CONF.ldap.password) ldif = ldap.modlist.addModlist(attrs) conn.add_s(dn, ldif) diff --git a/keystone/tests/unit/test_ldap_tls_livetest.py b/keystone/tests/unit/test_ldap_tls_livetest.py index b1541c5d44..bf785c9f8e 100644 --- a/keystone/tests/unit/test_ldap_tls_livetest.py +++ b/keystone/tests/unit/test_ldap_tls_livetest.py @@ -14,6 +14,7 @@ # under the License. import ldap.modlist +from six import PY2 from keystone.common import provider_api import keystone.conf @@ -28,7 +29,14 @@ PROVIDERS = provider_api.ProviderAPIs def create_object(dn, attrs): - conn = ldap.initialize(CONF.ldap.url) + if PY2: + # NOTE: Once https://github.com/python-ldap/python-ldap/issues/249 + # is released, we can pass bytes_strictness='warn' as a parameter to + # ldap.initialize instead of setting it after ldap.initialize. + conn = ldap.initialize(CONF.ldap.url, bytes_mode=False) + conn.bytes_strictness = 'warn' + else: + conn = ldap.initialize(CONF.ldap.url) conn.simple_bind_s(CONF.ldap.user, CONF.ldap.password) ldif = ldap.modlist.addModlist(attrs) conn.add_s(dn, ldif) diff --git a/lower-constraints.txt b/lower-constraints.txt index ceb813bead..f5e6e4641e 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -17,7 +17,7 @@ iso8601==0.1.12 jsonschema==2.6.0 keystoneauth1==3.4.0 keystonemiddleware==5.1.0 -ldappool===2.0.0 +ldappool===2.3.1 lxml==3.4.1 mock==2.0.0 msgpack==0.5.0 diff --git a/setup.cfg b/setup.cfg index 8d337f6fb0..132e29db45 100644 --- a/setup.cfg +++ b/setup.cfg @@ -28,7 +28,7 @@ packages = [extras] ldap = python-ldap>=3.0.0 # PSF - ldappool>=2.0.0 # MPL + ldappool>=2.3.1 # MPL memcache = python-memcached>=1.56 # PSF mongodb =