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

@@ -711,11 +711,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):
@@ -1024,29 +1023,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):
@@ -1060,7 +1046,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,
@@ -1071,7 +1057,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):
@@ -1081,7 +1067,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):
@@ -1090,7 +1076,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

@@ -77,10 +77,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()
 
@@ -120,10 +117,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():
             minssf = conn.get_option(_ldap.OPT_X_SASL_SSF_MIN)
             maxssf = conn.get_option(_ldap.OPT_X_SASL_SSF_MAX)
             # Always connect with at least an SSF of 56, confidentiality
@@ -137,15 +135,15 @@ class ldap2(CrudBackend, LDAPClient):
         ldapi = self.ldap_uri.startswith('ldapi://')
 
         if bind_pw:
-            self.simple_bind(bind_dn, bind_pw,
-                             server_controls=serverctrls,
-                             client_controls=clientctrls)
+            client.simple_bind(bind_dn, bind_pw,
+                               server_controls=serverctrls,
+                               client_controls=clientctrls)
         elif autobind != AUTOBIND_DISABLED and os.getegid() == 0 and ldapi:
             try:
                 pw_name = pwd.getpwuid(os.geteuid()).pw_name
-                self.external_bind(pw_name,
-                                   server_controls=serverctrls,
-                                   client_controls=clientctrls)
+                client.external_bind(pw_name,
+                                     server_controls=serverctrls,
+                                     client_controls=clientctrls)
             except errors.NotFound:
                 if autobind == AUTOBIND_ENABLED:
                     # autobind was required and failed, raise
@@ -153,7 +151,7 @@ class ldap2(CrudBackend, LDAPClient):
                     raise
         else:
             if ldapi:
-                with self.error_handler():
+                with client.error_handler():
                     conn.set_option(_ldap.OPT_HOST_NAME, self.api.env.host)
             if ccache is None:
                 os.environ.pop('KRB5CCNAME', None)
@@ -162,8 +160,8 @@ class ldap2(CrudBackend, LDAPClient):
 
             principal = krb_utils.get_principal(ccache_name=ccache)
 
-            self.gssapi_bind(server_controls=serverctrls,
-                             client_controls=clientctrls)
+            client.gssapi_bind(server_controls=serverctrls,
+                               client_controls=clientctrls)
             setattr(context, 'principal', principal)
 
         return conn
@@ -171,9 +169,8 @@ class ldap2(CrudBackend, LDAPClient):
     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