From 4b779b6fd63198b87fd308ea9ff661310b23b7d9 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 23 Jul 2015 08:10:25 -0400 Subject: [PATCH] allow ldap user search connection pool creation to be retried if it fails on startup Today, if a LDAP server is down and the LDAP realm uses the user search mechanism this will prevent the node from starting up. This is not ideal because users can still authenticate with another realm if it is configured. This change tries to create the connection pool on initialization but if it fails, creation will retried on each attempted authentication until the server is available again. Closes elastic/elasticsearch#107 Original commit: elastic/x-pack-elasticsearch@f2ccf858ff36076f4cfffe93d128872181222d3f --- .../ldap/LdapUserSearchSessionFactory.java | 37 ++++++++++--- .../LdapUserSearchSessionFactoryTests.java | 54 ++++++++++++++++++- 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactory.java b/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactory.java index 33fce6902db..42ad19648d0 100644 --- a/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactory.java +++ b/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactory.java @@ -8,6 +8,7 @@ package org.elasticsearch.shield.authc.ldap; import com.google.common.primitives.Ints; import com.unboundid.ldap.sdk.*; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.shield.ShieldSettingsFilter; @@ -36,15 +37,17 @@ public class LdapUserSearchSessionFactory extends SessionFactory { static final TimeValue DEFAULT_HEALTH_CHECK_INTERVAL = TimeValue.timeValueSeconds(60L); private final GroupsResolver groupResolver; - private final LDAPConnectionPool connectionPool; private final String userSearchBaseDn; private final LdapSearchScope scope; private final String userAttribute; private final ServerSet serverSet; + private final Settings settings; - public LdapUserSearchSessionFactory(RealmConfig config, ClientSSLService sslService) throws IOException { + private LDAPConnectionPool connectionPool; + + public LdapUserSearchSessionFactory(RealmConfig config, ClientSSLService sslService) { super(config); - Settings settings = config.settings(); + settings = config.settings(); userSearchBaseDn = settings.get("user_search.base_dn"); if (userSearchBaseDn == null) { throw new IllegalArgumentException("user_search base_dn must be specified"); @@ -52,17 +55,29 @@ public class LdapUserSearchSessionFactory extends SessionFactory { scope = LdapSearchScope.resolve(settings.get("user_search.scope"), LdapSearchScope.SUB_TREE); userAttribute = settings.get("user_search.attribute", DEFAULT_USERNAME_ATTRIBUTE); serverSet = serverSet(settings, sslService); - connectionPool = connectionPool(config.settings(), serverSet, timeout); + connectionPool = createConnectionPool(settings, serverSet, timeout, logger); groupResolver = groupResolver(settings); } + private synchronized LDAPConnectionPool connectionPool() throws IOException { + if (connectionPool == null) { + connectionPool = createConnectionPool(settings, serverSet, timeout, logger); + // if it is still null throw an exception + if (connectionPool == null) { + throw new IOException("failed to create a connection pool as no LDAP servers are available"); + } + } + + return connectionPool; + } + static void filterOutSensitiveSettings(String realmName, ShieldSettingsFilter filter) { filter.filterOut("shield.authc.realms." + realmName + ".bind_dn"); filter.filterOut("shield.authc.realms." + realmName + ".bind_password"); filter.filterOut("shield.authc.realms." + realmName + "." + HOSTNAME_VERIFICATION_SETTING); } - static LDAPConnectionPool connectionPool(Settings settings, ServerSet serverSet, TimeValue timeout) throws IOException { + static LDAPConnectionPool createConnectionPool(Settings settings, ServerSet serverSet, TimeValue timeout, ESLogger logger) { SimpleBindRequest bindRequest = bindRequest(settings); int initialSize = settings.getAsInt("user_search.pool.initial_size", DEFAULT_CONNECTION_POOL_INITIAL_SIZE); int size = settings.getAsInt("user_search.pool.size", DEFAULT_CONNECTION_POOL_SIZE); @@ -85,8 +100,13 @@ public class LdapUserSearchSessionFactory extends SessionFactory { } return pool; } catch (LDAPException e) { - throw new IOException("unable to connect to any LDAP servers", e); + if (logger.isDebugEnabled()) { + logger.debug("unable to create connection pool", e); + } else { + logger.error("unable to create connection pool: {}", e.getMessage()); + } } + return null; } static SimpleBindRequest bindRequest(Settings settings) { @@ -126,6 +146,7 @@ public class LdapUserSearchSessionFactory extends SessionFactory { public LdapSession session(String user, SecuredString password) throws Exception { SearchRequest request = new SearchRequest(userSearchBaseDn, scope.scope(), createEqualityFilter(userAttribute, encodeValue(user)), Strings.EMPTY_ARRAY); request.setTimeLimitSeconds(Ints.checkedCast(timeout.seconds())); + LDAPConnectionPool connectionPool = connectionPool(); try { SearchResultEntry entry = searchForEntry(connectionPool, request, logger); if (entry == null) { @@ -160,7 +181,9 @@ public class LdapUserSearchSessionFactory extends SessionFactory { * This method is used to cleanup the connections for tests */ void shutdown() { - connectionPool.close(); + if (connectionPool != null) { + connectionPool.close(); + } } static GroupsResolver groupResolver(Settings settings) { diff --git a/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactoryTests.java b/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactoryTests.java index 877ba313e61..18685b95bc6 100644 --- a/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactoryTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapUserSearchSessionFactoryTests.java @@ -5,12 +5,18 @@ */ package org.elasticsearch.shield.authc.ldap; +import com.carrotsearch.randomizedtesting.ThreadFilter; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; import com.unboundid.ldap.sdk.*; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.env.Environment; +import org.elasticsearch.license.plugin.LicensePlugin; +import org.elasticsearch.node.Node; +import org.elasticsearch.node.NodeBuilder; +import org.elasticsearch.shield.ShieldPlugin; import org.elasticsearch.shield.authc.RealmConfig; import org.elasticsearch.shield.authc.activedirectory.ActiveDirectorySessionFactoryTests; import org.elasticsearch.shield.authc.ldap.support.LdapSearchScope; @@ -19,6 +25,7 @@ import org.elasticsearch.shield.authc.ldap.support.LdapTest; import org.elasticsearch.shield.authc.support.SecuredString; import org.elasticsearch.shield.authc.support.SecuredStringTests; import org.elasticsearch.shield.ssl.ClientSSLService; +import org.elasticsearch.shield.support.NoOpLogger; import org.elasticsearch.test.junit.annotations.Network; import org.junit.Before; import org.junit.Test; @@ -27,11 +34,17 @@ import java.nio.file.Path; import java.text.MessageFormat; import java.util.List; import java.util.Locale; +import java.util.Map; import static org.elasticsearch.common.settings.Settings.settingsBuilder; import static org.elasticsearch.test.ShieldTestsUtils.assertAuthenticationException; import static org.hamcrest.Matchers.*; +// thread leak filter for UnboundID's background connect threads. The background connect threads do not always respect the +// timeout and linger. Will be fixed in a new version of the library, see http://sourceforge.net/p/ldap-sdk/discussion/1001257/thread/154e3b71/ +@ThreadLeakFilters(filters = { + LdapUserSearchSessionFactoryTests.BackgroundConnectThreadLeakFilter.class +}) public class LdapUserSearchSessionFactoryTests extends LdapTest { private ClientSSLService clientSSLService; @@ -306,7 +319,7 @@ public class LdapUserSearchSessionFactoryTests extends LdapTest { .put("bind_password", "pass") .build(), globalSettings); - LDAPConnectionPool connectionPool = LdapUserSearchSessionFactory.connectionPool(config.settings(), new SingleServerSet("localhost", ldapServer.getListenPort()), TimeValue.timeValueSeconds(5)); + LDAPConnectionPool connectionPool = LdapUserSearchSessionFactory.createConnectionPool(config.settings(), new SingleServerSet("localhost", ldapServer.getListenPort()), TimeValue.timeValueSeconds(5), NoOpLogger.INSTANCE); try { assertThat(connectionPool.getCurrentAvailableConnections(), is(LdapUserSearchSessionFactory.DEFAULT_CONNECTION_POOL_INITIAL_SIZE)); assertThat(connectionPool.getMaximumAvailableConnections(), is(LdapUserSearchSessionFactory.DEFAULT_CONNECTION_POOL_SIZE)); @@ -333,7 +346,7 @@ public class LdapUserSearchSessionFactoryTests extends LdapTest { .put("user_search.pool.health_check.enabled", false) .build(), globalSettings); - LDAPConnectionPool connectionPool = LdapUserSearchSessionFactory.connectionPool(config.settings(), new SingleServerSet("localhost", ldapServer.getListenPort()), TimeValue.timeValueSeconds(5)); + LDAPConnectionPool connectionPool = LdapUserSearchSessionFactory.createConnectionPool(config.settings(), new SingleServerSet("localhost", ldapServer.getListenPort()), TimeValue.timeValueSeconds(5), NoOpLogger.INSTANCE); try { assertThat(connectionPool.getCurrentAvailableConnections(), is(10)); assertThat(connectionPool.getMaximumAvailableConnections(), is(12)); @@ -377,4 +390,41 @@ public class LdapUserSearchSessionFactoryTests extends LdapTest { SimpleBindRequest simpleBindRequest = (SimpleBindRequest) request; assertThat(simpleBindRequest.getBindDN(), is("cn=ironman")); } + + @Test + public void testThatLDAPServerConnectErrorDoesNotPreventNodeFromStarting() { + String groupSearchBase = "DC=ad,DC=test,DC=elasticsearch,DC=com"; + String userSearchBase = "CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com"; + Settings ldapSettings = settingsBuilder() + .put(LdapTest.buildLdapSettings("ldaps://elastic.co:636", Strings.EMPTY_ARRAY, groupSearchBase, LdapSearchScope.SUB_TREE)) + .put("user_search.base_dn", userSearchBase) + .put("bind_dn", "ironman@ad.test.elasticsearch.com") + .put("bind_password", ActiveDirectorySessionFactoryTests.PASSWORD) + .put("user_search.attribute", "cn") + .put("timeout.tcp_connect", "500ms") + .put("type", "ldap") + .build(); + + Settings.Builder builder = settingsBuilder(); + for (Map.Entry entry : ldapSettings.getAsMap().entrySet()) { + builder.put("shield.authc.realms.ldap1." + entry.getKey(), entry.getValue()); + } + builder.put("path.home", createTempDir()); + builder.putArray("plugin.types", ShieldPlugin.class.getName(), LicensePlugin.class.getName()); + + try (Node node = NodeBuilder.nodeBuilder().loadConfigSettings(false).settings(builder.build()).build()) { + node.start(); + } + } + + public static class BackgroundConnectThreadLeakFilter implements ThreadFilter { + + @Override + public boolean reject(Thread thread) { + if (thread.getName().startsWith("Background connect thread for elastic.co")) { + return true; + } + return false; + } + } }