#143: fix salt generation for Argon2

Password4j · Mar 7, 2024 · ded2c06 · ded2c06
1 parent 33377fd
commit ded2c06
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 3 deletions.
diff --git a/src/main/java/com/password4j/Argon2Function.java b/src/main/java/com/password4j/Argon2Function.java
@@ -220,7 +220,7 @@ private Hash internalHash(byte[] plainTextPassword, byte[] salt, CharSequence pe
         initialize(plainTextPassword, salt, Utils.fromCharSequenceToBytes(pepper), null, blockMemory);
         fillMemoryBlocks(blockMemory);
         byte[] hash = ending(blockMemory);
-        Hash result = new Hash(this, encodeHash(hash, salt), hash, Utils.fromBytesToString(salt));
+        Hash result = new Hash(this, encodeHash(hash, salt), hash, salt);
         result.setPepper(pepper);
         return result;
     }

diff --git a/src/main/java/com/password4j/Hash.java b/src/main/java/com/password4j/Hash.java
@@ -120,7 +120,10 @@ private Hash()
      * @param bytes           the hash without additional information.
      * @param salt            the salt used for the computation.
      * @since 0.1.0
+     * @deprecated            As of 1.8.1 because of the salt conversion from {@link String} to byte[].
+     *                        {@link Hash#Hash(HashingFunction, String, byte[], byte[])} should be used instead.
      */
+    @Deprecated
     public Hash(HashingFunction hashingFunction, String result, byte[] bytes, String salt)
     {
         this(hashingFunction, Utils.fromCharSequenceToBytes(result), bytes, Utils.fromCharSequenceToBytes(salt));

diff --git a/src/main/java/com/password4j/ScryptFunction.java b/src/main/java/com/password4j/ScryptFunction.java
@@ -174,17 +174,18 @@ public Hash hash(byte[] plainTextPasswordAsBytes, byte[] salt)
 
     private Hash internalHash(byte[] plainTextPassword, byte[] salt)
     {
-        String stringedSalt = Utils.fromBytesToString(salt);
+
         try
         {
             byte[] derived = scrypt(plainTextPassword, salt, derivedKeyLength);
             String params = Long.toString((long) Utils.log2(workFactor) << 16 | (long) resources << 8 | parallelization, 16);
             String sb = "$" + params + '$' + Utils.encodeBase64(salt) + '$'
                     + Utils.encodeBase64(derived);
-            return new Hash(this, sb, derived, stringedSalt);
+            return new Hash(this, sb, derived, salt);
         }
         catch (IllegalArgumentException | GeneralSecurityException e)
         {
+            String stringedSalt = Utils.fromBytesToString(salt);
             String message = "Invalid specification with salt=" + stringedSalt + ", N=" + workFactor + ", r=" + resources + " and p=" + parallelization;
             throw new BadParametersException(message, e);
         }

diff --git a/src/test/com/password4j/Argon2FunctionTest.java b/src/test/com/password4j/Argon2FunctionTest.java
@@ -280,6 +280,28 @@ public void testOWASP()
         PropertyReader.properties = oldProps;
     }
 
+    @Test
+    public void testWithGeneratedSalt()
+    {
+        for (TestCase test : CASES)
+        {
+            Argon2Function f = getFunction(test.memory, test.iterations, test.parallelism, test.outLength, test.type, test.version);
+            Hash hash = Password.hash(test.plainTextPassword).addRandomSalt().with(f);
+            assertTrue(Password.check(test.plainTextPassword, hash));
+        }
+    }
+
+    @Test
+    public void testWithGeneratedSaltAndSetLenght()
+    {
+        for (TestCase test : CASES)
+        {
+            Argon2Function f = getFunction(test.memory, test.iterations, test.parallelism, test.outLength, test.type, test.version);
+            Hash hash = Password.hash(test.plainTextPassword).addRandomSalt(128).with(f);
+            assertTrue(Password.check(test.plainTextPassword, hash));
+        }
+    }
+