Merge pull request #27 from Password4j/bug-26

#26: Strings are always converted to byte[] with the same encoder
Password4j · Feb 4, 2021 · 750b49b · 750b49b
2 parents 423aff9 + e079318
commit 750b49b
Show file tree

Hide file tree

Showing 17 changed files with 314 additions and 84 deletions.
diff --git a/src/main/java/com/password4j/AbstractHashingFunction.java b/src/main/java/com/password4j/AbstractHashingFunction.java
@@ -78,6 +78,20 @@ public boolean check(CharSequence plainTextPassword, String hashed, String salt,
         return check(Utils.append(pepper, plainTextPassword), hashed, salt);
     }
 
+    /**
+     * Compares two {@link CharSequence}s as byte arrays in length-constant time. This comparison method
+     * is used so that password hashes cannot be extracted from an on-line
+     * system using a timing attack and then attacked off-line.
+     *
+     * @param a the first CharSequence
+     * @param b the second CharSequence
+     * @return true if both {@link CharSequence}s are the same, false if not
+     */
+    protected static boolean slowEquals(CharSequence a, CharSequence b)
+    {
+        return slowEquals(Utils.fromCharSequenceToBytes(a), Utils.fromCharSequenceToBytes(b));
+    }
+
     /**
      * Compares two byte arrays in length-constant time. This comparison method
      * is used so that password hashes cannot be extracted from an on-line

diff --git a/src/main/java/com/password4j/AlgorithmFinder.java b/src/main/java/com/password4j/AlgorithmFinder.java
@@ -57,32 +57,11 @@ public class AlgorithmFinder
      * @see #getSecureRandom()
      * @since 0.1.0
      */
-    private static final SecureRandom SR_SOURCE;
+    private static SecureRandom secureRandom;
 
     static
     {
-        SecureRandom sr;
-        if (useStrongRandom())
-        {
-            try
-            {
-                sr = SecureRandom.getInstanceStrong();
-            }
-            catch (NoSuchAlgorithmException nsae)
-            {
-                /* Even if there's no strong instance, execution
-                 * must continue with a less strong SecureRandom instance */
-                LOG.warn("No source of strong randomness found for this environment.");
-                sr = new SecureRandom();
-            }
-
-        }
-        else
-        {
-            sr = new SecureRandom();
-        }
-        SR_SOURCE = sr;
-
+        initialize();
     }
 
     private AlgorithmFinder()
@@ -109,7 +88,7 @@ private AlgorithmFinder()
      */
     public static SecureRandom getSecureRandom()
     {
-        return SR_SOURCE;
+        return secureRandom;
     }
 
     /**
@@ -381,6 +360,31 @@ private static boolean useStrongRandom()
         return PropertyReader.readBoolean("global.random.strong", false);
     }
 
+    static void initialize()
+    {
+        SecureRandom sr;
+        if (useStrongRandom())
+        {
+            try
+            {
+                sr = SecureRandom.getInstanceStrong();
+            }
+            catch (NoSuchAlgorithmException nsae)
+            {
+                /* Even if there's no strong instance, execution
+                 * must continue with a less strong SecureRandom instance */
+                LOG.warn("No source of strong randomness found for this environment.");
+                sr = new SecureRandom();
+            }
+
+        }
+        else
+        {
+            sr = new SecureRandom();
+        }
+        secureRandom = sr;
+    }
+
     private static class Param
     {
         String algorithm;

diff --git a/src/main/java/com/password4j/Argon2Function.java b/src/main/java/com/password4j/Argon2Function.java
@@ -162,7 +162,7 @@ public Hash hash(CharSequence plainTextPassword, String salt, CharSequence peppe
         {
             theSalt = salt;
         }
-        initialize(password, theSalt.getBytes(), Utils.fromCharSequenceToBytes(pepper), null, blockMemory);
+        initialize(password, Utils.fromCharSequenceToBytes(theSalt), Utils.fromCharSequenceToBytes(pepper), null, blockMemory);
         fillMemoryBlocks(blockMemory);
         byte[] hash = ending(blockMemory);
         return new Hash(this, encodeHash(hash, theSalt), theSalt);
@@ -190,7 +190,7 @@ public boolean check(CharSequence plainTextPassword, String hashed, String salt,
         }
 
         Hash internalHash = hash(plainTextPassword, theSalt, pepper);
-        return slowEquals(internalHash.getResult().getBytes(), hashed.getBytes());
+        return slowEquals(internalHash.getResult(), hashed);
     }
 
 
@@ -713,7 +713,8 @@ private long[][] copyOf(long[][] old)
     private String encodeHash(byte[] hash, String salt)
     {
         return "$argon2" + variant.name().toLowerCase() + "$v=" + version + "$m=" + memory
-                + ",t=" + iterations + ",p=" + parallelism + "$" + Base64.getEncoder().withoutPadding().encodeToString(salt.getBytes()) +
+                + ",t=" + iterations + ",p=" + parallelism +
+                "$" + Base64.getEncoder().withoutPadding().encodeToString(Utils.fromCharSequenceToBytes(salt)) +
                 "$" + Base64.getEncoder().withoutPadding().encodeToString(hash);
     }
 
@@ -732,7 +733,6 @@ private static Object[] decodeHash(String hash)
             result[4] = Integer.parseInt(StringUtils.removeStart(params[2], "p="));
             result[5] = Base64.getDecoder().decode(parts[4]);
             result[6] = Base64.getDecoder().decode(parts[5]);
-
             return result;
         }
         else

