From 96cadaec3c16b296627991ed517d7015fbe2882f Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jcholast@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
Reviewed-By: Tomas Babej <tbabej@redhat.com>
---
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