From 96cadaec3c16b296627991ed517d7015fbe2882f Mon Sep 17 00:00:00 2001 From: Jan Cholasta 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 --- 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