This is an automated email from the ASF dual-hosted git repository. martin_s pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/archiva-redback-core.git
commit ef075525c85be644ed4fc8ff712b14c85ed14ab7 Author: Martin Stockhammer <[email protected]> AuthorDate: Sun May 10 11:02:54 2020 +0200 Improving LDAP filters by escaping characters --- .../authentication/ldap/LdapBindAuthenticator.java | 3 +- .../ldap/LdapBindAuthenticatorTest.java | 11 +++++ .../archiva/redback/common/ldap/LdapUtils.java | 56 ++++++++++++++++++++++ .../archiva/redback/users/ldap/LdapUserQuery.java | 7 +-- .../redback/users/ldap/LdapUserManagerTest.java | 15 ++++++ 5 files changed, 88 insertions(+), 4 deletions(-) diff --git a/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/main/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticator.java b/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/main/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticator.java index 9c33ffd..3c46c78 100644 --- a/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/main/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticator.java +++ b/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/main/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticator.java @@ -20,6 +20,7 @@ package org.apache.archiva.redback.authentication.ldap; */ import org.apache.archiva.redback.authentication.AbstractAuthenticator; +import org.apache.archiva.redback.common.ldap.LdapUtils; import org.apache.archiva.redback.common.ldap.connection.DefaultLdapConnection; import org.apache.archiva.redback.common.ldap.connection.LdapConnection; import org.apache.archiva.redback.common.ldap.user.UserMapper; @@ -100,7 +101,7 @@ public class LdapBindAuthenticator String filter = "(&(objectClass=" + mapper.getUserObjectClass() + ")" + ( mapper.getUserFilter() != null ? mapper.getUserFilter() - : "" ) + "(" + mapper.getUserIdAttribute() + "=" + source.getUsername() + "))"; + : "" ) + "(" + mapper.getUserIdAttribute() + "=" + LdapUtils.encodeFilterValue( source.getUsername() ) + "))"; log.debug( "Searching for users with filter: '{}' from base dn: {}", filter, mapper.getUserBaseDn() ); diff --git a/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/test/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticatorTest.java b/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/test/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticatorTest.java index 84ee8be..e668f84 100644 --- a/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/test/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticatorTest.java +++ b/redback-authentication/redback-authentication-providers/redback-authentication-ldap/src/test/java/org/apache/archiva/redback/authentication/ldap/LdapBindAuthenticatorTest.java @@ -137,6 +137,17 @@ public class LdapBindAuthenticatorTest assertTrue( result.isAuthenticated() ); } + @Test + public void testAuthenticationWithInvalidChar() + throws Exception + { + PasswordBasedAuthenticationDataSource authDs = new PasswordBasedAuthenticationDataSource(); + authDs.setPrincipal( "jesse)(mail=foo" ); + authDs.setPassword( passwordEncoder.encodePassword( "foo" ) ); + AuthenticationResult result = authnr.authenticate( authDs ); + assertFalse( result.isAuthenticated() ); + } + // REDBACK-289/MRM-1488 @Test public void testAuthenticationFromCache() diff --git a/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java b/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java index 529a2d9..b1c45cb 100644 --- a/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java +++ b/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java @@ -34,6 +34,28 @@ import javax.naming.ldap.Rdn; */ public final class LdapUtils { + + private static String[] FILTER_ESCAPE_TABLE = new String['\\' + 1]; + + + // Characters that must be escaped in a user filter + static { + + // Filter encoding table ------------------------------------- + // fill with char itself + for (char c = 0; c < FILTER_ESCAPE_TABLE.length; c++) { + FILTER_ESCAPE_TABLE[c] = String.valueOf(c); + } + + // escapes (RFC2254) + FILTER_ESCAPE_TABLE['*'] = "\\2a"; + FILTER_ESCAPE_TABLE['('] = "\\28"; + FILTER_ESCAPE_TABLE[')'] = "\\29"; + FILTER_ESCAPE_TABLE['\\'] = "\\5c"; + FILTER_ESCAPE_TABLE[0] = "\\00"; + } + + private LdapUtils() { // no op @@ -172,4 +194,38 @@ public final class LdapUtils } return ""; } + + /** + * Escape a value for use in a filter. + * This method is copied from the spring framework class org.springframework.security.ldap.authentication.LdapEncoder + * + * @param value the value to escape. + * @return a properly escaped representation of the supplied value. + */ + public static String encodeFilterValue(String value) { + + if (value == null) { + return null; + } + + // make buffer roomy + StringBuilder encodedValue = new StringBuilder(value.length() * 2); + + int length = value.length(); + + for (int i = 0; i < length; i++) { + + char c = value.charAt(i); + + if (c < FILTER_ESCAPE_TABLE.length) { + encodedValue.append(FILTER_ESCAPE_TABLE[c]); + } + else { + // default: add the char + encodedValue.append(c); + } + } + + return encodedValue.toString(); + } } diff --git a/redback-users/redback-users-providers/redback-users-ldap/src/main/java/org/apache/archiva/redback/users/ldap/LdapUserQuery.java b/redback-users/redback-users-providers/redback-users-ldap/src/main/java/org/apache/archiva/redback/users/ldap/LdapUserQuery.java index 6f09794..d20063a 100644 --- a/redback-users/redback-users-providers/redback-users-ldap/src/main/java/org/apache/archiva/redback/users/ldap/LdapUserQuery.java +++ b/redback-users/redback-users-providers/redback-users-ldap/src/main/java/org/apache/archiva/redback/users/ldap/LdapUserQuery.java @@ -19,6 +19,7 @@ package org.apache.archiva.redback.users.ldap; * under the License. */ +import org.apache.archiva.redback.common.ldap.LdapUtils; import org.apache.archiva.redback.common.ldap.user.UserMapper; import org.apache.archiva.redback.users.AbstractUserQuery; @@ -49,13 +50,13 @@ public class LdapUserQuery String filter = ""; if (this.getEmail() != null ) { - filter += "(" + mapper.getEmailAddressAttribute() + "=" + this.getEmail() + ")"; + filter += "(" + mapper.getEmailAddressAttribute() + "=" + LdapUtils.encodeFilterValue( this.getEmail() ) + ")"; } if ( this.getFullName() != null ) { - filter += "(" + mapper.getUserFullNameAttribute() + "=" + this.getFullName() + ")"; + filter += "(" + mapper.getUserFullNameAttribute() + "=" + LdapUtils.encodeFilterValue( this.getFullName() ) + ")"; } - filter += "(" + mapper.getUserIdAttribute() + "=" + ( this.getUsername() != null ? this.getUsername() : "*" ) + ")"; + filter += "(" + mapper.getUserIdAttribute() + "=" + ( this.getUsername() != null ? LdapUtils.encodeFilterValue( this.getUsername() ) : "*" ) + ")"; return filter; } diff --git a/redback-users/redback-users-providers/redback-users-ldap/src/test/java/org/apache/archiva/redback/users/ldap/LdapUserManagerTest.java b/redback-users/redback-users-providers/redback-users-ldap/src/test/java/org/apache/archiva/redback/users/ldap/LdapUserManagerTest.java index 88ea922..ca06118 100644 --- a/redback-users/redback-users-providers/redback-users-ldap/src/test/java/org/apache/archiva/redback/users/ldap/LdapUserManagerTest.java +++ b/redback-users/redback-users-providers/redback-users-ldap/src/test/java/org/apache/archiva/redback/users/ldap/LdapUserManagerTest.java @@ -234,6 +234,21 @@ public class LdapUserManagerTest } @Test + public void testUserWithInvalidChars() + throws Exception + { + try + { + userManager.findUser( "jesse)([email protected]" ); + fail( "UserNotFoundException should be thrown, if invalid filter chars are in the username" ); + } + catch ( UserNotFoundException e ) + { + // cool it works ! + } + } + + @Test public void testWithManyUsers() throws Exception {
