feature 2727: improve password security with the use of PasswordHash class.

This class performs salt and multiple iterations. Already used in Wordpress,
Drupal, phpBB and many other web applications.

$conf['pass_convert'] is replaced by $conf['password_hash'] + $conf['password_verify']


git-svn-id: http://piwigo.org/svn/trunk@18889 68402e56-0260-453c-a942-63ccdbb3a9ee

8 files changed, 378 insertions, 16 deletions

diff --git a/admin/include/functions_upgrade.php b/admin/include/functions_upgrade.php
index a5b3f7e5a..35e45b032 100644
--- a/admin/include/functions_upgrade.php
+++ b/admin/include/functions_upgrade.php
@@ -247,12 +247,7 @@ WHERE '.$conf['user_fields']['username'].'=\''.$username.'\'
   }
   $row = pwg_db_fetch_assoc(pwg_query($query));
 
-  if (!isset($conf['pass_convert']))
-  {
-    $conf['pass_convert'] = create_function('$s', 'return md5($s);');
-  }
-
-  if ($row['password'] != $conf['pass_convert']($password))
+  if (!$conf['password_verify']($password, $row['password']))
   {
     array_push($page['errors'], l10n('Invalid password!'));
   }
diff --git a/include/config_default.inc.php b/include/config_default.inc.php
index b093e2bd1..d9b807f36 100644
--- a/include/config_default.inc.php
+++ b/include/config_default.inc.php
@@ -506,12 +506,20 @@ $conf['user_fields'] = array(
   'email' => 'mail_address'
   );
 
-// pass_convert : function to crypt or hash the clear user password to store
-// it in the database
-$conf['pass_convert'] = create_function('$s', 'return md5($s);');
+// password_hash: function hash the clear user password to store it in the
+// database. The function takes only one parameter: the clear password.
+$conf['password_hash'] = 'pwg_password_hash';
+
+// password_verify: function that checks the password against its hash. The
+// function takes 2 mandatory parameter : clear password, hashed password +
+// an optional parameter user_id. The user_id is used to update the password
+// with the new hash introduced in Piwigo 2.5. See function
+// pwg_password_verify in include/functions_user.inc.php
+$conf['password_verify'] = 'pwg_password_verify';
 
 // guest_id : id of the anonymous user
 $conf['guest_id'] = 2;
+
 // default_user_id : id of user used for default value
 $conf['default_user_id'] = $conf['guest_id'];
 
diff --git a/include/functions_user.inc.php b/include/functions_user.inc.php
index 924233bd0..0ba720167 100644
--- a/include/functions_user.inc.php
+++ b/include/functions_user.inc.php
@@ -182,7 +182,7 @@ SELECT MAX('.$conf['user_fields']['id'].') + 1
       array(
         $conf['user_fields']['id'] => $next_id,
         $conf['user_fields']['username'] => pwg_db_real_escape_string($login),
-        $conf['user_fields']['password'] => $conf['pass_convert']($password),
+        $conf['user_fields']['password'] => $conf['password_hash']($password),
         $conf['user_fields']['email'] => $mail_address
         );
 
@@ -1095,6 +1095,76 @@ function auto_login() {
 }
 
 /**
+ * hashes a password, with the PasswordHash class from phpass security
+ * library. We use an "pwg_" prefix because function password_hash is
+ * planned for PHP 5.5. Code inspired from Wordpress.
+ *
+ * @param string $password Plain text user password to hash
+ * @return string The hash string of the password
+ */
+function pwg_password_hash($password)
+{
+  global $pwg_hasher;
+
+  if (empty($pwg_hasher))
+  {
+    require_once(PHPWG_ROOT_PATH.'include/passwordhash.class.php');
+    
+    // We use the portable hash feature from phpass because we can't be sure
+    // Piwigo runs on PHP 5.3+ (and won't run on an older version in the
+    // future)
+    $pwg_hasher = new PasswordHash(13, true);
+  }
+  
+  return $pwg_hasher->HashPassword($password);
+}
+
+/**
+ * Verifies a password, with the PasswordHash class from phpass security
+ * library. We use an "pwg_" prefix because function password_verify is
+ * planned for PHP 5.5. Code inspired from Wordpress.
+ *
+ * @param string $password Plain text user password to hash
+ * @param string $hash may be md5 or phpass hashed password
+ * @param integer $account_id only useful to update password hash from md5 to phpass
+ * @return string The hash string of the password
+ */
+function pwg_password_verify($password, $hash, $user_id=null)
+{
+  global $conf, $pwg_hasher;
+
+  // If the hash is still md5...
+  if (strlen($hash) <= 32)
+  {
+    $check = ($hash == md5($password));
+    
+    if ($check and isset($user_id) and !$conf['external_authentification'])
+    {
+      // Rehash using new hash.
+      $hash = pwg_password_hash($password);
+
+      single_update(
+        USERS_TABLE,
+        array('password' => $hash),
+        array('id' => $user_id)
+        );
+    }
+  }
+
+  // If the stored hash is longer than an MD5, presume the
+  // new style phpass portable hash.
+  if (empty($pwg_hasher))
+  {
+    require_once(PHPWG_ROOT_PATH.'include/passwordhash.class.php');
+    
+    // We use the portable hash feature
+    $pwg_hasher = new PasswordHash(13, true);
+  }
+
+  return $pwg_hasher->CheckPassword($password, $hash);
+}
+
+/**
  * Tries to login a user given username and password (must be MySql escaped)
  * return true on success
  */
