Weak Password Hashing

[shackrock]

SHA256 is known to be weak. Please change to use crypt (specifically the latest adopted bcrypt) and user-specific salts that get regenerated from time to time. Then this will be a great class to use!

[panique]

This script uses SHA256, and that's the most advanced hashing algorithm i've ever seen in a php script. Most other scripts, even most tutorials use md5 or NO hashing, and no salting. A change to SHA512 would be even more secure, yes, but is this really necessary ? According to a lot of discussions on the web md5+salt is totally okay, so sha256 is definitly not weak. Anyway, i will have a deeper look on that again and implement sha512 if this makes sense ;)

A re-generated user salt sound good, but maybe it's a little bit too much. If there's time, then this will go into the advanced version of the script.

[shackrock]

I'd recommend reading over this: http://crackstation.net/hashing-security.htm

Or better yet: http://www.reddit.com/r/PHP/comments/1d7hns/reminder_use_bcrypt_for_your_users_passwords/

Great explanations and discussion of why SHA256 isn't truly good enough anymore.

[panique]

Big big merci, man! Excellent articles! I'll definitly change the hashing to SHA512 (and keep an eye on the current hash cracking development).

[shackrock]

No problem. I'd really really recommend using Crypt though, over SHA512. The ability to "scale up" the time it takes to hash the password by changing just one number in the code is what is so important for future applications (as computers continue to get faster, and servers continue to get cheaper, we can go in each year and change the factor so that it will always take .5 seconds to login - instead of "5 years from now we're in ms times again!").

[panique]

I just implemented crypt() (the script was using crypt() from the first day btw) using SHA512 with $6$rounds=5000$ (which allows scaling up) and a 123 character salt (on linux systems, on windows the max_salt_length might be smaller). Currently testing, will be released in the next few days ;)

Afaik SHA512 is the strongest algorithm available in PHP (?), so this should work for some years.

[iefdev]

Sorry to open this one again... Instead of sha512 I think you should use the "password_compat" by @ircmaxell to make use of the new functions in php5.5.

Please look at this video: https://www.youtube.com/watch?v=eNdW5HWBhG0#!&t=10m42s (sha512 is listed under "Very bad" and "Still bad") :)

So you could build the login upon those 4 functions, and for those not using 5.5 can use the password_compat: https://github.com/ircmaxell/password_compat

[panique]

Let's handle us this issue like this: Let's keep this password_compat thing in mind, but with low priority and maybe for 2014, when PHP 5.5. has reached a significant market share. I haven't realized that password_compat regenerates salt periodically, but this seems interesting.

[ircmaxell]

This thread shows a fundamental misunderstanding of how passwords are attacked. here are some resources for learning more. (TLDR: sha512 + salt is never strong enough in today's world):

• A breakdown of hashing algorithms
• The Rainbow Table Is Dead
• Password Hashing In PHP Talk
• How to safely store a password

And if you want to know why password_compat is so "totally weird" as you put it: Seven Ways To Screw Up BCrypt.

Additionally, if you want to know why the password API was designed as it is (functions and such), check this out: Designing An API

I strongly hope you reconsider.

[shackrock]

I have to continue to agree with @ircmaxell .... I don't see myself using this package unless some additional changes are made.

[J7mbo]

Just to point out, password_compat is built to work in the current PHP version, and provides forward compatibility for when we all upgrade to 5.5. It's a popular repo for a reason, and comes from someone who specialises in security. :+1:

[ghost]

I also agree with @ircmaxell.

...in my eyes way way way too much for this simply login script

Unfortunately, consumers define the purpose for which your library is used.

[shackrock]

@bracketworks even if @panique didn't want to use the password_compat for some reason, I don't see any reason not to use bcrypt in lieu of the SHA512. The articles @ircmaxell linked to (see the last one: http://codahale.com/how-to-safely-store-a-password/) tells pretty clearly why SHA512 just isn't good enough. There's no reason not to implement bcrypt from my eyes in the class.

The consumers using the class will have no idea anyway, unless they go look at the class more closely...right? So I think it's a no brainer.

[ghost]

@shackrock Precisely. Consumers should be able to maintain confidence in the library, and that the most appropriate algorithm has been chosen (hence my mention)

[shackrock]

@bracketworks ah, yes. I misunderstood.

[panique]

I like this discussion ;) ! But just to make something clear: I have started this script because ALL the scripts and ALL the tutorials on the web (regarding login systems) were very very very bad. We are talking about plain text password savings in DBs, md5 hashing etc. here. Some of the scripts used modern hashing algorithms, and ONE i found even had a simple salt in it. Even after reading a lot of threads regarding this topic, including strictly doing what experts claimed in the highest voted answers on stackoverflow, there's always somebody, somewhere in some threads who says "but you should do it more like this". And then somewhere else says "do 1000 mixed up salts" etc. After that, people argue about totally different methods to generate random chararcters etc...

So, it's not easy to say what exactly is "THE BEST" method to secure a login, and especially for a SIMPLE login system its difficult to find a balance between max security and beginner-friendly, readable, self-explaining hash/salt code.

I would like to note that the biggest IT companies of the world are saving their passwords in md5 hashed strings ;), so sha512 + system max salt is NOT THAT BAD, but,to sum this up: I will have a very deep look on the password_compat function and implement this, if possible ! Deal !? ;)

