Index: includes/session.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/session.inc,v retrieving revision 1.90 diff -u -p -r1.90 session.inc --- includes/session.inc 15 Oct 2010 04:15:41 -0000 1.90 +++ includes/session.inc 29 Oct 2010 22:10:36 -0000 @@ -88,7 +88,10 @@ function _drupal_session_read($sid) { // a HTTPS session or we are about to log in so we check the sessions table // for an anonymous session with the non-HTTPS-only cookie. if ($is_https) { - $user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.ssid = :ssid", array(':ssid' => $sid))->fetchObject(); + // Ensure that an empty secure session ID cannot be selected. + if ($sid) { + $user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.ssid = :ssid", array(':ssid' => $sid))->fetchObject(); + } if (!$user) { if (isset($_COOKIE[$insecure_session_name])) { $user = db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = :sid AND s.uid = 0", array( @@ -180,21 +183,23 @@ function _drupal_session_write($sid, $va 'timestamp' => REQUEST_TIME, ); - // 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'. + // Use the session ID as 'sid' and an empty string as 'ssid', which + // _drupal_session_read() will prevent from being selected as a valid + // secure session ID. + $key = array('sid' => $sid, 'ssid' => ''); + // On HTTPS connections, use the session ID as both 'sid' and 'ssid'. if ($is_https) { $key['ssid'] = $sid; - $insecure_session_name = substr(session_name(), 1); - if (variable_get('https', FALSE) && isset($_COOKIE[$insecure_session_name])) { - $key['sid'] = $_COOKIE[$insecure_session_name]; + // 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 (variable_get('https', FALSE)) { + $insecure_session_name = substr(session_name(), 1); + if (isset($_COOKIE[$insecure_session_name])) { + $key['sid'] = $_COOKIE[$insecure_session_name]; + } } } - else { - $key['sid'] = $sid; - } db_merge('sessions') ->key($key) Index: includes/update.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/update.inc,v retrieving revision 1.80 diff -u -p -r1.80 update.inc --- includes/update.inc 27 Oct 2010 19:31:53 -0000 1.80 +++ includes/update.inc 29 Oct 2010 22:10:36 -0000 @@ -701,7 +701,7 @@ function update_fix_d7_requirements() { } // Add ssid column and index. - db_add_field('sessions', 'ssid', array('description' => "Secure session ID. The value is generated by PHP's Session API.", 'type' => 'varchar', 'length' => 128, 'not null' => TRUE, 'default' => '')); + db_add_field('sessions', 'ssid', array('description' => "Secure session ID. The value is generated by Drupal's session handlers.", 'type' => 'varchar', 'length' => 128, 'not null' => TRUE, 'default' => '')); db_add_index('sessions', 'ssid', array('ssid')); // Drop existing primary key. db_drop_primary_key('sessions'); Index: modules/simpletest/tests/http.php =================================================================== RCS file: modules/simpletest/tests/http.php diff -N modules/simpletest/tests/http.php --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ modules/simpletest/tests/http.php 29 Oct 2010 22:10:36 -0000 @@ -0,0 +1,29 @@ + $value) { + $_SERVER[$key] = str_replace('modules/simpletest/tests/http.php', 'index.php', $value); + $_SERVER[$key] = str_replace('https://', 'http://', $_SERVER[$key]); +} + +require_once 'index.php'; Index: modules/simpletest/tests/session.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/session.test,v retrieving revision 1.33 diff -u -p -r1.33 session.test --- modules/simpletest/tests/session.test 15 Oct 2010 04:15:41 -0000 1.33 +++ modules/simpletest/tests/session.test 29 Oct 2010 22:10:36 -0000 @@ -316,7 +316,7 @@ class SessionHttpsTestCase extends Drupa // Check insecure cookie is not set. $this->assertFalse(isset($this->cookies[$insecure_session_name])); $ssid = $this->cookies[$secure_session_name]['value']; - $this->assertSessionIds('', $ssid, 'Session has NULL for SID and a correct secure SID.'); + $this->assertSessionIds($ssid, $ssid, 'Session has a non-empty SID and a correct secure SID.'); $cookie = $secure_session_name . '=' . $ssid; // Verify that user is logged in on secure URL. @@ -326,12 +326,36 @@ class SessionHttpsTestCase extends Drupa $this->assertResponse(200); // Verify that user is not logged in on non-secure URL. - if (!$is_https) { - $this->curlClose(); - $this->drupalGet('admin/config', array(), array('Cookie: ' . $cookie)); - $this->assertNoText(t('Configuration')); - $this->assertResponse(403); - } + $this->curlClose(); + $this->drupalGet($this->httpUrl('admin/config'), array(), array('Cookie: ' . $cookie)); + $this->assertNoText(t('Configuration')); + $this->assertResponse(403); + + // Verify that empty SID cannot be used on the non-secure site. + $this->curlClose(); + $cookie = $insecure_session_name . '='; + $this->drupalGet($this->httpUrl('admin/config'), array(), array('Cookie: ' . $cookie)); + $this->assertResponse(403); + + // Test HTTP session handling by altering the form action to submit the + // login form through http.php, which creates a mock HTTP request on HTTPS + // test environments. + $this->curlClose(); + $this->drupalGet('user'); + $form = $this->xpath('//form[@id="user-login"]'); + $form[0]['action'] = $this->httpUrl('user'); + $edit = array('name' => $user->name, 'pass' => $user->pass_raw); + $this->drupalPost(NULL, $edit, t('Log in')); + $this->drupalGet($this->httpUrl('admin/config')); + $this->assertResponse(200); + $sid = $this->cookies[$insecure_session_name]['value']; + $this->assertSessionIds($sid, '', 'Session has the correct SID and an empty secure SID.'); + + // Verify that empty secure SID cannot be used on the secure site. + $this->curlClose(); + $cookie = $secure_session_name . '='; + $this->drupalGet($this->httpsUrl('admin/config'), array(), array('Cookie: ' . $cookie)); + $this->assertResponse(403); // Clear browser cookie jar. $this->cookies = array(); @@ -458,9 +482,32 @@ class SessionHttpsTestCase extends Drupa return $this->assertTrue(db_query('SELECT timestamp FROM {sessions} WHERE sid = :sid AND ssid = :ssid', $args)->fetchField(), $assertion_text); } + /** + * Builds a URL for submitting a mock HTTPS request to HTTP test environments. + * + * @param $url + * A Drupal path such as 'user'. + * + * @return + * An absolute URL. + */ protected function httpsUrl($url) { global $base_url; return $base_url . '/modules/simpletest/tests/https.php?q=' . $url; } + + /** + * Builds a URL for submitting a mock HTTP request to HTTPS test environments. + * + * @param $url + * A Drupal path such as 'user'. + * + * @return + * An absolute URL. + */ + protected function httpUrl($url) { + global $base_url; + return $base_url . '/modules/simpletest/tests/http.php?q=' . $url; + } } Index: modules/simpletest/tests/upgrade/upgrade.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/upgrade/upgrade.test,v retrieving revision 1.9 diff -u -p -r1.9 upgrade.test --- modules/simpletest/tests/upgrade/upgrade.test 8 Oct 2010 18:19:11 -0000 1.9 +++ modules/simpletest/tests/upgrade/upgrade.test 29 Oct 2010 22:10:36 -0000 @@ -113,7 +113,12 @@ abstract class UpgradePathTestCase exten // Force our way into the session of the child site. drupal_save_session(TRUE); + // A session cannot be written without the ssid column which is missing on + // Drupal 6 sites. + db_add_field('sessions', 'ssid', array('description' => "Secure session ID. The value is generated by Drupal's session handlers.", 'type' => 'varchar', 'length' => 128, 'not null' => TRUE, 'default' => '')); _drupal_session_write($sid, ''); + // Remove the temporarily added ssid column. + db_drop_field('sessions', 'ssid'); drupal_save_session(FALSE); // Restore necessary variables. Index: modules/system/system.install =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.install,v retrieving revision 1.520 diff -u -p -r1.520 system.install --- modules/system/system.install 20 Oct 2010 00:47:44 -0000 1.520 +++ modules/system/system.install 29 Oct 2010 22:10:36 -0000 @@ -1469,14 +1469,13 @@ function system_schema() { 'not null' => TRUE, ), 'sid' => array( - 'description' => "A session ID. The value is generated by PHP's Session API.", + 'description' => "A session ID. The value is generated by Drupal's session handlers.", 'type' => 'varchar', 'length' => 128, 'not null' => TRUE, - 'default' => '', ), 'ssid' => array( - 'description' => "Secure session ID. The value is generated by PHP's Session API.", + 'description' => "Secure session ID. The value is generated by Drupal's session handlers.", 'type' => 'varchar', 'length' => 128, 'not null' => TRUE, @@ -2902,6 +2901,19 @@ function system_update_7064() { } /** + * Remove the default value for sid. + */ +function system_update_7065() { + $spec = array( + 'description' => "A session ID. The value is generated by Drupal's session handlers.", + 'type' => 'varchar', + 'length' => 128, + 'not null' => TRUE, + ); + db_change_field('sessions', 'sid', 'sid', $spec); +} + +/** * @} End of "defgroup updates-6.x-to-7.x" * The next series of updates should start at 8000. */