File 0001-Set-connection-timeout-for-LDAP-configuration.patch of Package openstack-keystone
From 6d7a207a85080d5eb6135b71358c62bca633f8ee Mon Sep 17 00:00:00 2001
From: Kam Nasim <kam.nasim@windriver.com>
Date: Wed, 11 Jan 2017 18:55:40 +0000
Subject: [PATCH] Set connection timeout for LDAP configuration
Presently the Identity LDAP driver does not set a connection timeout
option which has the disadvantage of causing the Identity LDAP backend
handler to stall indefinitely (or until TCP timeout) on LDAP bind, if
a) the LDAP URL is incorrect, or b) there is a connection failure/link
loss.
This commit add a new option to set the LDAP connection timeout to
set a new OPT_NETWORK_TIMEOUT option on the LDAP object. This will
raise ldap.SERVER_DOWN exceptions on timeout.
Signed-off-by: Kam Nasim <kam.nasim@windriver.com>
Closes-Bug: #1636950
Change-Id: I574e6368169ad60bef2cc990d2d410a638d1b770
(cherry picked from commit 2d239cfbc37573f245e6560b42117828b73d19b9)
---
keystone/conf/ldap.py | 16 +++-
keystone/exception.py | 7 ++
keystone/identity/backends/ldap/common.py | 95 +++++++++++++---------
keystone/tests/unit/fakeldap.py | 4 +-
.../unit/identity/backends/test_ldap_common.py | 53 ++++++++++++
.../notes/bug-1636950-8fa1a47fce440977.yaml | 10 +++
6 files changed, 141 insertions(+), 44 deletions(-)
create mode 100644 releasenotes/notes/bug-1636950-8fa1a47fce440977.yaml
diff --git a/keystone/conf/ldap.py b/keystone/conf/ldap.py
index 160ac0fa3..af2c1a3b5 100644
--- a/keystone/conf/ldap.py
+++ b/keystone/conf/ldap.py
@@ -489,6 +489,15 @@ always be requested but not required from the LDAP server. If set to `never`,
then a certificate will never be requested.
"""))
+connection_timeout = cfg.IntOpt(
+ 'connection_timeout',
+ default=-1,
+ min=-1,
+ help=utils.fmt("""
+The connection timeout to use with the LDAP server. A value of `-1` means that
+connections will never timeout.
+"""))
+
use_pool = cfg.BoolOpt(
'use_pool',
default=True,
@@ -529,9 +538,9 @@ pool_connection_timeout = cfg.IntOpt(
default=-1,
min=-1,
help=utils.fmt("""
-The connection timeout to use with the LDAP server. A value of `-1` means that
-connections will never timeout. This option has no effect unless `[ldap]
-use_pool` is also enabled.
+The connection timeout to use when pooling LDAP connections. A value of `-1`
+means that connections will never timeout. This option has no effect unless
+`[ldap] use_pool` is also enabled.
"""))
pool_connection_lifetime = cfg.IntOpt(
@@ -627,6 +636,7 @@ ALL_OPTS = [
tls_cacertdir,
use_tls,
tls_req_cert,
+ connection_timeout,
use_pool,
pool_size,
pool_retry_max,
diff --git a/keystone/exception.py b/keystone/exception.py
index d941f23ee..bb7c8247f 100644
--- a/keystone/exception.py
+++ b/keystone/exception.py
@@ -592,3 +592,10 @@ class UnsupportedDriverVersion(UnexpectedError):
class CredentialEncryptionError(Exception):
message_format = _("An unexpected error prevented the server "
"from accessing encrypted credentials.")
+
+
+class LDAPServerConnectionError(Error):
+ message_format = _('Timed out waiting to establish a '
+ 'connection to the LDAP Server (%(url)s).')
+ code = 504
+ title = 'Gateway Timeout'
diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py
index 09876ad02..e8d0b9872 100644
--- a/keystone/identity/backends/ldap/common.py
+++ b/keystone/identity/backends/ldap/common.py
@@ -428,8 +428,8 @@ class LDAPHandler(object):
def connect(self, url, page_size=0, alias_dereferencing=None,
use_tls=False, tls_cacertfile=None, tls_cacertdir=None,
tls_req_cert=ldap.OPT_X_TLS_DEMAND, chase_referrals=None,
- debug_level=None, use_pool=None, pool_size=None,
- pool_retry_max=None, pool_retry_delay=None,
+ debug_level=None, conn_timeout=None, use_pool=None,
+ pool_size=None, pool_retry_max=None, pool_retry_delay=None,
pool_conn_timeout=None, pool_conn_lifetime=None):
raise exception.NotImplemented() # pragma: no cover
@@ -496,8 +496,8 @@ class PythonLDAPHandler(LDAPHandler):
def connect(self, url, page_size=0, alias_dereferencing=None,
use_tls=False, tls_cacertfile=None, tls_cacertdir=None,
tls_req_cert=ldap.OPT_X_TLS_DEMAND, chase_referrals=None,
- debug_level=None, use_pool=None, pool_size=None,
- pool_retry_max=None, pool_retry_delay=None,
+ debug_level=None, conn_timeout=None, use_pool=None,
+ pool_size=None, pool_retry_max=None, pool_retry_delay=None,
pool_conn_timeout=None, pool_conn_lifetime=None):
_common_ldap_initialization(url=url,
@@ -505,7 +505,8 @@ class PythonLDAPHandler(LDAPHandler):
tls_cacertfile=tls_cacertfile,
tls_cacertdir=tls_cacertdir,
tls_req_cert=tls_req_cert,
- debug_level=debug_level)
+ debug_level=debug_level,
+ timeout=conn_timeout)
self.conn = ldap.initialize(url)
self.conn.protocol_version = ldap.VERSION3
@@ -569,7 +570,7 @@ class PythonLDAPHandler(LDAPHandler):
def _common_ldap_initialization(url, use_tls=False, tls_cacertfile=None,
tls_cacertdir=None, tls_req_cert=None,
- debug_level=None):
+ debug_level=None, timeout=None):
"""LDAP initialization for PythonLDAPHandler and PooledLDAPHandler."""
LOG.debug("LDAP init: url=%s", url)
LOG.debug('LDAP init: use_tls=%s tls_cacertfile=%s tls_cacertdir=%s '
@@ -582,6 +583,10 @@ def _common_ldap_initialization(url, use_tls=False, tls_cacertfile=None,
using_ldaps = url.lower().startswith("ldaps")
+ if timeout is not None and timeout > 0:
+ # set network connection timeout
+ ldap.set_option(ldap.OPT_NETWORK_TIMEOUT, timeout)
+
if use_tls and using_ldaps:
raise AssertionError(_('Invalid TLS / LDAPS combination'))
@@ -683,8 +688,8 @@ class PooledLDAPHandler(LDAPHandler):
def connect(self, url, page_size=0, alias_dereferencing=None,
use_tls=False, tls_cacertfile=None, tls_cacertdir=None,
tls_req_cert=ldap.OPT_X_TLS_DEMAND, chase_referrals=None,
- debug_level=None, use_pool=None, pool_size=None,
- pool_retry_max=None, pool_retry_delay=None,
+ debug_level=None, conn_timeout=None, use_pool=None,
+ pool_size=None, pool_retry_max=None, pool_retry_delay=None,
pool_conn_timeout=None, pool_conn_lifetime=None):
_common_ldap_initialization(url=url,
@@ -692,7 +697,8 @@ class PooledLDAPHandler(LDAPHandler):
tls_cacertfile=tls_cacertfile,
tls_cacertdir=tls_cacertdir,
tls_req_cert=tls_req_cert,
- debug_level=debug_level)
+ debug_level=debug_level,
+ timeout=pool_conn_timeout)
self.page_size = page_size
@@ -873,14 +879,15 @@ class KeystoneLDAPHandler(LDAPHandler):
def connect(self, url, page_size=0, alias_dereferencing=None,
use_tls=False, tls_cacertfile=None, tls_cacertdir=None,
tls_req_cert=ldap.OPT_X_TLS_DEMAND, chase_referrals=None,
- debug_level=None, use_pool=None, pool_size=None,
- pool_retry_max=None, pool_retry_delay=None,
+ debug_level=None, conn_timeout=None, use_pool=None,
+ pool_size=None, pool_retry_max=None, pool_retry_delay=None,
pool_conn_timeout=None, pool_conn_lifetime=None):
self.page_size = page_size
return self.conn.connect(url, page_size, alias_dereferencing,
use_tls, tls_cacertfile, tls_cacertdir,
tls_req_cert, chase_referrals,
debug_level=debug_level,
+ conn_timeout=conn_timeout,
use_pool=use_pool,
pool_size=pool_size,
pool_retry_max=pool_retry_max,
@@ -1147,6 +1154,7 @@ class BaseLdap(object):
self.attribute_mapping = {}
self.chase_referrals = conf.ldap.chase_referrals
self.debug_level = conf.ldap.debug_level
+ self.conn_timeout = conf.ldap.connection_timeout
# LDAP Pool specific attribute
self.use_pool = conf.ldap.use_pool
@@ -1258,35 +1266,42 @@ class BaseLdap(object):
conn = KeystoneLDAPHandler(conn=conn)
- conn.connect(self.LDAP_URL,
- page_size=self.page_size,
- alias_dereferencing=self.alias_dereferencing,
- use_tls=self.use_tls,
- tls_cacertfile=self.tls_cacertfile,
- tls_cacertdir=self.tls_cacertdir,
- tls_req_cert=self.tls_req_cert,
- chase_referrals=self.chase_referrals,
- debug_level=self.debug_level,
- use_pool=use_pool,
- pool_size=pool_size,
- pool_retry_max=self.pool_retry_max,
- pool_retry_delay=self.pool_retry_delay,
- pool_conn_timeout=self.pool_conn_timeout,
- pool_conn_lifetime=pool_conn_lifetime
- )
-
- if user is None:
- user = self.LDAP_USER
-
- if password is None:
- password = self.LDAP_PASSWORD
-
- # not all LDAP servers require authentication, so we don't bind
- # if we don't have any user/pass
- if user and password:
- conn.simple_bind_s(user, password)
-
- return conn
+ # The LDAP server may be down or a connection may not
+ # exist. If that is the case, the bind attempt will
+ # fail with a server down exception.
+ try:
+ conn.connect(self.LDAP_URL,
+ page_size=self.page_size,
+ alias_dereferencing=self.alias_dereferencing,
+ use_tls=self.use_tls,
+ tls_cacertfile=self.tls_cacertfile,
+ tls_cacertdir=self.tls_cacertdir,
+ tls_req_cert=self.tls_req_cert,
+ chase_referrals=self.chase_referrals,
+ debug_level=self.debug_level,
+ use_pool=use_pool,
+ pool_size=pool_size,
+ pool_retry_max=self.pool_retry_max,
+ pool_retry_delay=self.pool_retry_delay,
+ pool_conn_timeout=self.pool_conn_timeout,
+ pool_conn_lifetime=pool_conn_lifetime
+ )
+
+ if user is None:
+ user = self.LDAP_USER
+
+ if password is None:
+ password = self.LDAP_PASSWORD
+
+ # not all LDAP servers require authentication, so we don't bind
+ # if we don't have any user/pass
+ if user and password:
+ conn.simple_bind_s(user, password)
+
+ return conn
+ except ldap.SERVER_DOWN:
+ raise exception.LDAPServerConnectionError(
+ url=self.LDAP_URL)
def _id_to_dn_string(self, object_id):
return u'%s=%s,%s' % (self.id_attr,
diff --git a/keystone/tests/unit/fakeldap.py b/keystone/tests/unit/fakeldap.py
index 4ce20ae40..1c9524292 100644
--- a/keystone/tests/unit/fakeldap.py
+++ b/keystone/tests/unit/fakeldap.py
@@ -244,7 +244,8 @@ class FakeLdap(common.LDAPHandler):
tls_req_cert='demand', chase_referrals=None, debug_level=None,
use_pool=None, pool_size=None, pool_retry_max=None,
pool_retry_delay=None, pool_conn_timeout=None,
- pool_conn_lifetime=None):
+ pool_conn_lifetime=None,
+ conn_timeout=None):
if url.startswith('fake://memory'):
if url not in FakeShelves:
FakeShelves[url] = FakeShelve()
@@ -278,6 +279,7 @@ class FakeLdap(common.LDAPHandler):
self.pool_retry_delay = pool_retry_delay
self.pool_conn_timeout = pool_conn_timeout
self.pool_conn_lifetime = pool_conn_lifetime
+ self.conn_timeout = conn_timeout
def dn(self, dn):
return common.utf8_decode(dn)
diff --git a/keystone/tests/unit/identity/backends/test_ldap_common.py b/keystone/tests/unit/identity/backends/test_ldap_common.py
index 54b3d2ee1..d1f910460 100644
--- a/keystone/tests/unit/identity/backends/test_ldap_common.py
+++ b/keystone/tests/unit/identity/backends/test_ldap_common.py
@@ -297,6 +297,59 @@ class MultiURLTests(unit.TestCase):
self.assertEqual(urls, ldap_connection.conn.conn_pool.uri)
+class LDAPConnectionTimeoutTest(unit.TestCase):
+ """Test for Network Connection timeout on LDAP URL connection."""
+
+ def test_connectivity_timeout_no_conn_pool(self):
+ url = 'ldap://localhost'
+ conn_timeout = 1 # 1 second
+ self.config_fixture.config(group='ldap',
+ url=url,
+ connection_timeout=conn_timeout,
+ use_pool=False)
+ base_ldap = common_ldap.BaseLdap(CONF)
+ ldap_connection = base_ldap.get_connection()
+ self.assertIsInstance(ldap_connection.conn,
+ common_ldap.PythonLDAPHandler)
+
+ # Ensure that the Network Timeout option is set.
+ # Also ensure that the URL is set.
+ #
+ # We will not verify if an LDAP bind returns the timeout
+ # exception as that would fall under the realm of
+ # integration testing. If the LDAP option is set properly,
+ # and we get back a valid connection URI then that should
+ # suffice for this unit test.
+ self.assertEqual(conn_timeout,
+ ldap.get_option(ldap.OPT_NETWORK_TIMEOUT))
+ self.assertEqual(url, ldap_connection.conn.conn._uri)
+
+ def test_connectivity_timeout_with_conn_pool(self):
+ url = 'ldap://localhost'
+ conn_timeout = 1 # 1 second
+ self.config_fixture.config(group='ldap',
+ url=url,
+ pool_connection_timeout=conn_timeout,
+ use_pool=True,
+ pool_retry_max=1)
+ base_ldap = common_ldap.BaseLdap(CONF)
+ ldap_connection = base_ldap.get_connection()
+ self.assertIsInstance(ldap_connection.conn,
+ common_ldap.PooledLDAPHandler)
+
+ # Ensure that the Network Timeout option is set.
+ # Also ensure that the URL is set.
+ #
+ # We will not verify if an LDAP bind returns the timeout
+ # exception as that would fall under the realm of
+ # integration testing. If the LDAP option is set properly,
+ # and we get back a valid connection URI then that should
+ # suffice for this unit test.
+ self.assertEqual(conn_timeout,
+ ldap.get_option(ldap.OPT_NETWORK_TIMEOUT))
+ self.assertEqual(url, ldap_connection.conn.conn_pool.uri)
+
+
class SslTlsTest(unit.TestCase):
"""Test for the SSL/TLS functionality in keystone.common.ldap.core."""
diff --git a/releasenotes/notes/bug-1636950-8fa1a47fce440977.yaml b/releasenotes/notes/bug-1636950-8fa1a47fce440977.yaml
new file mode 100644
index 000000000..4756311dd
--- /dev/null
+++ b/releasenotes/notes/bug-1636950-8fa1a47fce440977.yaml
@@ -0,0 +1,10 @@
+---
+fixes:
+ - >
+ [`bug 1636950 <https://bugs.launchpad.net/keystone/+bug/1636950>`_]
+ New option ``[ldap] connection_timeout`` allows a deployer to set a
+ ``OPT_NETWORK_TIMEOUT`` value to use with the LDAP server.
+ This allows the LDAP server to return a ``SERVER_DOWN`` exception,
+ if the LDAP URL is incorrect if there is a connection failure. By default,
+ the value for ``[ldap] connection_timeout`` is -1, meaning it is disabled.
+ Set a postive value (in seconds) to enable the option.
--
2.14.2
