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

Argon2 not working as expected #92

Closed
anton-johansson opened this issue Dec 6, 2022 · 11 comments
Closed

Argon2 not working as expected #92

anton-johansson opened this issue Dec 6, 2022 · 11 comments
Milestone

Comments

@anton-johansson
Copy link

Describe the bug
I've been using Argon2 in an application not written in Java. Now, I want to start checking those Argon2 hashes in my Java application using password4j.

My other application has generated an Argon2 hash that looks like this:

$argon2id$v=19$m=16384,t=2,p=1$nlm7oNI5zquzSYkyby6oVw$JOkJAYrDB0i2gmiJrXC6o2r+u1rszCm/RO9gIQtnxlY

The clear text password for this is Test123!. If I fill in those values on this online checker, things work exactly the way I want it:
https://argon2.online

But when I use password4j, I always get false when running it like this:

        boolean verified = Password.check("Test123!", "$argon2id$v=19$m=16384,t=2,p=1$nlm7oNI5zquzSYkyby6oVw$JOkJAYrDB0i2gmiJrXC6o2r+u1rszCm/RO9gIQtnxlY").withArgon2();
        System.out.println(verified);

I've also noticed that it seems to ignore the configuration values in the hash itself (such as t=2 and p=1). I've tried setting them to match the hash, but that shouldn't be necessary...

To Reproduce
See Java code above.

Expected behavior
I expect the above to yield true.

Environment:

  • OS: Ubuntu 20.04
  • JDK Oracle JDK 17
  • Version 1.6.2

Additional context
N/A.

@anton-johansson
Copy link
Author

I realized that I have to use the method ArgonFunction.getInstanceFromHash to actually parse the hash and get the correct algorithm implementation. However, even if I do so, it doesn't validate:

@Test
public void test_verify()
{
    String hash = "$argon2id$v=19$m=16384,t=2,p=1$nlm7oNI5zquzSYkyby6oVw$JOkJAYrDB0i2gmiJrXC6o2r+u1rszCm/RO9gIQtnxlY";
    Argon2Function function = Argon2Function.getInstanceFromHash(hash);
    boolean verified = Password.check("Test123!", hash).with(function);
    assertTrue(verified);
}

@firaja
Copy link
Member

firaja commented Dec 7, 2022

Hi @anton-johansson,

thank you for opening this issue.

It might be an issue related to the salt encoding. When you use Password.check(...), password4j recalculates the hash and verifies if it is equal to the one in input. What is interesting is that for this particular hash password4j produces a different hash but that is still valid in https://argon2.online for Test123!.

At first glance, it seems something related to the bytes padding during the encoding/decoding of the salt.

Do you have other cases like this (password4j fails but not argon2.online)? What libraryhave you used to generate those?

@firaja firaja added this to the 1.6.3 milestone Dec 7, 2022
@anton-johansson
Copy link
Author

Do you have other cases like this (password4j fails but not argon2.online)? What libraryhave you used to generate those?

I've used the Node Argon2 library which is built on a native library written in C.

But I think you're right. When I generate new hashes using password4j, the salts look completely different than when I use the NodeJS library or when using https://argon2.online.

@firaja
Copy link
Member

firaja commented Dec 8, 2022

Hello @anton-johansson ,
I found the issue: Argon2 inefficiently encoded the salt first from string to bytes, then back to string, encoded to base64 and put in the final hash. This back and forth probably added some information that extended the final encoding with unwanted bytes. I didn't investigate on this because it is inefficient and now the bytes are directly converted in base64.

1.6.3 has been uploaded now. You can try it and let me know (or download from master branch if the maven remote cache is not updated yet)

Thank you very much for pointing out this issue!

@anton-johansson
Copy link
Author

@firaja That works like a charm, thanks for the quick support and fix!

I accidentally noticed something else though. See this test:

    @Test
    public void test_using_function_directly()
    {
        String hash = "$argon2id$v=19$m=16384,t=2,p=1$nlm7oNI5zquzSYkyby6oVw$JOkJAYrDB0i2gmiJrXC6o2r+u1rszCm/RO9gIQtnxlY";
        Argon2Function function = Argon2Function.getInstanceFromHash(hash);

        boolean test1 = Password.check("Test123!", hash).with(function);
        assertTrue(test1);

        boolean test2 = function.check("Test123!", hash);
        assertTrue(test2);
    }

When using the function directly to check, it doesn't work. This is not the case with for example Bcrypt. Not a big deal for me, I'll use the first way, but I thought you should know. Maybe extract this to a separate issue? Or ignore, it's up to you. :)

@firaja
Copy link
Member

firaja commented Dec 8, 2022

I will open a different issue but this one has lower priority: AbstractHashingFunction#hash(...) or AbstractHashingFunction#check(...) are intended for internal usage. They should be protected but for historical reason they are public, but it is worth to fix it.

Thank you again.

@RohanNagar
Copy link

Hello @firaja,

I am still seeing an issue with Argon2 in version 1.6.3

I have this test that passes with version 1.6.1:

@Test
  void testHashMatch() {
    String plaintext = "password";
    String hashed = "$argon2id$v=19$m=15,t=2,p=1$and1aHgwcThpM2EwMDAwMA$+GgRQ1NSPghlIAUWlO1mVTktS"
        + "QVSj35tUNvLiVfWiB0";

    assertTrue(Password.check(plaintext, hashed).withArgon2());
  }

But it fails with 1.6.2 and 1.6.3.

I generated the hashed version from the online generator https://argon2.online/ as well. Any ideas what is going on?

@anton-johansson
Copy link
Author

Hello @firaja,

I am still seeing an issue with Argon2 in version 1.6.3

I have this test that passes with version 1.6.1:

@Test
  void testHashMatch() {
    String plaintext = "password";
    String hashed = "$argon2id$v=19$m=15,t=2,p=1$and1aHgwcThpM2EwMDAwMA$+GgRQ1NSPghlIAUWlO1mVTktS"
        + "QVSj35tUNvLiVfWiB0";

    assertTrue(Password.check(plaintext, hashed).withArgon2());
  }

But it fails with 1.6.2 and 1.6.3.

I generated the hashed version from the online generator argon2.online as well. Any ideas what is going on?

@RohanNagar Hi! If you just use .withArgon2(), you must make sure that the settings of Argon2 are the same for your configuration as they are for your hash. If you plan on validating hashes that comes from different sources, you'll need to utilize the function Argon2Function.getInstanceFromHash(hash) to make sure you get a hashing function with the specification defined in the actual hash.

@RohanNagar
Copy link

Thanks @anton-johansson! I will try this. I am curious as to why it passes consistently with version 1.6.1 though. Did some default settings change?

@firaja
Copy link
Member

firaja commented Dec 13, 2022

Hi @RohanNagar ,
with #80 the default settings changed in order to be compliant with OWASP's recommendations.
We always suggest to put your algorithm's configurations inside psw4j.properties file or in the code via Argon2Function.getInstance(...) or Argon2Function.getInstanceFromHash(...).

When you use .withArgon2 the prototype of the function is generated from the property file or, in your case if you don't have such file, from default values taken from OWASP (that changed between 1.6.1 and 1.6.2). If you have a logger you should see a warning on startup.
The OWASP's recommendations are not always the best set of configurations: developers should tweak them depending on their security and functional requirements.

@RohanNagar
Copy link

Got it, thank you for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants