security MDL-18006 MDL-18807 MDL-20853 Several improvements to increa…

…se security * Installation o Adds a random $CFG->passwordsaltmain to config.php on install o Changes passwordpolicy default to on * Upgrade o If passwordpolicy is off it switches it on o Checks if passwordsaltmain has been set and if not recommends to the user and all admins to add it, both on the upgrade output and via messaging. o Forces all admin users to change their password at next login. * General o Extends the security overview report to check for passwordmainsalt and recommend if it not set (or weak)
moodle · Nov 17, 2009 · 0a4b736 · 0a4b736
1 parent 5953ae1
commit 0a4b736
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 2 deletions.
diff --git a/admin/index.php b/admin/index.php
@@ -635,6 +635,10 @@
 
     }
 
+    if (empty($CFG->passwordsaltmain)) {
+        print_box(get_string('upgrade197notice', 'admin')."\n".get_string('upgrade197salt', 'admin'));
+    }
+
     if (defined('WARN_DISPLAY_ERRORS_ENABLED')) {
         print_box(get_string('displayerrorswarning', 'admin'), 'generalbox adminwarning');
     }

diff --git a/admin/report/security/lib.php b/admin/report/security/lib.php
@@ -53,6 +53,7 @@ function report_security_get_issue_list() {
         'report_security_check_openprofiles',
         'report_security_check_google',
         'report_security_check_passwordpolicy',
+        'report_security_check_passwordsaltmain',
         'report_security_check_emailchangeconfirmation',
         'report_security_check_cookiesecure',
         'report_security_check_configrw',
@@ -485,6 +486,35 @@ function report_security_check_configrw($detailed=false) {
     return $result;
 }
 
+function report_security_check_passwordsaltmain($detailed=false) {
+    global $CFG;
+
+    $result = new object();
+    $result->issue   = 'report_security_check_passwordsaltmain';
+    $result->name    = get_string('check_passwordsaltmain_name', 'report_security');
+    $result->info    = null;
+    $result->details = null;
+    $result->status  = null;
+    $result->link    = null;
+
+    if (empty($CFG->passwordsaltmain)) {
+        $result->status = REPORT_SECURITY_WARNING;
+        $result->info   = get_string('check_passwordsaltmain_warning', 'report_security');
+    } else if (trim($CFG->passwordsaltmain)=='' || preg_match('/^([\w]+|[\d]+)$/i', $CFG->passwordsaltmain)) {
+        $result->status = REPORT_SECURITY_WARNING;
+        $result->info   = get_string('check_passwordsaltmain_weak', 'report_security');
+    } else {
+        $result->status = REPORT_SECURITY_OK;
+        $result->info   = get_string('check_passwordsaltmain_ok', 'report_security');
+    }
+
+    if ($detailed) {
+        $result->details = get_string('check_passwordsaltmain_details', 'report_security');
+    }
+
+    return $result;
+}
+
 /**
  * Lists all users with XSS risk, it would be great to combine this with risk trusts in user table,
  * unfortunately nobody implemented user trust UI yet :-(

diff --git a/admin/settings/security.php b/admin/settings/security.php
@@ -46,7 +46,7 @@
     $temp->add(new admin_setting_configcheckbox('cronclionly', get_string('cronclionly', 'admin'), get_string('configcronclionly', 'admin'), 0));
     $temp->add(new admin_setting_configpasswordunmask('cronremotepassword', get_string('cronremotepassword', 'admin'), get_string('configcronremotepassword', 'admin'), ''));
 
-    $temp->add(new admin_setting_configcheckbox('passwordpolicy', get_string('passwordpolicy', 'admin'), get_string('configpasswordpolicy', 'admin'), 0));
+    $temp->add(new admin_setting_configcheckbox('passwordpolicy', get_string('passwordpolicy', 'admin'), get_string('configpasswordpolicy', 'admin'), 1));
     $temp->add(new admin_setting_configtext('minpasswordlength', get_string('minpasswordlength', 'admin'), get_string('configminpasswordlength', 'admin'), 8, PARAM_INT));
     $temp->add(new admin_setting_configtext('minpassworddigits', get_string('minpassworddigits', 'admin'), get_string('configminpassworddigits', 'admin'), 1, PARAM_INT));
     $temp->add(new admin_setting_configtext('minpasswordlower', get_string('minpasswordlower', 'admin'), get_string('configminpasswordlower', 'admin'), 1, PARAM_INT));

diff --git a/install.php b/install.php
@@ -554,6 +554,9 @@
     $str .= '$CFG->directorypermissions = 00777;  // try 02777 on a server in Safe Mode'."\r\n";
     $str .= "\r\n";
 
+    $str .= '$CFG->passwordsaltmain = \''.addsingleslashes(complex_random_string()).'\';'."\r\n";
+    $str .= "\r\n";
+
     $str .= 'require_once("$CFG->dirroot/lib/setup.php");'."\r\n";
     $str .= '// MAKE SURE WHEN YOU EDIT THIS FILE THAT THERE ARE NO SPACES, BLANK LINES,'."\r\n";
     $str .= '// RETURNS, OR ANYTHING ELSE AFTER THE TWO CHARACTERS ON THE NEXT LINE.'."\r\n";

diff --git a/lang/en_utf8/admin.php b/lang/en_utf8/admin.php
@@ -758,6 +758,12 @@
 $string['uploadpicture_userupdated'] = 'Picture updated for user $a.';
 $string['uploadpicture_cannotsave'] = 'Cannot save picture for user $a. Check original picture file.';
 $string['updatetimezones'] = 'Update timezones';
+$string['upgrade197notice'] = '<p>Moodle 1.9.7 contains a number of security fixes to user passwords and backups to protect the user information on your site.<br />
+As a result some of your settings and permissions relating to backups may have changed.<br />
+Please see the <a href=\'http://docs.moodle.org/en/Moodle_1.9.7_release_notes\' target=\'_blank\'>Moodle 1.9.7 release</a> notes for full details.</p>';
+$string['upgrade197noticesubject'] = 'Moodle 1.9.7 upgrade security notices';
+$string['upgrade197salt'] = 'It is also now strongly recommended that you set a password salt to greatly reduce the risk of password theft.<br />
+Please refer to the Moodle security report for more information on this topic. The security report can be accessed by logging into your site as an administrator and go to Site Administration - Security - Site policies';
 $string['upgradeforumread'] = 'A new feature has been added in Moodle 1.5 to track read/unread forum posts.<br />To use this functionality you need to <a href=\"$a\">update your tables</a>.';
 $string['upgradeforumreadinfo'] = 'A new feature has been added in Moodle 1.5 to track read/unread forum posts.  To use this functionality you need to update your tables with all the tracking information for existing posts.  Depending on the size of your site this can take a long time (hours) and can be quite taxing on the database, so it\'s best to do it during a quiet period.  However, your site will continue functioning during this upgrade and users won\'t be affected.  Once you start this process you should let it finish (keep your browser window open).  However, if you stop the process by closing the window: don\'t worry, you can start over.<br /><br />Do you want to start the upgrading process now?';
 $string['upgradelogs'] = 'For full functionality, your old logs need to be upgraded.  <a href=\"$a\">More information</a>';

diff --git a/lang/en_utf8/report_security.php b/lang/en_utf8/report_security.php
@@ -119,6 +119,12 @@
 $string['check_passwordpolicy_name'] = 'Password policy';
 $string['check_passwordpolicy_ok'] = 'Password policy enabled.';
 
+$string['check_passwordsaltmain_name'] = 'Password salt';
+$string['check_passwordsaltmain_warning'] = 'No password salt has been set';
+$string['check_passwordsaltmain_ok'] = 'Password salt is OK';
+$string['check_passwordsaltmain_weak'] = 'Password salt is weak';
+$string['check_passwordsaltmain_details'] = '<p>It is strongly recommended that a password salt is set as it greatly reduces the risk of password theft.<br />To set a password salt add the following to your config.php file.</p><code>\$CFG->passwordsaltmain = \'arandomstringofcharacters\';</code><p>The random string of characters should be a mix of letters, numbers and other characters.</p>';
+
 $string['check_riskadmin_detailsok'] = '<p>Please verify the following list of system administrators:</p>$a';
 $string['check_riskadmin_detailswarning'] = '<p>Please verify the following list of system administrators:</p>$a->admins
 <p>It is recommended to assign administrator role in system context only. Following users have unsupported admin role assignments:</p>$a->unsupported';

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
@@ -3184,6 +3184,70 @@ function xmldb_main_upgrade($oldversion=0) {
         }
         upgrade_main_savepoint($result, 2007101551);
     }
+
+    if ($result && $oldversion < 2007101561.01) {
+        // As part of security changes password policy will now be enabled by default.
+        // If it has not already been enabled then we will enable it... Admins will still
+        // be able to switch it off after this upgrade
+        if (record_exists('config', 'name', 'passwordpolicy', 'value', 0)) {
+            unset_config('passwordpolicy');
+        }
+
+        $message = get_string('upgrade197notice', 'admin');
+        if (empty($CFG->passwordmainsalt)) {
+            $message .= "\n".get_string('upgrade197salt', 'admin');
+        }
+        notify($message, 'notifysuccess');
+
+        unset($message);
+
+        upgrade_main_savepoint($result, 2007101561.01);
+    }
+
+    if ($result && $oldversion < 2007101561.02) {
+
+        $messagesubject = get_string('upgrade197noticesubject', 'admin');
+        $message  = addslashes(get_string('upgrade197notice', 'admin'));
+        if (empty($CFG->passwordmainsalt)) {
+            $message .= "\n".get_string('upgrade197salt', 'admin');
+        }
+
+        // Force administrators to change password on next login
+        $sql = "SELECT DISTINCT u.id, u.firstname, u.lastname, u.picture, u.imagealt, u.email
+              FROM {$CFG->prefix}role_capabilities rc
+              JOIN {$CFG->prefix}role_assignments ra ON (ra.contextid = rc.contextid AND ra.roleid = rc.roleid)
+              JOIN {$CFG->prefix}user u ON u.id = ra.userid
+             WHERE rc.capability = 'moodle/site:doanything'
+                   AND rc.permission = ".CAP_ALLOW."
+                   AND u.deleted = 0
+                   AND rc.contextid = ".SYSCONTEXTID."";
+
+        $adminusers = get_records_sql($sql);
+        foreach ($adminusers as $adminuser) {
+            if ($preference = get_record('user_preferences', 'userid', $adminuser->id, 'name', 'auth_forcepasswordchange')) {
+                if ($preference->value == '1') {
+                    continue;
+                }
+                set_field('user_preferences', 'value', '1', 'id', $preference->id);
+            } else {
+                $preference = new stdClass;
+                $preference->userid = $adminuser->id;
+                $preference->name   = 'auth_forcepasswordchange';
+                $preference->value  = '1';
+                insert_record('user_preferences', $preference);
+            }
+            // Message them with the notice about upgrading
+            email_to_user($adminuser, $adminuser, $messagesubject, $message);
+        }
+
+        unset($adminusers);
+        unset($preference);
+        unset($message);
+        unset($messagesubject);
+
+        upgrade_main_savepoint($result, 2007101561.02);
+    }
+
     return $result;
 }
 

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
@@ -6831,6 +6831,30 @@ function random_string ($length=15) {
     return $string;
 }
 
+/**
+ * Generate a complex random string (usefull for md5 salts)
+ *
+ * This function is based on the above {@link random_string()} however it uses a
+ * larger pool of characters and generates a string between 24 and 32 characters
+ *
+ * @param int $length Optional if set generates a string to exactly this length
+ * @return string
+ */
+function complex_random_string($length=null) {
+    $pool  = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+    $pool .= '`~!@#%^&*()_+-=[];,./<>?:{} ';
+    $poollen = strlen($pool);
+    mt_srand ((double) microtime() * 1000000);
+    if ($length===null) {
+        $length = floor(rand(24,32));
+    }
+    $string = '';
+    for ($i = 0; $i < $length; $i++) {
+        $string .= $pool[(mt_rand()%$poollen)];
+    }
+    return $string;
+}
+
 /*
  * Given some text (which may contain HTML) and an ideal length,
  * this function truncates the text neatly on a word boundary if possible

diff --git a/version.php b/version.php
@@ -6,7 +6,7 @@
 // This is compared against the values stored in the database to determine
 // whether upgrades should be performed (see lib/db/*.php)
 
-    $version = 2007101560;  // YYYYMMDD      = date of the 1.9 branch (don't change)
+    $version = 2007101561;  // YYYYMMDD      = date of the 1.9 branch (don't change)
                             //         X     = release number 1.9.[0,1,2,3,4,5...]
                             //          Y.YY = micro-increments between releases