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.

6 files changed, 106 insertions, 85 deletions

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 @@
 <?php
 
 use r7r\ste;
+use r7r\cms\sys\PasswordHash;
 
 define("SETUP", true);
 
@@ -8,7 +9,6 @@ require_once(dirname(__FILE__) . "/../vendor/autoload.php");
 require_once(dirname(__FILE__) . "/../sys/init_ste.php");
 require_once(dirname(__FILE__) . "/../sys/translation.php");
 require_once(dirname(__FILE__) . "/../sys/db.php");
-require_once(dirname(__FILE__) . "/../sys/pwhash.php");
 require_once(dirname(__FILE__) . "/../languages.php");
 require_once(dirname(__FILE__) . "/create_tables.php");
 
@@ -221,7 +221,7 @@ STYLE;
             $ratatoeskr_settings->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 @@
+<?php
+
+
+namespace r7r\cms\sys;
+
+/**
+ * Functions for creating and checking password hashes. Mostly wrappers around php's builtin password_\* functions but
+ * can also verify our old legacy password hash.
+ */
+class PasswordHash
+{
+    private const PASSWORD_ALGO = \PASSWORD_DEFAULT;
+
+    /** @var bool */
+    private $isLegacy;
+
+    /** @var string */
+    private $hashData;
+
+    private function __construct(bool $isLegacy, string $hashData)
+    {
+        $this->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   - <PasswordHash> 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   - <PasswordHash> 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 @@
-<?php
-/*
- * File: ratatoeskr/sys/pwhash.php
- *
- * Hashing passwords
- *
- * License:
- * This file is part of Ratatöskr.
- * Ratatöskr is licensed unter the MIT / X11 License.
- * See "ratatoeskr/licenses/ratatoeskr" for more information.
- */
-
-/*
- * Class: PasswordHash
- * Contains static functions for password hashes.
- * Is just used as a namespace, can not be created.
- *
- * It should be fairly difficult to break these salted hashes via bruteforce attacks.
- */
-class PasswordHash
-{
-    private function __construct()
-    {
-    } /* Prevent construction */
-
-    private static $saltlen_min = 20;
-    private static $saltlen_max = 30;
-    private static $iterations_min = 200;
-    private static $iterations_max = 1000;
-
-    private static function hash($data, $salt, $iterations)
-    {
-        $hash = $data . $salt;
-        for ($i = $iterations ;$i--;) {
-            $hash = sha1($data . $hash . $salt, (bool) $i);
-        }
-        return $iterations . '$' . bin2hex($salt) . '$' . $hash;
-    }
-
-    /*
-     * Function: create
-     * Create a password hash string.
-     *
-     * Parameters:
-     *  $password - The password (or other data) to hash.
-     *
-     * Returns:
-     *  The salted hash as a string.
-     */
-    public static function create($password)
-    {
-        $salt = "";
-        $saltlen = mt_rand(self::$saltlen_min, self::$saltlen_max);
-        for ($i = 0; $i < $saltlen; $i++) {
-            $salt .= chr(mt_rand(0, 255));
-        }
-        return self::hash($password, $salt, mt_rand(self::$iterations_min, self::$iterations_max));
-    }
-
-    /*
-     * Function: validate
-     * Validate a salted hash.
-     *
-     * Parameters:
-     *  $password - The password to test.
-     *  $pwhash   - The hash to test against.
-     *
-     * Returns:
-     *  True, if $password was correct, False otherwise.
-     */
-    public static function validate($password, $pwhash)
-    {
-        list($iterations, $hexsalt) = explode('$', $pwhash);
-        return self::hash($password, pack("H*", $hexsalt), $iterations) == $pwhash;
-    }
-}
diff --git a/setup.php b/setup.php
index f496dfe..47f2202 100644
--- a/setup.php
+++ b/setup.php
@@ -69,7 +69,6 @@ $files = [
     "/ratatoeskr/sys/pluginpackage.php",
     "/ratatoeskr/sys/db.php",
     "/ratatoeskr/sys/utils.php",
-    "/ratatoeskr/sys/pwhash.php",
     "/ratatoeskr/sys/init_ste.php",
     "/ratatoeskr/sys/models.php",
     "/ratatoeskr/sys/textprocessors.php",