From 198908ec78b9a2dbdb802c3a094ec8f54b931d7a Mon Sep 17 00:00:00 2001 From: Jan Cholasta Date: Sep 04 2015 11:31:46 +0000 Subject: 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 --- diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 705d694..1279a18 100644 --- a/ipapython/ipaldap.py +++ b/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() - - 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): @@ -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): """ diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index acaf45f..abeb522 100644 --- a/ipaserver/plugins/ldap2.py +++ b/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