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

Add hashing support for Argon2 #1150

Closed
KonoromiHimaries opened this issue Mar 27, 2017 · 55 comments
Closed

Add hashing support for Argon2 #1150

KonoromiHimaries opened this issue Mar 27, 2017 · 55 comments
Assignees
Milestone

Comments

@KonoromiHimaries
Copy link

KonoromiHimaries commented Mar 27, 2017

https://github.com/AuthMe/AuthMeReloaded/blob/master/docs/hash_algorithms.md

argon2
https://github.com/p-h-c/phc-winner-argon2

table

table

@ljacqu ljacqu added this to the 5.4 (Future) milestone Mar 27, 2017
@ljacqu
Copy link
Member

ljacqu commented Mar 27, 2017

we can do it with https://github.com/wg/scrypt, which is even on Maven central

@Eyremba
Copy link

Eyremba commented Mar 30, 2017

This is useless. I already posted this some time ago:

Maybe take a look at Argon2? Argon2 is currently the most secure password hashing algorithm, even more secure than scrypt. It could be a milestone too for 5.4.

Original Argon2 source: https://github.com/P-H-C/phc-winner-argon2
Java implementation of Argon2: https://github.com/phxql/argon2-jvm

Argon2 is much more secure than Scrypt and much better. Implementing scrypt would just implement an old outdated algorithm...

@timvisee
Copy link
Contributor

timvisee commented Mar 30, 2017

Maybe, implementing some dynamic hashing solution might be the best idea. With that, I mean a solution that supports many known hashing algorithms without requiring changes in the code regarding hashing, with minimal code change (just for the initial implementation), and a little configuration property. It would be awesome if the server administrator would have the ability to configure what algorithm is used using some form of algorithm identifier string, whether that'd be a bcrypt, scrypt, Argon2 or /dev/random.

The problem is, that a single algorithm isn't ideal for all situations. Generally speaking, a hashing algorithm that takes longer (in time) to hash, is more secure against brute force attacks. However, that also means that the longer it takes to hash, the more resource expensive it is. And that for each hash calculation that needs to be done when a user enters a password through the login command. Some hashing algorithms even occupy all available CPU cores, which can cause great performance hits on servers running lots of users, or on low performance servers. I've heard that Facebook even uses dedicated hashing servers to minimize the performance impact for users that are logging in on their regular servers, although I don't know whether that is true.

Giving the server administrator freedom to choose an appropriate hashing algorithm would be ideal in my honest opinion. Purely implementing something like this might be a bit overkill solely for the reason mentioned above. But, there can be a constant debate on what algorithm is best, and all users seem to like different hashing implementations. Thus, something like this would be a perfect solution. Many other bigger projects that are focused on password security implement some form of this. And of course, it provides many pro's.

I've seen something like this a few weeks ago, with many supported hashing algorithm adapters, although I can't remember what it was called right now. Maybe it has been mentioned before.

If a feature like this is desired, I might be able to make some free time to implement this through a PR.

What does everybody think about this?

@ljacqu
Copy link
Member

ljacqu commented Mar 31, 2017

@timvisee Your efforts are very welcome (and we have tons of stuff that needs work!) but I think they'd be better spent on another subject.

It's generally a bad idea to mix hashing algorithms. The only legitimate reason for doing this is to hook into an existing system like a forum. There, admittedly, it needs the introduction of a new hash algorithm for it to work.

Given that we sometimes use the configured hash algorithm as an indiciation to perform forum-specific stuff (e.g. XFBCRYPT I think is just bcrypt but it triggers additional work in the data source) I fear that we won't be able to simplify the settings that way.

Bcrypt does a good job of hashing in many iterations. One improvement we can do there is in the settings. I don't think they're clear at all (on mobile, but I think one is called bcryptRoundsLog2 or similar and the comment just restates that xD)

http://softwareengineering.stackexchange.com/a/214451

Of course, I'm happy to be proven otherwise ;)

@ljacqu
Copy link
Member

ljacqu commented Mar 31, 2017

What could be interesting is hashing some password with the same algorithm multiple times: hash1(hash2(hash3(...hashn(pass + salt) + pass + salt) + pass + salt)...) + pass + salt) where n and maybe hash are configurable. But that's essentially Bcrypt, I'd still vouch for making Bcrypt more accessible

@Eyremba
Copy link

Eyremba commented Mar 31, 2017

But that's essentially Bcrypt, I'd still vouch for making Bcrypt more accessible

Bcrypt is OLD and INSECURE when you attack it with modern methods. Like stated multiple times, Bcrypt has been replaced by Scrypt, and Scrypt has been replaced by Argon2. Argon2 is the newest and currently most secure method.

