From 0d198d42aea1ecf643fdf3b39ddb30c8bd0b8e4f Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Sat, 27 Mar 2010 15:30:15 +0000 Subject: [PATCH] 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/SpringSecurityLdapTemplate.java | 17 +++++++++++------ .../ldap/authentication/BindAuthenticator.java | 8 ++++---- .../authentication/BindAuthenticatorTests.java | 14 +++++++------- .../search/FilterBasedLdapUserSearchTests.java | 4 +--- ldap/src/test/resources/test-server.ldif | 10 ++++++++++ 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java b/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java index 48d3b201e5..d1ca6b4795 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java +++ b/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 resultsEnum = ctx.search(base, filter, params, searchControls); + if (logger.isDebugEnabled()) { + logger.debug("Searching for entry in under DN '" + ctxBaseDn + + "', base = '" + base + "', filter = '" + filter + "'"); + } + Set results = new HashSet(); 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"); diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java index 7fe6feca8b..662023cf81 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/BindAuthenticator.java +++ b/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; } diff --git a/ldap/src/test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java b/ldap/src/test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java index 72d82683d3..d493fd8d2d 100644 --- a/ldap/src/test/java/org/springframework/security/ldap/authentication/BindAuthenticatorTests.java +++ b/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 diff --git a/ldap/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java b/ldap/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java index 5a1a072038..c5ecfcea9d 100644 --- a/ldap/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java +++ b/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) diff --git a/ldap/src/test/resources/test-server.ldif b/ldap/src/test/resources/test-server.ldif index 599c615c4f..1124c05077 100644 --- a/ldap/src/test/resources/test-server.ldif +++ b/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