diff --git a/core/core.services.yml b/core/core.services.yml index 9b9c80b..439cb7d 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -498,7 +498,7 @@ services: arguments: ['@state'] csrf_token: class: Drupal\Core\Access\CsrfTokenGenerator - arguments: ['@private_key'] + arguments: ['@private_key', '@session_manager'] access_manager: class: Drupal\Core\Access\AccessManager arguments: ['@router.route_provider', '@url_generator', '@paramconverter_manager'] diff --git a/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php b/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php index 9918610..3fd060e 100644 --- a/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php +++ b/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php @@ -10,6 +10,7 @@ use Drupal\Component\Utility\Crypt; use Drupal\Core\PrivateKey; use Drupal\Core\Session\AccountInterface; +use Drupal\Core\Session\SessionManagerInterface; /** * Generates and validates CSRF tokens. @@ -26,13 +27,21 @@ class CsrfTokenGenerator { protected $privateKey; /** + * The session manager. + * + * @var \Drupal\Core\Session\SessionManagerInterface + */ + protected $sessionManager; + + /** * Constructs the token generator. * * @param \Drupal\Core\PrivateKey $private_key * The private key service. */ - public function __construct(PrivateKey $private_key) { + public function __construct(PrivateKey $private_key, SessionManagerInterface $session_manager) { $this->privateKey = $private_key; + $this->sessionManager = $session_manager; } /** @@ -55,7 +64,7 @@ public function __construct(PrivateKey $private_key) { * @see \Drupal\Core\Session\SessionManager::start() */ public function get($value = '') { - return Crypt::hmacBase64($value, session_id() . $this->privateKey->get() . drupal_get_hash_salt()); + return Crypt::hmacBase64($value, $this->sessionManager->getCsrfTokenSeed(TRUE) . $this->privateKey->get() . drupal_get_hash_salt()); } /** @@ -70,7 +79,11 @@ public function get($value = '') { * TRUE for a valid token, FALSE for an invalid token. */ public function validate($token, $value = '') { - return $token === $this->get($value); + if (!$seed = $this->sessionManager->getCsrfTokenSeed()) { + return FALSE; + } + + return $token === Crypt::hmacBase64($value, $seed . $this->privateKey->get() . drupal_get_hash_salt()); } } diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php index 30ca61b..5b3fd1d 100644 --- a/core/lib/Drupal/Core/Session/SessionManager.php +++ b/core/lib/Drupal/Core/Session/SessionManager.php @@ -83,7 +83,7 @@ public function initialize() { // anonymous users not use a session cookie unless something is stored in // $_SESSION. This allows HTTP proxies to cache anonymous pageviews. $this->start(); - if ($user->isAuthenticated() || !empty($_SESSION)) { + if ($user->isAuthenticated() || !$this->isSessionObsolete()) { drupal_page_is_cacheable(FALSE); } } @@ -138,7 +138,7 @@ public function save() { return; } - if ($user->isAnonymous() && empty($_SESSION)) { + if ($user->isAnonymous() && $this->isSessionObsolete()) { // There is no session data to store, destroy the session if it was // previously started. if ($this->isStarted()) { @@ -204,6 +204,14 @@ public function regenerate() { } session_id(Crypt::randomBytesBase64()); + // @todo As soon as https://drupal.org/node/2238087 lands, the token seed + // can be moved onto Drupal\Core\Session\MetadataBag. The session manager + // then needs to notify the metadata bag when the token should be + // regenerated. + if (!empty($_SESSION)) { + unset($_SESSION['csrf_token_seed']); + } + if (isset($old_session_id)) { $params = session_get_cookie_params(); $expire = $params['lifetime'] ? REQUEST_TIME + $params['lifetime'] : 0; @@ -279,6 +287,17 @@ public function enable() { } /** + * {@inheritdoc} + */ + public function getCsrfTokenSeed($generate_if_empty = FALSE) { + if ($generate_if_empty && !isset($_SESSION['csrf_token_seed'])) { + $_SESSION['csrf_token_seed'] = Crypt::randomBytesBase64(); + } + + return isset($_SESSION['csrf_token_seed']) ? $_SESSION['csrf_token_seed'] : NULL; + } + + /** * Returns whether the current PHP process runs on CLI. * * Command line clients do not support cookies nor sessions. @@ -289,4 +308,30 @@ protected function isCli() { return PHP_SAPI === 'cli'; } + /** + * Determines whether the session contains user data. + * + * @return bool + * TRUE when the session does not contain any values and therefore can be + * destroyed. + */ + protected function isSessionObsolete() { + // Return early when $_SESSION is empty or not initialized. + if (empty($_SESSION)) { + return TRUE; + } + + // Ignore the CSRF token seed. + // + // @todo Anonymous users should not get a CSRF token at any time, or if they + // do, then the originating code is responsible for cleaning up the + // session once obsolete. Since that is not guaranteed to be the case, + // this check force-ignores the CSRF token, so as to avoid performance + // regressions. + // As soon as https://drupal.org/node/2238087 lands, the token seed can be + // moved onto \Drupal\Core\Session\MetadataBag. This will result in the + // CSRF token to be ignored automatically. + return count(array_diff_key($_SESSION, array('csrf_token_seed' => TRUE))) == 0; + } + } diff --git a/core/lib/Drupal/Core/Session/SessionManagerInterface.php b/core/lib/Drupal/Core/Session/SessionManagerInterface.php index 346bc0d..7ad79bd 100644 --- a/core/lib/Drupal/Core/Session/SessionManagerInterface.php +++ b/core/lib/Drupal/Core/Session/SessionManagerInterface.php @@ -77,4 +77,12 @@ public function disable(); */ public function enable(); + /** + * Gets the CSRF token seed for this session. + * + * @return string + * The unique token seed for this session. + */ + public function getCsrfTokenSeed(); + } diff --git a/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php b/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php index 02523d0..f1779f4 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php @@ -10,6 +10,7 @@ use Drupal\simpletest\WebTestBase; use Symfony\Component\HttpFoundation\Request; use Drupal\Component\Utility\Crypt; +use Drupal\Component\Utility\String; /** * Ensure that when running under HTTPS two session cookies are generated. @@ -215,6 +216,75 @@ protected function testHttpsSession() { } /** + * Ensure that a CSRF form token is shared in SSL mixed mode. + */ + protected function testCsrfTokenWithMixedModeSsl() { + if ($this->request->isSecure()) { + $secure_session_name = session_name(); + $insecure_session_name = substr(session_name(), 1); + } + else { + $secure_session_name = 'S' . session_name(); + $insecure_session_name = session_name(); + } + + // 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->randomName(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() { + $token_fields = $this->xpath('//input[@name="form_token"]'); + $this->assertEqual(count($token_fields), 1, 'One form token field on the page'); + return (string) $token_fields[0]['value']; + } + + /** * Test that there exists a session with two specific session IDs. * * @param $sid diff --git a/core/modules/system/tests/modules/session_test/lib/Drupal/session_test/Form/SessionTestForm.php b/core/modules/system/tests/modules/session_test/lib/Drupal/session_test/Form/SessionTestForm.php new file mode 100644 index 0000000..706c79c --- /dev/null +++ b/core/modules/system/tests/modules/session_test/lib/Drupal/session_test/Form/SessionTestForm.php @@ -0,0 +1,51 @@ + 'textfield', + '#title' => 'Input', + '#required' => TRUE, + ); + + $form['actions'] = array('#type' => 'actions'); + $form['actions']['submit'] = array( + '#type' => 'submit', + '#value' => 'Save', + ); + + return $form; + } + + /** + * {@inheritdoc} + */ + public function submitForm(array &$form, array &$form_state) { + drupal_set_message(String::format('Ok: @input', array('@input' => $form_state['values']['input']))); + } + +} diff --git a/core/modules/system/tests/modules/session_test/session_test.routing.yml b/core/modules/system/tests/modules/session_test/session_test.routing.yml index b1dc489..b044fc4 100644 --- a/core/modules/system/tests/modules/session_test/session_test.routing.yml +++ b/core/modules/system/tests/modules/session_test/session_test.routing.yml @@ -75,3 +75,11 @@ session_test.is_logged_in: _content: '\Drupal\session_test\Controller\SessionTestController::isLoggedIn' requirements: _user_is_logged_in: 'TRUE' + +session_test.form: + path: '/session-test/form' + defaults: + _form: '\Drupal\session_test\Form\SessionTestForm' + _title: 'Test form' + requirements: + _access: 'TRUE'