Commit 4971bfe9 authored by Tom Evans's avatar Tom Evans

Merge pull request #88 from tevans/master

OF-830: Properly handle LDAP search filters
parents b4bc7650 fd090d88
...@@ -107,7 +107,8 @@ public class LdapAuthorizationMapping implements AuthorizationMapping { ...@@ -107,7 +107,8 @@ public class LdapAuthorizationMapping implements AuthorizationMapping {
} }
constraints.setReturningAttributes(new String[] { usernameField }); constraints.setReturningAttributes(new String[] { usernameField });
NamingEnumeration answer = ctx.search("", princSearchFilter, new String[] {principal}, NamingEnumeration answer = ctx.search("", princSearchFilter,
new String[] {LdapManager.sanitizeSearchFilter(principal)},
constraints); constraints);
Log.debug("LdapAuthorizationMapping: ... search finished"); Log.debug("LdapAuthorizationMapping: ... search finished");
if (answer == null || !answer.hasMoreElements()) { if (answer == null || !answer.hasMoreElements()) {
......
...@@ -165,7 +165,7 @@ public class LdapGroupProvider extends AbstractGroupProvider { ...@@ -165,7 +165,7 @@ public class LdapGroupProvider extends AbstractGroupProvider {
StringBuilder filter = new StringBuilder(); StringBuilder filter = new StringBuilder();
filter.append("(&"); filter.append("(&");
filter.append(MessageFormat.format(manager.getGroupSearchFilter(), "*")); filter.append(MessageFormat.format(manager.getGroupSearchFilter(), "*"));
filter.append("(").append(key).append("=").append(value); filter.append("(").append(key).append("=").append(LdapManager.sanitizeSearchFilter(value));
filter.append("))"); filter.append("))");
if (Log.isDebugEnabled()) { if (Log.isDebugEnabled()) {
Log.debug("Trying to find group names using query: " + filter.toString()); Log.debug("Trying to find group names using query: " + filter.toString());
...@@ -188,13 +188,11 @@ public class LdapGroupProvider extends AbstractGroupProvider { ...@@ -188,13 +188,11 @@ public class LdapGroupProvider extends AbstractGroupProvider {
if (query == null || "".equals(query)) { if (query == null || "".equals(query)) {
return Collections.emptyList(); return Collections.emptyList();
} }
// Make the query be a wildcard search by default. So, if the user searches for
// "Test", make the search be "Test*" instead.
if (!query.endsWith("*")) {
query = query + "*";
}
StringBuilder filter = new StringBuilder(); StringBuilder filter = new StringBuilder();
filter.append("(").append(manager.getGroupNameField()).append("=").append(query).append(")"); // Make the query be a wildcard search by default. So, if the user searches for
// "Test", make the sanitized search be "Test*" instead.
filter.append("(").append(manager.getGroupNameField()).append("=")
.append(LdapManager.sanitizeSearchFilter(query)).append("*)");
// Perform the LDAP query // Perform the LDAP query
return manager.retrieveList( return manager.retrieveList(
manager.getGroupNameField(), manager.getGroupNameField(),
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
package org.jivesoftware.openfire.ldap; package org.jivesoftware.openfire.ldap;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder; import java.net.URLEncoder;
import java.text.MessageFormat; import java.text.MessageFormat;
import java.util.ArrayList; import java.util.ArrayList;
...@@ -971,7 +972,8 @@ public class LdapManager { ...@@ -971,7 +972,8 @@ public class LdapManager {
} }
constraints.setReturningAttributes(new String[] { usernameField }); constraints.setReturningAttributes(new String[] { usernameField });
NamingEnumeration<SearchResult> answer = ctx.search("", getSearchFilter(), new String[] {username}, NamingEnumeration<SearchResult> answer = ctx.search("", getSearchFilter(),
new String[] {sanitizeSearchFilter(username)},
constraints); constraints);
if (debug) { if (debug) {
...@@ -1115,7 +1117,7 @@ public class LdapManager { ...@@ -1115,7 +1117,7 @@ public class LdapManager {
} }
constraints.setReturningAttributes(new String[] { groupNameField }); constraints.setReturningAttributes(new String[] { groupNameField });
String filter = MessageFormat.format(getGroupSearchFilter(), groupname); String filter = MessageFormat.format(getGroupSearchFilter(), sanitizeSearchFilter(JID.unescapeNode(groupname)));
NamingEnumeration<SearchResult> answer = ctx.search("", filter, constraints); NamingEnumeration<SearchResult> answer = ctx.search("", filter, constraints);
if (debug) { if (debug) {
...@@ -2182,6 +2184,57 @@ public class LdapManager { ...@@ -2182,6 +2184,57 @@ public class LdapManager {
} }
return count; return count;
} }
/**
* Escapes any special chars (RFC 4515) from a string representing
* a search filter assertion value.
*
* @param input The input string.
*
* @return A assertion value string ready for insertion into a
* search filter string.
*/
public static String sanitizeSearchFilter(final String value) {
StringBuilder result = new StringBuilder();
for (int i=0; i< value.length(); i++) {
char c = value.charAt(i);
switch(c) {
case '!': result.append("\\21"); break;
case '&': result.append("\\26"); break;
case '(': result.append("\\28"); break;
case ')': result.append("\\29"); break;
case '*': result.append("\\2a"); break;
case ':': result.append("\\3a"); break;
case '\\': result.append("\\5c"); break;
case '|': result.append("\\7c"); break;
case '~': result.append("\\7e"); break;
case '\u0000': result.append("\\00"); break;
default:
if (c <= 0x7f) {
// regular 1-byte UTF-8 char
result.append(String.valueOf(c));
}
else if (c >= 0x080) {
// higher-order 2, 3 and 4-byte UTF-8 chars
try {
byte[] utf8bytes = String.valueOf(c).getBytes("UTF8");
for (byte b: utf8bytes)
{
result.append(String.format("\\%02x", b));
}
} catch (UnsupportedEncodingException e) {
// ignore
}
}
}
}
return result.toString();
}
/** /**
* Encloses DN values with " * Encloses DN values with "
......
...@@ -255,11 +255,6 @@ public class LdapUserProvider implements UserProvider { ...@@ -255,11 +255,6 @@ public class LdapUserProvider implements UserProvider {
if (!searchFields.keySet().containsAll(fields)) { if (!searchFields.keySet().containsAll(fields)) {
throw new IllegalArgumentException("Search fields " + fields + " are not valid."); throw new IllegalArgumentException("Search fields " + fields + " are not valid.");
} }
// Make the query be a wildcard search by default. So, if the user searches for
// "John", make the search be "John*" instead.
if (!query.endsWith("*")) {
query = query + "*";
}
StringBuilder filter = new StringBuilder(); StringBuilder filter = new StringBuilder();
//Add the global search filter so only those users the directory administrator wants to include //Add the global search filter so only those users the directory administrator wants to include
//are returned from the directory //are returned from the directory
...@@ -271,7 +266,10 @@ public class LdapUserProvider implements UserProvider { ...@@ -271,7 +266,10 @@ public class LdapUserProvider implements UserProvider {
} }
for (String field:fields) { for (String field:fields) {
String attribute = searchFields.get(field); String attribute = searchFields.get(field);
filter.append("(").append(attribute).append("=").append(query).append(")"); // Make the query be a wildcard search by default. So, if the user searches for
// "John", make the sanitized search be "John*" instead.
filter.append("(").append(attribute).append("=")
.append(LdapManager.sanitizeSearchFilter(query)).append("*)");
} }
if (fields.size() > 1) { if (fields.size() > 1) {
filter.append(")"); filter.append(")");
......
...@@ -9,11 +9,13 @@ ...@@ -9,11 +9,13 @@
*/ */
package org.jivesoftware.util; package org.jivesoftware.util;
import static org.junit.Assert.assertTrue;
import javax.naming.ldap.Rdn;
import org.jivesoftware.openfire.ldap.LdapManager; import org.jivesoftware.openfire.ldap.LdapManager;
import org.junit.Test; import org.junit.Test;
import static org.junit.Assert.assertTrue;
/** /**
* @author Daniel Henninger * @author Daniel Henninger
*/ */
...@@ -36,4 +38,45 @@ public class LDAPTest { ...@@ -36,4 +38,45 @@ public class LDAPTest {
converted = LdapManager.getEnclosedDN(before); converted = LdapManager.getEnclosedDN(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after)); assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
} }
@Test
public void testRdnEscapeValue() {
String before = "Jive Software, Inc";
String after = "Jive Software\\, Inc";
String converted = Rdn.escapeValue(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
before = "Test.User; (+1)";
after = "Test.User\\; (\\+1)";
converted = Rdn.escapeValue(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
before = "Wildcard *";
after = "Wildcard *";
converted = Rdn.escapeValue(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
before = "Group/Section";
after = "Group/Section";
converted = Rdn.escapeValue(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
}
@Test
public void testSanitizeSearchFilter() {
String before = "Test.User; (+1)";
String after = "Test.User; \\28+1\\29";
String converted = LdapManager.sanitizeSearchFilter(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
before = "Wildcard *";
after = "Wildcard \\2a";
converted = LdapManager.sanitizeSearchFilter(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
before = "~ Group|Section & Teams!";
after = "\\7e Group\\7cSection \\26 Teams\\21";
converted = LdapManager.sanitizeSearchFilter(before);
assertTrue("Conversion result "+before+" to "+converted, converted.equals(after));
}
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment