From d6df1dff3e519a26c1a12b6c32f9799484be5966 Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Mon, 18 Mar 2019 13:46:37 -0400 Subject: [PATCH] PY3: Ensure LDAP searches use unicode attributes This is a bug fix that corresponds to changes missed in commit eca0829c4c65e6b64f08023ce2d5a55dc329248f. 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]. This support was provided in commit eca0829c4c65e6b64f08023ce2d5a55dc329248f. In this patch we ensure that attribute names specified in searches are no longer encoded. [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 Change-Id: If3398e2d08ea14fa4b8c498b2a9a7c7edb47b9e5 Closes-Bug: #1820333 --- keystone/identity/backends/ldap/common.py | 17 ++++------------- keystone/tests/unit/fakeldap.py | 2 +- .../unit/identity/backends/test_ldap_common.py | 9 ++++++++- .../notes/bug-1820333-356dcc8bf9f73fed.yaml | 18 ++++++++++++++++++ 4 files changed, 31 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/bug-1820333-356dcc8bf9f73fed.yaml diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index 6b5fd66fa2..e2f85a7ee7 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -26,7 +26,7 @@ import ldappool from oslo_log import log from oslo_utils import reflection import six -from six.moves import map, zip +from six.moves import zip from six import PY2 from keystone.common import driver_hints @@ -951,13 +951,9 @@ class KeystoneLDAPHandler(LDAPHandler): ldap_result = self._paged_search_s(base, scope, filterstr, attrlist) else: - if attrlist is None: - attrlist_utf8 = None - else: - attrlist_utf8 = list(map(utf8_encode, attrlist)) try: ldap_result = self.conn.search_s(base, scope, filterstr, - attrlist_utf8, attrsonly) + attrlist, attrsonly) except ldap.SIZELIMIT_EXCEEDED: raise exception.LDAPSizeLimitExceeded() @@ -1001,15 +997,10 @@ class KeystoneLDAPHandler(LDAPHandler): cookie='') page_ctrl_oid = ldap.controls.SimplePagedResultsControl.controlType - 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, scope, filterstr, - attrlist_utf8, + attrlist, serverctrls=[lc]) # Endless loop request pages on ldap server until it has no data while True: @@ -1033,7 +1024,7 @@ class KeystoneLDAPHandler(LDAPHandler): msgid = self.conn.search_ext(base, scope, filterstr, - attrlist_utf8, + attrlist, serverctrls=[lc]) else: # Exit condition no more data on server diff --git a/keystone/tests/unit/fakeldap.py b/keystone/tests/unit/fakeldap.py index ab5de95432..ed748de7d7 100644 --- a/keystone/tests/unit/fakeldap.py +++ b/keystone/tests/unit/fakeldap.py @@ -516,7 +516,7 @@ class FakeLdap(common.LDAPHandler): raise AssertionError('No objectClass in search filter') # filter the attributes by attrlist attrs = {k: v for k, v in attrs.items() - if not attrlist or k in common.utf8_decode(attrlist)} + if not attrlist or k in attrlist} objects.append((dn, attrs)) return objects diff --git a/keystone/tests/unit/identity/backends/test_ldap_common.py b/keystone/tests/unit/identity/backends/test_ldap_common.py index 66dbd2c630..e464a8a142 100644 --- a/keystone/tests/unit/identity/backends/test_ldap_common.py +++ b/keystone/tests/unit/identity/backends/test_ldap_common.py @@ -404,7 +404,14 @@ class LDAPPagedResultsTest(unit.TestCase): conn = PROVIDERS.identity_api.user.get_connection() conn._paged_search_s('dc=example,dc=test', ldap.SCOPE_SUBTREE, - 'objectclass=*') + 'objectclass=*', + ['mail', 'userPassword']) + # verify search_ext() args - attrlist is tricky due to ordering + args, _ = mock_search_ext.call_args + self.assertEqual( + ('dc=example,dc=test', 2, 'objectclass=*'), args[0:3]) + attrlist = sorted([attr for attr in args[3] if attr]) + self.assertEqual(['mail', 'userPassword'], attrlist) class CommonLdapTestCase(unit.BaseTestCase): diff --git a/releasenotes/notes/bug-1820333-356dcc8bf9f73fed.yaml b/releasenotes/notes/bug-1820333-356dcc8bf9f73fed.yaml new file mode 100644 index 0000000000..0ee5ff812c --- /dev/null +++ b/releasenotes/notes/bug-1820333-356dcc8bf9f73fed.yaml @@ -0,0 +1,18 @@ +--- +fixes: + - | + [`bug 1798184 `_] + [`bug 1820333 `_] + 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]. + + The keystone LDAP backend is updated 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.