directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lucas Theisen <lucasthei...@pastdev.com>
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:54:51 GMT
On Jul 25, 2015 9:26 AM, "Kiran Ayyagari" <kayyagari@apache.org> wrote:
>
>
> 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.
>

It's not the information inside of the exception that is leaking.  It's the
fact that a PasswordPolicyException is thrown rather than invalid
credentials.  This would tell a potential attacker that scans through
possible names when they have found a valid one.  They could then use that
information to cause account lockout, or other nefarious things.

In general, I don't think we should provide any information other than
invalid credentials, until the user has successfully authenticated.

>>          // 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