@timvisee
Copy link
Contributor

timvisee commented Mar 31, 2017

@Eyremba Is it? Is there any proof for that statement?

Is it because it doesn't require much performance to brute force? The cool think about bcrypt is, that you can configure how much rounds (iterations ^2) to use while hashing, which increases the hashing time exponentially. Also, brypt uses a (secure) random salt for each hash further strengthening the security.

@Eyremba
Copy link

Eyremba commented Mar 31, 2017

@Eyremba Is it? Is there any proof for that statement?

Wtf just google it! Argon2 is the official successor of Bcrypt and Scrypt.

The cool think about bcrypt is, that you can configure how much iterations to use while hashing.

And...? Argon2 can do even more. You can specify the iterations, and also the memory consumption and other things to prevent other attacks.

Also, Argon2 has won the official PHC!

-->

Argon2 is a key derivation function that was selected as the winner of the Password Hashing Competition in July 2015.

--> https://en.wikipedia.org/wiki/Argon2

It has also won against Bcrypt and Scrypt....

--> https://github.com/p-h-c/phc-winner-argon2

@EbonJaeger
Copy link
Contributor

https://security.stackexchange.com/questions/4781/do-any-security-experts-recommend-bcrypt-for-password-storage/6415#6415

As a note with security algorithms, older does not inherently mean worse or less secure. Nor does something newer mean more secure. Older ones have been tested by time in the wild.

As another note, NIST recommends PKDBF (prolly misspelled, on mobile) for a security hashing algorithm.

@timvisee
Copy link
Contributor

timvisee commented Mar 31, 2017

@Eyremba Thank you for the quick reply. Argon2 really does seem promising!

@Gnat008 That's exactly what I ment. This doesn't mean that bcrypt isn't 'secure' anymore. However, I did find some articles where it was mentioned that more efficient ways to brute force bcrypt hashes have been found. Again, this doesn't instantly render it an 'unsecure' algorithm.

I must add, that if a single algorithm is used; Argon2 seems to be the best option.

@Eyremba
Copy link

Eyremba commented Mar 31, 2017

As another note, NIST recommends PKDBF (prolly misspelled, on mobile) for a security hashing algorithm.

That's because they have not tested Argon2 yet, however, Argon2 has won the official Password Hashing Competition in July 2015.

And just as another note, the popular password manager "KeePass" which is used by millions of people uses Argon2 too since the last version/release.

@KonoromiHimaries KonoromiHimaries changed the title pls. add support to scrypt pls. add support to scrypt and better argon2 Mar 31, 2017
@EbonJaeger
Copy link
Contributor

Congratulations. Doesn't mean I'm gonna necessarily trust the latest and greatest, for exactly the reasons I gave above.

Maybe it is better. But maybe it has a critical flaw that we don't know about yet because it hasn't spent too long in the wild.

See what I'm getting at here?

@timvisee timvisee changed the title pls. add support to scrypt and better argon2 Add hashing support for Argon2 and/or scrypt Mar 31, 2017
@timvisee timvisee changed the title Add hashing support for Argon2 and/or scrypt Add hashing support for Argon2 and/or scrypt Mar 31, 2017
sgdc3 added a commit that referenced this issue Apr 13, 2017
sgdc3 added a commit that referenced this issue Apr 14, 2017
* Implement ARGON2 hash

#1150

* Fix argon hash verify

* Add argon2 test

* #1150 Account for Argon2 managing salts internally
@sgdc3
Copy link
Member

sgdc3 commented Apr 14, 2017

We implemented Argon2 ;)

@sgdc3 sgdc3 closed this as completed Apr 14, 2017
@ljacqu ljacqu changed the title Add hashing support for Argon2 and/or scrypt Add hashing support for Argon2 Apr 14, 2017
@ljacqu ljacqu modified the milestones: 5.3, 5.4 (Future) Apr 14, 2017
@sgdc3
Copy link
Member

sgdc3 commented Jun 1, 2017

Due to devbukkit limitations we can't publish a jar containing binaries (argon2 implementation has some dlls)

@sgdc3 sgdc3 removed this from the 5.3 milestone Jun 1, 2017
@Eyremba
Copy link

Eyremba commented Jun 1, 2017

Due to devbukkit limitations we can't publish a jar containing binaries (argon2 implementation has some dlls)

Could you please make it so that if a user wants to use Argon2, he can download the Argon2 Library here on GitHub manually and put it into the AuthMe config folder?

@games647
Copy link
Member

