phxql / argon2-jvm

Argon2 Binding for the JVM
GNU Lesser General Public License v3.0
329 stars 32 forks source link

Salt lengths other than the pre-defined value do not produce an error or exception. #38

Closed seantcanavan closed 6 years ago

seantcanavan commented 6 years ago
public class SaltedArgon {

    public static final int ITERATIONS  = 10;     // number of 'rounds' to iterate over
    public static final int PARALLELISM = 35;     // number of java threads to use
    public static final int SALT_LENGTH = 32;     // 256 bits
    public static final int HASH_LENGTH = 128;    // 1024 bits
    public static final int MEMORY_USED = 350000; // 350,000 kibibytes = 358.4 megabytes

    private static final Argon2Advanced argon2 = Argon2Factory.createAdvanced(Argon2Factory.Argon2Types.ARGON2d, SALT_LENGTH, HASH_LENGTH);

    public static byte[] hash(String userInput, byte[] userSalt) {
        return argon2.rawHash(ITERATIONS, MEMORY_USED, PARALLELISM, userInput, userSalt);
    }

    public static boolean verify(String password, byte[] salt, byte[] argon) {
        byte[] result = hash(password, salt);
        return Arrays.equals(argon, result);
    }
}

...

@Test
public void hash_WithInvalidSaltLength_ShouldThrowException() {
    SecureRandom secureRandom = new SecureRandom();
    String password = StringUtils.randomAlphanumeric(100);
    byte[] salt = new byte[RandomUtils.nextInt(20, 30)]; // random length salt from 20 to 29
    secureRandom.nextBytes(salt);

    byte[] argon = SaltedArgon.hash(password, salt);

    assertThat(SaltedArgon.verify(password, salt, argon)).isFalse(); // FAILS
}

org.junit.ComparisonFailure: 
Expected :[fals]e
Actual   :[tru]e

I thought that using a salt length different than what the Argon2D algorithm was defined as would throw an error? Was this assumption wrong? I need help writing more tests to make sure my code is working as expected.

phxql commented 6 years ago

The salt length you can specify when creating Argon2 instances is only used to generate a salt of the given length if you don't supply one. If you supply the salt yourself, you can use every salt length.

seantcanavan commented 6 years ago

That's good to know. I poured through the code for a bit and couldn't find any relevant comments about the hash length constructor argument vs parameter so figured I'd open a question. For now I've implemented my own hash length check inside the verify and hash calls.

phxql commented 6 years ago

I'll add JavaDoc to clarify that.