Password Hashing

[ximex]

Drop the lib phpass and use the nativ password hashing functionality of php with the fallback lib password-compat to support PHP 5.3.7+

I will leave phpass in admidio the get backbard compatibility. If the user sucessfully logged in the next time, the password get rehashed with the new algorithm. So i hope to really remove phpass with Admidio 3.2 or 3.3

I only need some infos to the now used algorithms to detect which algo was used to got this hash:

• md5 (32 chars; HEX)
• private hash (34 chars; starts with: $P$)
• CRYPT_EXTDES (20 chars; starts with: ``)
• bcrypt (60 chars; starts with: $2$)

I will use the option PASSWORD_DEFAULT to get always the strongest possible algorithm (today only bcrypt)

If i understand the current code correct: it isn't possible to enter passwords longer than 30 chars because of the need to check if the password string is longer than 30 than it is allready an hash or if it less than it is the original unhashed password. I will try to fix that. there should never be a limit of password length or limitation to some chars

[Fasse]

:thumbsup:

There is no max length for passwords. We only save a hash and the hash is not longer then 35 chars. Also if your password has 100 chars the hash has 30. We never saved a unhashed password. In former versions we save a md5 hash password and since version 2.1 we use phpass.

I support your idea of a new hash if user has stored pw in old way. So if checkLogin was successfull then we should update password to new hash if its the old way. But we need a way to identify the password hash method. Now phpass stored a $ at first chars. If $ then phpass if not then md5. Now you must look if the new method has also a identifier.

We could not removed phpass because you always have old passwords. you dont know when users logged in again. Maybe after 5 years someone logged in again and had stored his password with phpass.

[ximex]

I want to (nearly) remove a max length limit. (i want to set it at least to 64 or 128 chars) the hashes should be saved completly (no trim at the 30th char) I don't mean that we saved unhased passwords. but what did this strlen($newValue) < 30 do in this line: https://github.com/Admidio/admidio/blob/master/adm_program/system/classes/tableusers.php#L204

i allready postet the possible detection patterns. i only need more infos to the _... hashes that phpass produce with his fallback. https://github.com/Admidio/admidio/blob/master/adm_program/system/classes/user.php#L106-L107

I anyway think we could drop phpass in the future. There are some possibilities:

• Check if all passwords are allready rehashed with new algorithm
• Warn the admin on update if not all are rehashed
• create new random passwords and send them to the emails of the users (password reset)

I want to make a new class PasswordHash where all checks and fallbacks, ... are moved to.

[ximex]

Some Links:

• http://www.php.net/security/crypt_blowfish.php
• https://secure.php.net/manual/de/function.crypt.php

What i found out:

• $2a$ and $2x$ are old identifiers for insecure implementations of crypt_blowfish
• $2y$ is the secure impl. of crypt_blowfish
• $P$ and $H$ are "portable hashes" from phpass (new PasswordHash($cost, TRUE))
• if you set $portable = true, you always get a "portable hash" (like a "use fallback yes" param)
• password_needs_rehash() always returns true if it isn't the defined algo and options (always true if hashed with phpass) -> helps a lot
• password_get_info() didn't know any of phpass and the broken crypt_blowfish hashes (_..., $2a$, $2x$, $P$, $H$)
• "portable hashes"/"private hashes" are a self made implementation from phpass with a salt and md5() algo
• "portable hashes"/"private hashes" (34 chars; starts with: $P$; used md5())
• CRYPT_EXT_DES (20 chars; starts with: _)
• CRYPT_BLOWFISH (60 chars; starts with: $2a$) (with phpass)

• maybe wo should really limit the password length to 64 chars.

  Achtung: Die Verwendung des CRYPT_BLOWFISH-Algorithmus hat zur Folge, dass der str-Parameter auf eine Länge von maximal 72 Zeichen gekürzt wird.

Summary: We only should get 4 different hashes:

• _... 20 chars -> "CRYPT_EXT_DES"
• $P$ or $H$ -> 34 chars -> "private/portable hash"
• $2a$ or $2x$ 60 chars -> "CRYPT_BLOWFISH" insecure
• $2y$ 60 chars -> "CRYPT_BLOWFISH" secure

[Fasse]

I don't know why there is the limit for 30 chars for usr_new_password. This field is only used if a new password is generated through "password forgotten". This limit could be removed.

If blowfish needs 60 chars for his hash, then we make varchar(60) fields for usr_password and usr_new_password. No problem.

Is CRYPT_BLOWFISH also a portable hash? This is important because if the database is moved to a new server we must also be able to build the same hash for login.

If possible we should only use one new method for password encryption.

A own class for password handling could be useful.

[ximex]

I don't know that there is a hashing function that you couldn't move them to an other db? they are only strings...

Ok i got it why there is this 30 chars check: Look at: https://github.com/Admidio/admidio/blob/master/adm_program/system/password_activation.php#L32 if you move the usr_new_password to usr_password the hashed password get rehashed. if think we need a $user->applyNewPassword() or something else for the move process.

[ximex]

see #91

[ximex]

merged