diff --git a/core/lib/Drupal/Core/Session/SessionHandler.php b/core/lib/Drupal/Core/Session/SessionHandler.php index c8d69fb..9d81327 100644 --- a/core/lib/Drupal/Core/Session/SessionHandler.php +++ b/core/lib/Drupal/Core/Session/SessionHandler.php @@ -78,44 +78,15 @@ public function read($sid) { // Handle the case of first time visitors and clients that don't store // cookies (eg. web crawlers). - $insecure_session_name = $this->sessionManager->getInsecureName(); $cookies = $this->requestStack->getCurrentRequest()->cookies; - if (!$cookies->has($this->getName()) && !$cookies->has($insecure_session_name)) { + if (!$cookies->has($this->getName())) { $user = new UserSession(); return ''; } - // Otherwise, if the session is still active, we have a record of the - // client's session in the database. If it's HTTPS then we are either have 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. The session ID - // that is in the user's cookie is hashed before being stored in the - // database as a security measure. Thus, we have to hash it to match the - // database. - if ($this->requestStack->getCurrentRequest()->isSecure()) { - // Try to load a session using the HTTPS-only secure session id. - $values = $this->connection->query("SELECT u.*, s.* FROM {users_field_data} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE u.default_langcode = 1 AND s.ssid = :ssid", array( - ':ssid' => Crypt::hashBase64($sid), - ))->fetchAssoc(); - if (!$values) { - // Fallback and try to load the anonymous non-HTTPS session. Use the - // non-HTTPS session id as the key. - if ($cookies->has($insecure_session_name)) { - $insecure_session_id = $cookies->get($insecure_session_name); - $args = array(':sid' => Crypt::hashBase64($insecure_session_id)); - $values = $this->connection->query("SELECT u.*, s.* FROM {users_field_data} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE u.default_langcode = 1 AND s.sid = :sid AND s.uid = 0", $args)->fetchAssoc(); - if ($values) { - $this->sessionSetObsolete($insecure_session_id); - } - } - } - } - else { - // Try to load a session using the non-HTTPS session id. - $values = $this->connection->query("SELECT u.*, s.* FROM {users_field_data} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE u.default_langcode = 1 AND s.sid = :sid", array( - ':sid' => Crypt::hashBase64($sid), - ))->fetchAssoc(); - } + $values = $this->connection->query("SELECT u.*, s.* FROM {users_field_data} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE u.default_langcode = 1 AND s.sid = :sid", array( + ':sid' => Crypt::hashBase64($sid), + ))->fetchAssoc(); // We found the client's session record and they are an authenticated, // active user. @@ -158,42 +129,17 @@ public function write($sid, $value) { return TRUE; } - // Either ssid or sid or both will be added from $key below. $fields = array( 'uid' => $user->id(), 'hostname' => $this->requestStack->getCurrentRequest()->getClientIP(), 'session' => $value, 'timestamp' => REQUEST_TIME, ); - // Use the session ID as 'sid' and an empty string as 'ssid' by default. - // read() does not allow empty strings so that's a safe default. - $key = array('sid' => Crypt::hashBase64($sid), 'ssid' => ''); - // On HTTPS connections, use the session ID as both 'sid' and 'ssid'. - if ($this->requestStack->getCurrentRequest()->isSecure()) { - $key['ssid'] = Crypt::hashBase64($sid); - $cookies = $this->requestStack->getCurrentRequest()->cookies; - // 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. The session ID from the cookie is - // hashed before being stored in the database as a security measure. - if ($this->sessionManager->isMixedMode()) { - $insecure_session_name = $this->sessionManager->getInsecureName(); - if ($cookies->has($insecure_session_name)) { - $key['sid'] = Crypt::hashBase64($cookies->get($insecure_session_name)); - } - } - } - elseif ($this->sessionManager->isMixedMode()) { - unset($key['ssid']); - } $this->connection->merge('sessions') - ->keys($key) + ->keys(array('sid' => Crypt::hashBase64($sid))) ->fields($fields) ->execute(); - // Remove obsolete sessions. - $this->cleanupObsoleteSessions(); - // Likewise, do not update access time more than once per 180 seconds. if ($user->isAuthenticated() && REQUEST_TIME - $user->getLastAccessedTime() > Settings::get('session_write_interval', 180)) { /** @var \Drupal\user\UserStorageInterface $storage */ @@ -231,10 +177,9 @@ public function destroy($sid) { if (!$this->sessionManager->isEnabled()) { return TRUE; } - $is_https = $this->requestStack->getCurrentRequest()->isSecure(); // Delete session data. $this->connection->delete('sessions') - ->condition($is_https ? 'ssid' : 'sid', Crypt::hashBase64($sid)) + ->condition('sid', Crypt::hashBase64($sid)) ->execute(); // Reset $_SESSION and $user to prevent a new session from being started @@ -244,15 +189,6 @@ public function destroy($sid) { // Unset the session cookies. $this->deleteCookie($this->getName()); - if ($is_https) { - $this->deleteCookie($this->sessionManager->getInsecureName(), FALSE); - } - elseif ($this->sessionManager->isMixedMode()) { - $this->deleteCookie('S' . $this->getName(), TRUE); - } - - // Remove obsolete sessions. - $this->cleanupObsoleteSessions(); return TRUE; } @@ -277,37 +213,14 @@ public function gc($lifetime) { * * @param string $name * Name of session cookie to delete. - * @param bool $secure - * Force the secure value of the cookie. */ - protected function deleteCookie($name, $secure = NULL) { + protected function deleteCookie($name) { $cookies = $this->requestStack->getCurrentRequest()->cookies; - if ($cookies->has($name) || (!$this->requestStack->getCurrentRequest()->isSecure() && $secure === TRUE)) { + if ($cookies->has($name)) { $params = session_get_cookie_params(); - if ($secure !== NULL) { - $params['secure'] = $secure; - } setcookie($name, '', REQUEST_TIME - 3600, $params['path'], $params['domain'], $params['secure'], $params['httponly']); $cookies->remove($name); } } - /** - * Mark a session for garbage collection upon session save. - */ - protected function sessionSetObsolete($sid, $https = FALSE) { - $this->obsoleteSessionIds[$sid] = $https ? 'ssid' : 'sid'; - } - - /** - * Remove sessions marked for garbage collection. - */ - protected function cleanupObsoleteSessions() { - foreach ($this->obsoleteSessionIds as $sid => $key) { - $this->connection->delete('sessions') - ->condition($key, Crypt::hashBase64($sid)) - ->execute(); - } - } - } diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php index efb3b7a..af36d5b 100644 --- a/core/lib/Drupal/Core/Session/SessionManager.php +++ b/core/lib/Drupal/Core/Session/SessionManager.php @@ -31,19 +31,11 @@ * necessary to subclass it at all. In order to reach the point where Drupal * can use the Symfony session management unmodified, the code implemented * here needs to be extracted either into a dedicated session handler proxy - * (e.g. mixed mode SSL, sid-hashing) or relocated to the authentication - * subsystem. + * (e.g. sid-hashing) or relocated to the authentication subsystem. */ class SessionManager extends NativeSessionStorage implements SessionManagerInterface { /** - * Whether or not the session manager is operating in mixed mode SSL. - * - * @var bool - */ - protected $mixedMode; - - /** * The request stack. * * @var \Symfony\Component\HttpFoundation\RequestStack @@ -100,8 +92,6 @@ public function __construct(RequestStack $request_stack, Connection $connection, parent::__construct($options, $write_check_handler, $metadata_bag); - $this->setMixedMode($settings->get('mixed_mode_sessions', FALSE)); - // @todo When not using the Symfony Session object, the list of bags in the // NativeSessionStorage will remain uninitialized. This will lead to // errors in NativeSessionHandler::loadSession. Remove this after @@ -121,10 +111,8 @@ public function start() { return $this->started; } - $is_https = $this->requestStack->getCurrentRequest()->isSecure(); $cookies = $this->requestStack->getCurrentRequest()->cookies; - $insecure_session_name = $this->getInsecureName(); - if (($cookies->has($this->getName()) && ($session_name = $cookies->get($this->getName()))) || ($is_https && $this->isMixedMode() && ($cookies->has($insecure_session_name) && ($session_name = $cookies->get($insecure_session_name))))) { + if ($cookies->get($this->getName())) { // If a session cookie exists, initialize the session. Otherwise the // session is only started on demand in save(), making // anonymous users not use a session cookie unless something is stored in @@ -144,10 +132,6 @@ public function start() { // default php session id instead of generating a custom one: // https://www.drupal.org/node/2238561 $this->setId(Crypt::randomBytesBase64()); - if ($is_https && $this->isMixedMode()) { - $session_id = Crypt::randomBytesBase64(); - $cookies->set($insecure_session_name, $session_id); - } // Initialize the session global and attach the Symfony session bags. $_SESSION = array(); @@ -214,13 +198,6 @@ public function save() { // started. if (!$this->getSaveHandler()->isActive()) { $this->startNow(); - if ($this->requestStack->getCurrentRequest()->isSecure() && $this->isMixedMode()) { - $insecure_session_name = $this->getInsecureName(); - $params = session_get_cookie_params(); - $expire = $params['lifetime'] ? REQUEST_TIME + $params['lifetime'] : 0; - $cookie_params = $this->requestStack->getCurrentRequest()->cookies; - setcookie($insecure_session_name, $cookie_params->get($insecure_session_name), $expire, $params['path'], $params['domain'], FALSE, $params['httponly']); - } } // Write the session data. parent::save(); @@ -246,21 +223,6 @@ public function regenerate($destroy = FALSE, $lifetime = NULL) { throw new \InvalidArgumentException('The optional parameters $destroy and $lifetime of SessionManager::regenerate() are not supported currently'); } - $is_https = $this->requestStack->getCurrentRequest()->isSecure(); - $cookies = $this->requestStack->getCurrentRequest()->cookies; - - if ($is_https && $this->isMixedMode()) { - $insecure_session_name = $this->getInsecureName(); - $params = session_get_cookie_params(); - $session_id = Crypt::randomBytesBase64(); - // If a session cookie lifetime is set, the session will expire - // $params['lifetime'] seconds from the current request. If it is not set, - // it will expire when the browser is closed. - $expire = $params['lifetime'] ? REQUEST_TIME + $params['lifetime'] : 0; - setcookie($insecure_session_name, $session_id, $expire, $params['path'], $params['domain'], FALSE, $params['httponly']); - $cookies->set($insecure_session_name, $session_id); - } - if ($this->isStarted()) { $old_session_id = $this->getId(); } @@ -273,17 +235,9 @@ public function regenerate($destroy = FALSE, $lifetime = NULL) { $expire = $params['lifetime'] ? REQUEST_TIME + $params['lifetime'] : 0; setcookie($this->getName(), $this->getId(), $expire, $params['path'], $params['domain'], $params['secure'], $params['httponly']); $fields = array('sid' => Crypt::hashBase64($this->getId())); - if ($is_https) { - $fields['ssid'] = Crypt::hashBase64($this->getId()); - // If the "secure pages" setting is enabled, use the newly-created - // insecure session identifier as the regenerated sid. - if ($this->isMixedMode()) { - $fields['sid'] = Crypt::hashBase64($session_id); - } - } $this->connection->update('sessions') ->fields($fields) - ->condition($is_https ? 'ssid' : 'sid', Crypt::hashBase64($old_session_id)) + ->condition('sid', Crypt::hashBase64($old_session_id)) ->execute(); } @@ -335,27 +289,6 @@ public function enable() { } /** - * {@inheritdoc} - */ - public function isMixedMode() { - return $this->mixedMode; - } - - /** - * {@inheritdoc} - */ - public function setMixedMode($mixed_mode) { - $this->mixedMode = (bool) $mixed_mode; - } - - /** - * {@inheritdoc} - */ - public function getInsecureName() { - return substr($this->getName(), 1); - } - - /** * Returns whether the current PHP process runs on CLI. * * Command line clients do not support cookies nor sessions. diff --git a/core/lib/Drupal/Core/Session/SessionManagerInterface.php b/core/lib/Drupal/Core/Session/SessionManagerInterface.php index b620c66..c86fb7d 100644 --- a/core/lib/Drupal/Core/Session/SessionManagerInterface.php +++ b/core/lib/Drupal/Core/Session/SessionManagerInterface.php @@ -50,28 +50,4 @@ public function disable(); */ public function enable(); - /** - * Returns whether mixed mode SSL sessions are enabled in the session manager. - * - * @return bool - * Value of the mixed mode SSL sessions flag. - */ - public function isMixedMode(); - - /** - * Enables or disables mixed mode SSL sessions in the session manager. - * - * @param bool $mixed_mode - * New value for the mixed mode SSL sessions flag. - */ - public function setMixedMode($mixed_mode); - - /** - * Returns the name of the insecure session when operating in mixed mode SSL. - * - * @return string - * The name of the insecure session. - */ - public function getInsecureName(); - } diff --git a/core/modules/system/src/Tests/Session/SessionHttpsTest.php b/core/modules/system/src/Tests/Session/SessionHttpsTest.php index 6d9cb25..0af7f49 100644 --- a/core/modules/system/src/Tests/Session/SessionHttpsTest.php +++ b/core/modules/system/src/Tests/Session/SessionHttpsTest.php @@ -64,7 +64,7 @@ protected function testHttpsSession() { // 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 a non-empty SID and a correct secure SID.'); + $this->assertSessionIds($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. @@ -110,177 +110,6 @@ protected function testHttpsSession() { } /** - * Tests sessions in SSL mixed mode. - */ - protected function testMixedModeSslSession() { - if ($this->request->isSecure()) { - // The functionality does not make sense when running on HTTPS. - return; - } - else { - $secure_session_name = 'S' . $this->getSessionName(); - $insecure_session_name = $this->getSessionName(); - } - - // Enable secure pages. - $this->settingsSet('mixed_mode_sessions', TRUE); - // Write that value also into the test settings.php file. - $settings['settings']['mixed_mode_sessions'] = (object) array( - 'value' => TRUE, - 'required' => TRUE, - ); - $this->writeSettings($settings); - - $user = $this->drupalCreateUser(array('access administration pages')); - - $this->curlClose(); - // Start an anonymous session on the insecure site. - $session_data = $this->randomMachineName(); - $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"]'); - $this->assertNotEqual(substr($form[0]['action'], 0, 6), 'https:', 'Password request form action is not secure'); - $form[0]['action'] = $this->httpsUrl('user'); - - // Check that user login form action is secure. - $this->drupalGet('user'); - $form = $this->xpath('//form[@id="user-login-form"]'); - $this->assertEqual(substr($form[0]['action'], 0, 6), 'https:', 'Login form action is secure'); - $form[0]['action'] = $this->httpsUrl('user'); - - $edit = array( - 'name' => $user->getUsername(), - 'pass' => $user->pass_raw, - ); - $this->drupalPostForm(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 on secure page. - $this->assertFalse($this->cookies[$insecure_session_name]['secure'], 'The insecure cookie does not have the secure attribute'); - - $sid = $this->cookies[$insecure_session_name]['value']; - $ssid = $this->cookies[$secure_session_name]['value']; - $this->assertSessionIds($sid, $ssid, 'Session has both secure and insecure SIDs'); - $cookies = array( - $insecure_session_name . '=' . $sid, - $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(); - - $this->drupalGet($url, array(), array('Cookie: ' . $cookie)); - if ($cookie_key == $url_key) { - $this->assertText(t('Configuration')); - $this->assertResponse(200); - } - else { - $this->assertNoText(t('Configuration')); - $this->assertResponse(403); - } - } - } - - // 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(), 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"]'); - $form[0]['action'] = $this->httpsUrl('user'); - $this->drupalPostForm(NULL, $edit, t('Log in')); - - // Test that the user is also authenticated on the insecure site. - $this->drupalGet("user/" . $user->id() . "/edit"); - $this->assertResponse(200); - } - - /** - * Ensure that a CSRF form token is shared in SSL mixed mode. - */ - protected function testCsrfTokenWithMixedModeSsl() { - if ($this->request->isSecure()) { - $secure_session_name = $this->getSessionName(); - $insecure_session_name = substr($this->getSessionName(), 1); - } - else { - $secure_session_name = 'S' . $this->getSessionName(); - $insecure_session_name = $this->getSessionName(); - } - - // Enable mixed mode SSL. - $this->settingsSet('mixed_mode_sessions', TRUE); - // Write that value also into the test settings.php file. - $settings['settings']['mixed_mode_sessions'] = (object) array( - 'value' => TRUE, - 'required' => TRUE, - ); - $this->writeSettings($settings); - - $user = $this->drupalCreateUser(array('access administration pages')); - - // Login using the HTTPS user-login form. - $this->drupalGet('user'); - $form = $this->xpath('//form[@id="user-login-form"]'); - $form[0]['action'] = $this->httpsUrl('user'); - $edit = array('name' => $user->getUsername(), 'pass' => $user->pass_raw); - $this->drupalPostForm(NULL, $edit, t('Log in')); - - // Collect session id cookies. - $sid = $this->cookies[$insecure_session_name]['value']; - $ssid = $this->cookies[$secure_session_name]['value']; - $this->assertSessionIds($sid, $ssid, 'Session has both secure and insecure SIDs'); - - // Retrieve the form via HTTP. - $this->curlClose(); - $this->drupalGet($this->httpUrl('session-test/form'), array(), array('Cookie: ' . $insecure_session_name . '=' . $sid)); - $http_token = $this->getFormToken(); - - // Verify that submitting form values via HTTPS to a form originally - // retrieved over HTTP works. - $form = $this->xpath('//form[@id="session-test-form"]'); - $form[0]['action'] = $this->httpsUrl('session-test/form'); - $edit = array('input' => $this->randomMachineName(32)); - $this->curlClose(); - $this->drupalPostForm(NULL, $edit, 'Save', array('Cookie: ' . $secure_session_name . '=' . $ssid)); - $this->assertText(String::format('Ok: @input', array('@input' => $edit['input']))); - - // Retrieve the same form via HTTPS. - $this->curlClose(); - $this->drupalGet($this->httpsUrl('session-test/form'), array(), array('Cookie: ' . $secure_session_name . '=' . $ssid)); - $https_token = $this->getFormToken(); - - // Verify that CSRF token values are the same for a form regardless of - // whether it was accessed via HTTP or HTTPS when SSL mixed mode is enabled. - $this->assertEqual($http_token, $https_token, 'Form token is the same on HTTP as well as HTTPS form'); - } - - /** * Return the token of the current form. */ protected function getFormToken() { @@ -294,8 +123,6 @@ protected function getFormToken() { * * @param $sid * The insecure session ID to search for. - * @param $ssid - * The secure session ID to search for. * @param $assertion_text * The text to display when we perform the assertion. * @@ -303,12 +130,11 @@ protected function getFormToken() { * The result of assertTrue() that there's a session in the system that * has the given insecure and secure session IDs. */ - protected function assertSessionIds($sid, $ssid, $assertion_text) { + protected function assertSessionIds($sid, $assertion_text) { $args = array( ':sid' => Crypt::hashBase64($sid), - ':ssid' => !empty($ssid) ? Crypt::hashBase64($ssid) : '', ); - return $this->assertTrue(db_query('SELECT timestamp FROM {sessions} WHERE sid = :sid AND ssid = :ssid', $args)->fetchField(), $assertion_text); + return $this->assertTrue(db_query('SELECT timestamp FROM {sessions} WHERE sid = :sid', $args)->fetchField(), $assertion_text); } /** diff --git a/core/modules/system/system.install b/core/modules/system/system.install index b9c1b83..49fadc5 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -887,13 +887,6 @@ function system_schema() { 'length' => 128, 'not null' => TRUE, ), - 'ssid' => array( - 'description' => "Secure session ID (hashed). The value is generated by Drupal's session handlers.", - 'type' => 'varchar', - 'length' => 128, - 'not null' => TRUE, - 'default' => '', - ), 'hostname' => array( 'description' => 'The IP address that last used this session ID (sid).', 'type' => 'varchar', @@ -916,12 +909,10 @@ function system_schema() { ), 'primary key' => array( 'sid', - 'ssid', ), 'indexes' => array( 'timestamp' => array('timestamp'), 'uid' => array('uid'), - 'ssid' => array('ssid'), ), 'foreign keys' => array( 'session_user' => array(