Fix issue 24 (Password4j#24)

When checking scrypt hash, respect the input hash's derived key length When checking bcrypt, scrypt, argon2, and compressed pkdf2 hashes, accept "non-standard" parameters
dpatriarche · Feb 4, 2021 · b0688ec · b0688ec
1 parent 423aff9
commit b0688ec
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 19 deletions.
diff --git a/src/main/java/com/password4j/HashChecker.java b/src/main/java/com/password4j/HashChecker.java
@@ -158,7 +158,7 @@ public boolean withPBKDF2()
      */
     public boolean withCompressedPBKDF2()
     {
-        PBKDF2Function pbkdf2 = AlgorithmFinder.getCompressedPBKDF2Instance();
+        PBKDF2Function pbkdf2 = CompressedPBKDF2Function.getInstanceFromHash(hashed);
         return with(pbkdf2);
     }
 
@@ -175,7 +175,7 @@ public boolean withCompressedPBKDF2()
      */
     public boolean withSCrypt()
     {
-        SCryptFunction scrypt = AlgorithmFinder.getSCryptInstance();
+        SCryptFunction scrypt = SCryptFunction.getInstanceFromHash(hashed);
         return with(scrypt);
     }
 
@@ -192,7 +192,7 @@ public boolean withSCrypt()
      */
     public boolean withBCrypt()
     {
-        return with(AlgorithmFinder.getBCryptInstance());
+        return with(BCryptFunction.getInstanceFromHash(hashed));
     }
 
     /**
@@ -224,7 +224,7 @@ public boolean withMessageDigest()
      */
     public boolean withArgon2()
     {
-        Argon2Function argon2 = AlgorithmFinder.getArgon2Instance();
+        Argon2Function argon2 = Argon2Function.getInstanceFromHash(hashed);
         return with(argon2);
     }
 

diff --git a/src/main/java/com/password4j/SCryptFunction.java b/src/main/java/com/password4j/SCryptFunction.java
@@ -34,6 +34,7 @@
  */
 public class SCryptFunction extends AbstractHashingFunction
 {
+    private static final int DERIVED_KEY_LENGTH = 64;
 
     private int workFactor; // N
 
@@ -122,7 +123,7 @@ public Hash hash(CharSequence plainTextPassword, String salt)
         try
         {
             byte[] saltAsBytes = salt.getBytes(StandardCharsets.UTF_8);
-            byte[] derived = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), saltAsBytes, 64);
+            byte[] derived = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), saltAsBytes, DERIVED_KEY_LENGTH);
             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);
@@ -145,22 +146,12 @@ 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];
-                    }
+                // Calculate the derived key using the key length from the input hash, whose key length may be different
+                // than this implementation's default DERIVED_KEY_LENGTH.
+                byte[] derived1 = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), salt, derived0.length);
 
-                    return result == 0;
-                }
+                return slowEquals(derived0, derived1);
             }
             else
             {

diff --git a/src/test/com/password4j/PasswordTest.java b/src/test/com/password4j/PasswordTest.java
@@ -704,6 +704,30 @@ public void testGenericUpdate5()
 
     }
 
+    @Test
+    public void testBCryptNonStandardParams()
+    {
+        Assert.assertTrue(Password.check("s$cret12",
+            "$2b$16$.1FczuSNl2iXHmLojhwBZO9vCfA5HIqrONkefhvn2qLQpth3r7Jwe").withBCrypt());
+    }
 
+    @Test
+    public void testSCryptNonStandardParams()
+    {
+        /*
+         * This password hash was generated using com.lambdaworks:scrypt, which has a derived key length (dkLen) of 32 bytes.
+         */
+        Assert.assertTrue(Password.check("Hello world!",
+            "$s0$e0801$fl+gNAicpGG4gLMkUTCvLw==$N5wE1IKsr4LPBoetJVW6jLzEH4kTVXuKGafvAA8Z+88=").withSCrypt());
+    }
 
+    @Test
+    public void testArgon2NonstandardParams()
+    {
+        /*
+         * This password hash comes from the Argon2 C reference implementation (https://github.com/P-H-C/phc-winner-argon2).
+         */
+        Assert.assertTrue(Password.check("password",
+            "$argon2i$v=19$m=65536,t=2,p=4$c29tZXNhbHQ$RdescudvJCsgt3ub+b+dWRWJTmaaJObG").withArgon2());
+    }
 }