honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[Security] Treat LDAP exceptions as auth failures (elastic/x-pack-elasticsearch#565) 

Any failure to communicate with an LDAP server should be treated as if authentication failed rather than a fatal "stop the world" error.

Because the AD realm validates the user's password by attempting to bind to the directory with that username/password combination, connection failures are to be expected and must be handled gracefully because the AD realm may not be the last realm in the order-chain, and any exception would prevent later realms from being tested.

Closes: elastic/x-pack-elasticsearch#564

Original commit: elastic/x-pack-elasticsearch@5282290b00
This commit is contained in:
Tim Vernum 2017-02-16 22:57:12 +11:00 committed by GitHub
parent e77a71827d
commit de10d26bfc
5 changed files with 133 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java
View File

@ -11,6 +11,8 @@ import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import com.unboundid.ldap.sdk.LDAPException;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.Strings;
@ -116,7 +118,7 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
// we submit to the threadpool because authentication using LDAP will execute blocking I/O for a bind request and we don't want
// network threads stuck waiting for a socket to connect. After the bind, then all interaction with LDAP should be async
threadPool.generic().execute(() -> sessionFactory.session(token.principal(), token.credentials(),
new LdapSessionActionListener(token.principal(), listener, roleMapper)));
new LdapSessionActionListener("authenticate", token.principal(), listener, roleMapper, logger)));
}
@Override
@ -125,7 +127,8 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
// we submit to the threadpool because authentication using LDAP will execute blocking I/O for a bind request and we don't want
// network threads stuck waiting for a socket to connect. After the bind, then all interaction with LDAP should be async
threadPool.generic().execute(() ->
sessionFactory.unauthenticatedSession(username, new LdapSessionActionListener(username, listener, roleMapper)));
sessionFactory.unauthenticatedSession(username,
new LdapSessionActionListener("lookup", username, listener, roleMapper, logger)));
} else {
listener.onResponse(null);
}
@ -173,14 +176,19 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
private static class LdapSessionActionListener implements ActionListener<LdapSession> {
private final AtomicReference<LdapSession> ldapSessionAtomicReference = new AtomicReference<>();
private String action;
private Logger logger;
private final String username;
private final ActionListener<User> userActionListener;
private final DnRoleMapper roleMapper;
LdapSessionActionListener(String username, ActionListener<User> userActionListener, DnRoleMapper roleMapper) {
LdapSessionActionListener(String action, String username, ActionListener<User> userActionListener,
DnRoleMapper roleMapper, Logger logger) {
this.action = action;
this.username = username;
this.userActionListener = userActionListener;
this.roleMapper = roleMapper;
this.logger = logger;
}
@Override
@ -197,10 +205,13 @@ public final class LdapRealm extends CachingUsernamePasswordRealm {
public void onFailure(Exception e) {
if (ldapSessionAtomicReference.get() != null) {
IOUtils.closeWhileHandlingException(ldapSessionAtomicReference.get());
userActionListener.onFailure(e);
} else {
userActionListener.onFailure(e);
}
logger.info("{} failed for user [{}]: {}", action, username, e.getMessage());
if (logger.isDebugEnabled()) {
logger.debug(new ParameterizedMessage("{} failed", action), e);
}
userActionListener.onResponse(null);
}
}
}

30
plugin/src/test/java/org/elasticsearch/integration/ldap/AbstractAdLdapRealmTestCase.java
View File

@ -52,6 +52,7 @@ public abstract class AbstractAdLdapRealmTestCase extends SecurityIntegTestCase
"Gods: [ \"cn=Gods,ou=people,dc=oldap,dc=test,dc=elasticsearch,dc=com\" ] \n" +
"Philanthropists: [ \"cn=Philanthropists,ou=people,dc=oldap,dc=test,dc=elasticsearch,dc=com\" ] \n";
protected static final String TESTNODE_KEYSTORE = "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks";
protected static RealmConfig realmConfig;
protected static boolean useGlobalSSL;
@ -70,8 +71,8 @@ public abstract class AbstractAdLdapRealmTestCase extends SecurityIntegTestCase
@Override
protected Settings nodeSettings(int nodeOrdinal) {
Path nodeFiles = createTempDir();
Path store = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks");
final RealmConfig realm = AbstractAdLdapRealmTestCase.realmConfig;
Path store = getDataPath(TESTNODE_KEYSTORE);
Settings.Builder builder = Settings.builder();
if (useGlobalSSL) {
builder.put(super.nodeSettings(nodeOrdinal).filter((s) -> s.startsWith("xpack.ssl.") == false))
@ -79,16 +80,23 @@ public abstract class AbstractAdLdapRealmTestCase extends SecurityIntegTestCase
} else {
builder.put(super.nodeSettings(nodeOrdinal));
}
builder.put(realmConfig.buildSettings(store, "testnode"))
builder.put(buildRealmSettings(realm, store));
return builder.build();
}
protected Settings buildRealmSettings(RealmConfig realm, Path store) {
Settings.Builder builder = Settings.builder();
Path nodeFiles = createTempDir();
builder.put(realm.buildSettings(store, "testnode"))
.put(XPACK_SECURITY_AUTHC_REALMS_EXTERNAL + ".files.role_mapping", writeFile(nodeFiles, "role_mapping.yml",
configRoleMappings()));
configRoleMappings(realm)));
return builder.build();
}
@Override
protected Settings transportClientSettings() {
if (useGlobalSSL) {
Path store = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks");
Path store = getDataPath(TESTNODE_KEYSTORE);
return Settings.builder()
.put(super.transportClientSettings().filter((s) -> s.startsWith("xpack.ssl.") == false))
.put(sslSettingsForStore(store, "testnode"))
@ -102,8 +110,8 @@ public abstract class AbstractAdLdapRealmTestCase extends SecurityIntegTestCase
return useGlobalSSL == false;
}
protected String configRoleMappings() {
return realmConfig.configRoleMappings();
protected String configRoleMappings(RealmConfig realm) {
return realm.configRoleMappings();
}
@Override
@ -238,7 +246,7 @@ public abstract class AbstractAdLdapRealmTestCase extends SecurityIntegTestCase
final boolean mapGroupsAsRoles;
final boolean loginWithCommonName;
private final String roleMappings;
private final Settings settings;
final Settings settings;
RealmConfig(boolean loginWithCommonName, String roleMappings, Settings settings) {
this.settings = settings;
@ -248,8 +256,12 @@ public abstract class AbstractAdLdapRealmTestCase extends SecurityIntegTestCase
}
public Settings buildSettings(Path store, String password) {
return buildSettings(store, password, 1);
}
protected Settings buildSettings(Path store, String password, int order) {
Settings.Builder builder = Settings.builder()
.put(XPACK_SECURITY_AUTHC_REALMS_EXTERNAL + ".order", 1)
.put(XPACK_SECURITY_AUTHC_REALMS_EXTERNAL + ".order", order)
.put(XPACK_SECURITY_AUTHC_REALMS_EXTERNAL + ".hostname_verification", false)
.put(XPACK_SECURITY_AUTHC_REALMS_EXTERNAL + ".unmapped_groups_as_roles", mapGroupsAsRoles)
.put(this.settings);

2
plugin/src/test/java/org/elasticsearch/integration/ldap/MultiGroupMappingTests.java
View File

@ -27,7 +27,7 @@ public class MultiGroupMappingTests extends AbstractAdLdapRealmTestCase {
}
@Override
protected String configRoleMappings() {
protected String configRoleMappings(RealmConfig realm) {
return "MarvelCharacters: \n" +
" - \"CN=SHIELD,CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com\"\n" +
" - \"CN=Avengers,CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com\"\n" +

69
plugin/src/test/java/org/elasticsearch/integration/ldap/MultipleAdRealmTests.java Normal file
View File

@ -0,0 +1,69 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.integration.ldap;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.junit.annotations.Network;
import org.junit.BeforeClass;
/**
* This tests that configurations that contain two AD realms work correctly.
* The required behaviour is that users from both realms (directory servers) can be authenticated using
* just their userid (the AuthenticationService tries them in order)
*/
@Network
public class MultipleAdRealmTests extends AbstractAdLdapRealmTestCase {
private static RealmConfig secondaryRealmConfig;
@BeforeClass
public static void setupSecondaryRealm() {
// Pick a secondary realm that has the inverse value for 'loginWithCommonName' compare with the primary realm
final List<RealmConfig> configs = new ArrayList<>();
for (RealmConfig config : RealmConfig.values()) {
if (config.loginWithCommonName != AbstractAdLdapRealmTestCase.realmConfig.loginWithCommonName) {
configs.add(config);
}
}
secondaryRealmConfig = randomFrom(configs);
ESLoggerFactory.getLogger("test")
.info("running test with secondary realm configuration [{}], with direct group to role mapping [{}]. Settings [{}]",
secondaryRealmConfig, secondaryRealmConfig.mapGroupsAsRoles, secondaryRealmConfig.settings.getAsMap());
}
@Override
protected Settings nodeSettings(int nodeOrdinal) {
Settings.Builder builder = Settings.builder();
builder.put(super.nodeSettings(nodeOrdinal));
Path store = getDataPath(TESTNODE_KEYSTORE);
final Settings secondarySettings = super.buildRealmSettings(secondaryRealmConfig, store);
secondarySettings.getAsMap().forEach((name, value) -> {
name = name.replace(XPACK_SECURITY_AUTHC_REALMS_EXTERNAL, XPACK_SECURITY_AUTHC_REALMS_EXTERNAL + "2");
builder.put(name, value);
});
return builder.build();
}
/**
* Test that both realms support user login. Implementation wise, this means that if the first realm reject the authentication attempt,
* then the second realm will be tried.
* Because one realm is using "common name" (cn) for login, and the other uses the "userid" (sAMAccountName) [see
* {@link #setupSecondaryRealm()}], this is simply a matter of checking that we can authenticate with both identifiers.
*/
public void testCanAuthenticateAgainstBothRealms() throws IOException {
assertAccessAllowed("Natasha Romanoff", "avengers");
assertAccessAllowed("blackwidow", "avengers");
}
}

25
plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealmTests.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.security.authc.ldap;
import com.unboundid.ldap.sdk.LDAPURL;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.settings.Settings;
@ -37,6 +38,7 @@ import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.spy;
@ -274,6 +276,29 @@ public class LdapRealmTests extends LdapTestCase {
assertThat(user.roles(), arrayContaining("avenger"));
}
/**
* The contract for {@link org.elasticsearch.xpack.security.authc.Realm} implementations is that they should log-and-return-null (and
* not call {@link ActionListener#onFailure(Exception)}) if there is an internal exception that prevented them from performing an
* authentication.
* This method tests that when an LDAP server is unavailable (invalid hostname), there is a <code>null</code> result
* rather than an exception.
*/
public void testLdapConnectionFailureIsTreatedAsAuthenticationFailure() throws Exception {
LDAPURL url = new LDAPURL("ldap", "..", 12345, null, null, null, null);
String groupSearchBase = "o=sevenSeas";
String userTemplate = VALID_USER_TEMPLATE;
Settings settings = buildLdapSettings(new String[] { url.toString() }, userTemplate, groupSearchBase, LdapSearchScope.SUB_TREE);
RealmConfig config = new RealmConfig("test-ldap-realm", settings, globalSettings);
LdapSessionFactory ldapFactory = new LdapSessionFactory(config, sslService);
LdapRealm ldap = new LdapRealm(LdapRealm.LDAP_TYPE, config, ldapFactory, buildGroupAsRoleMapper(resourceWatcherService),
threadPool);
PlainActionFuture<User> future = new PlainActionFuture<>();
ldap.authenticate(new UsernamePasswordToken(VALID_USERNAME, SecuredStringTests.build(PASSWORD)), future);
User user = future.actionGet();
assertThat(user, nullValue());
}
public void testUsageStats() throws Exception {
String groupSearchBase = "o=sevenSeas";
Settings.Builder settings = Settings.builder()