From 077af5e187cd916814335e46f9309c25c70d3925 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 26 Jan 2011 17:13:11 +0000 Subject: [PATCH] SEC-1661: Use a DistinguishedName to wrap the search base to avoid the need for JNDI escaping. --- ldap/openldaptest.ldif | 15 ++++++++++ ldap/run_slapd.sh | 4 +-- ldap/slapd.conf | 10 +++---- .../ldap/SpringSecurityLdapTemplate.java | 12 ++++---- .../ldap/AbstractLdapIntegrationTests.java | 10 +++++-- .../BindAuthenticatorTests.java | 30 +++++++++++++++++-- ldap/src/test/resources/test-server.ldif | 16 ++++++++++ 7 files changed, 79 insertions(+), 18 deletions(-) diff --git a/ldap/openldaptest.ldif b/ldap/openldaptest.ldif index 78f0650c167..bfc61c6803f 100755 --- a/ldap/openldaptest.ldif +++ b/ldap/openldaptest.ldif @@ -8,6 +8,21 @@ objectClass: organizationalUnit objectClass: top ou: users +dn: ou=\"quoted people\",dc=springsource,dc=com +objectclass: top +objectclass: organizationalUnit +ou: "quoted people" + +dn: cn=quoteguy,ou=\"quoted people\",dc=springsource,dc=com +objectclass: top +objectclass: person +objectclass: organizationalPerson +objectclass: inetOrgPerson +cn: quoteguy +sn: Quote +uid: quoteguy +userPassword: quoteguyspassword + dn: uid=luke,ou=users,dc=springsource,dc=com objectClass: person objectClass: organizationalPerson diff --git a/ldap/run_slapd.sh b/ldap/run_slapd.sh index 5ea13b9ff79..c8284b366f3 100755 --- a/ldap/run_slapd.sh +++ b/ldap/run_slapd.sh @@ -1,7 +1,7 @@ #! /bin/sh -rm -Rf target/openldap -mkdir -p target/openldap +rm -Rf build/openldap +mkdir -p build/openldap /opt/local/libexec/slapd -h ldap://localhost:22389 -d -1 -f slapd.conf & sleep 2 ldapadd -h localhost -p 22389 -D cn=admin,dc=springsource,dc=com -w password -x -f openldaptest.ldif \ No newline at end of file diff --git a/ldap/slapd.conf b/ldap/slapd.conf index e78aff339bd..d52cba0e5a2 100755 --- a/ldap/slapd.conf +++ b/ldap/slapd.conf @@ -4,8 +4,8 @@ include /opt/local/etc/openldap/schema/inetorgperson.schema include /opt/local/etc/openldap/schema/ppolicy.schema -pidfile ./target/slapd.pid -argsfile ./target/slapd.args +pidfile ./build/slapd.pid +argsfile ./build/slapd.args # Load dynamic backend modules: modulepath /usr/lib/openldap/modules @@ -14,7 +14,7 @@ modulepath /usr/lib/openldap/modules # moduleload back_monitor.la # moduleload back_perl.la -disallow bind_anon +#disallow bind_anon require authc access to dn.base="" @@ -28,7 +28,7 @@ rootdn "cn=admin,dc=springsource,dc=com" rootpw password -directory ./target/openldap +directory ./build/openldap index uid eq index cn eq @@ -49,5 +49,3 @@ ppolicy_default "cn=default,ou=policies,dc=springsource,dc=com" ppolicy_use_lockout ppolicy_hash_cleartext - - 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 384de306792..1e91e941093 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java +++ b/ldap/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java @@ -194,11 +194,13 @@ public DirContextOperations searchForSingleEntry(final String base, final String return (DirContextOperations) executeReadOnly(new ContextExecutor() { public Object executeWithContext(DirContext ctx) throws NamingException { - DistinguishedName ctxBaseDn = new DistinguishedName(ctx.getNameInNamespace()); - NamingEnumeration resultsEnum = ctx.search(base, filter, params, searchControls); + final DistinguishedName ctxBaseDn = new DistinguishedName(ctx.getNameInNamespace()); + final DistinguishedName searchBaseDn = new DistinguishedName(base); + final NamingEnumeration resultsEnum = ctx.search(searchBaseDn, filter, params, searchControls); + if (logger.isDebugEnabled()) { - logger.debug("Searching for entry in under DN '" + ctxBaseDn - + "', base = '" + base + "', filter = '" + filter + "'"); + logger.debug("Searching for entry under DN '" + ctxBaseDn + + "', base = '" + searchBaseDn + "', filter = '" + filter + "'"); } Set results = new HashSet(); @@ -209,7 +211,7 @@ public Object executeWithContext(DirContext ctx) throws NamingException { DistinguishedName dn = new DistinguishedName(searchResult.getName()); if (base.length() > 0) { - dn.prepend(new DistinguishedName(base)); + dn.prepend(searchBaseDn); } if (logger.isDebugEnabled()) { diff --git a/ldap/src/test/java/org/springframework/security/ldap/AbstractLdapIntegrationTests.java b/ldap/src/test/java/org/springframework/security/ldap/AbstractLdapIntegrationTests.java index cc72012c16a..ceaf823ce5a 100644 --- a/ldap/src/test/java/org/springframework/security/ldap/AbstractLdapIntegrationTests.java +++ b/ldap/src/test/java/org/springframework/security/ldap/AbstractLdapIntegrationTests.java @@ -40,7 +40,7 @@ public abstract class AbstractLdapIntegrationTests { // private static InMemoryXmlApplicationContext appContext; private static ApacheDSContainer server; - private static BaseLdapPathContextSource contextSource; + private static DefaultSpringSecurityContextSource contextSource; protected AbstractLdapIntegrationTests() { } @@ -48,7 +48,11 @@ protected AbstractLdapIntegrationTests() { @BeforeClass public static void startServer() throws Exception { contextSource = new DefaultSpringSecurityContextSource("ldap://127.0.0.1:53389/dc=springframework,dc=org"); - ((DefaultSpringSecurityContextSource)contextSource).afterPropertiesSet(); +// OpenLDAP option +// contextSource = new DefaultSpringSecurityContextSource("ldap://127.0.0.1:22389/dc=springsource,dc=com"); +// contextSource.setUserDn("cn=admin,dc=springsource,dc=com"); +// contextSource.setPassword("password"); + contextSource.afterPropertiesSet(); server = new ApacheDSContainer("dc=springframework,dc=org", "classpath:test-server.ldif"); server.afterPropertiesSet(); } @@ -98,7 +102,7 @@ private void clearSubContexts(DirContext ctx, Name name) throws NamingException try { enumeration = ctx.listBindings(name); while (enumeration.hasMore()) { - Binding element = (Binding) enumeration.next(); + Binding element = enumeration.next(); DistinguishedName childName = new DistinguishedName(element.getName()); childName.prepend((DistinguishedName) name); 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 d493fd8d2df..ce68f550018 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 @@ -17,7 +17,7 @@ import static org.junit.Assert.*; -import org.junit.Test; +import org.junit.*; import org.springframework.ldap.core.DirContextOperations; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; @@ -80,8 +80,34 @@ public void testAuthenticationWithUserSearch() throws Exception { authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=people", "(cn={0})", getContextSource())); authenticator.authenticate(new UsernamePasswordAuthenticationToken("mouse, jerry", "jerryspassword")); authenticator.authenticate(new UsernamePasswordAuthenticationToken("slash/guy", "slashguyspassword")); + // SEC-1661 + authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=\\\"quoted people\\\"", "(cn={0})", getContextSource())); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("quoteguy", "quoteguyspassword")); } - +/* + @Test + public void messingWithEscapedChars() throws Exception { + Hashtable env = new Hashtable(); + env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); + env.put(Context.PROVIDER_URL, "ldap://127.0.0.1:22389/dc=springsource,dc=com"); + env.put(Context.SECURITY_AUTHENTICATION, "simple"); + env.put(Context.SECURITY_PRINCIPAL, "cn=admin,dc=springsource,dc=com"); + env.put(Context.SECURITY_CREDENTIALS, "password"); + + InitialDirContext idc = new InitialDirContext(env); + SearchControls searchControls = new SearchControls(); + searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE); + DistinguishedName baseDn = new DistinguishedName("ou=\\\"quoted people\\\""); + NamingEnumeration matches = idc.search(baseDn, "(cn=*)", new Object[] {"quoteguy"}, searchControls); + + while(matches.hasMore()) { + SearchResult match = matches.next(); + DistinguishedName dn = new DistinguishedName(match.getName()); + System.out.println("**** Match: " + match.getName() + " ***** " + dn); + + } + } +*/ @Test public void testAuthenticationWithWrongPasswordFails() { authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"}); diff --git a/ldap/src/test/resources/test-server.ldif b/ldap/src/test/resources/test-server.ldif index 1124c050777..0e72399391d 100644 --- a/ldap/src/test/resources/test-server.ldif +++ b/ldap/src/test/resources/test-server.ldif @@ -13,6 +13,11 @@ objectclass: top objectclass: organizationalUnit ou: people +dn: ou=\"quoted people\",dc=springframework,dc=org +objectclass: top +objectclass: organizationalUnit +ou: "quoted people" + dn: ou=otherpeople,dc=springframework,dc=org objectclass: top objectclass: organizationalUnit @@ -68,6 +73,17 @@ sn: Slash uid: slashguy userPassword: slashguyspassword +dn: cn=quoteguy,ou=\"quoted people\",dc=springframework,dc=org +objectclass: top +objectclass: person +objectclass: organizationalPerson +objectclass: inetOrgPerson +cn: quoteguy +sn: Quote +uid: quoteguy +userPassword: quoteguyspassword + + dn: cn=developers,ou=groups,dc=springframework,dc=org objectclass: top objectclass: groupOfNames