From f45c9598955130a604ced6f7b5255887cec8c4d6 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 f3eb608..d3ef467 100644 --- a/report/security/lang/en/report_security.php +++ b/report/security/lang/en/report_security.php @@ -116,6 +116,12 @@ $string['check_unsecuredataroot_error'] = 'Your dataroot directory {$a} +

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 e9e654a..0ca90ab 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', @@ -139,6 +140,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 5dcc31da34ecd0a0ac8e93e72fd3fc36705270b8 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 | 40 ++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/report/security/lang/en/report_security.php b/report/security/lang/en/report_security.php index d3ef467..0c37d8b 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 0ca90ab..d50087f 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', @@ -417,9 +418,7 @@ function report_security_check_emailchangeconfirmation($detailed=false) { function report_security_check_cookiesecure($detailed=false) { global $CFG; - if (strpos($CFG->wwwroot, 'https://') !== 0) { - return null; - } + // Show this always, not just on HTTPS sites. $result = new stdClass(); $result->issue = 'report_security_check_cookiesecure'; @@ -513,6 +512,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); @@ -866,3 +870,33 @@ function report_security_check_riskbackup($detailed=false) { return $result; } + +/** + * 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