Commit 5a714bf7 authored by Hugh Davenport's avatar Hugh Davenport Committed by Francois Marier
Browse files

Add a sitewide salt that isn't in the db

This salt is used to add an extra layer of salting that
isn't visible from the database. This requires attackers
to obtain both the database, and the config.php file to
get the true salt value that is passed to crypt.

Bug #843568

See http://docs.moodle.org/20/en/Password_salting



Change-Id: Iaa575a4724e387104f9e436c07b336ef8c7ebef5
Signed-off-by: default avatarHugh Davenport <hugh@catalyst.net.nz>
Signed-off-by: default avatarFrancois Marier <francois@catalyst.net.nz>
parent 75ff6aae
......@@ -113,12 +113,13 @@ class AuthInternal extends Auth {
// Create a salted password and set it for the user
$user->salt = substr(md5(rand(1000000, 9999999)), 2, 8);
if ($quickhash) {
$user->password = $this->encrypt_password($password, $user->salt);
// $6$ is SHA512, used as a quick hash instead of bcrypt for now.
$user->password = $this->encrypt_password($password, $user->salt, '$6$', get_config('passwordsaltmain'));
}
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') . '$');
$user->password = $this->encrypt_password($password, $user->salt, '$2a$' . get_config('bcrypt_cost') . '$', get_config('passwordsaltmain'));
}
if ($resetpasswordchange) {
$user->passwordchange = 0;
......@@ -209,21 +210,22 @@ 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
* @param string $sitesalt A salt to combine with the user's salt to add an extra layer or salting
* @todo salt mandatory
*/
public function encrypt_password($password, $salt='', $alg='$6$') {
public function encrypt_password($password, $salt='', $alg='$6$', $sitesalt='') {
if ($salt == '') {
$salt = substr(md5(rand(1000000, 9999999)), 2, 8);
}
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
// Generate a salt based on a supplied salt and the passwordsaltmain
$fullsalt = substr(md5($sitesalt . $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
// Generate a salt based on a supplied salt and the passwordsaltmain
$fullsalt = substr(md5($sitesalt . $salt), 0, 22); // bcrypt expects 22 chars of salt
}
$hash = crypt($password, $alg . $fullsalt);
// Strip out the computed salt
......@@ -250,13 +252,15 @@ class AuthInternal extends Auth {
return false;
}
$sitesalt = get_config('passwordsaltmain');
$bcrypt = substr($wehave, 0, 4) == '$2a$';
if ($bcrypt) {
$alg = substr($wehave, 0, 7);
$hash = $this->encrypt_password($theysent, $salt, $alg);
$hash = $this->encrypt_password($theysent, $salt, $alg, $sitesalt);
}
else {
$hash = $this->encrypt_password($theysent, $salt);
$alg = substr($wehave, 0, 3);
$hash = $this->encrypt_password($theysent, $salt, $alg, $sitesalt);
}
if ($hash == $wehave) {
if (!$bcrypt || substr($alg, 4, 2) != get_config('bcrypt_cost')) {
......@@ -266,6 +270,24 @@ class AuthInternal extends Auth {
// Using bcrypt with the correct cost parameter, leave as is.
return 1;
}
// See http://docs.moodle.org/20/en/Password_salting#Changing_the_salt
if (!empty($sitesalt)) {
// There is a sitesalt set, try without it, and update if passes
$hash = $this->encrypt_password($theysent, $salt, $alg, '');
if ($hash == $wehave) {
return 2;
}
}
for ($i = 1; $i <= 20; ++ $i) {
// Try 20 alternate sitesalts
$alt = get_config('passwordsaltalt' . $i);
if (!empty($alt)) {
$hash = $this->encrypt_password($theysent, $salt, $alg, $alt);
if ($hash == $wehave) {
return 2;
}
}
}
// Nothing works, fail
return 0;
}
......
......@@ -88,4 +88,11 @@ $cfg->dataroot = '/path/to/uploaddir';
// user in the event of a false positive.
$cfg->emailcontact = '';
// Set this to enable a secondary hash that is only present in the config file
// $cfg->passwordsaltmain = 'some long random string here with lots of characters';
// When changing the salt (or disabling it), you will need to set the current salt as an alternate salt
// There are up to 20 alternate salts
// $cfg->passwordsaltalt1 = 'old salt value';
// closing php tag intentionally omitted to prevent whitespace issues
......@@ -139,3 +139,5 @@ $string['gdlibrarylacksgifsupport'] = 'The installed PHP GD Library does not sup
$string['gdlibrarylacksjpegsupport'] = 'The installed PHP GD Library does not support JPEG/JPG images. Full support is needed to upload JPEG/JPG images.';
$string['gdlibrarylackspngsupport'] = 'The installed PHP GD Library does not support PNG images. Full support is needed to upload PNG images.';
$string['nopasswordsaltset'] = 'No site-wide password salt has been set. Edit your config.php and set the "passwordsaltmain" parameter to a reasonable secret phrase.';
$string['passwordsaltweak'] = 'Your site-wide password salt is not strong enough. Edit your config.php and set the "passwordsaltmain" parameter to a longer secret phrase.';
......@@ -2804,7 +2804,7 @@ function xmldb_core_upgrade($oldversion=0) {
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));
$user->password = crypt($user->password, '$6$' . substr(md5(get_config('passwordsaltmain') . $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
......
......@@ -694,7 +694,7 @@ function core_install_lastcoredata_defaults() {
$user = new StdClass;
$user->username = 'admin';
$user->salt = auth_get_random_salt();
$user->password = crypt('mahara', '$2a$' . get_config('bcrypt_cost') . '$' . substr(md5($user->salt), 0, 22));
$user->password = crypt('mahara', '$2a$' . get_config('bcrypt_cost') . '$' . substr(md5(get_config('passwordsaltmain') . $user->salt), 0, 22));
$user->password = substr($user->password, 0, 7) . substr($user->password, 7+22);
$user->authinstance = $auth_instance->id;
$user->passwordchange = 1;
......@@ -1219,5 +1219,15 @@ function site_warnings() {
$warnings[] = get_string('openbasedirenabled', 'error') . ' ' . get_string('openbasedirwarning', 'error');
}
$sitesalt = get_config('passwordsaltmain');
if (empty($sitesalt)) {
$warnings[] = get_string('nopasswordsaltset', 'error');
}
else if ($sitesalt == 'some long random string here with lots of characters'
|| trim($sitesalt) === ''
|| preg_match('/^([a-zA-Z0-9]{0,10})$/', $sitesalt)) {
$warnings[] = get_string('passwordsaltweak', 'error');
}
return $warnings;
}
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