diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java index d030cd1eb18..dc7470d2f16 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java @@ -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"); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactory.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactory.java index d489ca6c602..3a3a10dfd5d 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactory.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactory.java @@ -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(); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactory.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactory.java index 112244ce944..cb76c08969c 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactory.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactory.java @@ -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); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapSession.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapSession.java index 490e93874fe..c9d11e1a7a3 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapSession.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapSession.java @@ -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 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> listener) { - groupsResolver.resolve(ldap, userDn, timeout, logger, attributes, listener); + groupsResolver.resolve(connection, userDn, timeout, logger, attributes, listener); } public void metaData(ActionListener> listener) { - metaDataResolver.resolve(ldap, userDn, timeout, logger, attributes, listener); + metaDataResolver.resolve(connection, userDn, timeout, logger, attributes, listener); } public void resolve(ActionListener listener) { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryIntegTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryIntegTests.java index ae2a97c735f..ce346bdfab2 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryIntegTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractActiveDirectoryIntegTests.java @@ -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() { + @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; + } + }); + } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactoryTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactoryTests.java index fec760b08ae..a51660a6695 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactoryTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactoryTests.java @@ -67,6 +67,7 @@ public class ActiveDirectorySessionFactoryTests extends AbstractActiveDirectoryI String userName = "ironman"; try (LdapSession ldap = session(sessionFactory, userName, SECURED_PASSWORD)) { + assertConnectionCanReconnect(ldap.getConnection()); List 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 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 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 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 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 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 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 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 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 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"))); } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java index 2a1d49738ad..7a97b23866a 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java @@ -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 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 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 groups = groups(ldap); assertThat(groups.size(), is(1)); assertThat(groups, contains("cn=HMS Lydia,ou=crews,ou=groups,o=sevenSeas")); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java index b08873f0063..dbfda854a58 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java @@ -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 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 groups = groups(ldap); assertThat(groups, containsInAnyOrder( diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java index 8bdfb7ff833..c913ab91506 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java @@ -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() { + @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; + } + }); + } }