File _service:obs_scm:CVE-2021-30640-pre3.patch of Package tomcat

From 94b22be79a82a7238b022bcaa61b574e6a56691e Mon Sep 17 00:00:00 2001
From: remm <remm@apache.org>
Date: Thu, 30 Jan 2020 17:22:51 +0100
Subject: [PATCH] Add connection pool to JNDI realm

This implements a TODO from the class javadoc header.
As described in the javadoc, the idea is to use a pool to avoid blocking
on a single connection, which could possibly become a bottleneck in some
cases. The message formats need to be kept along with the connection
since they are not thread safe.
Preserve the default behavior: sync without pooling (using a Lock object
which is more flexible).
I may backport this since this is limited to the JNDI realm, but only
once it is confirmed to be regression free. Tested with ApacheDS but my
LDAP skills are very limited.
---
 java/org/apache/catalina/realm/JNDIRealm.java | 442 ++++++++++--------
 .../catalina/realm/LocalStrings.properties    |   3 +-
 .../apache/catalina/realm/TestJNDIRealm.java  |   7 +-
 webapps/docs/changelog.xml                    |   3 +
 webapps/docs/config/realm.xml                 |   7 +
 5 files changed, 277 insertions(+), 185 deletions(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java
index 505dd13..b624c5b 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -33,6 +33,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import javax.naming.AuthenticationException;
 import javax.naming.CommunicationException;
@@ -62,6 +64,7 @@ import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocketFactory;
 
 import org.apache.catalina.LifecycleException;
+import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.ietf.jgss.GSSCredential;
 
 /**
@@ -166,10 +169,6 @@ import org.ietf.jgss.GSSCredential;
 * directory server itself.</li>
 * </ul>
 *
- * TODO - Support connection pooling (including message
- * format objects) so that <code>authenticate()</code> does not have to be
- * synchronized.
- *
 * WARNING - There is a reported bug against the Netscape
 * provider code (com.netscape.jndi.ldap.LdapContextFactory) with respect to
 * successfully authenticated a non-existing user. The
@@ -208,12 +207,6 @@ public class JNDIRealm extends RealmBase {
 protected String connectionURL = null;
 
 
- /**
- * The directory context linking us to our directory server.
- */
- protected DirContext context = null;
-
-
 /**
 * The JNDI context factory used to acquire our InitialContext. By
 * default, assumes use of an LDAP server using the standard JNDI LDAP
@@ -282,13 +275,6 @@ public class JNDIRealm extends RealmBase {
 private boolean userSearchAsUser = false;
 
 
- /**
- * The MessageFormat object associated with the current
- * <code>userSearch</code>.
- */
- protected MessageFormat userSearchFormat = null;
-
-
 /**
 * Should we search the entire subtree for matching users?
 */
@@ -328,32 +314,12 @@ public class JNDIRealm extends RealmBase {
 protected String userPattern = null;
 
 
- /**
- * An array of MessageFormat objects associated with the current
- * <code>userPatternArray</code>.
- */
- protected MessageFormat[] userPatternFormatArray = null;
-
 /**
 * The base element for role searches.
 */
 protected String roleBase = "";
 
 
- /**
- * The MessageFormat object associated with the current
- * <code>roleBase</code>.
- */
- protected MessageFormat roleBaseFormat = null;
-
-
- /**
- * The MessageFormat object associated with the current
- * <code>roleSearch</code>.
- */
- protected MessageFormat roleFormat = null;
-
-
 /**
 * The name of an attribute in the user's entry containing
 * roles for that user
@@ -497,6 +463,30 @@ public class JNDIRealm extends RealmBase {
 */
 private String sslProtocol;
 
+ /**
+ * Non pooled connection to our directory server.
+ */
+ protected JNDIConnection singleConnection = new JNDIConnection();
+
+
+ /**
+ * The lock to ensure single connection thread safety.
+ */
+ protected final Lock singleConnectionLock = new ReentrantLock();
+
+
+ /**
+ * Connection pool.
+ */
+ protected SynchronizedStack<JNDIConnection> connectionPool = null;
+
+
+ /**
+ * The pool size limit. If 1, pooling is not used.
+ */
+ protected int connectionPoolSize = 1;
+
+
 // ------------------------------------------------------------- Properties
 
 /**
@@ -717,13 +707,8 @@ public class JNDIRealm extends RealmBase {
 * @param userSearch The new user search pattern
 */
 public void setUserSearch(String userSearch) {
-
 this.userSearch = userSearch;
- if (userSearch == null)
- userSearchFormat = null;
- else
- userSearchFormat = new MessageFormat(userSearch);
-
+ singleConnection = create();
 }
 
 
@@ -796,13 +781,8 @@ public class JNDIRealm extends RealmBase {
 * @param roleBase The new base element
 */
 public void setRoleBase(String roleBase) {
-
 this.roleBase = roleBase;
- if (roleBase == null)
- roleBaseFormat = null;
- else
- roleBaseFormat = new MessageFormat(roleBase);
-
+ singleConnection = create();
 }
 
 
@@ -844,13 +824,8 @@ public class JNDIRealm extends RealmBase {
 * @param roleSearch The new role search pattern
 */
 public void setRoleSearch(String roleSearch) {
-
 this.roleSearch = roleSearch;
- if (roleSearch == null)
- roleFormat = null;
- else
- roleFormat = new MessageFormat(roleSearch);
-
+ singleConnection = create();
 }
 
 
@@ -960,18 +935,12 @@ public class JNDIRealm extends RealmBase {
 * @param userPattern The new user pattern
 */
 public void setUserPattern(String userPattern) {
-
 this.userPattern = userPattern;
- if (userPattern == null)
+ if (userPattern == null) {
 userPatternArray = null;
- else {
+ } else {
 userPatternArray = parseUserPatternString(userPattern);
- int len = this.userPatternArray.length;
- userPatternFormatArray = new MessageFormat[len];
- for (int i=0; i < len; i++) {
- userPatternFormatArray[i] =
- new MessageFormat(userPatternArray[i]);
- }
+ singleConnection = create();
 }
 }
 
@@ -1151,6 +1120,22 @@ public class JNDIRealm extends RealmBase {
 this.cipherSuites = suites;
 }
 
+ /**
+ * @return the connection pool size, or the default value 1 if pooling
+ * is disabled
+ */
+ public int getConnectionPoolSize() {
+ return connectionPoolSize;
+ }
+
+ /**
+ * Set the connection pool size
+ * @param connectionPoolSize the new pool size
+ */
+ public void setConnectionPoolSize(int connectionPoolSize) {
+ this.connectionPoolSize = connectionPoolSize;
+ }
+
 /**
 * @return name of the {@link HostnameVerifier} class used for connections
 * using StartTLS, or the empty string, if the default verifier
@@ -1269,20 +1254,20 @@ public class JNDIRealm extends RealmBase {
 @Override
 public Principal authenticate(String username, String credentials) {
 
- DirContext context = null;
+ JNDIConnection connection = null;
 Principal principal = null;
 
 try {
 
 // Ensure that we have a directory context available
- context = open();
+ connection = get();
 
 // Occasionally the directory context will timeout. Try one more
 // time before giving up.
 try {
 
 // Authenticate the specified username if possible
- principal = authenticate(context, username, credentials);
+ principal = authenticate(connection, username, credentials);
 
 } catch (NullPointerException | NamingException e) {
 /*
@@ -1304,19 +1289,18 @@ public class JNDIRealm extends RealmBase {
 containerLog.info(sm.getString("jndiRealm.exception.retry"), e);
 
 // close the connection so we know it will be reopened.
- if (context != null)
- close(context);
+ close(connection);
 
 // open a new directory context.
- context = open();
+ connection = get();
 
 // Try the authentication again.
- principal = authenticate(context, username, credentials);
+ principal = authenticate(connection, username, credentials);
 }
 
 
 // Release this context
- release(context);
+ release(connection);
 
 // Return the authenticated Principal (if any)
 return principal;
@@ -1327,8 +1311,7 @@ public class JNDIRealm extends RealmBase {
 containerLog.error(sm.getString("jndiRealm.exception"), e);
 
 // Close the connection so that it gets reopened next time
- if (context != null)
- close(context);
+ close(connection);
 
 // Return "not authenticated" for this request
 if (containerLog.isDebugEnabled())
@@ -1350,7 +1333,7 @@ public class JNDIRealm extends RealmBase {
 * Return the Principal associated with the specified username and
 * credentials, if there is one; otherwise return <code>null</code>.
 *
- * @param context The directory context
+ * @param connection The directory context
 * @param username Username of the Principal to look up
 * @param credentials Password or other credentials to use in
 * authenticating this username
@@ -1358,7 +1341,7 @@ public class JNDIRealm extends RealmBase {
 *
 * @exception NamingException if a directory server error occurs
 */
- public synchronized Principal authenticate(DirContext context,
+ public Principal authenticate(JNDIConnection connection,
 String username,
 String credentials)
 throws NamingException {
@@ -1372,16 +1355,16 @@ public class JNDIRealm extends RealmBase {
 
 if (userPatternArray != null) {
 for (int curUserPattern = 0;
- curUserPattern < userPatternFormatArray.length;
+ curUserPattern < userPatternArray.length;
 curUserPattern++) {
 // Retrieve user information
- User user = getUser(context, username, credentials, curUserPattern);
+ User user = getUser(connection, username, credentials, curUserPattern);
 if (user != null) {
 try {
 // Check the user's credentials
- if (checkCredentials(context, user, credentials)) {
+ if (checkCredentials(connection.context, user, credentials)) {
 // Search for additional roles
- List<String> roles = getRoles(context, user);
+ List<String> roles = getRoles(connection, user);
 if (containerLog.isDebugEnabled()) {
 containerLog.debug("Found roles: " + roles.toString());
 }
@@ -1400,16 +1383,16 @@ public class JNDIRealm extends RealmBase {
 return null;
 } else {
 // Retrieve user information
- User user = getUser(context, username, credentials);
+ User user = getUser(connection, username, credentials);
 if (user == null)
 return null;
 
 // Check the user's credentials
- if (!checkCredentials(context, user, credentials))
+ if (!checkCredentials(connection.context, user, credentials))
 return null;
 
 // Search for additional roles
- List<String> roles = getRoles(context, user);
+ List<String> roles = getRoles(connection, user);
 if (containerLog.isDebugEnabled()) {
 containerLog.debug("Found roles: " + roles.toString());
 }
@@ -1425,17 +1408,17 @@ public class JNDIRealm extends RealmBase {
 * with the specified username, if found in the directory;
 * otherwise return <code>null</code>.
 *
- * @param context The directory context
+ * @param connection The directory context
 * @param username Username to be looked up
 * @return the User object
 * @exception NamingException if a directory server error occurs
 *
- * @see #getUser(DirContext, String, String, int)
+ * @see #getUser(JNDIConnection, String, String, int)
 */
- protected User getUser(DirContext context, String username)
+ protected User getUser(JNDIConnection connection, String username)
 throws NamingException {
 
- return getUser(context, username, null, -1);
+ return getUser(connection, username, null, -1);
 }
 
 
@@ -1444,18 +1427,18 @@ public class JNDIRealm extends RealmBase {
 * with the specified username, if found in the directory;
 * otherwise return <code>null</code>.
 *
- * @param context The directory context
+ * @param connection The directory context
 * @param username Username to be looked up
 * @param credentials User credentials (optional)
 * @return the User object
 * @exception NamingException if a directory server error occurs
 *
- * @see #getUser(DirContext, String, String, int)
+ * @see #getUser(JNDIConnection, String, String, int)
 */
- protected User getUser(DirContext context, String username, String credentials)
+ protected User getUser(JNDIConnection connection, String username, String credentials)
 throws NamingException {
 
- return getUser(context, username, credentials, -1);
+ return getUser(connection, username, credentials, -1);
 }
 
 
@@ -1470,14 +1453,14 @@ public class JNDIRealm extends RealmBase {
 * configuration attribute is specified, all values of that
 * attribute are retrieved from the directory entry.
 *
- * @param context The directory context
+ * @param connection The directory context
 * @param username Username to be looked up
 * @param credentials User credentials (optional)
 * @param curUserPattern Index into userPatternFormatArray
 * @return the User object
 * @exception NamingException if a directory server error occurs
 */
- protected User getUser(DirContext context, String username,
+ protected User getUser(JNDIConnection connection, String username,
 String credentials, int curUserPattern)
 throws NamingException {
 
@@ -1496,8 +1479,8 @@ public class JNDIRealm extends RealmBase {
 list.toArray(attrIds);
 
 // Use pattern or search for user entry
- if (userPatternFormatArray != null && curUserPattern >= 0) {
- user = getUserByPattern(context, username, credentials, attrIds, curUserPattern);
+ if (userPatternArray != null && curUserPattern >= 0) {
+ user = getUserByPattern(connection, username, credentials, attrIds, curUserPattern);
 if (containerLog.isDebugEnabled()) {
 containerLog.debug("Found user by pattern [" + user + "]");
 }
@@ -1505,12 +1488,12 @@ public class JNDIRealm extends RealmBase {
 boolean thisUserSearchAsUser = isUserSearchAsUser();
 try {
 if (thisUserSearchAsUser) {
- userCredentialsAdd(context, username, credentials);
+ userCredentialsAdd(connection.context, username, credentials);
 }
- user = getUserBySearch(context, username, attrIds);
+ user = getUserBySearch(connection, username, attrIds);
 } finally {
 if (thisUserSearchAsUser) {
- userCredentialsRemove(context);
+ userCredentialsRemove(connection.context);
 }
 }
 if (containerLog.isDebugEnabled()) {
@@ -1588,7 +1571,7 @@ public class JNDIRealm extends RealmBase {
 * username and return a User object; otherwise return
 * <code>null</code>.
 *
- * @param context The directory context
+ * @param connection The directory context
 * @param username The username
 * @param credentials User credentials (optional)
 * @param attrIds String[]containing names of attributes to
@@ -1597,7 +1580,7 @@ public class JNDIRealm extends RealmBase {
 * @exception NamingException if a directory server error occurs
 * @see #getUserByPattern(DirContext, String, String[], String)
 */
- protected User getUserByPattern(DirContext context,
+ protected User getUserByPattern(JNDIConnection connection,
 String username,
 String credentials,
 String[] attrIds,
@@ -1606,25 +1589,25 @@ public class JNDIRealm extends RealmBase {
 
 User user = null;
 
- if (username == null || userPatternFormatArray[curUserPattern] == null)
+ if (username == null || userPatternArray[curUserPattern] == null)
 return null;
 
 // Form the dn from the user pattern
- String dn = userPatternFormatArray[curUserPattern].format(new String[] { username });
+ String dn = connection.userPatternFormatArray[curUserPattern].format(new String[] { username });
 
 try {
- user = getUserByPattern(context, username, attrIds, dn);
+ user = getUserByPattern(connection.context, username, attrIds, dn);
 } catch (NameNotFoundException e) {
 return null;
 } catch (NamingException e) {
 // If the getUserByPattern() call fails, try it again with the
 // credentials of the user that we're searching for
 try {
- userCredentialsAdd(context, dn, credentials);
+ userCredentialsAdd(connection.context, dn, credentials);
 
- user = getUserByPattern(context, username, attrIds, dn);
+ user = getUserByPattern(connection.context, username, attrIds, dn);
 } finally {
- userCredentialsRemove(context);
+ userCredentialsRemove(connection.context);
 }
 }
 return user;
@@ -1636,22 +1619,22 @@ public class JNDIRealm extends RealmBase {
 * information about the user with the specified username, if
 * found in the directory; otherwise return <code>null</code>.
 *
- * @param context The directory context
+ * @param connection The directory context
 * @param username The username
 * @param attrIds String[]containing names of attributes to retrieve.
 * @return the User object
 * @exception NamingException if a directory server error occurs
 */
- protected User getUserBySearch(DirContext context,
+ protected User getUserBySearch(JNDIConnection connection,
 String username,
 String[] attrIds)
 throws NamingException {
 
- if (username == null || userSearchFormat == null)
+ if (username == null || connection.userSearchFormat == null)
 return null;
 
 // Form the search filter
- String filter = userSearchFormat.format(new String[] { username });
+ String filter = connection.userSearchFormat.format(new String[] { username });
 
 // Set up the search controls
 SearchControls constraints = new SearchControls();
@@ -1670,9 +1653,10 @@ public class JNDIRealm extends RealmBase {
 if (attrIds == null)
 attrIds = new String[0];
 constraints.setReturningAttributes(attrIds);
+System.out.println("getUserBySearch " + username);
 
 NamingEnumeration<SearchResult> results =
- context.search(userBase, filter, constraints);
+ connection.context.search(userBase, filter, constraints);
 
 try {
 // Fail if no entries found
@@ -1693,8 +1677,9 @@ public class JNDIRealm extends RealmBase {
 // Check no further entries were found
 try {
 if (results.hasMore()) {
- if(containerLog.isInfoEnabled())
- containerLog.info("username " + username + " has multiple entries");
+ if (containerLog.isInfoEnabled()) {
+ containerLog.info(sm.getString("jndiRealm.multipleEntries", username));
+ }
 return null;
 }
 } catch (PartialResultException ex) {
@@ -1702,7 +1687,7 @@ public class JNDIRealm extends RealmBase {
 throw ex;
 }
 
- String dn = getDistinguishedName(context, userBase, result);
+ String dn = getDistinguishedName(connection.context, userBase, result);
 
 if (containerLog.isTraceEnabled())
 containerLog.trace(" entry found for " + username + " with dn " + dn);
@@ -1724,6 +1709,7 @@ public class JNDIRealm extends RealmBase {
 
 // Retrieve values of userRoleName attribute
 ArrayList<String> roles = null;
+System.out.println("userRoleName " + userRoleName + " " + attrs.get(userRoleName));
 if (userRoleName != null)
 roles = addAttributeValues(userRoleName, attrs, roles);
 
@@ -1902,12 +1888,12 @@ public class JNDIRealm extends RealmBase {
 * a directory search. If no roles are associated with this user,
 * a zero-length List is returned.
 *
- * @param context The directory context we are searching
+ * @param connection The directory context we are searching
 * @param user The User to be checked
 * @return the list of role names
 * @exception NamingException if a directory server error occurs
 */
- protected List<String> getRoles(DirContext context, User user)
+ protected List<String> getRoles(JNDIConnection connection, User user)
 throws NamingException {
 
 if (user == null)
@@ -1938,11 +1924,11 @@ public class JNDIRealm extends RealmBase {
 }
 
 // Are we configured to do role searches?
- if ((roleFormat == null) || (roleName == null))
+ if ((connection.roleFormat == null) || (roleName == null))
 return list;
 
 // Set up parameters for an appropriate search
- String filter = roleFormat.format(new String[] { doRFC2254Encoding(dn), username, userRoleId });
+ String filter = connection.roleFormat.format(new String[] { doRFC2254Encoding(dn), username, userRoleId });
 SearchControls controls = new SearchControls();
 if (roleSubtree)
 controls.setSearchScope(SearchControls.SUBTREE_SCOPE);
@@ -1951,20 +1937,20 @@ public class JNDIRealm extends RealmBase {
 controls.setReturningAttributes(new String[] {roleName});
 
 String base = null;
- if (roleBaseFormat != null) {
- NameParser np = context.getNameParser("");
+ if (connection.roleBaseFormat != null) {
+ NameParser np = connection.context.getNameParser("");
 Name name = np.parse(dn);
 String nameParts[] = new String[name.size()];
 for (int i = 0; i < name.size(); i++) {
 nameParts[i] = name.get(i);
 }
- base = roleBaseFormat.format(nameParts);
+ base = connection.roleBaseFormat.format(nameParts);
 } else {
 base = "";
 }
 
 // Perform the configured search and process the results
- NamingEnumeration<SearchResult> results = searchAsUser(context, user, base, filter, controls,
+ NamingEnumeration<SearchResult> results = searchAsUser(connection.context, user, base, filter, controls,
 isRoleSearchAsUser());
 
 if (results == null)
@@ -1977,7 +1963,7 @@ public class JNDIRealm extends RealmBase {
 Attributes attrs = result.getAttributes();
 if (attrs == null)
 continue;
- String dname = getDistinguishedName(context, roleBase, result);
+ String dname = getDistinguishedName(connection.context, roleBase, result);
 String name = getAttributeValue(roleName, attrs);
 if (name != null && dname != null) {
 groupMap.put(dname, name);
@@ -2010,14 +1996,14 @@ public class JNDIRealm extends RealmBase {
 Map<String, String> newThisRound = new HashMap<>(); // Stores the groups we find in this iteration
 
 for (Entry<String, String> group : newGroups.entrySet()) {
- filter = roleFormat.format(new String[] { doRFC2254Encoding(group.getKey()),
+ filter = connection.roleFormat.format(new String[] { doRFC2254Encoding(group.getKey()),
 group.getValue(), group.getValue() });
 
 if (containerLog.isTraceEnabled()) {
 containerLog.trace("Perform a nested group search with base "+ roleBase + " and filter " + filter);
 }
 
- results = searchAsUser(context, user, roleBase, filter, controls,
+ results = searchAsUser(connection.context, user, roleBase, filter, controls,
 isRoleSearchAsUser());
 
 try {
@@ -2026,7 +2012,7 @@ public class JNDIRealm extends RealmBase {
 Attributes attrs = result.getAttributes();
 if (attrs == null)
 continue;
- String dname = getDistinguishedName(context, roleBase, result);
+ String dname = getDistinguishedName(connection.context, roleBase, result);
 String name = getAttributeValue(roleName, attrs);
 if (name != null && dname != null && !groupMap.keySet().contains(dname)) {
 groupMap.put(dname, name);
@@ -2169,12 +2155,12 @@ public class JNDIRealm extends RealmBase {
 /**
 * Close any open connection to the directory server for this Realm.
 *
- * @param context The directory context to be closed
+ * @param connection The directory context to be closed
 */
- protected void close(DirContext context) {
+ protected void close(JNDIConnection connection) {
 
 // Do nothing if there is no opened connection
- if (context == null)
+ if (connection.context == null)
 return;
 
 // Close tls startResponse if used
@@ -2189,11 +2175,15 @@ public class JNDIRealm extends RealmBase {
 try {
 if (containerLog.isDebugEnabled())
 containerLog.debug("Closing directory context");
- context.close();
+ connection.context.close();
 } catch (NamingException e) {
 containerLog.error(sm.getString("jndiRealm.close"), e);
 }
- this.context = null;
+ connection.context = null;
+ // The lock will be reacquired before any manipulation of the connection
+ if (connectionPool == null) {
+ singleConnectionLock.unlock();
+ }
 
 }
 
@@ -2211,7 +2201,7 @@ public class JNDIRealm extends RealmBase {
 }
 
 try {
- User user = getUser(open(), username, null);
+ User user = getUser(get(), username, null);
 if (user == null) {
 // User should be found...
 return null;
@@ -2239,20 +2229,20 @@ public class JNDIRealm extends RealmBase {
 protected Principal getPrincipal(String username,
 GSSCredential gssCredential) {
 
- DirContext context = null;
+ JNDIConnection connection = null;
 Principal principal = null;
 
 try {
 
 // Ensure that we have a directory context available
- context = open();
+ connection = get();
 
 // Occasionally the directory context will timeout. Try one more
 // time before giving up.
 try {
 
 // Authenticate the specified username if possible
- principal = getPrincipal(context, username, gssCredential);
+ principal = getPrincipal(connection, username, gssCredential);
 
 } catch (CommunicationException | ServiceUnavailableException e) {
 
@@ -2260,20 +2250,19 @@ public class JNDIRealm extends RealmBase {
 containerLog.info(sm.getString("jndiRealm.exception.retry"), e);
 
 // close the connection so we know it will be reopened.
- if (context != null)
- close(context);
+ close(connection);
 
 // open a new directory context.
- context = open();
+ connection = get();
 
 // Try the authentication again.
- principal = getPrincipal(context, username, gssCredential);
+ principal = getPrincipal(connection, username, gssCredential);
 
 }
 
 
 // Release this context
- release(context);
+ release(connection);
 
 // Return the authenticated Principal (if any)
 return principal;
@@ -2284,8 +2273,7 @@ public class JNDIRealm extends RealmBase {
 containerLog.error(sm.getString("jndiRealm.exception"), e);
 
 // Close the connection so that it gets reopened next time
- if (context != null)
- close(context);
+ close(connection);
 
 // Return "not authenticated" for this request
 return null;
@@ -2298,19 +2286,20 @@ public class JNDIRealm extends RealmBase {
 
 /**
 * Get the principal associated with the specified certificate.
- * @param context The directory context
+ * @param connection The directory context
 * @param username The user name
 * @param gssCredential The credentials
 * @return the Principal associated with the given certificate.
 * @exception NamingException if a directory server error occurs
 */
- protected synchronized Principal getPrincipal(DirContext context,
+ protected Principal getPrincipal(JNDIConnection connection,
 String username, GSSCredential gssCredential)
 throws NamingException {
 
 User user = null;
 List<String> roles = null;
 Hashtable<?, ?> preservedEnvironment = null;
+ DirContext context = connection.context;
 
 try {
 if (gssCredential != null && isUseDelegatedCredential()) {
@@ -2326,9 +2315,9 @@ public class JNDIRealm extends RealmBase {
 // Note: Subject already set in SPNEGO authenticator so no need
 // for Subject.doAs() here
 }
- user = getUser(context, username);
+ user = getUser(connection, username);
 if (user != null) {
- roles = getRoles(context, user);
+ roles = getRoles(connection, user);
 }
 } finally {
 restoreEnvironmentParameter(context,
@@ -2363,50 +2352,100 @@ public class JNDIRealm extends RealmBase {
 /**
 * Open (if necessary) and return a connection to the configured
 * directory server for this Realm.
- * @return the directory context
+ * @return the connection
 * @exception NamingException if a directory server error occurs
 */
- protected DirContext open() throws NamingException {
+ protected JNDIConnection get() throws NamingException {
+ JNDIConnection connection = null;
+ // Use the pool if available, otherwise use the single connection
+ if (connectionPool != null) {
+ connection = connectionPool.pop();
+ if (connection == null) {
+ connection = create();
+ }
+ } else {
+ singleConnectionLock.lock();
+ connection = singleConnection;
+ }
+ if (connection.context == null) {
+ open(connection);
+ }
+ return connection;
+ }
 
- // Do nothing if there is a directory server connection already open
- if (context != null)
- return context;
+ /**
+ * Release our use of this connection so that it can be recycled.
+ *
+ * @param connection The directory context to release
+ */
+ protected void release(JNDIConnection connection) {
+ if (connectionPool != null) {
+ if (!connectionPool.push(connection)) {
+ // Any connection that doesn't end back to the pool must be closed
+ close(connection);
+ }
+ } else {
+ singleConnectionLock.unlock();
+ }
+ }
 
- try {
+ /**
+ * Create a new connection wrapper, along with the
+ * message formats.
+ * @return the new connection
+ */
+ protected JNDIConnection create() {
+ JNDIConnection connection = new JNDIConnection();
+ if (userSearch != null) {
+ connection.userSearchFormat = new MessageFormat(userSearch);
+ }
+ if (userPattern != null) {
+ int len = userPatternArray.length;
+ connection.userPatternFormatArray = new MessageFormat[len];
+ for (int i = 0; i < len; i++) {
+ connection.userPatternFormatArray[i] =
+ new MessageFormat(userPatternArray[i]);
+ }
+ }
+ if (roleBase != null) {
+ connection.roleBaseFormat = new MessageFormat(roleBase);
+ }
+ if (roleSearch != null) {
+ connection.roleFormat = new MessageFormat(roleSearch);
+ }
+ return connection;
+ }
 
+ /**
+ * Create a new connection to the directory server.
+ * @param connection The directory server connection wrapper
+ * @throws NamingException if a directory server error occurs
+ */
+ protected void open(JNDIConnection connection) throws NamingException {
+ try {
 // Ensure that we have a directory context available
- context = createDirContext(getDirectoryContextEnvironment());
-
+ connection.context = createDirContext(getDirectoryContextEnvironment());
 } catch (Exception e) {
 if (alternateURL == null || alternateURL.length() == 0) {
 // No alternate URL. Re-throw the exception.
 throw e;
 }
-
 connectionAttempt = 1;
-
 // log the first exception.
 containerLog.info(sm.getString("jndiRealm.exception.retry"), e);
-
 // Try connecting to the alternate url.
- context = createDirContext(getDirectoryContextEnvironment());
-
+ connection.context = createDirContext(getDirectoryContextEnvironment());
 } finally {
-
 // reset it in case the connection times out.
 // the primary may come back.
 connectionAttempt = 0;
-
 }
-
- return context;
-
 }
 
 @Override
 public boolean isAvailable() {
 // Simple best effort check
- return (context != null);
+ return (connectionPool != null || singleConnection.context != null);
 }
 
 private DirContext createDirContext(Hashtable<String, String> env) throws NamingException {
@@ -2559,18 +2598,6 @@ public class JNDIRealm extends RealmBase {
 }
 
 
- /**
- * Release our use of this connection so that it can be recycled.
- *
- * @param context The directory context to release
- */
- protected void release(DirContext context) {
-
- // NO-OP since we are not pooling anything
-
- }
-
-
 // ------------------------------------------------------ Lifecycle Methods
 
 
@@ -2585,15 +2612,22 @@ public class JNDIRealm extends RealmBase {
 @Override
 protected void startInternal() throws LifecycleException {
 
+ if (connectionPoolSize != 1) {
+ connectionPool = new SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE, connectionPoolSize);
+ }
+
 // Check to see if the connection to the directory can be opened
+ JNDIConnection connection = null;
 try {
- open();
+ connection = get();
 } catch (NamingException e) {
 // A failure here is not fatal as the directory may be unavailable
 // now but available later. Unavailability of the directory is not
 // fatal once the Realm has started so there is no reason for it to
 // be fatal when the Realm starts.
 containerLog.error(sm.getString("jndiRealm.open"), e);
+ } finally {
+ release(connection);
 }
 
 super.startInternal();
@@ -2610,12 +2644,18 @@ public class JNDIRealm extends RealmBase {
 */
 @Override
 protected void stopInternal() throws LifecycleException {
-
 super.stopInternal();
-
 // Close any open directory server connection
- close(this.context);
-
+ if (connectionPool == null) {
+ singleConnectionLock.lock();
+ close(singleConnection);
+ } else {
+ JNDIConnection connection = null;
+ while ((connection = connectionPool.pop()) != null) {
+ close(connection);
+ }
+ connectionPool = null;
+ }
 }
 
 /**
@@ -2812,5 +2852,43 @@ public class JNDIRealm extends RealmBase {
 
 
 }
+
+ /**
+ * Class holding the connection to the directory plus the associated
+ * non thread safe message formats.
+ */
+ protected static class JNDIConnection {
+
+ /**
+ * The MessageFormat object associated with the current
+ * <code>userSearch</code>.
+ */
+ protected MessageFormat userSearchFormat = null;
+
+ /**
+ * An array of MessageFormat objects associated with the current
+ * <code>userPatternArray</code>.
+ */
+ protected MessageFormat[] userPatternFormatArray = null;
+
+ /**
+ * The MessageFormat object associated with the current
+ * <code>roleBase</code>.
+ */
+ protected MessageFormat roleBaseFormat = null;
+
+ /**
+ * The MessageFormat object associated with the current
+ * <code>roleSearch</code>.
+ */
+ protected MessageFormat roleFormat = null;
+
+ /**
+ * The directory context linking us to our directory server.
+ */
+ protected DirContext context = null;
+
+ }
+
 }
 
diff --git a/java/org/apache/catalina/realm/LocalStrings.properties b/java/org/apache/catalina/realm/LocalStrings.properties
index 1a96cc4..b66ad94 100644
--- a/java/org/apache/catalina/realm/LocalStrings.properties
+++ b/java/org/apache/catalina/realm/LocalStrings.properties
@@ -48,6 +48,7 @@ jndiRealm.exception.retry=Exception performing authentication. Retrying...
 jndiRealm.invalidHostnameVerifier=[{0}] not a valid class name for a HostnameVerifier
 jndiRealm.invalidSslProtocol=Given protocol [{0}] is invalid. It has to be one of [{1}]
 jndiRealm.invalidSslSocketFactory=[{0}] not a valid class name for a SSLSocketFactory
+jndiRealm.multipleEntries=User name [{0}] has multiple entries
 jndiRealm.negotiatedTls=Negotiated tls connection using protocol [{0}]
 jndiRealm.open=Exception opening directory server connection
 jndiRealm.tlsClose=Exception closing tls response
@@ -93,4 +94,4 @@ lockOutRealm.removeWarning=User [{0}] was removed from the failed users cache af
 credentialHandler.invalidStoredCredential=The invalid stored credential string [{0}] was provided by the Realm to match with the user provided credentials
 credentialHandler.unableToMutateUserCredential=Failed to mutate user provided credentials. This typically means the CredentialHandler configuration is invalid
 mdCredentialHandler.unknownEncoding=The encoding [{0}] is not supported so the current setting of [{1}] will still be used
-pbeCredentialHandler.invalidKeySpec=Unable to generate a password based key
\ No newline at end of file
+pbeCredentialHandler.invalidKeySpec=Unable to generate a password based key
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java b/test/org/apache/catalina/realm/TestJNDIRealm.java
index b2a82e4..238106e 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -117,9 +117,12 @@ public class TestJNDIRealm {
 realm.setContainer(context);
 realm.setUserSearch("");
 
- Field field = JNDIRealm.class.getDeclaredField("context");
+ // Usually everything is created in create() but that's not the case here
+ Field field = JNDIRealm.class.getDeclaredField("singleConnection");
 field.setAccessible(true);
- field.set(realm, mockDirContext(mockSearchResults(password)));
+ Field field2 = JNDIRealm.JNDIConnection.class.getDeclaredField("context");
+ field2.setAccessible(true);
+ field2.set(field.get(realm), mockDirContext(mockSearchResults(password)));
 
 realm.start();
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7bcc3d9..173c209 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -60,6 +60,9 @@
 searching for nested groups when the JNDIRealm is configured with
 <code>roleNested</code> set to <code>true</code>. (markt)
 </fix>
+ <update>
+ Add connection pooling to JNDI realm. (remm)
+ </update>
 </changelog>
 </subsection>
 </section>
diff --git a/webapps/docs/config/realm.xml b/webapps/docs/config/realm.xml
index 1d5ae6e..a4bc5ef 100644
--- a/webapps/docs/config/realm.xml
+++ b/webapps/docs/config/realm.xml
@@ -433,6 +433,13 @@
 property.
 </attribute>
 
+ <attribute name="connectionPoolSize" required="false">
+ The JNDI realm can use a pool of connections to the directory server
+ to avoid blocking on a single connection. This attribute value is the
+ maximum pool size. If not specified, it will use <code>1</code>, which
+ means a single connection will be used.
+ </attribute>
+
 <attribute name="connectionTimeout" required="false">
 The timeout in milliseconds to use when establishing the connection
 to the LDAP directory. If not specified, a value of 5000 (5 seconds) is
-- 
2.23.0