This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 7.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e21eb4764ccda55e5a35a5a7c19a6fd2b0757fe9 Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Apr 13 16:09:56 2021 +0100 Fix BZ 65224. Correct escaping in JNDIRealm --- java/org/apache/catalina/realm/JNDIRealm.java | 161 ++++++++++++++++++++++---- webapps/docs/changelog.xml | 4 + 2 files changed, 142 insertions(+), 23 deletions(-) diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java index a9032cf..6425194 100644 --- a/java/org/apache/catalina/realm/JNDIRealm.java +++ b/java/org/apache/catalina/realm/JNDIRealm.java @@ -205,13 +205,11 @@ 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 @@ -291,7 +289,6 @@ public class JNDIRealm extends RealmBase { */ protected MessageFormat userSearchFormat = null; - /** * Should we search the entire subtree for matching users? */ @@ -915,8 +912,7 @@ public class JNDIRealm extends RealmBase { int len = this.userPatternArray.length; userPatternFormatArray = new MessageFormat[len]; for (int i=0; i < len; i++) { - userPatternFormatArray[i] = - new MessageFormat(userPatternArray[i]); + userPatternFormatArray[i] = new MessageFormat(userPatternArray[i]); } } } @@ -1462,7 +1458,7 @@ public class JNDIRealm extends RealmBase { * @exception NamingException if a directory server error occurs */ protected User getUser(DirContext context, String username, String credentials, int curUserPattern) - throws NamingException { + throws NamingException { User user = null; @@ -1589,8 +1585,11 @@ public class JNDIRealm extends RealmBase { return null; } - // Form the dn from the user pattern - String dn = userPatternFormatArray[curUserPattern].format(new String[] { username }); + // Form the DistinguishedName from the user pattern. + // Escape in case username contains a character with special meaning in + // an attribute value. + String dn = userPatternFormatArray[curUserPattern].format( + new String[] { doAttributeValueEscaping(username) }); try { user = getUserByPattern(context, username, attrIds, dn); @@ -1630,7 +1629,9 @@ public class JNDIRealm extends RealmBase { } // Form the search filter - String filter = userSearchFormat.format(new String[] { username }); + // Escape in case username contains a character with special meaning in + // a search filter. + String filter = userSearchFormat.format(new String[] { doFilterEscaping(username) }); // Set up the search controls SearchControls constraints = new SearchControls(); @@ -1798,6 +1799,8 @@ public class JNDIRealm extends RealmBase { return false; } + // This is returned from the directory so will be attribute value + // escaped if required String dn = user.getDN(); if (dn == null) { return false; @@ -1888,7 +1891,11 @@ public class JNDIRealm extends RealmBase { return null; } + // This is returned from the directory so will be attribute value + // escaped if required String dn = user.getDN(); + // This is the name the user provided to the authentication process so + // it will not be escaped String username = user.getUserName(); String userRoleId = user.getUserRoleId(); @@ -1920,8 +1927,13 @@ public class JNDIRealm extends RealmBase { return list; } - // Set up parameters for an appropriate search - String filter = roleFormat.format(new String[] { doRFC2254Encoding(dn), username, userRoleId }); + // Set up parameters for an appropriate search filter + // The dn is already attribute value escaped but the others are not + // This is a filter so all input will require filter escaping + String filter = roleFormat.format(new String[] { + doFilterEscaping(dn), + doFilterEscaping(doAttributeValueEscaping(username)), + doFilterEscaping(doAttributeValueEscaping(userRoleId)) }); SearchControls controls = new SearchControls(); if (roleSubtree) { controls.setSearchScope(SearchControls.SUBTREE_SCOPE); @@ -1936,7 +1948,9 @@ public class JNDIRealm extends RealmBase { Name name = np.parse(dn); String nameParts[] = new String[name.size()]; for (int i = 0; i < name.size(); i++) { - nameParts[i] = name.get(i); + // May have been returned with \<char> escaping rather than + // \<hex><hex>. Make sure it is \<hex><hex>. + nameParts[i] = convertToHexEscape(name.get(i)); } base = roleBaseFormat.format(nameParts); } else { @@ -1959,7 +1973,7 @@ public class JNDIRealm extends RealmBase { if (attrs == null) { continue; } - String dname = getDistinguishedName(context, roleBase, result); + String dname = getDistinguishedName(context, base, result); String name = getAttributeValue(roleName, attrs); if (name != null && dname != null) { groupMap.put(dname, name); @@ -1993,14 +2007,20 @@ public class JNDIRealm extends RealmBase { Map<String, String> newThisRound = new HashMap<String, String>(); // Stores the groups we find in this iteration for (Entry<String, String> group : newGroups.entrySet()) { - filter = roleFormat.format(new String[] { doRFC2254Encoding(group.getKey()), - group.getValue(), group.getValue() }); + // Group key is already value escaped if required + // Group value is not value escaped + // Everything needs to be filter escaped + filter = roleFormat.format(new String[] { + doFilterEscaping(group.getKey()), + doFilterEscaping(doAttributeValueEscaping(group.getValue())), + doFilterEscaping(doAttributeValueEscaping(group.getValue())) }); if (containerLog.isTraceEnabled()) { - containerLog.trace("Perform a nested group search with base "+ roleBase + " and filter " + filter); + containerLog.trace("Perform a nested group search with base "+ roleBase + + " and filter " + filter); } - results = searchAsUser(context, user, roleBase, filter, controls, isRoleSearchAsUser()); + results = searchAsUser(context, user, base, filter, controls, isRoleSearchAsUser()); try { while (results.hasMore()) { @@ -2501,11 +2521,9 @@ public class JNDIRealm extends RealmBase { } return sslContext.getSocketFactory(); } catch (NoSuchAlgorithmException e) { - List<String> allowedProtocols = Arrays - .asList(getSupportedSslProtocols()); - throw new IllegalArgumentException( - sm.getString("jndiRealm.invalidSslProtocol", protocol, - allowedProtocols), e); + List<String> allowedProtocols = Arrays.asList(getSupportedSslProtocols()); + throw new IllegalArgumentException(sm.getString("jndiRealm.invalidSslProtocol", + protocol, allowedProtocols), e); } catch (KeyManagementException e) { List<String> allowedProtocols = Arrays.asList(getSupportedSslProtocols()); throw new IllegalArgumentException(sm.getString("jndiRealm.invalidSslProtocol", @@ -2607,7 +2625,6 @@ public class JNDIRealm extends RealmBase { } return env; - } @@ -2722,10 +2739,36 @@ public class JNDIRealm extends RealmBase { * ) -> \29 * \ -> \5c * \0 -> \00 + * * @param inString string to escape according to RFC 2254 guidelines + * * @return String the escaped/encoded result + * + * @deprecated Will be removed in Tomcat 10.1.x onwards */ + @Deprecated protected String doRFC2254Encoding(String inString) { + return doFilterEscaping(inString); + } + + + /** + * Given an LDAP search string, returns the string with certain characters + * escaped according to RFC 2254 guidelines. + * The character mapping is as follows: + * char -> Replacement + * --------------------------- + * * -> \2a + * ( -> \28 + * ) -> \29 + * \ -> \5c + * \0 -> \00 + * + * @param inString string to escape according to RFC 2254 guidelines + * + * @return String the escaped/encoded result + */ + protected String doFilterEscaping(String inString) { StringBuilder buf = new StringBuilder(inString.length()); for (int i = 0; i < inString.length(); i++) { char c = inString.charAt(i); @@ -2810,6 +2853,78 @@ public class JNDIRealm extends RealmBase { } + /** + * Implements the necessary escaping to represent an attribute value as a + * String as per RFC 4514. + * + * @param input The original attribute value + * @return The string representation of the attribute value + */ + protected String doAttributeValueEscaping(String input) { + int len = input.length(); + StringBuilder result = new StringBuilder(); + + for (int i = 0; i < len; i++) { + char c = input.charAt(i); + switch (c) { + case ' ': { + if (i == 0 || i == (len -1)) { + result.append("\\20"); + } else { + result.append(c); + } + break; + } + case '#': { + if (i == 0 ) { + result.append("\\23"); + } else { + result.append(c); + } + break; + } + case '\"': { + result.append("\\22"); + break; + } + case '+': { + result.append("\\2B"); + break; + } + case ',': { + result.append("\\2C"); + break; + } + case ';': { + result.append("\\3B"); + break; + } + case '<': { + result.append("\\3C"); + break; + } + case '>': { + result.append("\\3E"); + break; + } + case '\\': { + result.append("\\5C"); + break; + } + case '\u0000': { + result.append("\\00"); + break; + } + default: + result.append(c); + } + + } + + return result.toString(); + } + + protected static String convertToHexEscape(String input) { if (input.indexOf('\\') == -1) { // No escaping present. Return original. diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 54255ef..b5fa4ef 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -122,6 +122,10 @@ <subsection name="Catalina"> <changelog> <fix> + <bug>65224</bug>: Ensure the correct escaping of attribute values and + search filters in the JNDIRealm. (markt) + </fix> + <fix> <bug>65226</bug>: Fix extraction of JAR name in some cases in StandardJarScanner. Submitted by Lynx. (remm) </fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org