AD authn: never clear passwords on Bind connections (elastic/x-pack-elasticsearch#3351)

It is unsafe to clear passwords of bind requests if the connection is live
and might be used latter (for eg for group searches). This is a temporary
fix that exposes passwords in memory.

Original commit: elastic/x-pack-elasticsearch@e2e1f1a358
This commit is contained in:
Albert Zaharovits 2017-12-27 19:17:07 +02:00 committed by GitHub
parent 220aa734ee
commit 3ecc433f43
9 changed files with 95 additions and 21 deletions

View File

@ -38,7 +38,6 @@ import org.elasticsearch.xpack.security.authc.ldap.support.SessionFactory;
import org.elasticsearch.xpack.security.authc.support.CharArrays;
import org.elasticsearch.xpack.ssl.SSLService;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@ -282,9 +281,7 @@ class ActiveDirectorySessionFactory extends PoolingSessionFactory {
listener.onResponse(new LdapSession(logger, realm, connection, entry.getDN(), groupsResolver,
metaDataResolver, timeout, null));
}
Arrays.fill(passwordBytes, (byte) 0);
}, e -> {
Arrays.fill(passwordBytes, (byte) 0);
listener.onFailure(e);
}));
}
@ -315,9 +312,7 @@ class ActiveDirectorySessionFactory extends PoolingSessionFactory {
listener.onResponse(
new LdapSession(logger, realm, pool, entry.getDN(), groupsResolver, metaDataResolver, timeout, null));
}
Arrays.fill(passwordBytes, (byte) 0);
}, e -> {
Arrays.fill(passwordBytes, (byte) 0);
listener.onFailure(e);
}));
}
@ -457,11 +452,9 @@ class ActiveDirectorySessionFactory extends PoolingSessionFactory {
ActionListener.wrap(
results -> {
IOUtils.close(searchConnection);
Arrays.fill(passwordBytes, (byte) 0);
handleSearchResults(results, netBiosDomainName, domainNameCache, listener);
}, e -> {
IOUtils.closeWhileHandlingException(searchConnection);
Arrays.fill(passwordBytes, (byte) 0);
listener.onFailure(e);
}),
"ncname");

View File

