Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue 24 (https://github.com/Password4j/password4j/issues/24) #25

Merged
merged 3 commits into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ boolean verification = Password.check(password, hash).addSalt(salt).addPepper(pe
boolean verification = Password.check(password, hash).withArgon2();
```

Some algorithms encode into the hash the parameters that were used to compute that hash, notably BCrypt, SCrypt, and Argon2.
When checking a hash, you can use the parameters from the hash rather than Password4j's configured defaults.
```java
// Verify with Argon2, reads the salt and parameters from the given hash.
boolean verification = Password.check(password, hash)..with(Argon2Function.getInstanceFromHash(hash)));
```

## Update the hash
When a configuration is not considered anymore secure you can
Expand Down
8 changes: 7 additions & 1 deletion src/main/java/com/password4j/AlgorithmFinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ public static BCryptFunction getBCryptInstance()
* <td>hash.scrypt.parallelization</td>
* <td>1</td>
* </tr>
* <tr>
* <td>Derived Key Length (dkLen)</td>
* <td>hash.scrypt.derivedKeyLength</td>
* <td>64</td>
* </tr>
* </table>
*
* @return a {@link SCryptFunction}
Expand All @@ -250,7 +255,8 @@ public static SCryptFunction getSCryptInstance()
int workFactor = PropertyReader.readInt("hash.scrypt.workfactor", 32_768, "SCrypt work factor (N) is not defined");
int resources = PropertyReader.readInt("hash.scrypt.resources", 8, "SCrypt resources (r) is not defined");
int parallelization = PropertyReader.readInt("hash.scrypt.parallelization", 1, "SCrypt parallelization (p) is not defined");
return SCryptFunction.getInstance(workFactor, resources, parallelization);
int derivedKeyLength = PropertyReader.readInt("hash.scrypt.derivedKeyLength", SCryptFunction.DERIVED_KEY_LENGTH, "SCrypt derivedKeyLength (dkLen) is not defined");
return SCryptFunction.getInstance(workFactor, resources, parallelization, derivedKeyLength);
}

public static MessageDigestFunction getMessageDigestInstance()
Expand Down
72 changes: 49 additions & 23 deletions src/main/java/com/password4j/SCryptFunction.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@
*/
public class SCryptFunction extends AbstractHashingFunction
{
public static final int DERIVED_KEY_LENGTH = 64;

private int workFactor; // N

private int resources; // r

private int parallelization; // p

private int derivedKeyLength; // dkLen

private static final ConcurrentMap<String, SCryptFunction> INSTANCES = new ConcurrentHashMap<>();

@SuppressWarnings("unused")
Expand All @@ -58,6 +61,21 @@ protected SCryptFunction(int workFactor, int resources, int parallelization)
this.resources = resources;
this.workFactor = workFactor;
this.parallelization = parallelization;
this.derivedKeyLength = DERIVED_KEY_LENGTH;
}

/**
* @param workFactor (N)
* @param resources (r)
* @param parallelization (p)
* @param derivedKeyLength (dkLen)
*/
protected SCryptFunction(int workFactor, int resources, int parallelization, int derivedKeyLength)
{
this.resources = resources;
this.workFactor = workFactor;
this.parallelization = parallelization;
this.derivedKeyLength = derivedKeyLength;
}

/**
Expand All @@ -77,8 +95,9 @@ public static SCryptFunction getInstanceFromHash(String hashed)
int workFactor = (int) Math.pow(2.0D, (double) (params >> 16 & 65535L));
int resources = (int) params >> 8 & 255;
int parallelization = (int) params & 255;
int derivedKeyLength = Base64.getDecoder().decode(parts[4]).length;

return SCryptFunction.getInstance(workFactor, resources, parallelization);
return SCryptFunction.getInstance(workFactor, resources, parallelization, derivedKeyLength);
}
throw new BadParametersException("`" + hashed + "` is not a valid hash");
}
Expand All @@ -95,14 +114,30 @@ public static SCryptFunction getInstanceFromHash(String hashed)
*/
public static SCryptFunction getInstance(int workFactor, int resources, int parallelization)
{
String key = getUID(resources, workFactor, parallelization);
return getInstance(workFactor, resources, parallelization, DERIVED_KEY_LENGTH);
}

/**
* Creates a singleton instance, depending on the provided
* N, r and p parameters.
*
* @param workFactor work factor (N)
* @param resources resources (r)
* @param parallelization parallelization (p)
* @param derivedKeyLength derived key length (dkLen)
* @return a singleton instance
* @since 0.3.0
*/
public static SCryptFunction getInstance(int workFactor, int resources, int parallelization, int derivedKeyLength)
{
String key = getUID(resources, workFactor, parallelization, derivedKeyLength);
if (INSTANCES.containsKey(key))
{
return INSTANCES.get(key);
}
else
{
SCryptFunction function = new SCryptFunction(workFactor, resources, parallelization);
SCryptFunction function = new SCryptFunction(workFactor, resources, parallelization, derivedKeyLength);
INSTANCES.put(key, function);
return function;
}
Expand All @@ -121,7 +156,7 @@ public Hash hash(CharSequence plainTextPassword, String salt)
try
{
byte[] saltAsBytes = Utils.fromCharSequenceToBytes(salt);
byte[] derived = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), saltAsBytes, 64);
byte[] derived = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), saltAsBytes, derivedKeyLength);
String params = Long.toString((long) Utils.log2(workFactor) << 16 | (long) resources << 8 | parallelization, 16);
String sb = "$s0$" + params + '$' + Base64.getEncoder().encodeToString(saltAsBytes) + '$' + Base64.getEncoder()
.encodeToString(derived);
Expand All @@ -144,22 +179,8 @@ public boolean check(CharSequence plainTextPassword, String hashed)
{
byte[] salt = Base64.getDecoder().decode(parts[3]);
byte[] derived0 = Base64.getDecoder().decode(parts[4]);
byte[] derived1 = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), salt, 64);
if (derived0.length != derived1.length)
{
return false;
}
else
{
int result = 0;

for (int i = 0; i < derived0.length; ++i)
{
result |= derived0[i] ^ derived1[i];
}

return result == 0;
}
byte[] derived1 = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), salt, derivedKeyLength);
return slowEquals(derived0, derived1);
}
else
{
Expand Down Expand Up @@ -199,6 +220,11 @@ public int getParallelization()
return parallelization;
}

public int getDerivedKeyLength()
{
return derivedKeyLength;
}

/**
* A more readable version of {@link #getRequiredBytes()},
* changing the unit (B, KB, MB) so that the number has at most
Expand Down Expand Up @@ -238,7 +264,7 @@ public boolean equals(Object obj)
@Override
public String toString()
{
return getClass().getSimpleName() + '[' + getUID(this.resources, this.workFactor, this.parallelization) + ']';
return getClass().getSimpleName() + '[' + getUID(this.resources, this.workFactor, this.parallelization, this.derivedKeyLength) + ']';
}

@Override
Expand All @@ -247,9 +273,9 @@ public int hashCode()
return Objects.hash(resources, workFactor, parallelization);
}

protected static String getUID(int resources, int workFactor, int parallelization)
protected static String getUID(int resources, int workFactor, int parallelization, int derivedKeyLength)
{
return workFactor + "|" + resources + "|" + parallelization;
return workFactor + "|" + resources + "|" + parallelization + "|" + derivedKeyLength;
}


Expand Down
24 changes: 24 additions & 0 deletions src/test/com/password4j/PasswordTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,30 @@ public void testGenericUpdate5()

}

@Test
public void testBCryptNonStandardParams()
{
final String testHash = "$2b$16$.1FczuSNl2iXHmLojhwBZO9vCfA5HIqrONkefhvn2qLQpth3r7Jwe";
Assert.assertTrue(Password.check("s$cret12", testHash).with(BCryptFunction.getInstanceFromHash(testHash)));
}

@Test
public void testSCryptNonStandardParams()
{
/*
* This password hash was generated using com.lambdaworks:scrypt, which has a derived key length (dkLen) of 32 bytes.
*/
final String testHash = "$s0$e0801$fl+gNAicpGG4gLMkUTCvLw==$N5wE1IKsr4LPBoetJVW6jLzEH4kTVXuKGafvAA8Z+88=";
Assert.assertTrue(Password.check("Hello world!", testHash).with(SCryptFunction.getInstanceFromHash(testHash)));
}

@Test
public void testArgon2NonstandardParams()
{
/*
* This password hash comes from the Argon2 C reference implementation (https://github.com/P-H-C/phc-winner-argon2).
*/
final String testHash = "$argon2i$v=19$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG";
Assert.assertTrue(Password.check("password", testHash).with(Argon2Function.getInstanceFromHash(testHash)));
}
}
6 changes: 4 additions & 2 deletions src/test/com/password4j/SCryptFunctionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,17 @@ public void testAccessors()
int workFactor = 3;
int resources = 5;
int parallelization = 7;
int derivedKeyLength = 32;

// WHEN
SCryptFunction scrypt = SCryptFunction.getInstance(workFactor, resources,parallelization);
SCryptFunction scrypt = SCryptFunction.getInstance(workFactor, resources, parallelization, derivedKeyLength);

// THEN
Assert.assertEquals(workFactor, scrypt.getWorkFactor());
Assert.assertEquals(resources, scrypt.getResources());
Assert.assertEquals(parallelization, scrypt.getParallelization());
Assert.assertEquals("SCryptFunction[3|5|7]", scrypt.toString());
Assert.assertEquals(derivedKeyLength, scrypt.getDerivedKeyLength());
Assert.assertEquals("SCryptFunction[3|5|7|32]", scrypt.toString());
}

}