From bb214ccd2f2a4ecd688568f28f195b1a0c9754bc Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Wed, 17 Jun 2015 17:14:21 +1200 Subject: [PATCH 1/2] MDL-50689 report_sercurityoverview: added user enumeration Change-Id: Iec51c0c25f4aadf2bfed0a2a67eded5df60796ce Reviewed-on: https://review.totaralms.com/8159 Tested-by: Jenkins Automation Reviewed-by: Petr Skoda Reviewed-by: Simon Player --- report/security/lang/en/report_security.php | 6 +++++ report/security/locallib.php | 36 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/report/security/lang/en/report_security.php b/report/security/lang/en/report_security.php index b08e19e..2dcabb2 100644 --- a/report/security/lang/en/report_security.php +++ b/report/security/lang/en/report_security.php @@ -120,6 +120,12 @@ $string['check_webcron_details'] = '

Running the cron from a web browser can e $string['check_webcron_warning'] = 'Anonymous users can access cron.'; $string['check_webcron_name'] = 'Web cron'; $string['check_webcron_ok'] = 'Anonymous users can not access cron.'; +$string['check_usernameenumeration_details'] = '

When a user\'s credentials are incorrect Totara is careful to be vague about the reason why so as to not let the user know whether the username or the password is incorrect. This means would be attackers must guess both the username and the password.

+

With Self registration turned on would be attackers can use the signup form to enumerate usernames and work out which are valid. Once a valid username has been identified they only need to guess the password.
To prevent this turn off Self registration.

+

With Protect usernames turned off would be attackers can use the forgotten password form to enumerate usernames and work out which are valid. Once a valid username has been identified they only need to guess the password.
To prevent this turn on Protect usernames.

'; +$string['check_usernameenumeration_name'] = 'Username enumeration'; +$string['check_usernameenumeration_ok'] = 'Protect usernames is enabled and Self registration is not enabled'; +$string['check_usernameenumeration_warning'] = 'With Self registration turned on or Protect usernames turned off unauthenticated users may be able to guess existing usernames'; $string['issue'] = 'Issue'; $string['pluginname'] = 'Security overview'; $string['security:view'] = 'View security report'; diff --git a/report/security/locallib.php b/report/security/locallib.php index 0add8e7..52f9728 100644 --- a/report/security/locallib.php +++ b/report/security/locallib.php @@ -48,6 +48,7 @@ function report_security_get_issue_list() { 'report_security_check_google', 'report_security_check_passwordpolicy', 'report_security_check_emailchangeconfirmation', + 'report_security_check_usernameenumeration', 'report_security_check_cookiesecure', 'report_security_check_configrw', 'report_security_check_riskxss', @@ -140,6 +141,41 @@ function report_security_check_passwordpolicy($detailed=false) { } /** + * Test if registerauth has been enabled or if protect usernames has been turned off and warn about possible user enumeration. + * + * @param bool $detailed + * @return stdClass + */ +function report_security_check_usernameenumeration($detailed = false) { + global $CFG; + + $result = new stdClass(); + $result->issue = __FUNCTION__; + $result->name = get_string('check_usernameenumeration_name', 'report_security'); + $result->info = null; + $result->details = null; + $result->status = null; + $result->link = "wwwroot/$CFG->admin/settings.php?section=manageauths\">".get_string('authsettings', 'admin').''; + + // We only check registerauth, we don't check that if its set the plugin actually facilitates it. + // This is because the plugin may have hidden logic, really if registerauth is set we can presume that "someone" can signup. + // We also check $CFG->protectusernames because that allows username enumeration as well. + if (empty($CFG->registerauth) && !empty($CFG->protectusernames)) { + $result->status = REPORT_SECURITY_OK; + $result->info = get_string('check_usernameenumeration_ok', 'report_security'); + } else { + $result->status = REPORT_SECURITY_WARNING; + $result->info = get_string('check_usernameenumeration_warning', 'report_security'); + } + + if ($detailed) { + $result->details = get_string('check_usernameenumeration_details', 'report_security'); + } + + return $result; +} + +/** * Verifies sloppy embedding - this should have been removed long ago!! * @param bool $detailed * @return object result -- 2.4.4 From b4dade0e26d7cbdc4dfecec6d23ccf99c235094c Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Mon, 15 Jun 2015 11:38:42 +1200 Subject: [PATCH 2/2] MDL-50689 report_sercurityoverview: add report warning for https Includes following fixes: * remove XSS warning on new install * show secure cookie warning always Change-Id: I38a839932fca74be0e9441e22d6673ac4aa9046e Reviewed-on: https://review.totaralms.com/8112 Reviewed-by: Sam Hemelryk Reviewed-by: Petr Skoda Tested-by: Jenkins Automation --- report/security/lang/en/report_security.php | 8 ++++-- report/security/locallib.php | 42 ++++++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/report/security/lang/en/report_security.php b/report/security/lang/en/report_security.php index 2dcabb2..a5fd982 100644 --- a/report/security/lang/en/report_security.php +++ b/report/security/lang/en/report_security.php @@ -31,8 +31,8 @@ Please note that this measure does not improve security of the server significan $string['check_configrw_name'] = 'Writable config.php'; $string['check_configrw_ok'] = 'config.php can not be modified by PHP scripts.'; $string['check_configrw_warning'] = 'PHP scripts may modify config.php.'; -$string['check_cookiesecure_details'] = '

If you enable https communication it is recommended that you also enable secure cookies. You should also add permanent redirection from http to https.

'; -$string['check_cookiesecure_error'] = 'Please enable secure cookies'; +$string['check_cookiesecure_details'] = '

If you enable https communication it is strongly recommended that you also enable secure cookies. You should also add permanent redirection from http to https.

'; +$string['check_cookiesecure_error'] = 'Enable secure cookies.'; $string['check_cookiesecure_name'] = 'Secure cookies'; $string['check_cookiesecure_ok'] = 'Secure cookies enabled.'; $string['check_defaultuserrole_details'] = '

All logged in users are given capabilities of the default user role. Please make sure no risky capabilities are allowed in this role.

@@ -72,6 +72,10 @@ $string['check_guestrole_error'] = 'The guest role "{$a}" is incorrectly defined $string['check_guestrole_name'] = 'Guest role'; $string['check_guestrole_notset'] = 'Guest role is not set.'; $string['check_guestrole_ok'] = 'Guest role definition is OK.'; +$string['check_https_details'] = '

HTTP protocol is easily exploitable, it is strongly recommended to use HTTPS protocol on all production servers.

'; +$string['check_https_warning'] = 'HTTPS protocol is not used.'; +$string['check_https_name'] = 'HTTPS protocol'; +$string['check_https_ok'] = 'HTTPS protocol is used.'; $string['check_mediafilterswf_details'] = '

Automatic swf embedding is very dangerous - any registered user may launch an XSS attack against other server users. Please disable it on production servers.

'; $string['check_mediafilterswf_error'] = 'Flash media filter is enabled - this is very dangerous for the majority of servers.'; $string['check_mediafilterswf_name'] = 'Enabled .swf media filter'; diff --git a/report/security/locallib.php b/report/security/locallib.php index 52f9728..cb3bbe0 100644 --- a/report/security/locallib.php +++ b/report/security/locallib.php @@ -49,6 +49,7 @@ function report_security_get_issue_list() { 'report_security_check_passwordpolicy', 'report_security_check_emailchangeconfirmation', 'report_security_check_usernameenumeration', + 'report_security_check_https', 'report_security_check_cookiesecure', 'report_security_check_configrw', 'report_security_check_riskxss', @@ -418,9 +419,7 @@ function report_security_check_emailchangeconfirmation($detailed=false) { function report_security_check_cookiesecure($detailed=false) { global $CFG; - if (!is_https()) { - return null; - } + // Show this always, not just on HTTPS sites. $result = new stdClass(); $result->issue = 'report_security_check_cookiesecure'; @@ -514,6 +513,11 @@ function report_security_check_riskxss($detailed=false) { $result->info = get_string('check_riskxss_warning', 'report_security', $count); + if ($count === 0) { + // No users means no warning, this is good for new installs. + $result->status = REPORT_SECURITY_OK; + } + if ($detailed) { $userfields = user_picture::fields('u'); $users = $DB->get_records_sql("SELECT DISTINCT $userfields $sqlfrom", $params); @@ -900,4 +904,34 @@ function report_security_check_webcron($detailed = false) { } return $result; -} \ No newline at end of file +} + +/** + * Verifies if https used in Totara. + * + * @param bool $detailed + * @return stdClass result + */ +function report_security_check_https($detailed = false) { + global $CFG; + + $result = new stdClass(); + $result->issue = 'report_security_check_https'; + $result->name = get_string('check_https_name', 'report_security'); + $result->details = null; + $result->link = ''; + + if ((strpos($CFG->httpswwwroot, 'https://') !== 0)) { + $result->status = REPORT_SECURITY_SERIOUS; + $result->info = get_string('check_https_warning', 'report_security'); + } else { + $result->status = REPORT_SECURITY_OK; + $result->info = get_string('check_https_ok', 'report_security'); + } + + if ($detailed) { + $result->details = get_string('check_https_details', 'report_security'); + } + + return $result; +} -- 2.4.4