[ghost]

I would like to note that the biggest IT companies of the world are saving their passwords in md5 hashed strings

Citation needed

Moreover, the best method for persisting credentials in a simple authentication system is the same as that of a complex authentication system. Concentrate on exposing a developer-friendly API, that "beginner" developers can use with ease, and advanced developers can use with assurance.

[panique]

@bracketworks In 2012 there were some hacks on major companies, like LinkedIn, eHarmony, the US Air Force, NBC, Sony, etc. including a nice discussion how they "secured" their user/employee passwords. It's been in all the major news, it even reached germany's biggest papers.

A quick google search brought up those:

http://www.zdnet.com/blog/btl/6-46-million-linkedin-passwords-leaked-online/79290 http://www.techspot.com/news/48923-eharmony-confirms-breach-about-15-million-passwords-stolen.html http://news.blogs.cnn.com/2012/01/16/zappos-com-hacked-24-million-customers-affected/

You can also find the entire databases of those companies on common filesharing platforms. And this is just the top of the iceberg. I mean, we are talking about BIG companies/organizations here, not simple hobby portals. Those companies have big IT teams, high paid security chiefs and millions of customers. And they totally failed !

[shackrock]

@panique IMO this is why we should use the latest accepted/adopted algorithms, so any sites created with this class, if their DB's are hacked, will not have passwords as easily exposed - if for no other reason except that the hashing algorithm takes a very long time, and can be scaled up with ease as computers continue to get faster. I think it's a no brainer =).

[PeeHaa]

According to a lot of discussions on the web md5+salt is totally okay

There are a lot of "discussions" on the web which advocate terrible practices and produce insecure applications just by being available for everyone to read. Please take your responsibility and stop this trend instead of saying everybody else is wrong and producing insecure code.

I have started this script because ALL the scripts and ALL the tutorials on the web (regarding login systems) were very very very bad.

And now you are happily continuing the trend of filling the web with bad examples / vulnerable applications.

This script uses sha512 and a salt and is therefore the most secure script i have ever seen on the entire web, using the most secure hash algorithm available in PHP (!)

Maybe your "entire web" is limited in some form, but this is just false.

mixin it with the longest salt available on your system (!). what else do you want ?

Listen to people who know more on the subject.

So, it's not easy to say what exactly is "THE BEST" method to secure a login, and especially for a SIMPLE login system its difficult to find a balance between max security and beginner-friendly, readable, self-explaining hash/salt code.

Actually it is pretty clear what currently THE BEST ™ method to hash passwords is. And @ircmaxell has been so nice to both provide a native API for this in PHP 5.5 as well as providing the same implementation in PHP for PHP 5.3.7+ with best practices in mind.

I would like to note that the biggest IT companies of the world are saving their passwords in md5 hashed strings ;), so sha512 + system max salt is NOT THAT BAD

Sometimes when using only a slightly better solution than others will still leave you with a crap solution. Using other weak implementations (the big IT companies you talk about) as an argument to not fix your own weak implementation is kinda... strange.

Reading back this comment I hate the fact it reads like a rant, which is something I didn't want it to be when starting to write this and I am glad you are considering looking into better implementations. It's just that I hate the fact that I keep seeing badly implemented stuff in regards to security. Especially when other people are going to use it (whether they are end users of the system or are developers using it).

[PeeHaa]

I ended my comment stating I hated the fact it read like a rant (which was not my intention when I started writing the comment at all) and my reasoning for this.

The reason it came to this is because of your attitude by keep saying your solution is just fine (even though you said you are going to look into other solutions linked, which is totally a good thin :+1: ) or reading information supplied to you including useful links which you didn't read otherwise you wouldn't had to ask me why sha512+salt is bad.

Again this is not meant as a rant nor as a personal attack nor am I trolling (sorry if you see it like that), but the attitude of putting your fingers in your ears and yelling NANANANANA I can't hear you is not a very good attitude security wise. Even though you are going to look into other solutions I hope you will also look into why some solution is better than another. Which would also prevent you from saying things like " password_compat uses $2y which is definitly totally outdated and insecure".

Regarding the missing PR from my side. When I see the owner of a repository saying he doesn't want feature / improvement X there is no point in sending a PR.

[shackrock]

