directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kiran Ayyagari <kayyag...@apache.org>
Subject Re: svn commit: r1692562 - in /directory/apacheds/trunk: interceptors/authn/src/main/java/org/apache/directory/server/core/authn/SimpleAuthenticator.java server-integ/src/test/java/org/apache/directory/server/ppolicy/PasswordPolicyIT.java
Date Sat, 25 Jul 2015 13:26:01 GMT
Lucas,

On Sat, Jul 25, 2015 at 2:11 AM, <lucastheisen@apache.org> wrote:

> Author: lucastheisen
> Date: Fri Jul 24 18:11:38 2015
> New Revision: 1692562
>
> URL: http://svn.apache.org/r1692562
> Log:
> fixed leak of information when password policy fails authentication attempt
>
> Modified:
>
> directory/apacheds/trunk/interceptors/authn/src/main/java/org/apache/directory/server/core/authn/SimpleAuthenticator.java
>
> directory/apacheds/trunk/server-integ/src/test/java/org/apache/directory/server/ppolicy/PasswordPolicyIT.java
>
> Modified:
> directory/apacheds/trunk/interceptors/authn/src/main/java/org/apache/directory/server/core/authn/SimpleAuthenticator.java
> URL:
> http://svn.apache.org/viewvc/directory/apacheds/trunk/interceptors/authn/src/main/java/org/apache/directory/server/core/authn/SimpleAuthenticator.java?rev=1692562&r1=1692561&r2=1692562&view=diff
>
> ==============================================================================
> ---
> directory/apacheds/trunk/interceptors/authn/src/main/java/org/apache/directory/server/core/authn/SimpleAuthenticator.java
> (original)
> +++
> directory/apacheds/trunk/interceptors/authn/src/main/java/org/apache/directory/server/core/authn/SimpleAuthenticator.java
> Fri Jul 24 18:11:38 2015
> @@ -25,8 +25,10 @@ import java.security.MessageDigest;
>  import java.security.NoSuchAlgorithmException;
>  import java.util.Arrays;
>
> +
>  import javax.naming.Context;
>
> +
>  import org.apache.commons.collections.map.LRUMap;
>  import org.apache.directory.api.ldap.model.constants.AuthenticationLevel;
>  import
> org.apache.directory.api.ldap.model.constants.LdapSecurityConstants;
> @@ -45,6 +47,7 @@ import org.apache.directory.server.core.
>  import org.apache.directory.server.core.api.InterceptorEnum;
>  import org.apache.directory.server.core.api.LdapPrincipal;
>  import
> org.apache.directory.server.core.api.authn.ppolicy.PasswordPolicyConfiguration;
> +import
> org.apache.directory.server.core.api.authn.ppolicy.PasswordPolicyException;
>  import org.apache.directory.server.core.api.entry.ClonedServerEntry;
>  import
> org.apache.directory.server.core.api.interceptor.context.BindOperationContext;
>  import
> org.apache.directory.server.core.api.interceptor.context.LookupOperationContext;
> @@ -222,11 +225,27 @@ public class SimpleAuthenticator extends
>          // Get the stored password, either from cache or from backend
>          byte[][] storedPasswords = principal.getUserPasswords();
>
> +        PasswordPolicyException ppe = null;
> +        try
> +        {
> +            checkPwdPolicy( bindContext.getEntry() );
> +        }
> +        catch ( PasswordPolicyException e )
> +        {
> +            ppe = e;
> +        }
> +
>
the information that checkPwdPolicy() throws is anyway meant to send out
through the ppolicy response
control, so there is nothing to prevent here from leaking and it is not
clear to me why you wait till comparing
the credentials to throw the exception.

         // Now, compare the passwords.
>          for ( byte[] storedPassword : storedPasswords )
>          {
>              if ( PasswordUtil.compareCredentials( credentials,
> storedPassword ) )
>              {
> +                if ( ppe != null )
> +                {
> +                    LOG.debug( "{} Authentication failed: {}",
> bindContext.getDn(), ppe.getMessage() );
> +                    throw ppe;
> +                }
> +
>                  if ( IS_DEBUG )
>                  {
>                      LOG.debug( "{} Authenticated", bindContext.getDn() );
> @@ -285,7 +304,7 @@ public class SimpleAuthenticator extends
>              throw e;
>          }
>
> -        checkPwdPolicy( userEntry );
> +        //checkPwdPolicy( userEntry );
>
>          DirectoryService directoryService = getDirectoryService();
>          String userPasswordAttribute = SchemaConstants.USER_PASSWORD_AT;
>
> Modified:
> directory/apacheds/trunk/server-integ/src/test/java/org/apache/directory/server/ppolicy/PasswordPolicyIT.java
> URL:
> http://svn.apache.org/viewvc/directory/apacheds/trunk/server-integ/src/test/java/org/apache/directory/server/ppolicy/PasswordPolicyIT.java?rev=1692562&r1=1692561&r2=1692562&view=diff
>
> ==============================================================================
> ---
> directory/apacheds/trunk/server-integ/src/test/java/org/apache/directory/server/ppolicy/PasswordPolicyIT.java
> (original)
> +++
> directory/apacheds/trunk/server-integ/src/test/java/org/apache/directory/server/ppolicy/PasswordPolicyIT.java
> Fri Jul 24 18:11:38 2015
> @@ -1131,7 +1131,7 @@ public class PasswordPolicyIT extends Ab
>          checkBind( userConnection, userDn, "badPassword", 3,
>              "INVALID_CREDENTIALS: Bind failed: ERR_229 Cannot
> authenticate user cn=userLockout,ou=system" );
>
> -        checkBind( userConnection, userDn, "badPassword", 1,
> +        checkBind( userConnection, userDn, "12345", 1,
>              "INVALID_CREDENTIALS: Bind failed: account was permanently
> locked" );
>
>          userConnection.close();
>
>
>


-- 
Kiran Ayyagari
http://keydap.com

Mime
View raw message