From 5e347e4efaa81c2108256dc927208cd55dc10baa Mon Sep 17 00:00:00 2001 From: Laria Carolin Chabowski Date: Fri, 25 Sep 2020 23:09:31 +0200 Subject: Use password_hash() and friends to hash and verify passwords Previously I rolled my own password hashing function. While it at least used some sort of salt, it's still a terrible idea. The newly created class PasswordHash wraps the password_hash() family of functions but can also check the old password hash format (to distinguish them, the new password hashes are prefixed with a '!'). In PasswordHash::needsRehash we then always report an hash of the old format as being in need of a rehash. That way, these old hashes will be replaced the next time the user successfully logs in. --- ratatoeskr/backend.php | 15 +++++-- ratatoeskr/setup/setup.php | 4 +- ratatoeskr/sys/PasswordHash.php | 91 +++++++++++++++++++++++++++++++++++++++++ ratatoeskr/sys/models.php | 4 +- ratatoeskr/sys/pwhash.php | 76 ---------------------------------- 5 files changed, 106 insertions(+), 84 deletions(-) create mode 100644 ratatoeskr/sys/PasswordHash.php delete mode 100644 ratatoeskr/sys/pwhash.php (limited to 'ratatoeskr') diff --git a/ratatoeskr/backend.php b/ratatoeskr/backend.php index 12a57de..fefa817 100644 --- a/ratatoeskr/backend.php +++ b/ratatoeskr/backend.php @@ -15,9 +15,9 @@ use r7r\ste\Transcompiler; use r7r\ste\Parser; use r7r\cms\sys\Env; use r7r\cms\sys\Esc; +use r7r\cms\sys\PasswordHash; require_once(dirname(__FILE__) . "/sys/models.php"); -require_once(dirname(__FILE__) . "/sys/pwhash.php"); require_once(dirname(__FILE__) . "/sys/textprocessors.php"); require_once(dirname(__FILE__) . "/sys/plugin_api.php"); require_once(dirname(__FILE__) . "/languages.php"); @@ -109,9 +109,16 @@ function build_backend_subactions() if (!empty($_POST["user"])) { try { $user = User::by_name($_POST["user"]); - if (!PasswordHash::validate($_POST["password"], $user->pwhash)) { + $password = (string)$_POST["password"]; + if (!PasswordHash::verify($password, $user->pwhash)) { throw new Exception(); } + + if (PasswordHash::needsRehash($user->pwhash)) { + $user->pwhash = PasswordHash::hash($password); + $user->save(); + } + if (!$user->member_of($admin_grp)) { throw new Exception(); } @@ -1342,7 +1349,7 @@ function build_backend_subactions() User::by_name($_POST["username"]); $ste->vars["error"] = $translation["user_already_exists"]; } catch (DoesNotExistError $e) { - User::create($_POST["username"], PasswordHash::create($_POST["initial_password"])); + User::create($_POST["username"], PasswordHash::hash($_POST["initial_password"])); $ste->vars["success"] = $translation["successfully_created_user"]; } } @@ -1466,7 +1473,7 @@ function build_backend_subactions() /* New Password? */ if (isset($_POST["new_password"])) { - $pwhash = PasswordHash::create($_POST["password"]); + $pwhash = PasswordHash::hash($_POST["password"]); $user->pwhash = $pwhash; if ($user->get_id() == $data["user"]->get_id()) { $_SESSION["ratatoeskr_pwhash"] = $pwhash; diff --git a/ratatoeskr/setup/setup.php b/ratatoeskr/setup/setup.php index 23205b8..c18fff0 100644 --- a/ratatoeskr/setup/setup.php +++ b/ratatoeskr/setup/setup.php @@ -1,6 +1,7 @@ save(); $admingrp = Group::create("admins"); - $admin = user::create($_POST["admin_username"], PasswordHash::create($_POST["admin_init_password"])); + $admin = User::create($_POST["admin_username"], PasswordHash::hash($_POST["admin_init_password"])); $admin->save(); $admingrp->include_user($admin); diff --git a/ratatoeskr/sys/PasswordHash.php b/ratatoeskr/sys/PasswordHash.php new file mode 100644 index 0000000..9228642 --- /dev/null +++ b/ratatoeskr/sys/PasswordHash.php @@ -0,0 +1,91 @@ +isLegacy = $isLegacy; + $this->hashData = $hashData; + } + + private static function verifyLegacy(string $password, string $pwhash): bool + { + list($iterations, $hexsalt) = explode('$', $pwhash); + return self::hashLegacy($password, pack("H*", $hexsalt), $iterations) == $pwhash; + } + + private static function hashLegacy(string $data, $salt, string $iterations): string + { + $hash = $data . $salt; + for ($i = $iterations ;$i--;) { + $hash = sha1($data . $hash . $salt, (bool) $i); + } + return $iterations . '$' . bin2hex($salt) . '$' . $hash; + } + + private function format(): string + { + return $this->isLegacy + ? $this->hashData + : '!' . $this->hashData; + } + + private static function parse(string $s): self + { + return substr($s, 0, 1) === '!' + ? new self(false, substr($s, 1)) + : new self(true, $s); + } + + /** + * Verifies that a given password is valid for the given hash + * @param string $password + * @param string $hash + * @return bool + */ + public static function verify(string $password, string $hash): bool + { + $hash = self::parse($hash); + return $hash->isLegacy + ? self::verifyLegacy($password, $hash->hashData) + : password_verify($password, $hash->hashData); + } + + /** + * Creates a hash for a password + * @param string $password + * @return string Treat this as opaque data. Don't rely on it being in a certain format, it might change in the future. + */ + public static function hash(string $password): string + { + return (new self(false, password_hash($password, self::PASSWORD_ALGO)))->format(); + } + + /** + * Checks, if a given hash should be recomputed (because it's not considered secure any more) if the password is known. + * @param string $hash + * @return bool + */ + public static function needsRehash(string $hash): bool + { + $hash = self::parse($hash); + return $hash->isLegacy + ? true + : password_needs_rehash($hash->hashData, self::PASSWORD_ALGO); + } +} diff --git a/ratatoeskr/sys/models.php b/ratatoeskr/sys/models.php index b820e6f..1b196d6 100644 --- a/ratatoeskr/sys/models.php +++ b/ratatoeskr/sys/models.php @@ -253,7 +253,7 @@ class User extends BySQLRowEnabled * Variables: Public class properties * * $username - The username. - * $pwhash - of the password. + * $pwhash - Hash of the password. * $mail - E-Mail-address. * $fullname - The full name of the user. * $language - Users language @@ -270,7 +270,7 @@ class User extends BySQLRowEnabled * * Parameters: * $username - The username - * $pwhash - of the password + * $pwhash - Hash of the password * * Returns: * An User object diff --git a/ratatoeskr/sys/pwhash.php b/ratatoeskr/sys/pwhash.php deleted file mode 100644 index f52ae03..0000000 --- a/ratatoeskr/sys/pwhash.php +++ /dev/null @@ -1,76 +0,0 @@ -