From 92cdd4cc7f6955760236e134126d8a68efa13360 Mon Sep 17 00:00:00 2001 From: Andreas Baumann Date: Wed, 12 Feb 2020 19:21:35 +0100 Subject: hand-picked changes for better password functions from tyzoids branch --- db_update.php | 6 +++- include/common.php | 2 +- include/functions.php | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++- install.php | 6 ++-- login.php | 38 +++++++++------------ profile.php | 8 ++--- register.php | 4 +-- 7 files changed, 120 insertions(+), 35 deletions(-) diff --git a/db_update.php b/db_update.php index 2964bf3..b26bd93 100644 --- a/db_update.php +++ b/db_update.php @@ -9,7 +9,7 @@ // The FluxBB version this script updates to define('UPDATE_TO', '1.5.11'); -define('UPDATE_TO_DB_REVISION', 21); +define('UPDATE_TO_DB_REVISION', 22); define('UPDATE_TO_SI_REVISION', 2); define('UPDATE_TO_PARSER_REVISION', 2); @@ -704,6 +704,10 @@ switch ($stage) $db->alter_field('users', 'jabber', 'VARCHAR(80)', true) or error('Unable to alter jabber field', __FILE__, __LINE__, $db->error()); $db->alter_field('users', 'msn', 'VARCHAR(80)', true) or error('Unable to alter msn field', __FILE__, __LINE__, $db->error()); $db->alter_field('users', 'activate_string', 'VARCHAR(80)', true) or error('Unable to alter activate_string field', __FILE__, __LINE__, $db->error()); + + // Make password field VARCHAR(255) to support password_hash + // 255 is recommended by the PHP manual: http://php.net/manual/en/function.password-hash.php + $db->alter_field('users', 'password', 'VARCHAR(255)', false) or error('Unable to alter password field', __FILE__, __LINE__, $db->error()); // Make all IP fields VARCHAR(39) to support IPv6 $db->alter_field('posts', 'poster_ip', 'VARCHAR(39)', true) or error('Unable to alter poster_ip field', __FILE__, __LINE__, $db->error()); diff --git a/include/common.php b/include/common.php index ad34b5e..4f64128 100644 --- a/include/common.php +++ b/include/common.php @@ -12,7 +12,7 @@ if (!defined('PUN_ROOT')) // Define the version and database revision that this code was written for define('FORUM_VERSION', '1.5.11'); -define('FORUM_DB_REVISION', 21); +define('FORUM_DB_REVISION', 22); define('FORUM_SI_REVISION', 2); define('FORUM_PARSER_REVISION', 2); diff --git a/include/functions.php b/include/functions.php index ace2934..8e9265c 100644 --- a/include/functions.php +++ b/include/functions.php @@ -164,7 +164,7 @@ function authenticate_user($user, $password, $password_is_hash = false) $pun_user = $db->fetch_assoc($result); $is_password_authorized = pun_hash_equals($password, $pun_user['password']); - $is_hash_authorized = pun_hash_equals(pun_hash($password), $pun_user['password']); + $is_hash_authorized = pun_password_verify($password, $pun_user['password']); if (!isset($pun_user['id']) || ($password_is_hash && !$is_password_authorized || @@ -1130,6 +1130,95 @@ function validate_redirect($redirect_url, $fallback_url) } +// +// Compute the hash of a password +// using a secure password hashing algorithm, if available +// As of PHP 7.2, this is BLOWFISH. +// This function will fall back to unsecure defaults if +// password_hash does not exist (requires >=PHP5.5) +// +function pun_password_hash($pass) +{ + global $password_hash_cost; + + $cost = $password_hash_cost; + if (empty($cost)) + $cost = 10; + + if (function_exists('password_hash')) + return password_hash($pass, PASSWORD_DEFAULT, array('cost' => $cost)); + else + return pun_hash($pass); +} + + +// +// Verify that $pass and $hash match +// This supports any password hashing algorithm +// used by pun_password_hash +// +function pun_password_verify($pass, $hash) +{ + if (!empty($hash) && $hash[0] !== '$') + return pun_hash_equals(pun_hash($pass), $hash); + else + return password_verify($pass, $hash); +} + +// +// Verify that $pass and $hash match +// This supports any password hashing algorithm +// used by pun_password_hash, but is also +// backwards-compatable with older versions of this software. +// +function pun_password_verify_legacy($pass, $hash, $salt = null) +{ + // MD5 from 1.2 + if (strlen($hash) < 40) + return pun_hash_equals(md5($pass), $hash); + + // SHA1-With-Salt from 1.3 + if (!empty($salt)) + return pun_hash_equals(sha1($salt . sha1($pass)), $hash); + + // SHA1-Without-Salt from 1.4 + if (strlen($hash) == 40) + return pun_hash_equals(sha1($pass), $hash); + + // Support current password standard + return pun_password_verify($pass, $hash); +} + + +// +// Check if $hash is outdated and needs to be rehashed +// +function pun_password_needs_rehash($hash) +{ + global $password_hash_cost; + + // Determine appropriate cost + $cost = $password_hash_cost; + if (empty($cost)) + $cost = 10; + + // Check for legacy md5 hash + if (strlen($hash) < 40) + return true; + + // Check for legacy sha1 hash. Note: legacy sha1 is used + // if password_hash is not available + if (function_exists('password_hash') && strlen($hash) == 40) + return true; + + // Check for out-of-date hash type or cost + if (function_exists('password_needs_rehash')) + return password_needs_rehash($hash, PASSWORD_DEFAULT, array('cost' => $cost)); + + return false; +} + + // // Generate a random password of length $len // Compatibility wrapper for random_key diff --git a/install.php b/install.php index 979caf9..26fbae6 100644 --- a/install.php +++ b/install.php @@ -9,7 +9,7 @@ // The FluxBB version this script installs define('FORUM_VERSION', '1.5.11'); -define('FORUM_DB_REVISION', 21); +define('FORUM_DB_REVISION', 22); define('FORUM_SI_REVISION', 2); define('FORUM_PARSER_REVISION', 2); @@ -110,7 +110,7 @@ function generate_config_file() { global $db_type, $db_host, $db_name, $db_username, $db_password, $db_prefix, $cookie_name, $cookie_seed; - return 'query('INSERT INTO '.$db_prefix.'users (group_id, username, password, email) VALUES(3, \''.$db->escape($lang_install['Guest']).'\', \''.$db->escape($lang_install['Guest']).'\', \''.$db->escape($lang_install['Guest']).'\')') or error('Unable to add guest user. Please check your configuration and try again', __FILE__, __LINE__, $db->error()); - $db->query('INSERT INTO '.$db_prefix.'users (group_id, username, password, email, language, style, num_posts, last_post, registered, registration_ip, last_visit) VALUES(1, \''.$db->escape($username).'\', \''.pun_hash($password1).'\', \''.$email.'\', \''.$db->escape($default_lang).'\', \''.$db->escape($default_style).'\', 1, '.$now.', '.$now.', \''.$db->escape(get_remote_address()).'\', '.$now.')') + $db->query('INSERT INTO '.$db_prefix.'users (group_id, username, password, email, language, style, num_posts, last_post, registered, registration_ip, last_visit) VALUES(1, \''.$db->escape($username).'\', \''.$db->escape(pun_password_hash($password1)).'\', \''.$email.'\', \''.$db->escape($default_lang).'\', \''.$db->escape($default_style).'\', 1, '.$now.', '.$now.', \''.$db->escape(get_remote_address()).'\', '.$now.')') or error('Unable to add administrator user. Please check your configuration and try again', __FILE__, __LINE__, $db->error()); // Enable/disable avatars depending on file_uploads setting in PHP configuration diff --git a/login.php b/login.php index 323629a..63d1663 100644 --- a/login.php +++ b/login.php @@ -38,33 +38,25 @@ if (isset($_POST['form_sent']) && $action == 'in') if (!empty($cur_user['password'])) { - $form_password_hash = pun_hash($form_password); // Will result in a SHA-1 hash + // Represents the hash of the user's password + // If it's transparently changed in this function, + // this allows the cookie token to reflect the new hash + $user_password = $cur_user['password']; - // If there is a salt in the database we have upgraded from 1.3-legacy though haven't yet logged in - if (!empty($cur_user['salt'])) + if (pun_password_verify_legacy($form_password, $user_password, $cur_user['salt'])) { - $is_salt_authorized = pun_hash_equals(sha1($cur_user['salt'].sha1($form_password)), $cur_user['password']); - if ($is_salt_authorized) // 1.3 used sha1(salt.sha1(pass)) - { - $authorized = true; - - $db->query('UPDATE '.$db->prefix.'users SET password=\''.$form_password_hash.'\', salt=NULL WHERE id='.$cur_user['id']) or error('Unable to update user password', __FILE__, __LINE__, $db->error()); - } - } - // If the length isn't 40 then the password isn't using sha1, so it must be md5 from 1.2 - else if (strlen($cur_user['password']) != 40) - { - $is_md5_authorized = pun_hash_equals(md5($form_password), $cur_user['password']); - if ($is_md5_authorized) + $authorized = true; + + $remove_salt = !empty($cur_user['salt']); + $rehash = $remove_salt || pun_password_needs_rehash($user_password); + + if ($rehash) { - $authorized = true; - - $db->query('UPDATE '.$db->prefix.'users SET password=\''.$form_password_hash.'\' WHERE id='.$cur_user['id']) or error('Unable to update user password', __FILE__, __LINE__, $db->error()); + $user_password = pun_password_hash($form_password); + $salt_sql = ($remove_salt ? 'salt=NULL,' : ''); + $db->query('UPDATE '.$db->prefix.'users SET '.$salt_sql.' password=\''.$db->escape($user_password).'\' WHERE id='.$cur_user['id']) or error('Unable to update user password', __FILE__, __LINE__, $db->error()); } } - // Otherwise we should have a normal sha1 password - else - $authorized = pun_hash_equals($cur_user['password'], $form_password_hash); } if (!$authorized) @@ -91,7 +83,7 @@ if (isset($_POST['form_sent']) && $action == 'in') $db->query('DELETE FROM '.$db->prefix.'online WHERE ident=\''.$db->escape(get_remote_address()).'\'') or error('Unable to delete from online list', __FILE__, __LINE__, $db->error()); $expire = ($save_pass == '1') ? time() + 1209600 : time() + $pun_config['o_timeout_visit']; - pun_setcookie($cur_user['id'], $form_password_hash, $expire); + pun_setcookie($cur_user['id'], $user_password, $expire); // Reset tracked topics set_tracked_topics(null); diff --git a/profile.php b/profile.php index 8b0eb86..a5e21e4 100644 --- a/profile.php +++ b/profile.php @@ -102,16 +102,16 @@ if ($action == 'change_pass') { $old_password_hash = pun_hash($old_password); - if ($cur_user['password'] == $old_password_hash || $pun_user['is_admmod']) + if (pun_password_verify($old_password, $cur_user['password']) || $pun_user['is_admmod']) $authorized = true; } if (!$authorized) message($lang_profile['Wrong pass']); - $new_password_hash = pun_hash($new_password1); + $new_password_hash = pun_password_hash($new_password1); - $db->query('UPDATE '.$db->prefix.'users SET password=\''.$new_password_hash.'\''.(!empty($cur_user['salt']) ? ', salt=NULL' : '').' WHERE id='.$id) or error('Unable to update password', __FILE__, __LINE__, $db->error()); + $db->query('UPDATE '.$db->prefix.'users SET password=\''.$db->escape($new_password_hash).'\''.(!empty($cur_user['salt']) ? ', salt=NULL' : '').' WHERE id='.$id) or error('Unable to update password', __FILE__, __LINE__, $db->error()); if ($pun_user['id'] == $id) pun_setcookie($pun_user['id'], $new_password_hash, time() + $pun_config['o_timeout_visit']); @@ -193,7 +193,7 @@ else if ($action == 'change_email') } else if (isset($_POST['form_sent'])) { - if (pun_hash($_POST['req_password']) !== $pun_user['password']) + if (!pun_password_verify($_POST['req_password'], $pun_user['password'])) message($lang_profile['Wrong pass']); // Make sure they got here from the site diff --git a/register.php b/register.php index 04a6417..111c850 100644 --- a/register.php +++ b/register.php @@ -157,10 +157,10 @@ if (isset($_POST['form_sent'])) $now = time(); $intial_group_id = ($pun_config['o_regs_verify'] == '0') ? $pun_config['o_default_user_group'] : PUN_UNVERIFIED; - $password_hash = pun_hash($password1); + $password_hash = pun_password_hash($password1); // Add the user - $db->query('INSERT INTO '.$db->prefix.'users (username, group_id, password, email, email_setting, timezone, dst, language, style, registered, registration_ip, last_visit) VALUES(\''.$db->escape($username).'\', '.$intial_group_id.', \''.$password_hash.'\', \''.$db->escape($email1).'\', '.$email_setting.', '.$timezone.' , '.$dst.', \''.$db->escape($language).'\', \''.$pun_config['o_default_style'].'\', '.$now.', \''.$db->escape(get_remote_address()).'\', '.$now.')') or error('Unable to create user', __FILE__, __LINE__, $db->error()); + $db->query('INSERT INTO '.$db->prefix.'users (username, group_id, password, email, email_setting, timezone, dst, language, style, registered, registration_ip, last_visit) VALUES(\''.$db->escape($username).'\', '.$intial_group_id.', \''.$db->escape($password_hash).'\', \''.$db->escape($email1).'\', '.$email_setting.', '.$timezone.' , '.$dst.', \''.$db->escape($language).'\', \''.$pun_config['o_default_style'].'\', '.$now.', \''.$db->escape(get_remote_address()).'\', '.$now.')') or error('Unable to create user', __FILE__, __LINE__, $db->error()); $new_uid = $db->insert_id(); if ($pun_config['o_regs_verify'] == '0') -- cgit v1.2.3