@@ -1112,7 +1182,7 @@ SELECT '.$conf['user_fields']['id'].' AS id,
   WHERE '.$conf['user_fields']['username'].' = \''.pwg_db_real_escape_string($username).'\'
 ;';
   $row = pwg_db_fetch_assoc(pwg_query($query));
-  if ($row['password'] == $conf['pass_convert']($password))
+  if ($conf['password_verify']($password, $row['password'], $row['id']))
   {
     log_user($row['id'], $remember_me);
     trigger_action('login_success', stripslashes($username));
diff --git a/include/passwordhash.class.php b/include/passwordhash.class.php
new file mode 100644
index 000000000..12958c7f1
--- /dev/null
+++ b/include/passwordhash.class.php
@@ -0,0 +1,253 @@
+<?php
+#
+# Portable PHP password hashing framework.
+#
+# Version 0.3 / genuine.
+#
+# Written by Solar Designer <solar at openwall.com> in 2004-2006 and placed in
+# the public domain.  Revised in subsequent years, still public domain.
+#
+# There's absolutely no warranty.
+#
+# The homepage URL for this framework is:
+#
+#	http://www.openwall.com/phpass/
+#
+# Please be sure to update the Version line if you edit this file in any way.
+# It is suggested that you leave the main version number intact, but indicate
+# your project name (after the slash) and add your own revision information.
+#
+# Please do not change the "private" password hashing method implemented in
+# here, thereby making your hashes incompatible.  However, if you must, please
+# change the hash type identifier (the "$P$") to something different.
+#
+# Obviously, since this code is in the public domain, the above are not
+# requirements (there can be none), but merely suggestions.
+#
+class PasswordHash {
+	var $itoa64;
+	var $iteration_count_log2;
+	var $portable_hashes;
+	var $random_state;
+
+	function PasswordHash($iteration_count_log2, $portable_hashes)
+	{
+		$this->itoa64 = './0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
+
+		if ($iteration_count_log2 < 4 || $iteration_count_log2 > 31)
+			$iteration_count_log2 = 8;
+		$this->iteration_count_log2 = $iteration_count_log2;
+
+		$this->portable_hashes = $portable_hashes;
+
+		$this->random_state = microtime();
+		if (function_exists('getmypid'))
+			$this->random_state .= getmypid();
+	}
+
+	function get_random_bytes($count)
+	{
+		$output = '';
+		if (is_readable('/dev/urandom') &&
+		    ($fh = @fopen('/dev/urandom', 'rb'))) {
+			$output = fread($fh, $count);
+			fclose($fh);
+		}
+
+		if (strlen($output) < $count) {
+			$output = '';
+			for ($i = 0; $i < $count; $i += 16) {
+				$this->random_state =
+				    md5(microtime() . $this->random_state);
+				$output .=
+				    pack('H*', md5($this->random_state));
+			}
+			$output = substr($output, 0, $count);
+		}
+
+		return $output;
+	}
+
+	function encode64($input, $count)
+	{
+		$output = '';
+		$i = 0;
+		do {
+			$value = ord($input[$i++]);
+			$output .= $this->itoa64[$value & 0x3f];
+			if ($i < $count)
+				$value |= ord($input[$i]) << 8;
+			$output .= $this->itoa64[($value >> 6) & 0x3f];
+			if ($i++ >= $count)
+				break;
+			if ($i < $count)
+				$value |= ord($input[$i]) << 16;
+			$output .= $this->itoa64[($value >> 12) & 0x3f];
+			if ($i++ >= $count)
+				break;
+			$output .= $this->itoa64[($value >> 18) & 0x3f];
+		} while ($i < $count);
+
+		return $output;
+	}
+
+	function gensalt_private($input)
+	{
+		$output = '$P$';
+		$output .= $this->itoa64[min($this->iteration_count_log2 +
+			((PHP_VERSION >= '5') ? 5 : 3), 30)];
+		$output .= $this->encode64($input, 6);
+
+		return $output;
+	}
+
+	function crypt_private($password, $setting)
+	{
+		$output = '*0';
+		if (substr($setting, 0, 2) == $output)
+			$output = '*1';
+
+		$id = substr($setting, 0, 3);
+		# We use "$P$", phpBB3 uses "$H$" for the same thing
+		if ($id != '$P$' && $id != '$H$')
+			return $output;
+
+		$count_log2 = strpos($this->itoa64, $setting[3]);
+		if ($count_log2 < 7 || $count_log2 > 30)
+			return $output;
+
+		$count = 1 << $count_log2;
+
+		$salt = substr($setting, 4, 8);
+		if (strlen($salt) != 8)
+			return $output;
+
+		# We're kind of forced to use MD5 here since it's the only
+		# cryptographic primitive available in all versions of PHP
+		# currently in use.  To implement our own low-level crypto
+		# in PHP would result in much worse performance and
+		# consequently in lower iteration counts and hashes that are
+		# quicker to crack (by non-PHP code).
+		if (PHP_VERSION >= '5') {
+			$hash = md5($salt . $password, TRUE);
+			do {
+				$hash = md5($hash . $password, TRUE);
+			} while (--$count);
+		} else {
+			$hash = pack('H*', md5($salt . $password));
+			do {
+				$hash = pack('H*', md5($hash . $password));
+			} while (--$count);
+		}
+
+		$output = substr($setting, 0, 12);
+		$output .= $this->encode64($hash, 16);
+
+		return $output;
+	}
+
+	function gensalt_extended($input)
+	{
+		$count_log2 = min($this->iteration_count_log2 + 8, 24);
+		# This should be odd to not reveal weak DES keys, and the
+		# maximum valid value is (2**24 - 1) which is odd anyway.
+		$count = (1 << $count_log2) - 1;
+
+		$output = '_';
+		$output .= $this->itoa64[$count & 0x3f];
+		$output .= $this->itoa64[($count >> 6) & 0x3f];
+		$output .= $this->itoa64[($count >> 12) & 0x3f];
+		$output .= $this->itoa64[($count >> 18) & 0x3f];
+
+		$output .= $this->encode64($input, 3);
+
+		return $output;
+	}
+
+	function gensalt_blowfish($input)
+	{
+		# This one needs to use a different order of characters and a
+		# different encoding scheme from the one in encode64() above.
+		# We care because the last character in our encoded string will
+		# only represent 2 bits.  While two known implementations of
+		# bcrypt will happily accept and correct a salt string which
+		# has the 4 unused bits set to non-zero, we do not want to take
+		# chances and we also do not want to waste an additional byte
+		# of entropy.
+		$itoa64 = './ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+
+		$output = '$2a$';
+		$output .= chr(ord('0') + $this->iteration_count_log2 / 10);
+		$output .= chr(ord('0') + $this->iteration_count_log2 % 10);
+		$output .= '$';
+
+		$i = 0;
+		do {
+			$c1 = ord($input[$i++]);
+			$output .= $itoa64[$c1 >> 2];
+			$c1 = ($c1 & 0x03) << 4;
+			if ($i >= 16) {
+				$output .= $itoa64[$c1];
+				break;
+			}
+
+			$c2 = ord($input[$i++]);
+			$c1 |= $c2 >> 4;
+			$output .= $itoa64[$c1];
+			$c1 = ($c2 & 0x0f) << 2;
+
+			$c2 = ord($input[$i++]);
+			$c1 |= $c2 >> 6;
+			$output .= $itoa64[$c1];
+			$output .= $itoa64[$c2 & 0x3f];
+		} while (1);
+
+		return $output;
+	}
+
+	function HashPassword($password)
+	{
+		$random = '';
+
+		if (CRYPT_BLOWFISH == 1 && !$this->portable_hashes) {
+			$random = $this->get_random_bytes(16);
+			$hash =
+			    crypt($password, $this->gensalt_blowfish($random));
+			if (strlen($hash) == 60)
+				return $hash;
+		}
+
+		if (CRYPT_EXT_DES == 1 && !$this->portable_hashes) {
+			if (strlen($random) < 3)
+				$random = $this->get_random_bytes(3);
+			$hash =
+			    crypt($password, $this->gensalt_extended($random));
+			if (strlen($hash) == 20)
+				return $hash;
+		}
+
+		if (strlen($random) < 6)
+			$random = $this->get_random_bytes(6);
+		$hash =
+		    $this->crypt_private($password,
+		    $this->gensalt_private($random));
+		if (strlen($hash) == 34)
+			return $hash;
+
+		# Returning '*' on error is safe here, but would _not_ be safe
+		# in a crypt(3)-like function used _both_ for generating new
+		# hashes and for validating passwords against existing hashes.
+		return '*';
+	}
+
+	function CheckPassword($password, $stored_hash)
+	{
+		$hash = $this->crypt_private($password, $stored_hash);
+		if ($hash[0] == '*')
+			$hash = crypt($password, $stored_hash);
+
+		return $hash == $stored_hash;
+	}
+}
+
+?>
diff --git a/install/db/132-database.php b/install/db/132-database.php
new file mode 100644
index 000000000..744e8e3ba
--- /dev/null
+++ b/install/db/132-database.php
@@ -0,0 +1,36 @@
+<?php
+// +-----------------------------------------------------------------------+
+// | Piwigo - a PHP based photo gallery                                    |
+// +-----------------------------------------------------------------------+
+// | Copyright(C) 2008-2012 Piwigo Team                  http://piwigo.org |
+// | Copyright(C) 2003-2008 PhpWebGallery Team    http://phpwebgallery.net |
+// | Copyright(C) 2002-2003 Pierrick LE GALL   http://le-gall.net/pierrick |
+// +-----------------------------------------------------------------------+
+// | 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                                          |
+// |                                                                       |
+// | 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 the Free Software           |
+// | Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, |
+// | USA.                                                                  |
+// +-----------------------------------------------------------------------+
+
+defined('PHPWG_ROOT_PATH') or die('Hacking attempt!');
+
+$upgrade_description = 'Enlarge #users.password to increase security.';
+
+global $prefixeTable;
+
+// we don't use USERS_TABLE because it might be an external table, here we
+// want to change to users table specific to Piwigo
+$query = 'ALTER TABLE '.$prefixeTable.'users CHANGE password password varchar(255) default NULL';
+pwg_query($query);
+
+echo "\n".$upgrade_description."\n";
+?>
\ No newline at end of file
diff --git a/install/piwigo_structure-mysql.sql b/install/piwigo_structure-mysql.sql
index c75dbaae3..cde461bcd 100644
--- a/install/piwigo_structure-mysql.sql
+++ b/install/piwigo_structure-mysql.sql
@@ -441,7 +441,7 @@ DROP TABLE IF EXISTS `piwigo_users`;
 CREATE TABLE `piwigo_users` (
   `id` smallint(5) NOT NULL auto_increment,
   `username` varchar(100) binary NOT NULL default '',
-  `password` varchar(32) default NULL,
+  `password` varchar(255) default NULL,
   `mail_address` varchar(255) default NULL,
   PRIMARY KEY  (`id`),
   UNIQUE KEY `users_ui1` (`username`)
diff --git a/password.php b/password.php
index 845a00a3d..cae85cec0 100644
--- a/password.php
+++ b/password.php
@@ -221,7 +221,7 @@ function reset_password()
     
   single_update(
     USERS_TABLE,
-    array($conf['user_fields']['password'] => $conf['pass_convert']($_POST['use_new_pwd'])),
+    array($conf['user_fields']['password'] => $conf['password_hash']($_POST['use_new_pwd'])),
     array($conf['user_fields']['id'] => $user_id)
     );
 
diff --git a/profile.php b/profile.php
index 259b0e382..e60ef4f36 100644
--- a/profile.php
+++ b/profile.php
@@ -177,7 +177,7 @@ function save_profile_from_post($userdata, &$errors)
   ;';
       list($current_password) = pwg_db_fetch_row(pwg_query($query));
 
-      if ($conf['pass_convert']($_POST['password']) != $current_password)
+      if (!$conf['password_verify']($_POST['password'], $current_password))
       {
         $errors[] = l10n('Current password is wrong');
       }
@@ -202,8 +202,8 @@ function save_profile_from_post($userdata, &$errors)
       if (!empty($_POST['use_new_pwd']))
       {
         array_push($fields, $conf['user_fields']['password']);
-        // password is encrpyted with function $conf['pass_convert']
-        $data{$conf['user_fields']['password']} = $conf['pass_convert']($_POST['use_new_pwd']);
+        // password is hashed with function $conf['password_hash']
+        $data{$conf['user_fields']['password']} = $conf['password_hash']($_POST['use_new_pwd']);
       }
       
       // username is updated only if allowed