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 output does not conform to the specs #42

Closed
jdoe0000000 opened this issue Feb 10, 2021 · 3 comments
Closed

Argon2 output does not conform to the specs #42

jdoe0000000 opened this issue Feb 10, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@jdoe0000000
Copy link

jdoe0000000 commented Feb 10, 2021

According to the specs, = should be omitted from the b64 encoded parts of the output string. Currently, the Argon2 module includes =s in the b64 encoded parts.

>>> let salt = Salt "abcdefghijklmnop"
>>> hashPasswordWithSalt defaultParams salt (mkPassword "foobar")
PasswordHash {unPasswordHash = "$argon2id$v=19$m=65536,t=2,p=1$YWJjZGVmZ2hpamtsbW5vcA==$BztdyfEefG5V18ZNlztPrfZaU5duVFKZiI6dJeWht0o="}

In contrast, the reference implementation produces the following output string for the same input.

$ echo -n foobar | argon2 abcdefghijklmnop -id -t 2 -k 65536 -l 32
Type:		Argon2id
Iterations:	2
Memory:		65536 KiB
Parallelism:	1
Hash:		073b5dc9f11e7c6e55d7c64d973b4fadf65a53976e545299888e9d25e5a1b74a
Encoded:	$argon2id$v=19$m=65536,t=2,p=1$YWJjZGVmZ2hpamtsbW5vcA$BztdyfEefG5V18ZNlztPrfZaU5duVFKZiI6dJeWht0o
0.072 seconds
Verification ok

Moreover, checkPassword will not accept hash strings without padding =s.

>>> checkPassword (mkPassword "foobar") (PasswordHash "$argon2id$v=19$m=65536,t=2,p=1$YWJjZGVmZ2hpamtsbW5vcA$BztdyfEefG5V18ZNlztPrfZaU5duVFKZiI6dJeWht0o")
PasswordCheckFail

But the same string with padding =s works fine.

>>> checkPassword (mkPassword "foobar") (PasswordHash "$argon2id$v=19$m=65536,t=2,p=1$YWJjZGVmZ2hpamtsbW5vcA==$BztdyfEefG5V18ZNlztPrfZaU5duVFKZiI6dJeWht0o=")
PasswordCheckSuccess
@Vlix
Copy link
Collaborator

Vlix commented Feb 12, 2021

@jdoe0000000 Thanks for bringing this up!
I think I've either overlooked this detail or mistakenly implemented it incorrectly.
I can't remember what I used as a reference, tbh. The Argon2 specification doesn't describe the "required" format for printing a hash AFAICT. Is this format official, or just how every other library does it?

Regardless, it'd be a good idea to accept technically correct hashes.
(I was already planning on doing more with hashing formats: #11)

Do you want to make a PR for this? If not, I'll pick this up before the version 3.0.0.0 release.

@Vlix Vlix added the bug Something isn't working label Feb 12, 2021
@Vlix Vlix mentioned this issue Feb 14, 2021
@jdoe0000000
Copy link
Author

@Vlix Thank you for looking into this.

tbh. The Argon2 specification doesn't describe the "required" format for printing a hash AFAICT. Is this format official, or just how every other library does it?

The specs I linked is just for the PHC string format. The specs is official AFAICT.

I guess the PHC format is technically not the official format for Argon2 since it's not in its specs, but it has a special status since it's what the reference implementation uses.

@Vlix
Copy link
Collaborator

Vlix commented Mar 7, 2021

Merged and will be included in the 3.0.0.0 release

@Vlix Vlix closed this as completed Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants