deprecated-packages / pehapkari.cz-old

Website of Czech and Slovak PHP Community
https://pehapkari.cz
31 stars 85 forks source link

Suggest better way of rehashing passwords in Symfony #221

Closed spaze closed 7 years ago

spaze commented 7 years ago

Hey, the process outlined in https://pehapkari.cz/blog/2017/02/06/how-to-rehash-legacy-passwords-in-symfony/ is sub-optimal. Here are the reasons, mostly copied from my comment below the article. Sorry I can't help with the code.

You should not keep hashes in your database unless they are one of

So you want to rehash all the passwords at once using a batch job or similar. Do not keep passwords and legacy hashes in use until users log in, because they might not log in even if they actually use the site. Remember the "permanent login" checkbox you've added few months ago? Users can go months without actually typing the password.

So here's what you should do, and what the article should suggest instead:

  1. Rehash all the password hashes you have with let's say bcrypt, mark these in db
  2. Rehashing might take a while so update your code to handle both legacy hashes and bcrypt(legacy hashes)
  3. Since you've also stored the hash type you don't need to do the double check, you know exactly which hash verification algo to use
  4. The "rehash all" script might also crash for example, and you want to skip the already hashed records, so saving the hash type is not optional
  5. When the user logs in, after successful password verification, or when they set their new password after a password reset, or when they are changing the password, store just bcrypt(plaintext password)
  6. Make the code work with just bcrypt hashes

So, you need to able to handle three types of hashes, depending of the type attribute:

  1. legacy(password)
  2. bcrypt(legacy(password))
  3. bcrypt(password)

Unfortunately, Symfony does not support auto upgrades for hashes created with password_hash so it's not possible to create automatically upgraded hashes. In other cases we should plug in password_needs_rehash after the verification, so it can be automatically upgraded when needed.

So, remember:

TomasVotruba commented 7 years ago

Ping @ikvasnica

TomasVotruba commented 7 years ago

I think this should be written on the blog. Could I rephrase this into post? Me as reviewer, you as author.

spaze commented 7 years ago

@TomasVotruba I don't know Symfony that much to actually write the code to do what my original post says, sorry. What I'd like to see is a note added to @ikvasnica's piece saying there are different ways to do it (and also maybe slightly better ways :-)) And then somebody write a post following this advice. I don't really mind who but if @ikvasnica would like to I'd be happy panda. I think Ivan said something along that lines in a comment below his post.

But of course, if you want rephrase this bug into a post, feel free to do it. I think it's just not worth it without the code. Or maybe it is, we could have part one – the general thing outlined in this bug report, then some other parts for particular frameworks. So this is me thinking out loud :-)

TomasVotruba commented 7 years ago

@spaze Not related to Symfony, just standalone general PHP "how to passwords" post.

It could be linked from original post, but also useful for other projects to read. Sb using Symfony would not read this if hidden in Symfony post.

I like the out loud thinking, doing the same here :)

ikvasnica commented 7 years ago

@TomasVotruba I suggest a code example in a framework. In this scenario, you have a login system, saving user-related stuff into the database, I don't know if it's a good idea to use just a plain PHP example. Do you have any idea?

Or maybe a general PHP step-by-step tutorial, basically an extended version of the @spaze's post, plus a link to GitHub on a Symfony example?

I can write the post later this week (perhaps over the weekend).

TomasVotruba commented 7 years ago

@ikvasnica

@TomasVotruba I suggest a code example in a framework. In this scenario, you have a login system, saving user-related stuff into the database, I don't know if it's a good idea to use just a plain PHP example. Do you have any idea?

Good idea. I can add my part for Nette, so people can relate to it.

I can write the post later this week (perhaps over the weekend).

Awesome. We can make writing hackathon over Sunday, coming to Brno @posobota :)

TomasVotruba commented 7 years ago

Closing for now.

Feel free to reopen, if somebody will start writing a post.

spaze commented 7 years ago

As the article still has issues I've at least created #249 to highlight my comment that it can be done in a better and more secure way.

TomasVotruba commented 7 years ago

@spaze Perfect solution! Feel free to merge it.