Commit 75ff6aae authored by Hugh Davenport's avatar Hugh Davenport Committed by Francois Marier
Browse files

Change internal password algorithm to bcrypt

This changes the internal authentication plugin to use
bcrypt instead of sha1.

It also introduces a fast hash (SHA512) for bulk operations.
This hash is updated on user login to the bcrypt hash.

Bug #843568

See https://wiki.mahara.org/index.php/Developer_Area/Specifications_in_Development/Improve_Password_Storage



Change-Id: Ibf2f71bb5b5a5279dbc16ccda781ad99e81c59b8
Signed-off-by: default avatarHugh Davenport <hugh@catalyst.net.nz>
parent 65c27b46
......@@ -536,13 +536,13 @@ function uploadcsv_submit(Pieform $form, $values) {
if (!$values['updateusers'] || !isset($UPDATES[$user->username])) {
$user->passwordchange = (int)$values['forcepasswordchange'];
$user->id = create_user($user, $profilefields, $institution, $authrecord, $remoteuser, $values);
$user->id = create_user($user, $profilefields, $institution, $authrecord, $remoteuser, $values, true);
$addedusers[] = $user;
log_debug('added user ' . $user->username);
}
else if (isset($UPDATES[$user->username])) {
$updated = update_user($user, $profilefields, $remoteuser, $values, true);
$updated = update_user($user, $profilefields, $remoteuser, $values, true, true);
if (empty($updated)) {
// Nothing changed for this user
......
......@@ -59,7 +59,19 @@ class AuthInternal extends Auth {
*/
public function authenticate_user_account($user, $password) {
$this->must_be_ready();
return $this->validate_password($password, $user->password, $user->salt);
$result = $this->validate_password($password, $user->password, $user->salt);
// If result == 1, password is correct
// If result > 1, password is correct but using old settings, should be changed
if ($result > 1 ) {
if ($user->passwordchange != 1) {
$userobj = new User();
$userobj->find_by_id($user->id);
$this->change_password($userobj, $password);
$user->password = $userobj->password;
$user->salt = $userobj->salt;
}
}
return $result > 0;
}
/**
......@@ -96,11 +108,18 @@ class AuthInternal extends Auth {
* @param boolean $resetpasswordchange Whether to reset the passwordchange variable or not
* @return string The new password, or empty if the password could not be set
*/
public function change_password(User $user, $password, $resetpasswordchange = true) {
public function change_password(User $user, $password, $resetpasswordchange = true, $quickhash = false) {
$this->must_be_ready();
// Create a salted password and set it for the user
$user->salt = substr(md5(rand(1000000, 9999999)), 2, 8);
$user->password = $this->encrypt_password($password, $user->salt);
if ($quickhash) {
$user->password = $this->encrypt_password($password, $user->salt);
}
else {
// $2a$ is bcrypt hash. See http://php.net/manual/en/function.crypt.php
// It requires a cost, a 2 digit number in the range 04-31
$user->password = $this->encrypt_password($password, $user->salt, '$2a$' . get_config('bcrypt_cost') . '$');
}
if ($resetpasswordchange) {
$user->passwordchange = 0;
}
......@@ -189,13 +208,28 @@ class AuthInternal extends Auth {
*
* @param string $password The password to encrypt
* @param string $salt The salt to use to encrypt the password
* @param string $alg The algorithm to use, defaults to $6$ which is SHA512
* @todo salt mandatory
*/
public function encrypt_password($password, $salt='') {
public function encrypt_password($password, $salt='', $alg='$6$') {
if ($salt == '') {
$salt = substr(md5(rand(1000000, 9999999)), 2, 8);
}
return sha1($salt . $password);
if ($alg == '$6$') { // $6$ is the identifier for the SHA512 algorithm
// Return a hash which is sha512(originalHash, salt), where original is sha1(salt + password)
$password = sha1($salt . $password);
// Generate a salt based on a supplied salt
$fullsalt = substr(md5($salt), 0, 16); // SHA512 expects 16 chars of salt
}
else { // This is most likely bcrypt $2a$, but any other algorithm can take up to 22 chars of salt
// Generate a salt based on a supplied salt
$fullsalt = substr(md5($salt), 0, 22); // bcrypt expects 22 chars of salt
}
$hash = crypt($password, $alg . $fullsalt);
// Strip out the computed salt
// We strip out the salt hide the computed salt (in case the sitesalt was used which isn't in the database)
$hash = substr($hash, 0, strlen($alg)) . substr($hash, strlen($alg)+strlen($fullsalt));
return $hash;
}
/**
......@@ -203,8 +237,9 @@ class AuthInternal extends Auth {
* and the salt we have, see if the password they sent is correct.
*
* @param string $theysent The password the user sent
* @param string $wehave The password we have in the database for them
* @param string $wehave The salted and hashed password we have in the database for them
* @param string $salt The salt we have.
* @returns int 0 means not validated, 1 means validated, 2 means validated but needs updating
*/
private function validate_password($theysent, $wehave, $salt) {
$this->must_be_ready();
......@@ -215,9 +250,24 @@ class AuthInternal extends Auth {
return false;
}
// The main type - a salted sha1
$sha1sent = $this->encrypt_password($theysent, $salt);
return $sha1sent == $wehave;
$bcrypt = substr($wehave, 0, 4) == '$2a$';
if ($bcrypt) {
$alg = substr($wehave, 0, 7);
$hash = $this->encrypt_password($theysent, $salt, $alg);
}
else {
$hash = $this->encrypt_password($theysent, $salt);
}
if ($hash == $wehave) {
if (!$bcrypt || substr($alg, 4, 2) != get_config('bcrypt_cost')) {
// Either not using bcrypt yet, or the cost parameter has changed, update the hash
return 2;
}
// Using bcrypt with the correct cost parameter, leave as is.
return 1;
}
// Nothing works, fail
return 0;
}
}
......
......@@ -1813,12 +1813,12 @@ function auth_get_random_salt() {
}
// Add salt and encrypt the pw for a user, if their auth instance allows for it
function reset_password($user, $resetpasswordchange=true) {
function reset_password($user, $resetpasswordchange=true, $quickhash=false) {
$userobj = new User();
$userobj->find_by_id($user->id);
$authobj = AuthFactory::create($user->authinstance);
if (isset($user->password) && $user->password != '' && method_exists($authobj, 'change_password')) {
$authobj->change_password($userobj, $user->password, $resetpasswordchange);
$authobj->change_password($userobj, $user->password, $resetpasswordchange, $quickhash);
}
else {
$userobj->password = '';
......
......@@ -258,6 +258,16 @@ if (!get_config('searchplugin')) {
$CFG->searchplugin = 'internal';
}
}
$bcrypt_cost = get_config('bcrypt_cost');
// bcrypt_cost is the cost parameter passed as part of the bcrypt hash
// See http://php.net/manual/en/function.crypt.php
// The value is a 2 digit number in the range of 04-31
if (!$bcrypt_cost || !is_int($bcrypt_cost) || $bcrypt_cost < 4 || $bcrypt_cost > 31) {
$bcrypt_cost = 12;
}
$CFG->bcrypt_cost = sprintf('%02d', $bcrypt_cost);
header('Content-type: text/html; charset=UTF-8');
// Only do authentication once we know the page theme, so that the login form
......
......@@ -2797,5 +2797,21 @@ function xmldb_core_upgrade($oldversion=0) {
add_field($table, $field);
}
if ($oldversion < 2012021700) {
$users = get_records_sql_array('SELECT u.id, u.password, u.salt FROM {usr} u JOIN {auth_instance} ai ON (u.authinstance = ai.id) WHERE ai.authname = ?', array('internal'));
foreach ($users as $user) {
if ($user->password == '*' || $user->salt == '*') {
continue;
}
// Wrap the old hashed password inside a SHA512 hash ($6$ is the identifier for SHA512)
$user->password = crypt($user->password, '$6$' . substr(md5($user->salt), 0, 16));
// Drop the salt from the password as it may contain secrets that are not stored in the db
// for example, the passwordsaltmain value
$user->password = substr($user->password, 0, 3) . substr($user->password, 3+16);
set_field('usr', 'password', $user->password, 'id', $user->id);
}
}
return $status;
}
......@@ -694,7 +694,8 @@ function core_install_lastcoredata_defaults() {
$user = new StdClass;
$user->username = 'admin';
$user->salt = auth_get_random_salt();
$user->password = sha1($user->salt . 'mahara');
$user->password = crypt('mahara', '$2a$' . get_config('bcrypt_cost') . '$' . substr(md5($user->salt), 0, 22));
$user->password = substr($user->password, 0, 7) . substr($user->password, 7+22);
$user->authinstance = $auth_instance->id;
$user->passwordchange = 1;
$user->admin = 1;
......
......@@ -1866,7 +1866,7 @@ function addfriend_submit(Pieform $form, $values) {
* @param array $accountprefs user account preferences to set
* @return integer id of the new user
*/
function create_user($user, $profile=array(), $institution=null, $remoteauth=null, $remotename=null, $accountprefs=array()) {
function create_user($user, $profile=array(), $institution=null, $remoteauth=null, $remotename=null, $accountprefs=array(), $quickhash=false) {
db_begin();
if ($user instanceof User) {
......@@ -1944,7 +1944,7 @@ function create_user($user, $profile=array(), $institution=null, $remoteauth=nul
$userobj->find_by_id($user->id);
$userobj->copy_views(get_column('view', 'id', 'institution', 'mahara', 'copynewuser', 1), $checkviewaccess);
reset_password($user, false);
reset_password($user, false, $quickhash);
handle_event('createuser', $user);
db_commit();
......@@ -1961,7 +1961,7 @@ function create_user($user, $profile=array(), $institution=null, $remoteauth=nul
* @param bool $forceupdateremote force delete of remotename before update attempted
* @return array list of updated fields
*/
function update_user($user, $profile, $remotename=null, $accountprefs=array(), $forceupdateremote=false) {
function update_user($user, $profile, $remotename=null, $accountprefs=array(), $forceupdateremote=false, $quickhash=false) {
require_once(get_config('docroot') . 'auth/session.php');
if (!empty($user->id)) {
......@@ -1991,7 +1991,7 @@ function update_user($user, $profile, $remotename=null, $accountprefs=array(), $
update_record('usr', $newrecord);
if (!empty($newrecord->password)) {
$newrecord->authinstance = $user->authinstance;
reset_password($newrecord, false);
reset_password($newrecord, false, $quickhash);
}
}
......
......@@ -28,7 +28,7 @@
defined('INTERNAL') || die();
$config = new StdClass;
$config->version = 2012021000;
$config->version = 2012021700;
$config->release = '1.5.0dev';
$config->minupgradefrom = 2008040200;
$config->minupgraderelease = '1.0.0 (release tag 1.0.0_RELEASE)';
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment