Commit 6edbf02a authored by Praveena G's avatar Praveena G Committed by GitHub
Browse files

Merge pull request #10168 from OliviaYtterbrink/3.0-inaccessible-credentials-arithmetic

Handle INACCESSIBLE credentials
parents cad75813 86d5547b
Showing with 39 additions and 7 deletions
+39 -7
......@@ -63,10 +63,10 @@ public class Credential
return byteEquals( passwordHash, hash( salt, password ) );
}
/**
* <p>Utility method that replaces Arrays.equals() to avoid timing attacks.
* The length of the loop executed will always be the length of the given password</p>
* The length of the loop executed will always be the length of the given password.
* Remember {@link #INACCESSIBLE} credentials should still execute loop for the length of given password.</p>
*
* @param actual the actual password
* @param given password given by the user
......@@ -83,15 +83,26 @@ public class Credential
return false;
}
boolean result = true;
boolean accessible = true;
int actualLength = actual.length;
int givenLength = given.length;
for ( int i = 0; i < givenLength; ++i )
{
result &= actual[i % actualLength] == given[i];
if ( actualLength == 0 )
{
accessible = false;
}
else
{
result &= actual[i % actualLength] == given[i];
}
}
return result && actualLength == givenLength;
return result && actualLength == givenLength && accessible;
}
/**
* <p>Equality to always check for both salt and password hash as a safeguard against timing attack.</p>
*/
@Override
public boolean equals( Object o )
{
......@@ -106,7 +117,9 @@ public class Credential
Credential that = (Credential) o;
return byteEquals( this.salt, that.salt ) && byteEquals( this.passwordHash, that.passwordHash );
boolean saltEquals = byteEquals( this.salt, that.salt );
boolean passwordEquals = byteEquals( this.passwordHash, that.passwordHash );
return saltEquals && passwordEquals;
}
@Override
......
......@@ -24,6 +24,7 @@ import org.junit.Test;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.neo4j.server.security.auth.Credential.INACCESSIBLE;
public class CredentialTest
{
......@@ -44,4 +45,22 @@ public class CredentialTest
Credential sameCredential = new Credential( credential.salt(), credential.passwordHash() );
assertTrue( credential.equals( sameCredential ) );
}
@Test
public void testInaccessibleCredentials()
{
Credential credential = new Credential( INACCESSIBLE.salt(), INACCESSIBLE.passwordHash() );
//equals
assertTrue( INACCESSIBLE.equals( credential ) );
assertTrue( credential.equals( INACCESSIBLE ) );
assertTrue( INACCESSIBLE.equals( INACCESSIBLE ) );
assertFalse( INACCESSIBLE.equals( Credential.forPassword( "" ) ) );
assertFalse( Credential.forPassword( "" ).equals( INACCESSIBLE ) );
//matchesPassword
assertFalse( INACCESSIBLE.matchesPassword( new String( new byte[]{} )) );
assertFalse( INACCESSIBLE.matchesPassword( "foo" ) );
assertFalse( INACCESSIBLE.matchesPassword( "" ) );
}
}
......@@ -212,10 +212,10 @@ public class FileUserRepositoryTest
FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
User user = new User( "jake", Credential.INACCESSIBLE, true );
users.create( user );
User modifiedUser = new User( "jake", Credential.forPassword( "foo" ), false );
User modifiedUser = new User( "jake", Credential.forPassword( "foo" ), true );
// When
User updatedUser = new User( "jake", Credential.forPassword( "bar" ), false );
User updatedUser = new User( "jake", Credential.forPassword( "bar" ), true );
try
{
users.update( modifiedUser, updatedUser );
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment