Skip to content

Commit

Permalink
#102: prevention of hash update when not needed
Browse files Browse the repository at this point in the history
  • Loading branch information
firaja committed Feb 15, 2023
1 parent 748043e commit 74983bc
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/password4j/HashBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ public class HashBuilder
{
private CharSequence plainTextPassword;

private String salt;
protected String salt;

private CharSequence pepper;
protected CharSequence pepper;

@SuppressWarnings("unused")
private HashBuilder()
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/password4j/HashChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
*/
public class HashChecker
{
private String hashed;
protected String hashed;

private String salt;
protected String salt;

private CharSequence pepper;

Expand Down
25 changes: 25 additions & 0 deletions src/main/java/com/password4j/HashUpdate.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public class HashUpdate

private Hash hash;

private boolean updated;

private HashUpdate()
{
//
Expand All @@ -47,6 +49,18 @@ public HashUpdate(Hash hash)
this.hash = hash;
}

/**
* @param hash the new hash
* @param updated flag for updated hash
* @throws BadParametersException if hash is null but verified is true
* @since 1.7.0
*/
public HashUpdate(Hash hash, boolean updated)
{
this(hash);
this.updated = updated;
}

/**
* Returns the hash generated after a verification + update
* process.
Expand All @@ -73,4 +87,15 @@ public boolean isVerified()
return hash != null;
}

/**
* True if the update process changed the original hash due to changes on parameters.
* Changing the algorithms always set this flag to true.
*
* @return true if the hash was updated
* @since 1.7.0
*/
public boolean isUpdated()
{
return updated;
}
}
34 changes: 32 additions & 2 deletions src/main/java/com/password4j/HashUpdater.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public class HashUpdater

protected HashBuilder hashBuilder;

private boolean force;

HashUpdater(HashChecker hashChecker, HashBuilder hashBuilder)
{
this.hashChecker = hashChecker;
Expand Down Expand Up @@ -55,10 +57,25 @@ public HashUpdate with(HashingFunction oldHashingFunction, HashingFunction newHa
{
throw new BadParametersException("New hashing function cannot be null.");
}

boolean toBeUpdated = force || //
!oldHashingFunction.equals(newHashingFunction) //
|| hashBuilder.salt != null //
|| hashBuilder.pepper != null;

if (hashChecker.with(oldHashingFunction))
{
Hash hash = this.hashBuilder.with(newHashingFunction);
return new HashUpdate(hash);
if (toBeUpdated)
{
Hash hash = this.hashBuilder.with(newHashingFunction);
return new HashUpdate(hash, true);
}
else
{
Hash hash = new Hash(oldHashingFunction, hashChecker.hashed, null, hashChecker.salt);
return new HashUpdate(hash, false);
}

}
else
{
Expand Down Expand Up @@ -137,6 +154,19 @@ public HashUpdater addNewPepper(CharSequence pepper)
return this;
}

/**
* Forces the update of the hash even if function, salt and pepper did not change.
* This can be helpful with algorithm like Bcrypt that generates their own salt.
*
* @return this builder
* @since 1.7.0
*/
public HashUpdater forceUpdate()
{
this.force = true;
return this;
}

/**
* Hashes the previously given plain text password
* with {@link PBKDF2Function}.
Expand Down
55 changes: 47 additions & 8 deletions src/test/com/password4j/PasswordTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
*/
package com.password4j;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.security.Provider;
Expand All @@ -33,6 +30,8 @@
import com.password4j.types.Bcrypt;
import com.password4j.types.Hmac;

import static org.junit.Assert.*;


public class PasswordTest
{
Expand Down Expand Up @@ -596,7 +595,7 @@ public void testHashChecker()
boolean result = hc.with(AlgorithmFinder.getPBKDF2Instance());

// THEN
Assert.assertFalse(result);
assertFalse(result);
}

@Test
Expand Down Expand Up @@ -642,7 +641,7 @@ public void testGenericUpdate1()
Assert.assertEquals(pepper, hash.getPepper());
Assert.assertEquals(prefix + salt, update.getHash().getSalt());
Assert.assertEquals(prefix + salt, update.getHash().getSalt());

Assert.assertTrue(update.isUpdated());
}

@Test
Expand All @@ -658,7 +657,7 @@ public void testGenericUpdate2()
assertTrue(update.isVerified());
Assert.assertEquals(hash.getSalt(), update.getHash().getSalt());
Assert.assertEquals(hash.getPepper(), update.getHash().getPepper());

Assert.assertTrue(update.isUpdated());
}


Expand All @@ -672,9 +671,10 @@ public void testGenericUpdate3()
HashUpdate update = Password.check(password, "hash").addSalt("salt")
.andUpdate().addNewSalt(hash.getSalt()).withPBKDF2();

Assert.assertFalse(update.isVerified());
assertFalse(update.isVerified());
Assert.assertNotNull(update);
Assert.assertNull(update.getHash());
assertFalse(update.isUpdated());
}

@Test
Expand All @@ -687,9 +687,10 @@ public void testGenericUpdate4()

HashUpdate update = Password.check(password, "hash")
.andUpdate().addNewSalt(hash.getSalt()).withPBKDF2();
Assert.assertFalse(update.isVerified());
assertFalse(update.isVerified());
Assert.assertNotNull(update);
Assert.assertNull(update.getHash());
assertFalse(update.isUpdated());
} catch (Exception ex) {
assertTrue(ex instanceof BadParametersException);
}
Expand All @@ -716,7 +717,45 @@ public void testGenericUpdate5()
assertTrue(updateSalt.getHash().getPepper() == null && updateFixedSalt.getHash().getPepper() == null);
assertTrue(updateSalt.getHash().getSalt() != null && updateFixedSalt.getHash().getSalt() != null && updateFixedSaltPepper.getHash().getSalt() != null);
Assert.assertEquals(PropertyReader.readString("global.pepper", null, null), updateFixedSaltPepper.getHash().getPepper());
Assert.assertTrue(updateSalt.isUpdated());
Assert.assertTrue(updateFixedSalt.isUpdated());
Assert.assertTrue(updateFixedSaltPepper.isUpdated());
}


@Test
public void testGenericUpdate6()
{
String password = "password";

HashingFunction oldFunction = ScryptFunction.getInstance(1024, 4, 2);
Hash hash = Password.hash(password).with(oldFunction);

HashUpdate notUpdated1 = Password.check(password, hash.getResult())
.andUpdate().with(oldFunction, oldFunction);

HashUpdate updated1 = Password.check(password, hash.getResult())
.andUpdate().addNewRandomSalt().with(oldFunction, oldFunction);

HashUpdate updated2 = Password.check(password, hash.getResult())
.andUpdate().addNewPepper("pepper").with(oldFunction, oldFunction);

HashUpdate updated3 = Password.check(password, hash.getResult())
.andUpdate().with(oldFunction, BcryptFunction.getInstance(5));

HashUpdate updated4 = Password.check(password, hash.getResult())
.andUpdate().with(oldFunction, ScryptFunction.getInstance(512, 4, 2));

HashUpdate updated5 = Password.check(password, hash.getResult())
.andUpdate().forceUpdate().with(oldFunction, oldFunction);


assertFalse(notUpdated1.isUpdated());
assertTrue(updated1.isUpdated());
assertTrue(updated2.isUpdated());
assertTrue(updated3.isUpdated());
assertTrue(updated4.isUpdated());
assertTrue(updated5.isUpdated());
}

@Test
Expand Down

0 comments on commit 74983bc

Please sign in to comment.