Commit 36adc757 authored by webchick's avatar webchick

#575280 follow-up by mfb and chx: Fixed impersonation attack when an https session exists.

parent 59b7e23b
......@@ -151,12 +151,19 @@ function _drupal_session_write($sid, $value) {
'session' => $value,
'timestamp' => REQUEST_TIME,
);
$insecure_session_name = substr(session_name(), 1);
if ($is_https && isset($_COOKIE[$insecure_session_name])) {
$fields['sid'] = $_COOKIE[$insecure_session_name];
$key = array('sid' => $sid);
if ($is_https) {
$key['ssid'] = $sid;
$insecure_session_name = substr(session_name(), 1);
// The "secure pages" setting allows a site to simultaneously use both
// secure and insecure session cookies. If enabled, use the insecure session
// identifier as the sid.
if (variable_get('https', FALSE) && isset($_COOKIE[$insecure_session_name])) {
$key['sid'] = $_COOKIE[$insecure_session_name];
}
}
db_merge('sessions')
->key(array($is_https ? 'ssid' : 'sid' => $sid))
->key($key)
->fields($fields)
->execute();
......
......@@ -760,6 +760,8 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
}
function testDrupalHTTPRequest() {
global $is_https;
// Parse URL schema.
$missing_scheme = drupal_http_request('example.com/path');
$this->assertEqual($missing_scheme->code, -1002, t('Returned with "-1002" error code.'));
......@@ -781,18 +783,23 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
$this->assertEqual($result->code, '404', t('Result code is 404'));
$this->assertEqual($result->status_message, 'Not Found', t('Result status message is "Not Found"'));
// Test that timeout is respected. The test machine is expected to be able
// to make the connection (i.e. complete the fsockopen()) in 2 seconds and
// return within a total of 5 seconds. If the test machine is extremely
// slow, the test will fail. fsockopen() has been seen to time out in
// slightly less than the specified timeout, so allow a little slack on the
// minimum expected time (i.e. 1.8 instead of 2).
timer_start(__METHOD__);
$result = drupal_http_request(url('system-test/sleep/10', array('absolute' => TRUE)), array('timeout' => 2));
$time = timer_read(__METHOD__) / 1000;
$this->assertTrue(1.8 < $time && $time < 5, t('Request timed out (%time seconds).', array('%time' => $time)));
$this->assertTrue($result->error, t('An error message was returned.'));
$this->assertEqual($result->code, HTTP_REQUEST_TIMEOUT, t('Proper error code was returned.'));
// Skip the timeout tests when the testing environment is HTTPS because
// stream_set_timeout() does not work for SSL connections.
// @link http://bugs.php.net/bug.php?id=47929
if (!$is_https) {
// Test that timeout is respected. The test machine is expected to be able
// to make the connection (i.e. complete the fsockopen()) in 2 seconds and
// return within a total of 5 seconds. If the test machine is extremely
// slow, the test will fail. fsockopen() has been seen to time out in
// slightly less than the specified timeout, so allow a little slack on
// the minimum expected time (i.e. 1.8 instead of 2).
timer_start(__METHOD__);
$result = drupal_http_request(url('system-test/sleep/10', array('absolute' => TRUE)), array('timeout' => 2));
$time = timer_read(__METHOD__) / 1000;
$this->assertTrue(1.8 < $time && $time < 5, t('Request timed out (%time seconds).', array('%time' => $time)));
$this->assertTrue($result->error, t('An error message was returned.'));
$this->assertEqual($result->code, HTTP_REQUEST_TIMEOUT, t('Proper error code was returned.'));
}
}
function testDrupalHTTPRequestBasicAuth() {
......@@ -800,7 +807,7 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
$password = $this->randomName();
$url = url('system-test/auth', array('absolute' => TRUE));
$auth = str_replace('http://', 'http://' . $username . ':' . $password . '@', $url);
$auth = str_replace('://', '://' . $username . ':' . $password . '@', $url);
$result = drupal_http_request($auth);
$this->drupalSetContent($result->data);
......
......@@ -12,6 +12,9 @@
exit;
}
// Set a global variable to indicate a mock HTTPS request.
$is_https_mock = empty($_SERVER['HTTPS']);
// Change to https.
$_SERVER['HTTPS'] = 'on';
......
......@@ -271,19 +271,63 @@ class SessionHttpsTestCase extends DrupalWebTestCase {
protected function testHttpsSession() {
global $is_https;
if ($is_https) {
$secure_session_name = session_name();
$insecure_session_name = substr(session_name(), 1);
}
else {
$secure_session_name = 'S' . session_name();
$insecure_session_name = session_name();
}
$user = $this->drupalCreateUser(array('access administration pages'));
// Test HTTPS session handling by altering the form action to submit the
// login form through https.php, which creates a mock HTTPS request.
$this->drupalGet('user');
$form = $this->xpath('//form[@id="user-login"]');
$form[0]['action'] = $this->httpsUrl('user');
$edit = array('name' => $user->name, 'pass' => $user->pass_raw);
$this->drupalPost(NULL, $edit, t('Log in'));
// Test a second concurrent session.
$this->curlClose();
$this->drupalGet('user');
$form = $this->xpath('//form[@id="user-login"]');
$form[0]['action'] = $this->httpsUrl('user');
$this->drupalPost(NULL, $edit, t('Log in'));
// Check secure cookie on secure page.
$this->assertTrue($this->cookies[$secure_session_name]['secure'], 'The secure cookie has the secure attribute');
// Check insecure cookie is not set.
$this->assertFalse(isset($this->cookies[$insecure_session_name]));
$args = array_fill_keys(array(':sid', ':ssid'), $this->cookies[$secure_session_name]['value']);
$this->assertTrue(db_query('SELECT sid FROM {sessions} WHERE sid = :sid AND ssid = :ssid', $args)->fetchField(), 'Session has both SIDs');
$cookie = $secure_session_name . '=' . $args[':ssid'];
// Verify that user is logged in on secure URL.
$this->curlClose();
$this->drupalGet($this->httpsUrl('admin'), array(), array('Cookie: ' . $cookie));
$this->assertText(t('Administer'));
// Verify that user is not logged in on non-secure URL.
if (!$is_https) {
$this->curlClose();
$this->drupalGet('admin', array(), array('Cookie: ' . $cookie));
$this->assertNoText(t('Administer'));
}
// Clear browser cookie jar.
$this->cookies = array();
if ($is_https) {
// The functionality does not make sense when running on https.
return;
}
$insecure_session_name = session_name();
$secure_session_name = "S$insecure_session_name";
// Enable secure pages.
variable_set('https', TRUE);
$user = $this->drupalCreateUser(array('access administration pages'));
$this->curlClose();
$this->drupalGet('session-test/set/1');
// Check secure cookie on insecure page.
......
......@@ -157,6 +157,11 @@ function session_test_form_user_login_alter(&$form) {
* page through https.php.
*/
function session_test_drupal_goto_alter(&$path, &$options, &$http_response_code) {
global $base_insecure_url;
$path = $base_insecure_url . '/' . $path;
global $base_insecure_url, $is_https_mock;
// Alter the redirect to use HTTP when using a mock HTTPS request through
// https.php because form submissions would otherwise redirect to a
// non-existent HTTPS site.
if (!empty($is_https_mock)) {
$path = $base_insecure_url . '/' . $path;
}
}
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