@ -86,7 +86,6 @@ public class LdapSessionFactory extends SessionFactory {
listener.onResponse(
(new LdapSession(logger, config, connection, ((SimpleBindRequest) connection.getLastBindRequest()).getBindDN(),
groupResolver, metaDataResolver, timeout, null)));
Arrays.fill(passwordBytes, (byte) 0);
}
@Override
@ -103,7 +102,6 @@ public class LdapSessionFactory extends SessionFactory {
} else if (loopIndex == userDnTemplates.length) {
// loop break
IOUtils.closeWhileHandlingException(connection);
Arrays.fill(passwordBytes, (byte) 0);
listener.onFailure(containerException);
} else {
loop();

View File

@ -30,8 +30,6 @@ import org.elasticsearch.xpack.security.authc.ldap.support.SessionFactory;
import org.elasticsearch.xpack.security.authc.support.CharArrays;
import org.elasticsearch.xpack.ssl.SSLService;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Function;
@ -109,12 +107,10 @@ class LdapUserSearchSessionFactory extends PoolingSessionFactory {
protected void doRun() throws Exception {
listener.onResponse(new LdapSession(logger, config, connectionPool, dn, groupResolver, metaDataResolver, timeout,
entry.getAttributes()));
Arrays.fill(passwordBytes, (byte) 0);
}
@Override
public void onFailure(Exception e) {
Arrays.fill(passwordBytes, (byte) 0);
listener.onFailure(e);
}
});
@ -155,12 +151,10 @@ class LdapUserSearchSessionFactory extends PoolingSessionFactory {
protected void doRun() throws Exception {
listener.onResponse(new LdapSession(logger, config, connection, dn, groupResolver, metaDataResolver,
timeout, entry.getAttributes()));
Arrays.fill(passwordBytes, (byte) 0);
}
@Override
public void onFailure(Exception e) {
Arrays.fill(passwordBytes, (byte) 0);
IOUtils.closeWhileHandlingException(connection);
listener.onFailure(e);
}

View File

@ -25,7 +25,7 @@ public class LdapSession implements Releasable {
protected final Logger logger;
protected final RealmConfig realm;
protected final LDAPInterface ldap;
protected final LDAPInterface connection;
protected final String userDn;
protected final GroupsResolver groupsResolver;
private LdapMetaDataResolver metaDataResolver;
@ -44,7 +44,7 @@ public class LdapSession implements Releasable {
LdapMetaDataResolver metaDataResolver, TimeValue timeout, Collection<Attribute> attributes) {
this.logger = logger;
this.realm = realm;
this.ldap = connection;
this.connection = connection;
this.userDn = userDn;
this.groupsResolver = groupsResolver;
this.metaDataResolver = metaDataResolver;
@ -59,8 +59,8 @@ public class LdapSession implements Releasable {
public void close() {
// Only if it is an LDAPConnection do we need to close it, otherwise it is a connection pool and we will close all of the
// connections in the pool
if (ldap instanceof LDAPConnection) {
((LDAPConnection) ldap).close();
if (connection instanceof LDAPConnection) {
((LDAPConnection) connection).close();
}
}
@ -78,15 +78,22 @@ public class LdapSession implements Releasable {
return realm;
}
/**
* @return the connection to the LDAP/AD server of this session
*/
public LDAPInterface getConnection() {
return connection;
}
/**
* Asynchronously retrieves a list of group distinguished names
*/
public void groups(ActionListener<List<String>> listener) {
groupsResolver.resolve(ldap, userDn, timeout, logger, attributes, listener);
groupsResolver.resolve(connection, userDn, timeout, logger, attributes, listener);
}
public void metaData(ActionListener<Map<String, Object>> listener) {
metaDataResolver.resolve(ldap, userDn, timeout, logger, attributes, listener);
metaDataResolver.resolve(connection, userDn, timeout, logger, attributes, listener);
}
public void resolve(ActionListener<LdapUserData> listener) {

View File

@ -5,6 +5,11 @@
*/
package org.elasticsearch.xpack.security.authc.ldap;
import com.unboundid.ldap.sdk.LDAPConnection;
import com.unboundid.ldap.sdk.LDAPConnectionPool;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.LDAPInterface;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
@ -16,6 +21,8 @@ import org.elasticsearch.xpack.ssl.VerificationMode;
import org.junit.Before;
import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
@Network
public class AbstractActiveDirectoryIntegTests extends ESTestCase {
@ -78,4 +85,24 @@ public class AbstractActiveDirectoryIntegTests extends ESTestCase {
}
return builder.build();
}
protected static void assertConnectionCanReconnect(LDAPInterface conn) {
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
try {
if (conn instanceof LDAPConnection) {
((LDAPConnection) conn).reconnect();
} else if (conn instanceof LDAPConnectionPool) {
try (LDAPConnection c = ((LDAPConnectionPool) conn).getConnection()) {
c.reconnect();
}
}
} catch (LDAPException e) {
fail("Connection is not valid. It will not work on follow referral flow.");
}
return null;
}
});
}
}

View File

@ -67,6 +67,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String userName = "ironman";
try (LdapSession ldap = session(sessionFactory, userName, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
containsString("Geniuses"),
@ -90,6 +91,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String userName = "ades\\ironman";
try (LdapSession ldap = session(sessionFactory, userName, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
containsString("Geniuses"),
@ -132,6 +134,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String[] users = new String[]{"cap", "hawkeye", "hulk", "ironman", "thor", "blackwidow"};
for (String user : users) {
try (LdapSession ldap = session(sessionFactory, user, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
assertThat("group avenger test for user " + user, groups(ldap), hasItem(containsString("Avengers")));
}
}
@ -148,6 +151,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String userName = "hulk";
try (LdapSession ldap = session(sessionFactory, userName, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
@ -172,6 +176,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String userName = "hulk";
try (LdapSession ldap = session(sessionFactory, userName, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
@ -200,6 +205,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String userName = "hulk";
try (LdapSession ldap = session(sessionFactory, userName, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, hasItem(containsString("Avengers")));
@ -218,6 +224,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
//Login with the UserPrincipalName
String userDN = "CN=Erik Selvig,CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com";
try (LdapSession ldap = session(sessionFactory, "erik.selvig", SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(ldap.userDn(), is(userDN));
assertThat(groups, containsInAnyOrder(
@ -238,6 +245,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
//login with sAMAccountName
String userDN = "CN=Erik Selvig,CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com";
try (LdapSession ldap = session(sessionFactory, "selvig", SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
assertThat(ldap.userDn(), is(userDN));
List<String> groups = groups(ldap);
@ -263,6 +271,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
//Login with the UserPrincipalName
try (LdapSession ldap = session(sessionFactory, "erik.selvig", SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
containsString("CN=Geniuses"),
@ -297,6 +306,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String user = "Bruce Banner";
try (LdapSession ldap = session(sessionFactory, user, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
@ -308,6 +318,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
}
@SuppressWarnings("unchecked")
@AwaitsFix(bugUrl = "https://github.com/elastic/x-pack-elasticsearch/issues/3369")
public void testHandlingLdapReferralErrors() throws Exception {
String groupSearchBase = "DC=ad,DC=test,DC=elasticsearch,DC=com";
String userTemplate = "CN={0},CN=Users,DC=ad,DC=test,DC=elasticsearch,DC=com";
@ -361,6 +372,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
String user = "Bruce Banner";
try (LdapSession ldap = session(sessionFactory, user, SECURED_PASSWORD)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
@ -419,6 +431,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI
"ADES\\cap", "ADES\\hawkeye", "ADES\\hulk", "ADES\\ironman", "ADES\\thor", "ADES\\blackwidow"));
for (String user : users) {
try (LdapSession ldap = unauthenticatedSession(sessionFactory, user)) {
assertConnectionCanReconnect(ldap.getConnection());
assertNotNull("ldap session was null for user " + user, ldap);
assertThat("group avenger test for user " + user, groups(ldap), hasItem(containsString("Avengers")));
}

View File

@ -7,6 +7,7 @@ package org.elasticsearch.xpack.security.authc.ldap;
import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.LDAPInterface;
import com.unboundid.ldap.sdk.LDAPURL;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
@ -124,6 +125,7 @@ public class LdapSessionFactoryTests extends LdapTestCase {
SecureString userPass = new SecureString("pass");
try (LdapSession ldap = session(sessionFactory, user, userPass)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString(user));
}
@ -163,6 +165,7 @@ public class LdapSessionFactoryTests extends LdapTestCase {
SecureString userPass = new SecureString("pass");
try (LdapSession ldap = session(ldapFac, user, userPass)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, contains("cn=HMS Lydia,ou=crews,ou=groups,o=sevenSeas"));
}
@ -178,6 +181,7 @@ public class LdapSessionFactoryTests extends LdapTestCase {
String user = "Horatio Hornblower";
try (LdapSession ldap = session(ldapFac, user, new SecureString("pass"))) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, contains("cn=HMS Lydia,ou=crews,ou=groups,o=sevenSeas"));
}
@ -195,6 +199,7 @@ public class LdapSessionFactoryTests extends LdapTestCase {
SecureString userPass = new SecureString("pass");
try (LdapSession ldap = session(ldapFac, user, userPass)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups.size(), is(1));
assertThat(groups, contains("cn=HMS Lydia,ou=crews,ou=groups,o=sevenSeas"));

View File

@ -138,12 +138,14 @@ public class LdapUserSearchSessionFactoryTests extends LdapTestCase {
try {
// auth
try (LdapSession ldap = session(sessionFactory, user, userPass)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString(user));
}
//lookup
try (LdapSession ldap = unauthenticatedSession(sessionFactory, user)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString(user));
}
@ -221,12 +223,14 @@ public class LdapUserSearchSessionFactoryTests extends LdapTestCase {
try {
// auth
try (LdapSession ldap = session(sessionFactory, user, userPass)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString(user));
}
//lookup
try (LdapSession ldap = unauthenticatedSession(sessionFactory, user)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString(user));
}
@ -304,12 +308,14 @@ public class LdapUserSearchSessionFactoryTests extends LdapTestCase {
try {
//auth
try (LdapSession ldap = session(sessionFactory, user, userPass)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString(user));
}
//lookup
try (LdapSession ldap = unauthenticatedSession(sessionFactory, user)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString(user));
}
@ -378,12 +384,14 @@ public class LdapUserSearchSessionFactoryTests extends LdapTestCase {
try {
//auth
try (LdapSession ldap = session(sessionFactory, user, userPass)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString("William Bush"));
}
//lookup
try (LdapSession ldap = unauthenticatedSession(sessionFactory, user)) {
assertConnectionCanReconnect(ldap.getConnection());
String dn = ldap.userDn();
assertThat(dn, containsString("William Bush"));
}
@ -422,6 +430,7 @@ public class LdapUserSearchSessionFactoryTests extends LdapTestCase {
try {
//auth
try (LdapSession ldap = session(sessionFactory, user, new SecureString(ActiveDirectorySessionFactoryTests.PASSWORD))) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(
@ -433,6 +442,7 @@ public class LdapUserSearchSessionFactoryTests extends LdapTestCase {
//lookup
try (LdapSession ldap = unauthenticatedSession(sessionFactory, user)) {
assertConnectionCanReconnect(ldap.getConnection());
List<String> groups = groups(ldap);
assertThat(groups, containsInAnyOrder(

View File

@ -7,8 +7,13 @@ package org.elasticsearch.xpack.security.authc.ldap.support;
import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.sdk.Attribute;
import com.unboundid.ldap.sdk.LDAPConnection;
import com.unboundid.ldap.sdk.LDAPConnectionPool;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.LDAPInterface;
import com.unboundid.ldap.sdk.LDAPURL;
import org.elasticsearch.SpecialPermission;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.SecureString;
@ -27,6 +32,8 @@ import org.junit.Before;
import org.junit.BeforeClass;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
import java.util.List;
@ -160,4 +167,24 @@ public abstract class LdapTestCase extends ESTestCase {
factory.unauthenticatedSession(username, future);
return future.actionGet();
}
protected static void assertConnectionCanReconnect(LDAPInterface conn) {
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
try {
if (conn instanceof LDAPConnection) {
((LDAPConnection) conn).reconnect();
} else if (conn instanceof LDAPConnectionPool) {
try (LDAPConnection c = ((LDAPConnectionPool) conn).getConnection()) {
c.reconnect();
}
}
} catch (LDAPException e) {
fail("Connection is not valid. It will not work on follow referral flow.");
}
return null;
}
});
}
}