SEC-1444: Fix JNDI escaping problems in LDAP authentication.

CompositeName adds quotes to names which contain a forward slash ("/") character. These are automatically removed by Spring LDAP's DistinguishedName, but only if they are at the ends of the String. Since we were preprending the base to the (quoted) DN, resulting in something like ["cn=joe/b",ou=people], this was causing problems with the DN value returned from the search. Additionally, the bind succeeds when a DN is used with a slash, but the subsequent call to getAttributes() fails. This call now passes in a DistinguishedName for the user DN instance instead of a String.

ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java

@@ -193,21 +193,26 @@ public class SpringSecurityLdapTemplate extends LdapTemplate {
                 public Object executeWithContext(DirContext ctx) throws NamingException {
                     DistinguishedName ctxBaseDn = new DistinguishedName(ctx.getNameInNamespace());
                     NamingEnumeration<SearchResult> resultsEnum = ctx.search(base, filter, params, searchControls);
+                    if (logger.isDebugEnabled()) {
+                        logger.debug("Searching for entry in under DN '" + ctxBaseDn
+                                + "', base = '" + base + "', filter = '" + filter + "'");
+                    }
+
                     Set<DirContextOperations> results = new HashSet<DirContextOperations>();
                     try {
                         while (resultsEnum.hasMore()) {
-
                             SearchResult searchResult = resultsEnum.next();
                             // Work out the DN of the matched entry
-                            StringBuilder dn = new StringBuilder(searchResult.getName());
+                            DistinguishedName dn = new DistinguishedName(searchResult.getName());
 
                             if (base.length() > 0) {
-                                dn.append(",");
-                                dn.append(base);
+                                dn.prepend(new DistinguishedName(base));
                             }
 
-                            results.add(new DirContextAdapter(searchResult.getAttributes(),
-                                    new DistinguishedName(dn.toString()), ctxBaseDn));
+                            if (logger.isDebugEnabled()) {
+                                logger.debug("Found DN: " + dn);
+                            }
+                            results.add(new DirContextAdapter(searchResult.getAttributes(), dn, ctxBaseDn));
                         }
                     } catch (PartialResultException e) {
                         logger.info("Ignoring PartialResultException");

ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java

@@ -99,8 +99,9 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
         return user;
     }
 
-    private DirContextOperations bindWithDn(String userDn, String username, String password) {
+    private DirContextOperations bindWithDn(String userDnStr, String username, String password) {
         BaseLdapPathContextSource ctxSource = (BaseLdapPathContextSource) getContextSource();
+        DistinguishedName userDn = new DistinguishedName(userDnStr);
         DistinguishedName fullDn = new DistinguishedName(userDn);
         fullDn.prepend(ctxSource.getBaseLdapPath());
 
@@ -114,8 +115,7 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
 
             Attributes attrs = ctx.getAttributes(userDn, getUserAttributes());
 
-            DirContextAdapter result = new DirContextAdapter(attrs, new DistinguishedName(userDn),
-                    ctxSource.getBaseLdapPath());
+            DirContextAdapter result = new DirContextAdapter(attrs, userDn, ctxSource.getBaseLdapPath());
 
             if (ppolicy != null) {
                 result.setAttributeValue(ppolicy.getID(), ppolicy);
@@ -128,7 +128,7 @@ public class BindAuthenticator extends AbstractLdapAuthenticator {
             // unless a subclass wishes to implement more specialized behaviour.
             if ((e instanceof org.springframework.ldap.AuthenticationException)
                     || (e instanceof org.springframework.ldap.OperationNotSupportedException)) {
-                handleBindException(userDn, username, e);
+                handleBindException(userDnStr, username, e);
             } else {
                 throw e;
             }

ldap/src/test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java

@@ -18,14 +18,13 @@ package org.springframework.security.ldap.authentication;
 import static org.junit.Assert.*;
 
 import org.junit.Test;
-import org.springframework.ldap.core.DirContextAdapter;
 import org.springframework.ldap.core.DirContextOperations;
-import org.springframework.ldap.core.DistinguishedName;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.SpringSecurityMessageSource;
 import org.springframework.security.ldap.AbstractLdapIntegrationTests;
+import org.springframework.security.ldap.search.FilterBasedLdapUserSearch;
 
 /**
  * Tests for {@link BindAuthenticator}.
@@ -37,7 +36,6 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests {
 
     private BindAuthenticator authenticator;
     private Authentication bob;
-//    private Authentication ben;
 
 
     //~ Methods ========================================================================================================
@@ -46,7 +44,6 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests {
         authenticator = new BindAuthenticator(getContextSource());
         authenticator.setMessageSource(new SpringSecurityMessageSource());
         bob = new UsernamePasswordAuthenticationToken("bob", "bobspassword");
-//        ben = new UsernamePasswordAuthenticationToken("ben", "benspassword");
 
     }
 
@@ -75,11 +72,14 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests {
 
     @Test
     public void testAuthenticationWithUserSearch() throws Exception {
-        DirContextAdapter ctx = new DirContextAdapter(new DistinguishedName("uid=bob,ou=people"));
-
-        authenticator.setUserSearch(new MockUserSearch(ctx));
+        //DirContextAdapter ctx = new DirContextAdapter(new DistinguishedName("uid=bob,ou=people"));
+        authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=people", "(uid={0})", getContextSource()));
         authenticator.afterPropertiesSet();
         authenticator.authenticate(bob);
+        // SEC-1444
+        authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=people", "(cn={0})", getContextSource()));
+        authenticator.authenticate(new UsernamePasswordAuthenticationToken("mouse, jerry", "jerryspassword"));
+        authenticator.authenticate(new UsernamePasswordAuthenticationToken("slash/guy", "slashguyspassword"));
     }
 
     @Test

ldap/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java

@@ -70,13 +70,11 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapIntegrationTests
     @Test
     public void extraFilterPartToExcludeBob() throws Exception {
         FilterBasedLdapUserSearch locator = new FilterBasedLdapUserSearch("ou=people",
-                "(&(cn=*)(!(|(uid={0})(uid=rod)(uid=jerry))))", dirCtxFactory);
+                "(&(cn=*)(!(|(uid={0})(uid=rod)(uid=jerry)(uid=slashguy))))", dirCtxFactory);
 
         // Search for bob, get back ben...
         DirContextOperations ben = locator.searchForUser("bob");
         assertEquals("Ben Alex", ben.getStringAttribute("cn"));
-
-//        assertEquals("uid=ben,ou=people,"+ROOT_DN, ben.getDn());
     }
 
     @Test(expected=IncorrectResultSizeDataAccessException.class)

ldap/src/test/resources/test-server.ldif

@@ -58,6 +58,16 @@ sn: Mouse
 uid: jerry
 userPassword: jerryspassword
 
+dn: cn=slash/guy,ou=people,dc=springframework,dc=org
+objectclass: top
+objectclass: person
+objectclass: organizationalPerson
+objectclass: inetOrgPerson
+cn: slash/guy
+sn: Slash
+uid: slashguy
+userPassword: slashguyspassword
+
 dn: cn=developers,ou=groups,dc=springframework,dc=org
 objectclass: top
 objectclass: groupOfNames