On 2.9.2015 14:51, Martin Basti wrote:



On 09/02/2015 02:32 PM, Jan Cholasta wrote:
Hi,

the attached patch fixes <https://fedorahosted.org/freeipa/ticket/5268>.

Honza




This patch needs a big rebase to ipa-4-2 branch

Patch attached.

--
Jan Cholasta
From bde838afa9ebd7dabda03208c79959df21369006 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@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
---
 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 ef7c41a..4443db0 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 68feee4..deb0592 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.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to