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::Password.verify_password can only verify passwords with default Salt lengths #14

Closed
asynchrony-ringo opened this issue Sep 19, 2016 · 7 comments

Comments

@asynchrony-ringo
Copy link

The verify_password method will return false when attempting to validate Argon2 encodings generated with a salt length different from the default 16.

This means encodings generated within the gem validate properly but encodings generated outside of the gem will fail when validating with the gem unless the Salt length matches the gem's default.

Moving existing Argon2 encodings into a system using this gem may then require regenerating all encodings.

Below I've put together test cases using the gem's validation and encodings generated via command line using Argon2 v1.3.

Gem Generated Password Validates

irb(main):001:0> require 'argon2'
=> true
irb(main):002:0> test = Argon2::Password.create("password")
=> "$argon2i$v=19$m=65536,t=2,p=1$FMTtGRKnyJlUMvCyoXKZ9g$dQFsMAFHBBdMnp/nQ/wScDqz+fJrUiO+UAGNailZIYU"
irb(main):003:0> Argon2::Password.verify_password("password",test)
=> true

Argon2 1.3 Generated Password Following Gem Defaults Validates

echo -n "password" | ./argon2 LongEnoughSaltLn -t 2 -m 16 -l 32
Type:       Argon2i
Iterations: 2
Memory:     65536 KiB
Parallelism:    1
Hash:       4c172bab9fe7cb8f2063623f97845374f01902864e187a0cba7150255b865f9e
Encoded:    $argon2i$v=19$m=65536,t=2,p=1$TG9uZ0Vub3VnaFNhbHRMbg$TBcrq5/ny48gY2I/l4RTdPAZAoZOGHoMunFQJVuGX54
0.124 seconds
Verification ok
irb(main):004:0> Argon2::Password.verify_password("password","$argon2i$v=19$m=65536,t=2,p=1$TG9uZ0Vub3VnaFNhbHRMbg$TBcrq5/ny48gY2I/l4RTdPAZAoZOGHoMunFQJVuGX54")
=> true

Argon2 1.3 Generated Password With Shorter Salt Fails Validation

$ echo -n "password" | ./argon2 TooShortSaleLen -t 2 -m 16 -l 32
Type:       Argon2i
Iterations: 2
Memory:     65536 KiB
Parallelism:    1
Hash:       8b9f442e0026e46e89fbdfa86703be924578f2d272a1c361e9b1dd923f49e659
Encoded:    $argon2i$v=19$m=65536,t=2,p=1$VG9vU2hvcnRTYWxlTGVu$i59ELgAm5G6J+9+oZwO+kkV48tJyocNh6bHdkj9J5lk
0.122 seconds
Verification ok
irb(main):005:0> Argon2::Password.verify_password("password","$argon2i$v=19$m=65536,t=2,p=1$VG9vU2hvcnRTYWxlTGVu$i59ELgAm5G6J+9+oZwO+kkV48tJyocNh6bHdkj9J5lk")
=> false

Argon2 1.3 Generated Password With Longer Salt Fails Validation

$ echo -n "password" | ./argon2 TooLongSaleLength -t 2 -m 16 -l 32
Type:       Argon2i
Iterations: 2
Memory:     65536 KiB
Parallelism:    1
Hash:       99895e047b06e8dd3e1f8246274c57a0844eeab58d67037f790410f1e1c80e69
Encoded:    $argon2i$v=19$m=65536,t=2,p=1$VG9vTG9uZ1NhbGVMZW5ndGg$mYleBHsG6N0+H4JGJ0xXoIRO6rWNZwN/eQQQ8eHIDmk
0.122 seconds
Verification ok
irb(main):006:0> Argon2::Password.verify_password("password","$argon2i$v=19$m=65536,t=2,p=1$VG9vTG9uZ1NhbGVMZW5ndGg$mYleBHsG6N0+H4JGJ0xXoIRO6rWNZwN/eQQQ8eHIDmk")
=> false
@technion
Copy link
Owner

Hi,

Thanks for this report. I'm going to take some time to look into an appropriate solution. Trying to mess with this has a knock-on effect of messing with the ENCODE_LEN constant, which exists in Ruby outside the C wrapper.

Being in this position would also involve messing with a recommended default, which I've been trying to discourage people from doing.

Regardless, let me look into a solution.

@technion
Copy link
Owner

It would appear that my concerns related only to creating a hash.
Given the position of encouraging people not to come up with their own salting scheme in the first place, I'm happy to stay firm on SALT_LEN as a hard coded input.

I'm pushing a fix now however that should allow it to verify existing hashes, which I've tested against your examples.

@micahhainline
Copy link

Any idea when we might see this in a release?

@technion
Copy link
Owner

@micahhainline Given it's a bugfix, I will push a new release if no issues come up in another 24 hours.

technion added a commit that referenced this issue Sep 20, 2016
@micahhainline
Copy link

@technion wow! This is setting the bar for responsiveness! Thanks for turning it around so quickly!

@technion
Copy link
Owner

@micahhainline No worries - it's a legit bug and can be fixed in a non-breaking way.

@technion
Copy link
Owner

@micahhainline @asynchrony-ringo Version 1.1.1 has been tagged and pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants