[SECURITY] Protect persisted session IDs from being used directly

Instead of storing session IDs with their corresponding storage
backends in plain text, their HMAC-SHA256 (Redis) or HMAC-MD5 (DB)
is being used. HMAC-MD5 had to be chosen to avoid breaking changes
for limited field size in database fields (32 characters currently).

This change also allows a fallback to non-hashed-session values,
meaning that
* set() and update() will create new session records with the hashed
  identifier
* get() contains a fallback to the non-hashed-version when no session
  with a hashed identifier is found

Resolves: #91854
Releases: master, 10.4, 9.5
Change-Id: Ia57acc5e0d0cf71088af1aaff1ab894bd1d4e3dd
Security-Bulletin: TYPO3-CORE-SA-2020-011
Security-References: CVE-2020-26228
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/66660
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>

typo3/sysext/beuser/Classes/Controller/BackendUserController.php

@@ -297,6 +297,7 @@ public function removeAllFromCompareListAction(): void
      */
     protected function terminateBackendUserSessionAction(BackendUser $backendUser, $sessionId)
     {
+        // terminating value of persisted session ID (probably hashed value)
         $sessionBackend = $this->getSessionBackend();
         $success = $sessionBackend->remove($sessionId);
 
@@ -334,8 +335,7 @@ protected function switchUser($switchUser)
                 ]
             );
 
-            $sessionBackend = $this->getSessionBackend();
-            $sessionBackend->update(
+            $this->getSessionBackend()->update(
                 $this->getBackendUserAuthentication()->getSessionId(),
                 [
                     'ses_userid' => (int)$targetUser['uid'],

typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php

@@ -32,6 +32,7 @@
 use TYPO3\CMS\Core\Exception;
 use TYPO3\CMS\Core\Http\CookieHeaderTrait;
 use TYPO3\CMS\Core\Session\Backend\Exception\SessionNotFoundException;
+use TYPO3\CMS\Core\Session\Backend\HashableSessionBackendInterface;
 use TYPO3\CMS\Core\Session\Backend\SessionBackendInterface;
 use TYPO3\CMS\Core\Session\SessionManager;
 use TYPO3\CMS\Core\SysLog\Action\Login as SystemLogLoginAction;
@@ -743,6 +744,17 @@ public function checkAuthentication()
                     ]);
                 }
             } elseif ($haveSession) {
+                // Validate the session ID and promote it
+                // This check can be removed in TYPO3 v11
+                if ($this->getSessionBackend() instanceof HashableSessionBackendInterface && !empty($authInfo['userSession']['ses_id'] ?? '')) {
+                    // The session is stored in plaintext, promote it
+                    if ($authInfo['userSession']['ses_id'] === $this->id) {
+                        $authInfo['userSession'] = $this->getSessionBackend()->update(
+                            $this->id,
+                            ['ses_data' => $authInfo['userSession']['ses_data']]
+                        );
+                    }
+                }
                 // if we come here the current session is for sure not anonymous as this is a pre-condition for $authenticated = true
                 $this->user = $authInfo['userSession'];
             }

typo3/sysext/core/Classes/Session/Backend/DatabaseSessionBackend.php

@@ -32,7 +32,7 @@
  * This session backend requires the 'table' configuration option. If the backend is used to holds non-authenticated
  * sessions (default if 'TYPO3_MODE' is 'FE'), the 'ses_anonymous' configuration option must be set to true.
  */
