openstack/keystone
Code Issues Proposed changes

PY3: switch to using unicode text values

Browse Source 
In Python 3, python-ldap no longer allows bytes for some fields (DNs,
RDNs, attribute names, queries). Instead, text values are represented
as str, the Unicode text type. Compatibility support is provided for
Python 2 by setting bytes_mode=False [1].

Update the keystone LDAP backend to adhere to this behavior by using
bytes_mode=False for Python 2 and dropping UTF-8 encoding and decoding
fields that are now represented as text in python-ldap.

[1] More details about byte/str usage in python-ldap can be found at:
http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode

Note that at a minimum python-ldappool 2.3.1 is required. For more
details see Depends-On's below.

Change-Id: Ifdd0644cd7042407a008c85c0b2c40a971c90bc3
Closes-Bug: #1798184
Depends-On: https://review.openstack.org/611401
Depends-On: https://review.openstack.org/613632
Depends-On: https://review.openstack.org/614052
This commit is contained in:
Corey Bryant 2018-10-16 16:19:15 -04:00
parent f81afc7ce6
commit eca0829c4c
7 changed files with 110 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

99
keystone/identity/backends/ldap/common.py
View File

@ -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):

71
keystone/tests/unit/fakeldap.py
View File

@ -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,

12
keystone/tests/unit/test_backend_ldap_pool.py
View File

@ -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)

10
keystone/tests/unit/test_ldap_livetest.py
View File

@ -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)

10
keystone/tests/unit/test_ldap_tls_livetest.py
View File

@ -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)

2
lower-constraints.txt
View File

@ -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

2
setup.cfg
View File

@ -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 =