Merge pull request #77 from glensc/password_hash

refactor password handling

composer.json

@@ -51,6 +51,7 @@
 		"ext-session": "*",
 		"ext-spl": "*",
 		"fonts/liberation":"*",
+		"ircmaxell/password-compat": "~1.0.4",
 		"ircmaxell/random-lib": "~1.1.0",
 		"pear-pear.php.net/mail_mimedecode": "*",
 		"pear-pear.php.net/math_stats": "*",

htdocs/preferences.php

@@ -72,12 +72,30 @@
 } elseif ($cat == 'update_email') {
     $res = User::updateEmail($usr_id);
 } elseif ($cat == 'update_password') {
-    $res = Auth::updatePassword($usr_id, $_POST['new_password'], $_POST['confirm_password']);
+    // verify current password
+    if (!Auth::isCorrectPassword(Auth::getUserLogin(), $_POST['password'])) {
+        Misc::setMessage(ev_gettext('Incorrect password'), Misc::MSG_ERROR);
+        $res = -3;
+    } elseif ($_POST['new_password'] != $_POST['confirm_password']) {
+        Misc::setMessage(ev_gettext('New passwords mismatch'), Misc::MSG_ERROR);
+        $res = -2;
+    } elseif ($_POST['password'] == $_POST['new_password']) {
+        Misc::setMessage(ev_gettext('Please set different password than current'), Misc::MSG_ERROR);
+        $res = -2;
+    } else {
+        try {
+            User::updatePassword($usr_id, $_POST['new_password']);
+            $res = 1;
+        } catch (Exception $e) {
+            error_log($e->getMessage());
+            $res = -1;
+        }
+    }
 }
 
 if ($res == 1) {
     Misc::setMessage(ev_gettext('Your information has been updated'));
-} elseif ($res == -1) {
+} elseif ($res !== null) {
     Misc::setMessage(ev_gettext('Sorry, there was an error updating your information'), Misc::MSG_ERROR);
 }
 

lib/eventum/auth/AuthPassword.php

@@ -0,0 +1,96 @@
+<?php
+/* vim: set expandtab tabstop=4 shiftwidth=4 encoding=utf-8: */
+// +----------------------------------------------------------------------+
+// | Eventum - Issue Tracking System                                      |
+// +----------------------------------------------------------------------+
+// | Copyright (c) 2015 Eventum Team.                                     |
+// |                                                                      |
+// | This program is free software; you can redistribute it and/or modify |
+// | it under the terms of the GNU General Public License as published by |
+// | the Free Software Foundation; either version 2 of the License, or    |
+// | (at your option) any later version.                                  |
+// |                                                                      |
+// | This program is distributed in the hope that it will be useful,      |
+// | but WITHOUT ANY WARRANTY; without even the implied warranty of       |
+// | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the        |
+// | GNU General Public License for more details.                         |
+// |                                                                      |
+// | You should have received a copy of the GNU General Public License    |
+// | along with this program; if not, write to:                           |
+// |                                                                      |
+// | Free Software Foundation, Inc.                                       |
+// | 51 Franklin Street, Suite 330                                        |
+// | Boston, MA 02110-1301, USA.                                          |
+// +----------------------------------------------------------------------+
+// | Authors: Elan Ruusamäe <glen@delfi.ee>                               |
+// +----------------------------------------------------------------------+
+
+/**
+ * Class dealing with user passwords
+ */
+class AuthPassword
+{
+    /**
+     * Hash the password
+     *
+     * @param string $password The password to hash
+     * @return string The hashed password, throws on error.
+     * @throws RuntimeException
+     */
+    public static function hash($password)
+    {
+        $res = password_hash($password, PASSWORD_DEFAULT);
+        if (!$res) {
+            throw new RuntimeException("password hashing failed");
+        }
+        return $res;
+    }
+
+    /**
+     * Verify a password against a hash using a timing attack resistant approach
+     *
+     * @param string $hash The hash to verify against
+     * @param string $password The password to verify
+     * @return boolean If the password matches the hash
+     * @throws InvalidArgumentException in case non-strings were passed as hash or password
+     */
+    public static function verify($password, $hash)
+    {
+        if (!is_string($password) || !is_string($hash)) {
+            throw new InvalidArgumentException("password and hash need to be strings");
+        }
+
+        // verify passwords in constant time, i.e always do all checks
+        $cmp = 0;
+
+        $cmp |= (int)password_verify($password, $hash);
+
+        // legacy authentication methods
+        $cmp |= (int)self::cmp($hash, base64_encode(pack('H*', md5($password))));
+        $cmp |= (int)self::cmp($hash, md5($password));
+
+        return (bool)$cmp;
+    }
+
+    /**
+     * Compare strings using a timing attack resistant approach
+     *
+     * @param string $str1
+     * @param string $str2
+     * @return bool
+     */
+    private static function cmp($str1, $str2)
+    {
+        if (Misc::countBytes($str1) != Misc::countBytes($str2) || Misc::countBytes($str1) <= 13) {
+            return false;
+        }
+
+        $status = 0;
+        $length = Misc::countBytes($str1);
+        for ($i = 0; $i < $length; $i++) {
+            $status |= (ord($str1[$i]) ^ ord($str2[$i]));
+        }
+
+        return $status === 0;
+    }
+}

