From 96cadaec3c16b296627991ed517d7015fbe2882f Mon Sep 17 00:00:00 2001

From: Jan Cholasta <jcholast@redhat.com>

Date: Wed, 2 Sep 2015 14:04:17 +0200

Subject: [PATCH] ldap: Make ldap2 connection management thread-safe again

This fixes the connection code in LDAPClient to not store the LDAP connection

in an attribute of the object, which in combination with ldap2's per-thread

connections lead to race conditions resulting in connection failures. ldap2

code was updated accordingly.

https://fedorahosted.org/freeipa/ticket/5268

Reviewed-By: Tomas Babej <tbabej@redhat.com>

---

 ipapython/ipaldap.py       | 32 +++++++++-----------------------

 ipaserver/plugins/ldap2.py | 29 +++++++++++++----------------

 2 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py

index ef7c41a37c1f46290afbb11f912d7a4b8ea224c9..4443db03bcee25033abf63786016a7931f7eed20 100644

--- a/ipapython/ipaldap.py

+++ b/ipapython/ipaldap.py

@@ -710,11 +710,10 @@ class LDAPClient(object):

         self._decode_attrs = decode_attrs

         self.log = log_mgr.get_logger(self)

-        self._conn = None

         self._has_schema = False

         self._schema = None

-        self._connect()

+        self._conn = self._connect()

     @property

     def conn(self):

@@ -1023,29 +1022,16 @@ class LDAPClient(object):

         """

         Close the connection.

         """

-        if self._conn is not None:

-            self._disconnect()

+        self._conn = None

     def _connect(self):

-        if self._conn is not None:

-            raise errors.DatabaseError(

-                desc="Can't connect to server", info="Already connected")

-

         with self.error_handler():

-            # bypass ldap2's locking

-            object.__setattr__(self, '_conn',

-                               ldap.initialize(self.ldap_uri))

+            conn = ldap.initialize(self.ldap_uri)

             if self._start_tls:

-                self._conn.start_tls_s()

-

-    def _disconnect(self):

-        if self._conn is None:

-            raise errors.DatabaseError(

-                desc="Can't disconnect from server", info="Not connected")

+                conn.start_tls_s()

-        # bypass ldap2's locking

-        object.__setattr__(self, '_conn', None)

+        return conn

     def simple_bind(self, bind_dn, bind_password, server_controls=None,

                     client_controls=None):

@@ -1059,7 +1045,7 @@ class LDAPClient(object):

             assert isinstance(bind_dn, DN)

             bind_dn = str(bind_dn)

             bind_password = self.encode(bind_password)

-            self._conn.simple_bind_s(

+            self.conn.simple_bind_s(

                 bind_dn, bind_password, server_controls, client_controls)

     def external_bind(self, user_name, server_controls=None,

@@ -1070,7 +1056,7 @@ class LDAPClient(object):

         with self.error_handler():

             auth_tokens = ldap.sasl.external(user_name)

             self._flush_schema()

-            self._conn.sasl_interactive_bind_s(

+            self.conn.sasl_interactive_bind_s(

                 '', auth_tokens, server_controls, client_controls)

     def gssapi_bind(self, server_controls=None, client_controls=None):

@@ -1080,7 +1066,7 @@ class LDAPClient(object):

         with self.error_handler():

             auth_tokens = ldap.sasl.sasl({}, 'GSSAPI')

             self._flush_schema()

-            self._conn.sasl_interactive_bind_s(

+            self.conn.sasl_interactive_bind_s(

                 '', auth_tokens, server_controls, client_controls)

     def unbind(self):

@@ -1089,7 +1075,7 @@ class LDAPClient(object):

         """

         with self.error_handler():

             self._flush_schema()

-            self._conn.unbind_s()

+            self.conn.unbind_s()

     def make_dn_from_attr(self, attr, value, parent_dn=None):

         """

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py

index 68feee4f09eb12e50867dfbe3c482a359838aa82..deb0592ab68ab8eb712a6d29fdffd8776e2e289a 100644

--- a/ipaserver/plugins/ldap2.py

+++ b/ipaserver/plugins/ldap2.py

@@ -76,10 +76,7 @@ class ldap2(CrudBackend, LDAPClient):

         # do not set it

         pass

-    def _disconnect(self):

-        pass

-

-    def __del__(self):

+    def close(self):

         if self.isconnected():

             self.disconnect()

@@ -118,10 +115,11 @@ class ldap2(CrudBackend, LDAPClient):

         if debug_level:

             _ldap.set_option(_ldap.OPT_DEBUG_LEVEL, debug_level)

-        LDAPClient._connect(self)

-        conn = self._conn

+        client = LDAPClient(self.ldap_uri,

+                            force_schema_updates=self._force_schema_updates)

+        conn = client._conn

-        with self.error_handler():

+        with client.error_handler():

             if self.ldap_uri.startswith('ldapi://') and ccache:

                 conn.set_option(_ldap.OPT_HOST_NAME, self.api.env.host)

             minssf = conn.get_option(_ldap.OPT_X_SASL_SSF_MIN)

@@ -147,29 +145,28 @@ class ldap2(CrudBackend, LDAPClient):

                     context=krbV.default_context()).principal().name

             os.environ['KRB5CCNAME'] = ccache

-            self.gssapi_bind(server_controls=serverctrls,

-                             client_controls=clientctrls)

+            client.gssapi_bind(server_controls=serverctrls,

+                               client_controls=clientctrls)

             setattr(context, 'principal', principal)

         else:

             # no kerberos ccache, use simple bind or external sasl

             if autobind:

                 pent = pwd.getpwuid(os.geteuid())

-                self.external_bind(pent.pw_name,

+                client.external_bind(pent.pw_name,

+                                     server_controls=serverctrls,

+                                     client_controls=clientctrls)

+            else:

+                client.simple_bind(bind_dn, bind_pw,

                                    server_controls=serverctrls,

                                    client_controls=clientctrls)

-            else:

-                self.simple_bind(bind_dn, bind_pw,

-                                 server_controls=serverctrls,

-                                 client_controls=clientctrls)

         return conn

     def destroy_connection(self):

         """Disconnect from LDAP server."""

         try:

-            if self._conn is not None:

+            if self.conn is not None:

                 self.unbind()

-                LDAPClient._disconnect(self)

         except errors.PublicError:

             # ignore when trying to unbind multiple times

             pass

-- 

2.5.1