diff --git a/includes/session.inc b/includes/session.inc index ce5524a2226a430f818e24805479b1deeaae06e8..2d2c2b638298ccbb784eea6f2ef5dfa3ee47dce0 100644 --- a/includes/session.inc +++ b/includes/session.inc @@ -147,6 +147,7 @@ function _drupal_session_write($sid, $value) { return; } + // Either ssid or sid or both will be added from $key below. $fields = array( 'uid' => $user->uid, 'cache' => isset($user->cache) ? $user->cache : 0, @@ -154,17 +155,22 @@ function _drupal_session_write($sid, $value) { 'session' => $value, 'timestamp' => REQUEST_TIME, ); - $key = array('sid' => $sid); + + // The "secure pages" setting allows a site to simultaneously use both secure + // and insecure session cookies. If enabled and both cookies are presented + // then use both keys. If not enabled but on HTTPS then use the PHP session + // id as 'ssid'. If on HTTP then use the PHP session id as '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]; } } + else { + $key['sid'] = $sid; + } + db_merge('sessions') ->key($key) ->fields($fields) @@ -198,11 +204,11 @@ function _drupal_session_write($sid, $value) { * Initialize the session handler, starting a session if needed. */ function drupal_session_initialize() { - global $user; + global $user, $is_https; session_set_save_handler('_drupal_session_open', '_drupal_session_close', '_drupal_session_read', '_drupal_session_write', '_drupal_session_destroy', '_drupal_session_garbage_collection'); - if (isset($_COOKIE[session_name()])) { + if (isset($_COOKIE[session_name()]) || ($is_https && variable_get('https', FALSE) && isset($_COOKIE[substr(session_name(), 1)]))) { // If a session cookie exists, initialize the session. Otherwise the // session is only started on demand in drupal_session_commit(), making // anonymous users not use a session cookie unless something is stored in @@ -298,6 +304,9 @@ function drupal_session_regenerate() { global $user, $is_https; if ($is_https && variable_get('https', FALSE)) { $insecure_session_name = substr(session_name(), 1); + if (isset($_COOKIE[$insecure_session_name])) { + $old_insecure_session_id = $_COOKIE[$insecure_session_name]; + } $params = session_get_cookie_params(); $session_id = drupal_hash_base64(uniqid(mt_rand(), TRUE) . drupal_random_bytes(55)); setcookie($insecure_session_name, $session_id, REQUEST_TIME + $params['lifetime'], $params['path'], $params['domain'], FALSE, $params['httponly']); @@ -318,11 +327,27 @@ function drupal_session_regenerate() { } if (isset($old_session_id)) { + $fields = array('sid' => session_id()); + if ($is_https) { + $fields['ssid'] = session_id(); + // If the "secure pages" setting is enabled, use the newly-created + // insecure session identifier as the regenerated sid. + if (variable_get('https', FALSE)) { + $fields['sid'] = $session_id; + } + } + db_update('sessions') + ->fields($fields) + ->condition($is_https ? 'ssid' : 'sid', $old_session_id) + ->execute(); + } + elseif (isset($old_insecure_session_id)) { + // If logging in to the secure site, and there was no active session on the + // secure site but a session was active on the insecure site, update the + // insecure session with the new session identifiers. db_update('sessions') - ->fields(array( - $is_https ? 'ssid' : 'sid' => session_id() - )) - ->condition('sid', $old_session_id) + ->fields(array('sid' => $session_id, 'ssid' => session_id())) + ->condition('sid', $old_insecure_session_id) ->execute(); } date_default_timezone_set(drupal_get_user_timezone()); diff --git a/modules/simpletest/tests/session.test b/modules/simpletest/tests/session.test index 1b74405fd3b5b1fcaa11b63f7668154526be2076..600843bb80eec89f8042646259a24071878f41d5 100644 --- a/modules/simpletest/tests/session.test +++ b/modules/simpletest/tests/session.test @@ -274,7 +274,7 @@ class SessionHttpsTestCase extends DrupalWebTestCase { // Check insecure cookie is not set. $this->assertFalse(isset($this->cookies[$insecure_session_name])); $ssid = $this->cookies[$secure_session_name]['value']; - $this->assertSessionIds($ssid, $ssid, 'Session has two secure SIDs'); + $this->assertSessionIds('', $ssid, 'Session has NULL for SID and a correct secure SID.'); $cookie = $secure_session_name . '=' . $ssid; // Verify that user is logged in on secure URL. @@ -303,12 +303,18 @@ class SessionHttpsTestCase extends DrupalWebTestCase { variable_set('https', TRUE); $this->curlClose(); - $this->drupalGet('session-test/set/1'); + // Start an anonymous session on the insecure site. + $session_data = $this->randomName(); + $this->drupalGet('session-test/set/' . $session_data); // Check secure cookie on insecure page. $this->assertFalse(isset($this->cookies[$secure_session_name]), 'The secure cookie is not sent on insecure pages.'); // Check insecure cookie on insecure page. $this->assertFalse($this->cookies[$insecure_session_name]['secure'], 'The insecure cookie does not have the secure attribute'); + // Store the anonymous cookie so we can validate that its session is killed + // after login. + $anonymous_cookie = $insecure_session_name . '=' . $this->cookies[$insecure_session_name]['value']; + // Check that password request form action is not secure. $this->drupalGet('user/password'); $form = $this->xpath('//form[@id="user-pass"]'); @@ -339,6 +345,11 @@ class SessionHttpsTestCase extends DrupalWebTestCase { $secure_session_name . '=' . $ssid, ); + // Test that session data saved before login is still available on the + // authenticated session. + $this->drupalGet('session-test/get'); + $this->assertText($session_data, 'Session correctly returned the stored data set by the anonymous session.'); + foreach ($cookies as $cookie_key => $cookie) { foreach (array('admin/config', $this->httpsUrl('admin/config')) as $url_key => $url) { $this->curlClose(); @@ -354,6 +365,33 @@ class SessionHttpsTestCase extends DrupalWebTestCase { } } } + + // Test that session data saved before login is not available using the + // pre-login anonymous cookie. + $this->cookies = array(); + $this->drupalGet('session-test/get', array('Cookie: ' . $anonymous_cookie)); + $this->assertNoText($session_data, 'Initial anonymous session is inactive after login.'); + + // Clear browser cookie jar. + $this->cookies = array(); + + // Start an anonymous session on the secure site. + $this->drupalGet($this->httpsUrl('session-test/set/1')); + + // Mock a login to the secure site using the secure session cookie. + $this->drupalGet('user'); + $form = $this->xpath('//form[@id="user-login"]'); + $form[0]['action'] = $this->httpsUrl('user'); + $this->drupalPost(NULL, $edit, t('Log in'), array(), array('Cookie: ' . $secure_session_name . '=' . $this->cookies[$secure_session_name]['value'])); + + // Get the insecure session cookie set by the secure login POST request. + $headers = $this->drupalGetHeaders(TRUE); + strtok($headers[0]['set-cookie'], ';='); + $session_id = strtok(';='); + + // Test that the user is also authenticated on the insecure site. + $this->drupalGet("user/{$user->uid}/edit", array(), array('Cookie: ' . $insecure_session_name . '=' . $session_id)); + $this->assertResponse(200); } /** @@ -375,7 +413,7 @@ class SessionHttpsTestCase extends DrupalWebTestCase { ':sid' => $sid, ':ssid' => $ssid, ); - return $this->assertTrue(db_query('SELECT sid FROM {sessions} WHERE sid = :sid AND ssid = :ssid', $args)->fetchField(), $assertion_text); + return $this->assertTrue(db_query('SELECT timestamp FROM {sessions} WHERE sid = :sid AND ssid = :ssid', $args)->fetchField(), $assertion_text); } protected function httpsUrl($url) { @@ -383,3 +421,4 @@ class SessionHttpsTestCase extends DrupalWebTestCase { return $base_url . '/modules/simpletest/tests/https.php?q=' . $url; } } + diff --git a/modules/system/system.install b/modules/system/system.install index 9a57f1bc97e2ad05ba9c4584148139be4789b331..c6d0470f5a89cead4fd62aca34a89faac245520d 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -1413,8 +1413,8 @@ function system_schema() { 'description' => "Unique key: Secure session ID. The value is generated by PHP's Session API.", 'type' => 'varchar', 'length' => 64, - 'not null' => FALSE, - 'default' => NULL, + 'not null' => TRUE, + 'default' => '', ), 'hostname' => array( 'description' => 'The IP address that last used this session ID (sid).', @@ -1442,14 +1442,14 @@ function system_schema() { 'size' => 'big', ), ), - 'primary key' => array('sid'), + 'primary key' => array( + 'sid', + 'ssid', + ), 'indexes' => array( 'timestamp' => array('timestamp'), 'uid' => array('uid'), ), - 'unique keys' => array( - 'ssid' => array('ssid'), - ), 'foreign keys' => array( 'uid' => array('users' => 'uid'), ),