-class DatabaseSessionBackend implements SessionBackendInterface
+class DatabaseSessionBackend implements SessionBackendInterface, HashableSessionBackendInterface
 {
     /**
      * @var array
@@ -75,6 +75,14 @@ public function validateConfiguration(): bool
         return true;
     }
 
+    public function hash(string $sessionId): string
+    {
+        // The sha1 hash ensures we have good length for the key.
+        $key = sha1($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] . 'core-session-backend');
+        // @todo md5 is used as be_sessions.ses_id field only supports 32 characters in stable branches
+        return hash_hmac('md5', $sessionId, $key);
+    }
+
     /**
      * Read session data
      *
@@ -88,15 +96,25 @@ public function get(string $sessionId): array
 
         $query->select('*')
             ->from($this->configuration['table'])
-            ->where($query->expr()->eq('ses_id', $query->createNamedParameter($sessionId, \PDO::PARAM_STR)));
+            ->where($query->expr()->eq('ses_id', $query->createNamedParameter($this->hash($sessionId), \PDO::PARAM_STR)));
 
         $result = $query->execute()->fetch();
 
         if (!is_array($result)) {
-            throw new SessionNotFoundException(
-                'The session with identifier ' . $sessionId . ' was not found ',
-                1481885483
-            );
+            // Check for a non-hashed-version, will be removed in TYPO3 v11
+            $query = $this->getQueryBuilder();
+
+            $result = $query->select('*')
+                ->from($this->configuration['table'])
+                ->where($query->expr()->eq('ses_id', $query->createNamedParameter($sessionId, \PDO::PARAM_STR)))
+                ->execute()
+                ->fetch();
+            if (!is_array($result)) {
+                throw new SessionNotFoundException(
+                    'The session with identifier ' . $sessionId . ' was not found ',
+                    1481885483
+                );
+            }
         }
         return $result;
     }
@@ -111,7 +129,12 @@ public function remove(string $sessionId): bool
     {
         $query = $this->getQueryBuilder();
         $query->delete($this->configuration['table'])
-            ->where($query->expr()->eq('ses_id', $query->createNamedParameter($sessionId, \PDO::PARAM_STR)));
+            ->where(
+                $query->expr()->orX(
+                    $query->expr()->eq('ses_id', $query->createNamedParameter($this->hash($sessionId), \PDO::PARAM_STR)),
+                    $query->expr()->eq('ses_id', $query->createNamedParameter($sessionId, \PDO::PARAM_STR))
+                )
+            );
 
         return (bool)$query->execute();
     }
@@ -128,6 +151,7 @@ public function remove(string $sessionId): bool
      */
     public function set(string $sessionId, array $sessionData): array
     {
+        $sessionId = $this->hash($sessionId);
         $sessionData['ses_id'] = $sessionId;
         $sessionData['ses_tstamp'] = $GLOBALS['EXEC_TIME'] ?? time();
 
@@ -160,11 +184,19 @@ public function set(string $sessionId, array $sessionData): array
      */
     public function update(string $sessionId, array $sessionData): array
     {
-        $sessionData['ses_id'] = $sessionId;
+        $hashedSessionId = $this->hash($sessionId);
+        $sessionData['ses_id'] = $hashedSessionId;
         $sessionData['ses_tstamp'] = $GLOBALS['EXEC_TIME'] ?? time();
 
         try {
             // allow 0 records to be affected, happens when no columns where changed
+            $this->getConnection()->update(
+                $this->configuration['table'],
+                $sessionData,
+                ['ses_id' => $hashedSessionId],
+                ['ses_data' => \PDO::PARAM_LOB]
+            );
+            // Migrate old session data as well to remove old entries and promote them to migrated entries
             $this->getConnection()->update(
                 $this->configuration['table'],
                 $sessionData,

typo3/sysext/core/Classes/Session/Backend/HashableSessionBackendInterface.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Session\Backend;
+
+interface HashableSessionBackendInterface
+{
+    public function hash(string $sessionId): string;
+}

typo3/sysext/core/Classes/Session/Backend/RedisSessionBackend.php

@@ -29,7 +29,7 @@
  * This session backend takes these optional configuration options: 'hostname' (default '127.0.0.1'),
  * 'database' (default 0), 'port' (default 3679) and 'password' (no default value).
  */
-class RedisSessionBackend implements SessionBackendInterface, LoggerAwareInterface
+class RedisSessionBackend implements SessionBackendInterface, HashableSessionBackendInterface, LoggerAwareInterface
 {
     use LoggerAwareTrait;
 
@@ -116,6 +116,13 @@ public function validateConfiguration()
         }
     }
 
+    public function hash(string $sessionId): string
+    {
+        // The sha1 hash ensures we have good length for the key.
+        $key = sha1($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] . 'core-session-backend');
+        return hash_hmac('sha256', $sessionId, $key);
+    }
+
     /**
      * Read session data
      *
@@ -127,9 +134,16 @@ public function get(string $sessionId): array
     {
         $this->initializeConnection();
 
-        $key = $this->getSessionKeyName($sessionId);
-        $rawData = $this->redis->get($key);
-
+        $hashedSessionId = $this->hash($sessionId);
+        $rawData = $this->redis->get($this->getSessionKeyName($hashedSessionId));
+        if ($rawData !== false) {
+            $decodedValue = json_decode($rawData, true);
+            if (is_array($decodedValue)) {
+                return $decodedValue;
+            }
+        }
+        // Fallback to the non-hashed-value, will be removed in TYPO3 v11
+        $rawData = $this->redis->get($this->getSessionKeyName($sessionId));
         if ($rawData !== false) {
             $decodedValue = json_decode($rawData, true);
             if (is_array($decodedValue)) {
@@ -149,8 +163,11 @@ public function get(string $sessionId): array
     public function remove(string $sessionId): bool
     {
         $this->initializeConnection();
+        $status = $this->redis->del($this->getSessionKeyName($this->hash($sessionId))) >= 1;
+        // Checking for non-hashed-identifier, will be removed in TYPO3 v11
+        $statusLegacy = $this->redis->del($this->getSessionKeyName($sessionId)) >= 1;
 
-        return $this->redis->del($this->getSessionKeyName($sessionId)) >= 1;
+        return $status || $statusLegacy;
     }
 
     /**
@@ -166,15 +183,15 @@ public function remove(string $sessionId): bool
     public function set(string $sessionId, array $sessionData): array
     {
         $this->initializeConnection();
-        $sessionData['ses_id'] = $sessionId;
-        $sessionData['ses_tstamp'] = $GLOBALS['EXEC_TIME'] ?? time();
 
-        $key = $this->getSessionKeyName($sessionId);
+        $hashedSessionId = $this->hash($sessionId);
+        $sessionData['ses_id'] = $hashedSessionId;
+        $sessionData['ses_tstamp'] = $GLOBALS['EXEC_TIME'] ?? time();
 
         // nx will not allow overwriting existing keys
         $jsonString = json_encode($sessionData);
         $wasSet = is_string($jsonString) && $this->redis->set(
-            $key,
+            $this->getSessionKeyName($hashedSessionId),
             $jsonString,
             ['nx']
         );
@@ -198,15 +215,16 @@ public function set(string $sessionId, array $sessionData): array
      */
     public function update(string $sessionId, array $sessionData): array
     {
+        $hashedSessionId = $this->hash($sessionId);
         try {
-            $sessionData = array_merge($this->get($sessionId), $sessionData);
+            $sessionData = array_merge($this->get($hashedSessionId), $sessionData);
         } catch (SessionNotFoundException $e) {
             throw new SessionNotUpdatedException('Cannot update non-existing record', 1484389971, $e);
         }
-        $sessionData['ses_id'] = $sessionId;
+        $sessionData['ses_id'] = $hashedSessionId;
         $sessionData['ses_tstamp'] = $GLOBALS['EXEC_TIME'] ?? time();
 
-        $key = $this->getSessionKeyName($sessionId);
+        $key = $this->getSessionKeyName($hashedSessionId);
         $jsonString = json_encode($sessionData);
         $wasSet = is_string($jsonString) && $this->redis->set($key, $jsonString);
 

typo3/sysext/core/Classes/Session/SessionManager.php

@@ -18,6 +18,7 @@
 namespace TYPO3\CMS\Core\Session;
 
 use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication;
+use TYPO3\CMS\Core\Session\Backend\HashableSessionBackendInterface;
 use TYPO3\CMS\Core\Session\Backend\SessionBackendInterface;
 use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -75,15 +76,21 @@ public function getSessionBackend(string $identifier): SessionBackendInterface
     public function invalidateAllSessionsByUserId(SessionBackendInterface $backend, int $userId, AbstractUserAuthentication $userAuthentication = null)
     {
         $sessionToRenew = '';
+        $hashedSessionToRenew = '';
         // Prevent destroying the session of the current user session, but renew session id
         if ($userAuthentication !== null && (int)$userAuthentication->user['uid'] === $userId) {
             $sessionToRenew = $userAuthentication->getSessionId();
         }
+        if ($sessionToRenew !== '' && $backend instanceof HashableSessionBackendInterface) {
+            $hashedSessionToRenew = $backend->hash($sessionToRenew);
+        }
 
         foreach ($backend->getAll() as $session) {
-            if ($userAuthentication !== null && $session['ses_id'] === $sessionToRenew) {
-                $userAuthentication->enforceNewSessionId();
-                continue;
+            if ($userAuthentication !== null) {
+                if ($session['ses_id'] === $sessionToRenew || $session['ses_id'] === $hashedSessionToRenew) {
+                    $userAuthentication->enforceNewSessionId();
+                    continue;
+                }
             }
             if ((int)$session['ses_userid'] === $userId) {
                 $backend->remove($session['ses_id']);

typo3/sysext/core/Tests/Acceptance/Fixtures/be_sessions.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="utf-8"?>
+<dataset>
+	<be_sessions>
+        <!-- hash_hmac('md5', '886526ce72b86870739cc41991144ec1', sha1('iAmInvalid' . 'core-session-backend')) -->
+		<ses_id>a7475832dbc0aa7ed07bb1f800520d16</ses_id>
+		<ses_iplock>[DISABLED]</ses_iplock>
+		<ses_userid>1</ses_userid>
+		<ses_tstamp>1777777777</ses_tstamp>
+		<ses_data></ses_data>
+		<ses_backuserid>0</ses_backuserid>
+	</be_sessions>
+	<be_sessions>
+        <!-- hash_hmac('md5', 'ff83dfd81e20b34c27d3e97771a4525a', sha1('iAmInvalid' . 'core-session-backend')) -->
+		<ses_id>b99a7e54850ef064b7181d0de7d67900</ses_id>
+		<ses_iplock>[DISABLED]</ses_iplock>
+		<ses_userid>2</ses_userid>
+		<ses_tstamp>1777777777</ses_tstamp>
+		<ses_data></ses_data>
+		<ses_backuserid>0</ses_backuserid>
+	</be_sessions>
+</dataset>

typo3/sysext/core/Tests/Acceptance/Support/Extension/BackendCoreEnvironment.php

@@ -59,7 +59,7 @@ class BackendCoreEnvironment extends BackendEnvironment
         ],
         'xmlDatabaseFixtures' => [
             'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/be_users.xml',
-            'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/be_sessions.xml',
+            'typo3/sysext/core/Tests/Acceptance/Fixtures/be_sessions.xml',
             'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/be_groups.xml',
             'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/sys_category.xml',
             'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/tx_extensionmanager_domain_model_extension.xml',

typo3/sysext/core/Tests/Acceptance/Support/Extension/PageTreeCoreEnvironment.php

@@ -57,7 +57,7 @@ class PageTreeCoreEnvironment extends BackendEnvironment
         'xmlDatabaseFixtures' => [
             'typo3/sysext/core/Tests/Acceptance/Fixtures/pages.xml',
             'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/be_users.xml',
-            'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/be_sessions.xml',
+            'typo3/sysext/core/Tests/Acceptance/Fixtures/be_sessions.xml',
             'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/be_groups.xml',
             'PACKAGE:typo3/testing-framework/Resources/Core/Acceptance/Fixtures/sys_category.xml',
         ],

typo3/sysext/core/Tests/Functional/Session/Backend/DatabaseSessionBackendTest.php

@@ -36,7 +36,8 @@ class DatabaseSessionBackendTest extends FunctionalTestCase
      * @var array
      */
     protected $testSessionRecord = [
-        'ses_id' => 'randomSessionId',
+        // DatabaseSessionBackend::hash('randomSessionId') with encryption key 12345
+        'ses_id' => '76898588caa1baee7984f4dc8adfed3b',
         'ses_userid' => 1,
         // serialize(['foo' => 'bar', 'boo' => 'far'])
         'ses_data' => 'a:2:{s:3:"foo";s:3:"bar";s:3:"boo";s:3:"far";}',
@@ -48,6 +49,7 @@ class DatabaseSessionBackendTest extends FunctionalTestCase
     protected function setUp(): void
     {
         parent::setUp();
+        $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = '12345';
 
         $this->subject = new DatabaseSessionBackend();
         $this->subject->initialize('default', [

typo3/sysext/core/Tests/Functional/Session/Backend/RedisSessionBackendTest.php

@@ -39,7 +39,8 @@ class RedisSessionBackendTest extends FunctionalTestCase
      * @var array
      */
     protected $testSessionRecord = [
-        'ses_id' => 'randomSessionId',
+        // RedisSessionBackend::hash('randomSessionId') with encryption key 12345
+        'ses_id' => '21c0e911565a67315cdc384889c470fd291feafbfa62e31ecf7409430640bc7a',
         'ses_userid' => 1,
         // serialize(['foo' => 'bar', 'boo' => 'far'])
         'ses_data' => 'a:2:{s:3:"foo";s:3:"bar";s:3:"boo";s:3:"far";}',
@@ -51,6 +52,7 @@ class RedisSessionBackendTest extends FunctionalTestCase
     protected function setUp(): void
     {
         parent::setUp();
+        $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = '12345';
 
         if (!getenv('typo3TestingRedisHost')) {
             self::markTestSkipped('environment variable "typo3TestingRedisHost" must be set to run this test');