On Jul 25, 2015 9:26 AM, "Kiran Ayyagari" <[email protected]> wrote:
>
>
> Lucas,
>
> On Sat, Jul 25, 2015 at 2:11 AM, <[email protected]> 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
