From a73846717f5c884e0eef0b5591ff7ad374375a0b Mon Sep 17 00:00:00 2001 From: plegall Date: Fri, 2 Nov 2012 13:59:07 +0000 Subject: 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 --- admin/include/functions_upgrade.php | 7 +- include/config_default.inc.php | 14 +- include/functions_user.inc.php | 74 ++++++++++- include/passwordhash.class.php | 253 ++++++++++++++++++++++++++++++++++++ install/db/132-database.php | 36 +++++ install/piwigo_structure-mysql.sql | 2 +- password.php | 2 +- profile.php | 6 +- 8 files changed, 378 insertions(+), 16 deletions(-) create mode 100644 include/passwordhash.class.php create mode 100644 install/db/132-database.php 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 ); @@ -1094,6 +1094,76 @@ function auto_login() { return false; } +/** + * 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 @@ + 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 @@ + \ 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 -- cgit v1.2.3