Commit 657ddbf5 authored by Brendan Heywood's avatar Brendan Heywood
Browse files

MDL-55273 admin: Change $CFG->cookiesecure default to on

parent 3ca3cc77
......@@ -109,7 +109,7 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page
// "httpsecurity" settingpage
$temp = new admin_settingpage('httpsecurity', new lang_string('httpsecurity', 'admin'));
$temp->add(new admin_setting_configcheckbox('loginhttps', new lang_string('loginhttps', 'admin'), new lang_string('configloginhttps', 'admin'), 0));
$temp->add(new admin_setting_configcheckbox('cookiesecure', new lang_string('cookiesecure', 'admin'), new lang_string('configcookiesecure', 'admin'), 0));
$temp->add(new admin_setting_configcheckbox('cookiesecure', new lang_string('cookiesecure', 'admin'), new lang_string('configcookiesecure', 'admin'), 1));
$temp->add(new admin_setting_configcheckbox('cookiehttponly', new lang_string('cookiehttponly', 'admin'), new lang_string('configcookiehttponly', 'admin'), 0));
$temp->add(new admin_setting_configcheckbox('allowframembedding', new lang_string('allowframembedding', 'admin'), new lang_string('allowframembedding_help', 'admin'), 0));
$temp->add(new admin_setting_configcheckbox('loginpasswordautocomplete', new lang_string('loginpasswordautocomplete', 'admin'), new lang_string('loginpasswordautocomplete_help', 'admin'), 0));
......
......@@ -151,7 +151,7 @@ $string['configbloglevel'] = 'This setting allows you to restrict the level to w
$string['configcalendarcustomexport'] = 'Enable custom date range export of calendar';
$string['configcalendarexportsalt'] = 'This random text is used for improving of security of authentication tokens used for exporting of calendars. Please note that all current tokens are invalidated if you change this hash salt.';
$string['configcookiehttponly'] = 'Enables new PHP 5.2.0 feature - browsers are instructed to send cookie with real http requests only, cookies should not be accessible by scripting languages. This is not supported in all browsers and it may not be fully compatible with current code. It helps to prevent some types of XSS attacks.';
$string['configcookiesecure'] = 'If server is accepting only https connections it is recommended to enable sending of secure cookies. If enabled please make sure that web server is not accepting http:// or set up permanent redirection to https:// address. When <em>wwwroot</em> address does not start with https:// this setting is turned off automatically.';
$string['configcookiesecure'] = 'If server is accepting only https connections it is recommended to enable sending of secure cookies. If enabled please make sure that web server is not accepting http:// or set up permanent redirection to https:// address and ideally send HSTS headers. When <em>wwwroot</em> address does not start with https:// this setting is ignored.';
$string['configcountry'] = 'If you set a country here, then this country will be selected by default on new user accounts. To force users to choose a country, just leave this unset.';
$string['configcourseoverviewfilesext'] = 'A comma-separated list of allowed course summary files extensions.';
$string['configcourseoverviewfileslimit'] = 'The maximum number of files that can be attached to a course summary.';
......
......@@ -191,9 +191,7 @@ class manager {
protected static function prepare_cookies() {
global $CFG;
if (!isset($CFG->cookiesecure) or (!is_https() and empty($CFG->sslproxy))) {
$CFG->cookiesecure = 0;
}
$cookiesecure = is_moodle_cookie_secure();
if (!isset($CFG->cookiehttponly)) {
$CFG->cookiehttponly = 0;
......@@ -254,7 +252,7 @@ class manager {
// Set configuration.
session_name($sessionname);
session_set_cookie_params(0, $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $CFG->cookiesecure, $CFG->cookiehttponly);
session_set_cookie_params(0, $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $cookiesecure, $CFG->cookiehttponly);
ini_set('session.use_trans_sid', '0');
ini_set('session.use_only_cookies', '1');
ini_set('session.hash_function', '0'); // For now MD5 - we do not have room for sha-1 in sessions table.
......
......@@ -86,6 +86,25 @@ function require_sesskey() {
}
}
/**
* Determine wether the secure flag should be set on cookies
* @return bool
*/
function is_moodle_cookie_secure() {
global $CFG;
if (!isset($CFG->cookiesecure)) {
return false;
}
if (!empty($CFG->loginhttps)) {
return false;
}
if (!is_https() and empty($CFG->sslproxy)) {
return false;
}
return !empty($CFG->cookiesecure);
}
/**
* Sets a moodle cookie with a weakly encrypted username
*
......@@ -111,12 +130,14 @@ function set_moodle_cookie($username) {
$cookiename = 'MOODLEID1_'.$CFG->sessioncookie;
// delete old cookie
setcookie($cookiename, '', time() - HOURSECS, $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $CFG->cookiesecure, $CFG->cookiehttponly);
$cookiesecure = is_moodle_cookie_secure();
// Delete old cookie.
setcookie($cookiename, '', time() - HOURSECS, $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $cookiesecure, $CFG->cookiehttponly);
if ($username !== '') {
// set username cookie for 60 days
setcookie($cookiename, rc4encrypt($username), time()+(DAYSECS*60), $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $CFG->cookiesecure, $CFG->cookiehttponly);
// Set username cookie for 60 days.
setcookie($cookiename, rc4encrypt($username), time() + (DAYSECS * 60), $CFG->sessioncookiepath, $CFG->sessioncookiedomain, $cookiesecure, $CFG->cookiehttponly);
}
}
......
......@@ -154,6 +154,110 @@ class core_sessionlib_testcase extends advanced_testcase {
$this->assertSame($GLOBALS['USER'], $USER);
}
/**
* Test provided for secure cookie
*
* @return array of config and secure result
*/
public function moodle_cookie_secure_provider() {
return array(
array(
// Non ssl, not set.
'config' => array(
'wwwroot' => 'http://example.com',
'httpswwwroot' => 'http://example.com',
'sslproxy' => null,
'loginhttps' => null,
'cookiesecure' => null,
),
'secure' => false,
),
array(
// Non ssl, off and ignored.
'config' => array(
'wwwroot' => 'http://example.com',
'httpswwwroot' => 'http://example.com',
'sslproxy' => null,
'loginhttps' => null,
'cookiesecure' => false,
),
'secure' => false,
),
array(
// Non ssl, on and ignored.
'config' => array(
'wwwroot' => 'http://example.com',
'httpswwwroot' => 'http://example.com',
'sslproxy' => null,
'loginhttps' => null,
'cookiesecure' => true,
),
'secure' => false,
),
array(
// SSL via proxy, off.
'config' => array(
'wwwroot' => 'http://example.com',
'httpswwwroot' => 'http://example.com',
'sslproxy' => true,
'loginhttps' => null,
'cookiesecure' => false,
),
'secure' => false,
),
array(
// SSL via proxy, on.
'config' => array(
'wwwroot' => 'http://example.com',
'httpswwwroot' => 'http://example.com',
'sslproxy' => true,
'loginhttps' => null,
'cookiesecure' => true,
),
'secure' => true,
),
array(
// SSL and off.
'config' => array(
'wwwroot' => 'https://example.com',
'httpswwwroot' => 'https://example.com',
'sslproxy' => null,
'loginhttps' => null,
'cookiesecure' => false,
),
'secure' => false,
),
array(
// SSL and on.
'config' => array(
'wwwroot' => 'https://example.com',
'httpswwwroot' => 'https://example.com',
'sslproxy' => null,
'loginhttps' => null,
'cookiesecure' => true,
),
'secure' => true,
),
);
}
/**
* Test for secure cookie
*
* @dataProvider moodle_cookie_secure_provider
*
* @param array $config Array of key value config settings
* @param bool $secure Wether cookies should be secure or not
*/
public function test_is_moodle_cookie_secure($config, $secure) {
$this->resetAfterTest();
foreach ($config as $key => $value) {
set_config($key, $value);
}
$this->assertEquals($secure, is_moodle_cookie_secure());
}
public function test_sesskey() {
global $USER;
$this->resetAfterTest();
......
......@@ -31,7 +31,7 @@ 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'] = '<p>If you enable https communication it is recommended that you also enable secure cookies. You should also add permanent redirection from http to https.</p>';
$string['check_cookiesecure_details'] = '<p>If you enable https communication it is recommended that you also enable secure cookies. You should also add permanent redirection from http to https. Ideally also serve HSTS headers well.</p>';
$string['check_cookiesecure_error'] = 'Please enable secure cookies';
$string['check_cookiesecure_name'] = 'Secure cookies';
$string['check_cookiesecure_ok'] = 'Secure cookies enabled.';
......
......@@ -394,7 +394,7 @@ function report_security_check_cookiesecure($detailed=false) {
$result->status = null;
$result->link = "<a href=\"$CFG->wwwroot/$CFG->admin/settings.php?section=httpsecurity\">".get_string('httpsecurity', 'admin').'</a>';
if (empty($CFG->cookiesecure)) {
if (!is_moodle_cookie_secure()) {
$result->status = REPORT_SECURITY_SERIOUS;
$result->info = get_string('check_cookiesecure_error', 'report_security');
} else {
......
Supports Markdown
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