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

@@ -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()
+                conn.start_tls_s()
 
-    def _disconnect(self):
-        if self._conn is None:
-            raise errors.DatabaseError(
-                desc="Can't disconnect from server", info="Not connected")
-
-        # 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):
         """

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