ldap: closes NamingEnumerations

This checkin ensures all objects allocated by jndi requests are freed up.  It does this by wrapping NamingEnumerations with a ClosableNamingEnumeration that is placed in a try-with-resources block.

Original commit: elastic/x-pack-elasticsearch@8bed9585bd

src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnection.java

@@ -9,7 +9,9 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.ImmutableList;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.shield.authc.ldap.LdapException;
 import org.elasticsearch.shield.authc.support.ldap.AbstractLdapConnection;
+import org.elasticsearch.shield.authc.support.ldap.ClosableNamingEnumeration;
 
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
@@ -64,16 +66,15 @@ public class ActiveDirectoryConnection extends AbstractLdapConnection {
         groupsSearchCtls.setTimeLimit(timeoutMilliseconds);
 
         ImmutableList.Builder<String> groups = ImmutableList.builder();
-        try {
+        try (ClosableNamingEnumeration groupsAnswer = new ClosableNamingEnumeration(
-            //Search for objects using the filter
+                jndiContext.search(groupSearchDN, groupsSearchFilter, groupsSearchCtls))) {
-            NamingEnumeration groupsAnswer = jndiContext.search(groupSearchDN, groupsSearchFilter, groupsSearchCtls);
 
             //Loop through the search results
             while (groupsAnswer.hasMoreElements()) {
                 SearchResult sr = (SearchResult) groupsAnswer.next();
                 groups.add(sr.getNameInNamespace());
             }
-        } catch (NamingException ne) {
+        } catch (NamingException | LdapException ne) {
             throw new ActiveDirectoryException("Exception occurred fetching AD groups", bindDn, ne);
         }
         List<String> groupList = groups.build();
@@ -93,29 +94,33 @@ public class ActiveDirectoryConnection extends AbstractLdapConnection {
             String userSearchFilter = "(objectClass=user)";
             String userReturnedAtts[] = { "tokenGroups" };
             userSearchCtls.setReturningAttributes(userReturnedAtts);
-            NamingEnumeration userAnswer = jndiContext.search(authenticatedUserDn(), userSearchFilter, userSearchCtls);
+            try (ClosableNamingEnumeration userAnswer = new ClosableNamingEnumeration(
+                    jndiContext.search(authenticatedUserDn(), userSearchFilter, userSearchCtls))) {
 
-            //Loop through the search results
+                //Loop through the search results
-            while (userAnswer.hasMoreElements()) {
+                while (userAnswer.hasMoreElements()) {
 
-                SearchResult sr = (SearchResult) userAnswer.next();
+                    SearchResult sr = (SearchResult) userAnswer.next();
-                Attributes attrs = sr.getAttributes();
+                    Attributes attrs = sr.getAttributes();
 
-                if (attrs != null) {
+                    if (attrs != null) {
-                    for (NamingEnumeration ae = attrs.getAll(); ae.hasMore(); ) {
+                        try (ClosableNamingEnumeration<? extends Attribute> ae = new ClosableNamingEnumeration<>(attrs.getAll())) {
-                        Attribute attr = (Attribute) ae.next();
+                            while (ae.hasMore() ) {
-                        for (NamingEnumeration e = attr.getAll(); e.hasMore(); ) {
+                                Attribute attr = (Attribute) ae.next();
-                            byte[] sid = (byte[]) e.next();
+                                for (NamingEnumeration e = attr.getAll(); e.hasMore(); ) {
-                            groupsSearchFilter.append("(objectSid=");
+                                    byte[] sid = (byte[]) e.next();
-                            groupsSearchFilter.append(binarySidToStringSid(sid));
+                                    groupsSearchFilter.append("(objectSid=");
-                            groupsSearchFilter.append(")");
+                                    groupsSearchFilter.append(binarySidToStringSid(sid));
+                                    groupsSearchFilter.append(")");
+                                }
+                                groupsSearchFilter.append(")");
+                            }
                         }
-                        groupsSearchFilter.append(")");
                     }
                 }
             }
 
-        } catch (NamingException ne) {
+        } catch (NamingException | LdapException ne) {
             throw new ActiveDirectoryException("Exception occurred fetching AD groups", bindDn, ne);
         }
         return groupsSearchFilter.toString();

src/main/java/org/elasticsearch/shield/authc/active_directory/ActiveDirectoryConnectionFactory.java

@@ -10,12 +10,12 @@ import org.elasticsearch.common.collect.ImmutableMap;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.shield.ShieldSettingsException;
+import org.elasticsearch.shield.authc.ldap.LdapException;
 import org.elasticsearch.shield.authc.support.SecuredString;
+import org.elasticsearch.shield.authc.support.ldap.ClosableNamingEnumeration;
 import org.elasticsearch.shield.authc.support.ldap.ConnectionFactory;
-import org.elasticsearch.shield.authc.support.ldap.AbstractLdapSslSocketFactory;
 
 import javax.naming.Context;
-import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.InitialDirContext;
@@ -86,24 +86,23 @@ public class ActiveDirectoryConnectionFactory extends ConnectionFactory {
             searchCtls.setReturningAttributes(Strings.EMPTY_ARRAY);
             searchCtls.setTimeLimit(timeoutMilliseconds);
             String searchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
-            NamingEnumeration<SearchResult> results = ctx.search(userSearchDN, searchFilter, new Object[] { userPrincipal }, searchCtls);
+            try (ClosableNamingEnumeration<SearchResult> results = new ClosableNamingEnumeration(
+                    ctx.search(userSearchDN, searchFilter, new Object[] { userPrincipal }, searchCtls))) {
 
-            if (results.hasMore()) {
+                if(results.hasMore()){
-                SearchResult entry = results.next();
+                    SearchResult entry = results.next();
-                String name = entry.getNameInNamespace();
+                    String name = entry.getNameInNamespace();
 
-                if (!results.hasMore()) {
+                    if (!results.hasMore()) {
-                    return new ActiveDirectoryConnection(ctx, name, userSearchDN, timeoutMilliseconds);
+                        return new ActiveDirectoryConnection(ctx, name, userSearchDN, timeoutMilliseconds);
+                    }
+                    throw new ActiveDirectoryException("Search for user [" + userName + "] by principle name yielded multiple results");
                 }
-                results.close();
+
                 ctx.close();
                 throw new ActiveDirectoryException("Search for user [" + userName + "] by principle name yielded multiple results");
             }
-            results.close();
+        } catch (NamingException | LdapException e) {
-            ctx.close();
-            throw new ActiveDirectoryException("Search for user [" + userName + "], search root [" + userSearchDN + "] yielded no results");
-
-        } catch (NamingException e) {
             if (ctx != null) {
                 try {
                     ctx.close();

src/main/java/org/elasticsearch/shield/authc/ldap/LdapConnection.java

@@ -9,6 +9,7 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.shield.authc.support.ldap.AbstractLdapConnection;
+import org.elasticsearch.shield.authc.support.ldap.ClosableNamingEnumeration;
 
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
@@ -81,12 +82,12 @@ public class LdapConnection extends AbstractLdapConnection {
                 "(|(objectclass=groupOfNames)(objectclass=groupOfUniqueNames)(objectclass=group)) " +
                 "(|(uniqueMember={0})(member={0})))";
 
-        try {
+        try (ClosableNamingEnumeration<SearchResult> results = new ClosableNamingEnumeration<>(
-            NamingEnumeration<SearchResult> results = jndiContext.search(groupSearchDN, filter, new Object[] { userDn }, search);
+                jndiContext.search(groupSearchDN, filter, new Object[] { userDn }, search))) {
             while (results.hasMoreElements()) {
                 groups.add(results.next().getNameInNamespace());
             }
-        } catch (NamingException e) {
+        } catch (NamingException | LdapException e ) {
             throw new LdapException("Could not search for an LDAP group for user [" + userDn + "]", e);
         }
         return groups;
@@ -103,17 +104,19 @@ public class LdapConnection extends AbstractLdapConnection {
         List<String> groupDns = new LinkedList<>();
         try {
             Attributes results = jndiContext.getAttributes(userDn, new String[] { groupAttribute });
-            for (NamingEnumeration ae = results.getAll(); ae.hasMore(); ) {
+            try (ClosableNamingEnumeration<? extends Attribute> ae = new ClosableNamingEnumeration<>(results.getAll())) {
-                Attribute attr = (Attribute) ae.next();
+                while (ae.hasMore()) {
-                for (NamingEnumeration attrEnum = attr.getAll(); attrEnum.hasMore(); ) {
+                    Attribute attr = (Attribute) ae.next();
-                    Object val = attrEnum.next();
+                    for (NamingEnumeration attrEnum = attr.getAll(); attrEnum.hasMore(); ) {
-                    if (val instanceof String) {
+                        Object val = attrEnum.next();
-                        String stringVal = (String) val;
+                        if (val instanceof String) {
-                        groupDns.add(stringVal);
+                            String stringVal = (String) val;
+                            groupDns.add(stringVal);
+                        }
                     }
                 }
             }
-        } catch (NamingException e) {
+        } catch (NamingException | LdapException e) {
             throw new LdapException("Could not look up group attributes for user [" + userDn + "]", e);
         }
         return groupDns;

src/main/java/org/elasticsearch/shield/authc/support/ldap/ClosableNamingEnumeration.java

@@ -0,0 +1,52 @@
+/*
+ * 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.shield.authc.support.ldap;
+
+import org.elasticsearch.shield.authc.ldap.LdapException;
+
+import javax.naming.NamingEnumeration;
+import javax.naming.NamingException;
+import java.io.Closeable;
+
+/**
+ * ClosableNamingEnumeration wraps a NamingEnumeration so it can be used in a try with resources block and auto-closed.
+ */
+public class ClosableNamingEnumeration<T> implements Closeable, NamingEnumeration<T> {
+    private final NamingEnumeration<T> namingEnumeration;
+
+    public ClosableNamingEnumeration(NamingEnumeration<T> namingEnumeration) {
+        this.namingEnumeration = namingEnumeration;
+    }
+
+    @Override
+    public T next() throws NamingException {
+        return namingEnumeration.next();
+    }
+
+    @Override
+    public boolean hasMore() throws NamingException {
+        return namingEnumeration.hasMore();
+    }
+
+    @Override
+    public void close() {
+        try {
+            namingEnumeration.close();
+        } catch (NamingException e) {
+            throw new LdapException("Error occurred trying to close a naming enumeration", e);
+        }
+    }
+
+    @Override
+    public boolean hasMoreElements() {
+        return namingEnumeration.hasMoreElements();
+    }
+
+    @Override
+    public T nextElement() {
+        return namingEnumeration.nextElement();
+    }
+}