$2y$ - is blowfish in crypt, and fixes vulnerabilities in $2a - making $2y$ the recommended algorithm to use for password hashing (it's the "official" algo for bcrypt).

$6 - this is the sha512 version of crypt, instead of blowfish. I am not enlightened enough to know if using 6 vs. 2y is going to yeild similar results... I think it should be fine, but you may need to push your cost up in order to hit the "time" you would like your hash to take in this case?

A good explanation is here: http://security.stackexchange.com/questions/20541/insecure-versions-of-crypt-hashes

I should also add this stackoverflow thread which has enormous community feedback on this question: http://stackoverflow.com/questions/4795385/how-do-you-use-bcrypt-for-hashing-passwords-in-php

-----UPDATE----- Ok latest update, I found a another really great article that helps a little extra: http://crackstation.net/hashing-security.htm

Based on this, $6 is a secure option as well. Just know that it's SHA512 not Blowfish.

[panique]

A little note: Before this script used SHA512+maxSalt, it used SHA256, before that it used the $2y-blowfish-algorithm that is currently requested here by some people ;) ... I find this very interesting, as i implemented the sha-versions because people were complaining that the $2y-algorithm is not secure enough. @shackrock Yeah, i have read all those articles quite often... The thing is, there is no definitive, clear way-to-go.

I like this question [SHA512 vs. Blowfish] on StackOverflow, but as usually it has been closed due to "not constructive" (what's wrong with stackoverflow-moderators ?). http://stackoverflow.com/questions/1561174/sha512-vs-blowfish-and-bcrypt

In the next days I'll try to contact ircmaxwell/Antony Ferrara (who has just posted in this github issue and is the creator of the new PHP 5.5 password hashing API [in the PHP core code!]) and try to get a simple statement why BLOWFISH is the choice, and not SHA512. I'm confused because SHA512-salt-hashed strings are 118 chars, while BLOWFISH-salt-hashed strings are just 60 chars.

[shackrock]

@panique I actually didn't realize you made the change to $6 during this thread's life... sorry about that.

When I created this thread, you were using $2a which is known to be less secure by PHP the devs. I actually believe that the latest commit may very will be a good hashing algo... but yea, I think it's best to get in touch with @ircmaxell and review his password compatibility script with yours, to see if anything was missed - and if so, if it's necessary and why.

If nothing else, as a developer I think I'd love to have the compatibility functions used so that when php 5.5 comes out, I can just swap out @ircmaxell 's library for PHP's native functions with ease.

Thanks

[J7mbo]

Can I just point out that, you include password_compat.php in your bootstrap, making this global state.

And this isn't bad, because the functions used are named exactly the same as the functions to be released with PHP 5.5. The point is that, once 5.5 comes out, you simply remove the include "password_compat.php" and it'll all still work (you don't need to rename any functions or anything, they're identical).

So, this library is the equivalent of core php functions, which are most of the time the way forward :)

I just implemented this in my own project, and it was so simple. Literally two method calls :)

[panique]

@J7mbo Yeah I know ;) and i find this library really interesting, but the current state of the password_compat library is hard to read and hard to understand (i mean the code itself). so the question is: should we include this, and tell the users not to touch this part of the script, or writing a reduced version of this (which is also bad because then it's not an equivalent of the PHP functions anymore). And: is it really necessary to use/write something that runs seamless on PHP 5.3, 5.4 and 5.5 ? Let's think about this for some time, as this is a big decision!

[J7mbo]

@panique Imho you're already including a custom hashing function that you need to tell your users not to touch - how is this anything different?

[panique]

@J7mbo Good point!

[shackrock]

@panique glad to read about the upcoming changes. If you can please post in this thread when it's done so I'm notified, it would much appreciated, I'd like to try it out.

Don't forget to change the marketing text here: http://www.php-login.net/

Thanks again.

[panique]

You can already test this when you download the develop branch (you can change the branch on the upper left corner of the github project home screen).

[ircmaxell]

Additionally, this line should be removed: https://github.com/panique/php-login/blob/develop/1-minimal/classes/Registration.php#L102

// cut password to 1024 chars to prevent too much calculation
$this->user_password        = substr($this->user_password, 0, 1024);

Additionally, this should be removed as well: https://github.com/panique/php-login/blob/develop/1-minimal/classes/Registration.php#L99

$this->user_password        = $this->db_connection->real_escape_string($_POST['user_password_new']);

Never edit the password submitted by the user, and never escape data that isn't going in the database...

[panique]

@ircmaxell Merci! ;) By the way, this 1024 limiter was implemented because i'm afraid that extremely long passwords [let's say somebody maxes out the POST data limit] could slow down the crypt() function massivly.

[ircmaxell]

@panique Long data won't affect crypt(). In fact, anything past 72 chars is basically ignored with bcrypt anyway. Here's why that's not a problem. But I wouldn't do that on your end. Let the hashing algorithm deal with that...

[panique]

Done! Thanks for all your feedback & guidance. Find the changes in the develop branch and some notices in the changelog.