diff --git a/src/main/java/com/password4j/BCryptFunction.java b/src/main/java/com/password4j/BCryptFunction.java
@@ -822,7 +822,7 @@ protected boolean checkPw(CharSequence plaintext, String hashed)
 
     static boolean equalsNoEarlyReturn(String a, String b)
     {
-        return MessageDigest.isEqual(a.getBytes(StandardCharsets.UTF_8), b.getBytes(StandardCharsets.UTF_8));
+        return MessageDigest.isEqual(Utils.fromCharSequenceToBytes(a), Utils.fromCharSequenceToBytes(b));
     }
 
 }
diff --git a/src/main/java/com/password4j/CompressedPBKDF2Function.java b/src/main/java/com/password4j/CompressedPBKDF2Function.java
@@ -153,7 +153,7 @@ public static CompressedPBKDF2Function getInstanceFromHash(String hashed)
     protected String getHash(SecretKey key, String salt)
     {
         String params = Long.toString((((long) getIterations()) << 32) | (getLength() & 0xffffffffL));
-        String salt64 = Base64.getEncoder().encodeToString(salt.getBytes());
+        String salt64 = Base64.getEncoder().encodeToString(Utils.fromCharSequenceToBytes(salt));
         String hash64 = super.getHash(key, salt);
         return "$" + algorithm.code() + "$" + params + "$" + salt64 + "$" + hash64;
     }
@@ -164,23 +164,23 @@ public boolean check(CharSequence plainTextPassword, String hashed)
         String salt = getSaltFromHash(hashed);
         Hash internalHas = hash(plainTextPassword, salt);
 
-        return slowEquals(internalHas.getResult().getBytes(), hashed.getBytes());
+        return slowEquals(internalHas.getResult(), hashed);
     }
 
     @Override
     public boolean check(CharSequence plainTextPassword, String hashed, String salt)
     {
         String realSalt = getSaltFromHash(hashed);
         Hash internalHas = hash(plainTextPassword, realSalt);
-        return slowEquals(internalHas.getResult().getBytes(), hashed.getBytes());
+        return slowEquals(internalHas.getResult(), hashed);
     }
 
     private String getSaltFromHash(String hashed)
     {
         String[] parts = getParts(hashed);
         if (parts.length == 5)
         {
-            return new String(Base64.getDecoder().decode(parts[3].getBytes()));
+            return new String(Base64.getDecoder().decode(Utils.fromCharSequenceToBytes(parts[3])));
         }
         throw new BadParametersException("`" + hashed + "` is not a valid hash");
     }

diff --git a/src/main/java/com/password4j/MessageDigestFunction.java b/src/main/java/com/password4j/MessageDigestFunction.java
@@ -117,14 +117,26 @@ protected Hash internalHash(CharSequence plainTextPassword, String salt)
     public boolean check(CharSequence plainTextPassword, String hashed)
     {
         Hash hash = internalHash(plainTextPassword, null);
-        return slowEquals(hash.getResult().getBytes(), hashed.getBytes());
+        return slowEquals(hash.getResult(), hashed);
     }
 
     @Override
     public boolean check(CharSequence plainTextPassword, String hashed, String salt)
     {
         Hash hash = internalHash(plainTextPassword, salt);
-        return slowEquals(hash.getResult().getBytes(), hashed.getBytes());
+        return slowEquals(hash.getResult(), hashed);
+    }
+
+    /**
+     * The salt option describes if the Salt has been appended or prepended to
+     * the plain test password.
+     *
+     * @return how the salt is concatenated
+     * @since 1.6.1
+     */
+    public SaltOption getSaltOption()
+    {
+        return saltOption;
     }
 
     private CharSequence concatenateSalt(CharSequence plainTextPassword, CharSequence salt)

diff --git a/src/main/java/com/password4j/PBKDF2Function.java b/src/main/java/com/password4j/PBKDF2Function.java
@@ -146,7 +146,7 @@ protected static SecretKey internalHash(CharSequence plainTextPassword, String s
         {
             throw new IllegalArgumentException("Salt cannot be null");
         }
-        return internalHash(Utils.fromCharSequenceToChars(plainTextPassword), salt.getBytes(), algorithm, iterations, length);
+        return internalHash(Utils.fromCharSequenceToChars(plainTextPassword), Utils.fromCharSequenceToBytes(salt), algorithm, iterations, length);
     }
 
     protected static SecretKey internalHash(char[] plain, byte[] salt, String algorithm, int iterations, int length)
@@ -173,7 +173,7 @@ protected String getHash(SecretKey key, String salt)
     public boolean check(CharSequence plainTextPassword, String hashed, String salt)
     {
         Hash internalHash = hash(plainTextPassword, salt);
-        return slowEquals(internalHash.getResult().getBytes(), hashed.getBytes());
+        return slowEquals(internalHash.getResult(), hashed);
     }
 
     @Override

diff --git a/src/main/java/com/password4j/SCryptFunction.java b/src/main/java/com/password4j/SCryptFunction.java
@@ -121,7 +121,7 @@ public Hash hash(CharSequence plainTextPassword, String salt)
     {
         try
         {
-            byte[] saltAsBytes = salt.getBytes(StandardCharsets.UTF_8);
+            byte[] saltAsBytes = Utils.fromCharSequenceToBytes(salt);
             byte[] derived = scrypt(Utils.fromCharSequenceToBytes(plainTextPassword), saltAsBytes, 64);
             String params = Long.toString((long) Utils.log2(workFactor) << 16 | (long) resources << 8 | parallelization, 16);
             String sb = "$s0$" + params + '$' + Base64.getEncoder().encodeToString(saltAsBytes) + '$' + Base64.getEncoder()