You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/04/13 15:21:23 UTC
[tomcat] 02/02: Fix BZ 65224. Correct escaping in JNDIRealm
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org