Question about SHA1/256 (github issue #60)

Farjo

  • "a valuable asset to the community"
  • Posts: 492

CerealGuy

  • Posts: 343
Re: Question about SHA1/256 (github issue #60)
« Reply #1, on April 9th, 2017, 10:35 PM »Last edited on April 9th, 2017, 10:44 PM
Bcrypt would be the algorithm i would choose too. But I don't know what i should think of the password_hash() functions. It's always a bit critical if you change the password hashing algorithm. Because in the database you still have the hashes with the old algorithm, so you have to hash it once with the old algorithm to make sure it's the correct password and after that insert/update the hashed password with the new algorithm. It's getting even more complicated because wedge already hashes the password on client side to don't transfer the password in plaintext (which i like). I want to change it, but it's a bit critical and I'm not an expert for this. Therefore I want to do it well planned and with a bit more knowledge as i have in the moment. The thing I worry about with password_hash() is that if you use a database backup from a new php version on an old php version where the password_hash() function would use a different (less secure) algorithm, this would maybe break things. But I only scanned the documentation.
Besides that, it's not super important to change the hashing algorithm now, as we "only" use it for passwords. So if you don't have to fear NSA or something like that, your passwords aren't currently in danger.

Farjo

  • "a valuable asset to the community"
  • Posts: 492
Re: Question about SHA1/256 (github issue #60)
« Reply #2, on April 15th, 2017, 01:53 AM »
Thanks for the reply, very interesting. How does Wedge hash on client side?

Perhaps my next question is to the Elkarte crew - why, after you'd looked into it all, did you choose SHA256 over bcript / password_hash() ?

I see that Nao has posted to issue #60 and I will watch with interest.

CerealGuy

  • Posts: 343
Re: Question about SHA1/256 (github issue #60)
« Reply #3, on April 15th, 2017, 11:10 AM »
On client side, if the browser supports js, the password gets hashed once without any salt and with sha1. If the browser doesn't support js, it doesnt get hashed. The hashed or unhashed password gets send to the server, if it's unhashed/plain, wedge hashes it first. Now it's in any ways hashed once with sha1. Now it gets hashed again with sha1 but this time with the salt connected to this user. If the hash is now the same as in the Database, it's the correct password.
This hashing on the client side was quite cool when you didn't had ssl in all places because you don't send your password in plain. Still if someone can hijack the Session (mitm attack) he/she can readout the hashed password and log in with that again at any time (until you Change your password).
I didn't dig through all the code, so not 100% sure it works exactly like this. Especially this 2x hashing + salting is interesting, it makes all of it a bit harder to crack.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Question about SHA1/256 (github issue #60)
« Reply #4, on April 16th, 2017, 04:13 PM »
If I do password_hash() with PASSWORD_DEFAULT, then it means I can't be sure what algorithm will be used to hash the password, right..?

So, how exactly can I hash the password on the client side if I don't know which kind of crypt system PHP will use to compare it with my own hash...?

CerealGuy

  • Posts: 343
Re: Question about SHA1/256 (github issue #60)
« Reply #5, on April 17th, 2017, 05:20 PM »
Quote from Nao on April 16th, 2017, 04:13 PM
If I do password_hash() with PASSWORD_DEFAULT, then it means I can't be sure what algorithm will be used to hash the password, right..?

So, how exactly can I hash the password on the client side if I don't know which kind of crypt system PHP will use to compare it with my own hash...?
As long as you always hash it with the same algorithm on client side, it wouldn't matter, because you always hash it again on server side. The problem i see is that the unclear nature of password_hash() would result in inconsistencies across different php versions. So you couldn't use a backup from a newer php version on one with a server with an older php version.

Suki

  • Posts: 59
Re: Question about SHA1/256 (github issue #60)
« Reply #6, on April 18th, 2017, 01:09 AM »
If there are any changes, it will be known pretty early, any change in the default algo will be made in the next full release after a proposed algo was included. Plenty of time IMO.

And you will always know what thing was changed and to what. Theres a similar problem if you chose an user defined algo, sooner or later you will have to upgrade your script to get rid of obsolete ones.

Nao

  • Dadman with a boy
  • Posts: 16,079

Suki

  • Posts: 59
Re: Question about SHA1/256 (github issue #60)
« Reply #8, on April 20th, 2017, 01:48 AM »
I pop up every once in a while and since I'm dealing with the same thing myself I chime in.


I wouldn't use bcrypt on the client side, too much of an issue and as far as I know it wasn't designed to do hashing but to encrypt.  Besides, everyone is going towards https so client hashing will become less of an issue.

So, use bycrypt yes but on server side only.

CerealGuy

  • Posts: 343
Re: Question about SHA1/256 (github issue #60)
« Reply #9, on May 1st, 2017, 04:45 PM »Last edited on May 1st, 2017, 05:05 PM
Quote from Suki on April 20th, 2017, 01:48 AM
I pop up every once in a while and since I'm dealing with the same thing myself I chime in.


I wouldn't use bcrypt on the client side, too much of an issue and as far as I know it wasn't designed to do hashing but to encrypt.  Besides, everyone is going towards https so client hashing will become less of an issue.

So, use bycrypt yes but on server side only.
I liked the client side hashing, but could be hard to do on client side, because we would
need to pass costs and salts to client (if we use bcrypt). Or we use another algorithm on client
side, which could make things worse. Or we use stable values for this. Costs of 20, salt of blubb.
As long as we don't get hash collissions on the first round, this shouldn't be a big problem?
Besides that, client side hashing doesn't  protect against mitm. Simply serving a evil js would
destroy all the benifits. Only scenario where this could help is if there's no ssl. But as soon
as an evil person can hook between server and user, this doesn't help anymore.

I looked into password_hash function, i think it's safe to use. Because in the resulting
hash there are information about the algorithm (bcrypt), the salt, and costs (some bcrypt
specific parameter which defines how many rounds of bcrypt shall be used to make brute
force attacks harder.) and therefore password_verify will take the right algorithm and parameters
to verfify the password.
Things to think about:
- how to define costs for bcrypt, Settings.php?
- How do we change the password values in running environments? We have to plan this for the future
(if the default hash algo changes) and for now.


EDIT:
Easiest thing would be to send the passwords always in plain text, on server side check if we have an old password saved with sha1, if so, hash it in the old way and verify it. If it's fine, hash it in the new way and update the value. If not, password's wrong, error message and so. If it's already with the new algo, just do password_verify.

Nao

  • Dadman with a boy
  • Posts: 16,079

CerealGuy

  • Posts: 343
Re: Question about SHA1/256 (github issue #60)
« Reply #11, on May 21st, 2017, 02:13 AM »Last edited on May 25th, 2017, 09:16 PM

Finally found some time to do this.It was actually not that hard as you may thought.
The password_hash/verify functions are very nice, definetly a good thing!

Commit:
https://github.com/C3realGuy/wedge/commit/e1eb7f2129974cca89c3cd81c76e579b8f63922b

PR (WIP):
https://github.com/Wedge/wedge/pull/68

What still need's to be done:
- Update database so that passwrd field is a VARCHAR(255) (see https://github.com/ircmaxell/password_compat#usage)
- Remove sha1.js
- test it


Only thing we still need to think about is how we now want to create the cookies. Currently this is done by doing some sha1 hashing over the password concatenated with the salt. Because we don't have a salt anymore, this is maybe bad (or not).
And i'm still unsure about dropping client side hashing. Maybe I will just switch to sha256 on client side?!

What's not working yet:
- Admin login