@krusic22 Could you run file /usr/lib/libargon2.so maybe it's broken?

@krusic22
Copy link
Member

I get "/usr/lib/libargon2.so: symbolic link to libargon2.so.0"
/usr/lib/libargon2.so.0: ELF 64-bit LIB shared object,.....

@games647
Copy link
Member

games647 commented Jul 25, 2017

@krusic22 You use the topic/argon2 branch? What exact error do you get in Spigot? Do you use the OpenJDK or the OracleJDK/JRE?

EDIT:
Adding this to the startup parameter
-Djna.debug_load=true

@krusic22
Copy link
Member

krusic22 commented Jul 25, 2017

Yes. It doesn't give a error just reverts to SHA256. OracleJDK.
Will try after I get back from vacation.

@games647
Copy link
Member

games647 commented Jul 25, 2017

What config value is now in your passwordHash option?

@krusic22
Copy link
Member

ARGON2 also tried just ARGON.

@games647
Copy link
Member

For me it crashes if I put ARGON2 in there and don't have the library installed:

[WARN]: [AuthMe] Cannot find libargon2 [UnsatisfiedLinkError]: no argon2 in java.library.path
[WARN]: [AuthMe] WARNING!!! You use Argon2 Hash Algorithm method but we can't found any Argon2 library on your system !
[WARN]: [AuthMe] THE SERVER IS GOING TO SHUT DOWN AS DEFINED IN THE CONFIGURATION!!

If the library is installed, it works fine.

@KonoromiHimaries
Copy link
Author

@games647 How to run Argon2 in windows 10?

@games647
Copy link
Member

@mat41997
Download a compiled library for your architecture from here or compile it yourself. Then install the dll to one folder specified in your PATH variable. You could execute set path to view all possible locations separated by a semicolon. You could also use -Djava.library.path=<absolute path> as your Java startup parameter.

@sgdc3
Copy link
Member

sgdc3 commented Sep 14, 2017

@Xephi any news about that? ;)

@Xephi
Copy link
Contributor

Xephi commented Sep 15, 2017

It actually works, i've to write the wiki page T.T

@Eyremba
Copy link

Eyremba commented Sep 17, 2017

Just by the way, this here: https://github.com/andreas1327250/argon2-java is a pure Java implementation of Argon2.

It could be maybe implemented into AuthMe, and it would not require any external libraries or dlls or stuff.

@ljacqu
Copy link
Member

ljacqu commented Sep 17, 2017

That's pretty cool, I would immediately favor that over anything that requires additional steps. Looks like it still needs a little to be mature, though. Maybe I could help out a little.

@Eyremba
Copy link

Eyremba commented Sep 17, 2017

Looks like it still needs a little to be mature, though.

Yes. The last changes in the repo were made 5 days ago, so yes it is currently in active developement.

But maybe it could be used in AuthMe soon. I have tested this Argon2 version, and it seems to be pretty stable. Also, AuthMe could pull the latest version of Argon2 everytime before a AuthMe release is compiled.

@ljacqu ljacqu modified the milestones: 5.4, 5.5 Oct 15, 2017
Xephi added a commit that referenced this issue Oct 22, 2017
- Add argon2 implementation

- Extract argon2 library check to method on Argon 2
- Add link to Wiki page on errors
- Check within Argon2Test if the test cases should be run, not in the abstract parent
@Xephi
Copy link
Contributor

Xephi commented Oct 22, 2017

Available on master branch

@Xephi Xephi closed this as completed Oct 22, 2017
@ljacqu ljacqu modified the milestones: 5.5, 5.4 Oct 23, 2017
@ljacqu
Copy link
Member

ljacqu commented Oct 23, 2017

Hi @Eyremba @mat41997, we'd be thankful for some feedback :)

@ljacqu ljacqu reopened this Oct 23, 2017
@ljacqu
Copy link
Member

ljacqu commented Oct 23, 2017

Confirmed by krusic22 on Discord that it works, so I close again xP Sorry

@lenis0012
Copy link

Intresting conversation.

I implemented Argon2 a while ago, but never documented it because I was unable to find enough evidence proving that the algorithm is "battle tested".

I also considered a custom hashing method where multiple algorithms could be combined. but for the same reasons decided not to implement it.

I am getting Déjà vu scrolling through this page.

On another note, nice work guys.
AuthMe Reloaded appears to have improved a lot in the past 2 years or so.

@sgdc3
Copy link
Member

sgdc3 commented Nov 26, 2017

@lenis0012 Thank you man ;)

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

No branches or pull requests

10 participants