diff options
| -rw-r--r-- | .zuul.yaml | 4 | ||||
| -rw-r--r-- | keystone/identity/backends/ldap/common.py | 118 | ||||
| -rw-r--r-- | keystone/tests/unit/identity/backends/test_ldap_common.py | 14 | ||||
| -rw-r--r-- | lower-constraints.txt | 2 | ||||
| -rw-r--r-- | releasenotes/notes/bug-1794527-866b1caff67977f3.yaml | 2 | ||||
| -rw-r--r-- | releasenotes/notes/bug-1889936-78d6853b5212b8f1.yaml | 5 | ||||
| -rw-r--r-- | releasenotes/notes/bug-1896125-b17a4d12730fe493.yaml | 7 |
7 files changed, 108 insertions, 44 deletions
diff --git a/.zuul.yaml b/.zuul.yaml index 8fa6bb277..edeca5efa 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -176,7 +176,6 @@ - project: templates: - openstack-cover-jobs - - openstack-lower-constraints-jobs - openstack-python-jobs - openstack-python3-train-jobs - publish-openstack-docs-pti @@ -204,6 +203,7 @@ voting: false irrelevant-files: *irrelevant-files - keystone-dsvm-py3-functional-federation-opensuse15-k2k: + voting: false irrelevant-files: *irrelevant-files - keystoneclient-devstack-functional: voting: false @@ -236,8 +236,6 @@ irrelevant-files: *irrelevant-files - keystone-dsvm-py3-functional: irrelevant-files: *irrelevant-files - - keystone-dsvm-py3-functional-federation-opensuse15-k2k: - irrelevant-files: *irrelevant-files - tempest-full: irrelevant-files: *tempest-irrelevant-files - tempest-full-py3: diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index cef0d9bdf..879da2986 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -14,10 +14,10 @@ import abc import codecs -import functools import os.path import re import sys +import uuid import weakref import ldap.controls @@ -95,7 +95,16 @@ def utf8_decode(value): :raises UnicodeDecodeError: for invalid UTF-8 encoding """ if isinstance(value, six.binary_type): - return _utf8_decoder(value)[0] + try: + return _utf8_decoder(value)[0] + except UnicodeDecodeError: + # NOTE(lbragstad): We could be dealing with a UUID in byte form, + # which some LDAP implementations use. + uuid_byte_string_length = 16 + if len(value) == uuid_byte_string_length: + return six.text_type(uuid.UUID(bytes_le=value)) + else: + raise return six.text_type(value) @@ -635,10 +644,36 @@ def _common_ldap_initialization(url, use_tls=False, tls_cacertfile=None, tls_req_cert) -class MsgId(list): - """Wrapper class to hold connection and msgid.""" +class AsynchronousMessage(object): + """A container for handling asynchronous LDAP responses. + + Some LDAP APIs, like `search_ext`, are asynchronous and return a message ID + when the server successfully initiates the operation. Clients can use this + message ID and the original connection to make the request to fetch the + results using `result3`. + + This object holds the message ID, the original connection, and a callable + weak reference Finalizer that cleans up context managers specific to the + connection associated to the message ID. + + :param message_id: The message identifier (str). + :param connection: The connection associated with the message identifier + (ldappool.StateConnector). + + The `clean` attribute is a callable that cleans up the context manager used + to create or return the connection object (weakref.finalize). - pass + """ + + def __init__(self, message_id, connection, context_manager): + self.id = message_id + self.connection = connection + self.clean = weakref.finalize( + self, self._cleanup_connection_context_manager, context_manager + ) + + def _cleanup_connection_context_manager(self, context_manager): + context_manager.__exit__(None, None, None) def use_conn_pool(func): @@ -792,15 +827,17 @@ class PooledLDAPHandler(LDAPHandler): filterstr='(objectClass=*)', attrlist=None, attrsonly=0, serverctrls=None, clientctrls=None, timeout=-1, sizelimit=0): - """Return a ``MsgId`` instance, it asynchronous API. + """Return an AsynchronousMessage instance, it asynchronous API. - The ``MsgId`` instance can be safely used in a call to ``result3()``. + The AsynchronousMessage instance can be safely used in a call to + `result3()`. - To work with ``result3()`` API in predictable manner, the same LDAP - connection is needed which originally provided the ``msgid``. So, this - method wraps the existing connection and ``msgid`` in a new ``MsgId`` - instance. The connection associated with ``search_ext`` is released - once last hard reference to the ``MsgId`` instance is freed. + To work with `result3()` API in predictable manner, the same LDAP + connection is needed which originally provided the `msgid`. So, this + method wraps the existing connection and `msgid` in a new + `AsynchronousMessage` instance. The connection associated with + `search_ext()` is released after `result3()` fetches the data + associated with `msgid`. """ conn_ctxt = self._get_pool_connection() @@ -813,30 +850,33 @@ class PooledLDAPHandler(LDAPHandler): except Exception: conn_ctxt.__exit__(*sys.exc_info()) raise - res = MsgId((conn, msgid)) - weakref.ref(res, functools.partial(conn_ctxt.__exit__, - None, None, None)) - return res + return AsynchronousMessage(msgid, conn, conn_ctxt) - def result3(self, msgid, all=1, timeout=None, + def result3(self, message, all=1, timeout=None, resp_ctrl_classes=None): - """Wait for and return the result. + """Wait for and return the result to an asynchronous message. This method returns the result of an operation previously initiated by - one of the LDAP asynchronous operation routines (eg search_ext()). It - returned an invocation identifier (a message id) upon successful - initiation of their operation. + one of the LDAP asynchronous operation routines (e.g., `search_ext()`). + The `search_ext()` method in python-ldap returns an invocation + identifier, or a message ID, upon successful initiation of the + operation by the LDAP server. - Input msgid is expected to be instance of class MsgId which has LDAP - session/connection used to execute search_ext and message idenfier. + The `message` is expected to be instance of class + `AsynchronousMessage`, which contains the message ID and the connection + used to make the original request. - The connection associated with search_ext is released once last hard - reference to MsgId object is freed. This will happen when function - which requested msgId and used it in result3 exits. + The connection and context manager associated with `search_ext()` are + cleaned up when message.clean() is called. """ - conn, msg_id = msgid - return conn.result3(msg_id, all, timeout) + results = message.connection.result3(message.id, all, timeout) + + # Now that we have the results from the LDAP server for the message, we + # don't need the the context manager used to create the connection. + message.clean() + + return results @use_conn_pool def modify_s(self, conn, dn, modlist): @@ -997,15 +1037,15 @@ class KeystoneLDAPHandler(LDAPHandler): cookie='') page_ctrl_oid = ldap.controls.SimplePagedResultsControl.controlType - msgid = self.conn.search_ext(base, - scope, - filterstr, - attrlist, - serverctrls=[lc]) + message = self.conn.search_ext(base, + scope, + filterstr, + attrlist, + serverctrls=[lc]) # Endless loop request pages on ldap server until it has no data while True: # Request to the ldap server a page with 'page_size' entries - rtype, rdata, rmsgid, serverctrls = self.conn.result3(msgid) + rtype, rdata, rmsgid, serverctrls = self.conn.result3(message) # Receive the data res.extend(rdata) pctrls = [c for c in serverctrls @@ -1021,11 +1061,11 @@ class KeystoneLDAPHandler(LDAPHandler): if cookie: # There is more data still on the server # so we request another page - msgid = self.conn.search_ext(base, - scope, - filterstr, - attrlist, - serverctrls=[lc]) + message = self.conn.search_ext(base, + scope, + filterstr, + attrlist, + serverctrls=[lc]) else: # Exit condition no more data on server break diff --git a/keystone/tests/unit/identity/backends/test_ldap_common.py b/keystone/tests/unit/identity/backends/test_ldap_common.py index e464a8a14..2a0d9ab28 100644 --- a/keystone/tests/unit/identity/backends/test_ldap_common.py +++ b/keystone/tests/unit/identity/backends/test_ldap_common.py @@ -520,6 +520,20 @@ class CommonLdapTestCase(unit.BaseTestCase): # The user name should still be a string value. self.assertEqual(user_name, py_result[0][1]['user_name'][0]) + def test_user_id_attribute_is_uuid_in_byte_form(self): + results = [( + 'cn=alice,dc=example,dc=com', + { + 'cn': [b'cn=alice'], + 'objectGUID': [b'\xdd\xd8Rt\xee]bA\x8e(\xe39\x0b\xe1\xf8\xe8'], + 'email': [uuid.uuid4().hex], + 'sn': [uuid.uuid4().hex] + } + )] + py_result = common_ldap.convert_ldap_result(results) + exp_object_guid = '7452d8dd-5dee-4162-8e28-e3390be1f8e8' + self.assertEqual(exp_object_guid, py_result[0][1]['objectGUID'][0]) + class LDAPFilterQueryCompositionTest(unit.BaseTestCase): """These test cases test LDAP filter generation.""" diff --git a/lower-constraints.txt b/lower-constraints.txt index 6ccbdc230..a6fa74847 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -1,4 +1,4 @@ -amqp==2.2.2 +amqp==5.0.0 Babel==2.3.4 bashate==0.5.1 bcrypt==3.1.3 diff --git a/releasenotes/notes/bug-1794527-866b1caff67977f3.yaml b/releasenotes/notes/bug-1794527-866b1caff67977f3.yaml index 6a8134fd2..04435288d 100644 --- a/releasenotes/notes/bug-1794527-866b1caff67977f3.yaml +++ b/releasenotes/notes/bug-1794527-866b1caff67977f3.yaml @@ -18,4 +18,4 @@ features: the old one, and having spurious failures, or undesirecd domain_id matching. - https://specs.openstack.org/openstack/keystone-specs/specs/keystone/stein/explicit-domains-ids.html
\ No newline at end of file + https://specs.openstack.org/openstack/keystone-specs/specs/keystone/train/explicit-domains-ids.html diff --git a/releasenotes/notes/bug-1889936-78d6853b5212b8f1.yaml b/releasenotes/notes/bug-1889936-78d6853b5212b8f1.yaml new file mode 100644 index 000000000..de96b27f7 --- /dev/null +++ b/releasenotes/notes/bug-1889936-78d6853b5212b8f1.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + [`bug 1889936 <https://bugs.launchpad.net/keystone/+bug/1889936>`_] + Properly decode octet strings, or byte arrays, returned from LDAP. diff --git a/releasenotes/notes/bug-1896125-b17a4d12730fe493.yaml b/releasenotes/notes/bug-1896125-b17a4d12730fe493.yaml new file mode 100644 index 000000000..7a758b9ab --- /dev/null +++ b/releasenotes/notes/bug-1896125-b17a4d12730fe493.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`bug 1896125 <https://bugs.launchpad.net/keystone/+bug/1896125>`_] + Introduced more robust connection handling for asynchronous LDAP requests + to address memory leaks fetching data from LDAP backends with low page + sizes. |