lib/eventum/auth/class.mysql_auth_backend.php

@@ -44,22 +44,21 @@ public function verifyPassword($login, $password)
     {
         $usr_id = User::getUserIDByEmail($login, true);
         $user = User::getDetails($usr_id);
-        if ($user['usr_password'] == Auth::hashPassword($password)) {
-            self::resetFailedLogins($usr_id);
 
+        if (AuthPassword::verify($password, $user['usr_password'])) {
+            self::resetFailedLogins($usr_id);
             return true;
-        } else {
-            self::incrementFailedLogins($usr_id);
-
-            return false;
         }
+
+        self::incrementFailedLogins($usr_id);
+        return false;
     }
 
     /**
      * Method used to update the account password for a specific user.
      *
      * @param   integer $usr_id The user ID
-     * @param   string  $password The password.
+     * @param   string $password The password.
      * @return  boolean
      */
     public function updatePassword($usr_id, $password)
@@ -70,14 +69,14 @@ public function updatePassword($usr_id, $password)
                     usr_password=?
                  WHERE
                     usr_id=?';
-        $params = array(Auth::hashPassword($password), $usr_id);
+        $params = array(AuthPassword::hash($password), $usr_id);
         try {
             DB_Helper::getInstance()->query($stmt, $params);
         } catch (DbException $e) {
             return false;
         }
 
-        # NOTE: this will say updated failed if password is identical to old one
+        # NOTE: this will say update failed if password is identical to old one
         $updated = DB_Helper::getInstance()->affectedRows();
 
         return $updated > 0;
@@ -88,7 +87,7 @@ public function getUserIDByLogin($login)
         return User::getUserIDByEmail($login, true);
     }
 
-     /**
+    /**
      * Increment the failed logins attempts for this user
      *
      * @param   integer $usr_id The ID of the user
@@ -203,6 +202,7 @@ public function checkAuthentication()
 
     /**
      * Called when a user logs out.
+     *
      * @return mixed
      */
     public function logout()

lib/eventum/class.auth.php

@@ -349,32 +349,6 @@ public static function isCorrectPassword($email, $password)
         return self::getAuthBackend()->verifyPassword($email, $password);
     }
 
-    /**
-     * Method used to update the account password for a specific user.
-     *
-     * @param   integer $usr_id The user ID
-     * @param $password
-     * @param $password_confirm
-     * @param   boolean $send_notification Whether to send the notification email or not
-     * @return  integer 1 if the update worked, -1 otherwise
-     */
-    public static function updatePassword($usr_id, $password, $password_confirm, $send_notification = false)
-    {
-        if ($password != $password_confirm) {
-            return -2;
-        }
-
-        $res = self::getAuthBackend()->updatePassword($usr_id, $password);
-        if (!$res) {
-            return -1;
-        }
-        if ($send_notification) {
-            Notification::notifyUserPassword($usr_id, $password);
-        }
-
-        return 1;
-    }
-
     /**
      * Returns the true if the account is currently locked becouse of Back-Off lock
      *
@@ -576,22 +550,6 @@ public static function getFallBackAuthBackend()
         return $instance;
     }
 
-    /**
-     * Hashes the password according to APP_HASH_TYPE constant
-     *
-     * @param   string $password The plain text password
-     * @return  string The hashed password
-     */
-    public static function hashPassword($password)
-    {
-        if (APP_HASH_TYPE == 'MD5-64') {
-            return base64_encode(pack('H*', md5($password)));
-        } else {
-            // default to md5
-            return md5($password);
-        }
-    }
-
     /**
      * Returns the user ID for the specified login. This can be the email address, an alias,
      * the external login id or any other info the backend can handle.