[SECURITY] Destroy sessions on password change 04/60704/2
authorFrank Naegler <frank.naegler@typo3.org>
Tue, 7 May 2019 09:44:26 +0000 (11:44 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Tue, 7 May 2019 09:44:38 +0000 (11:44 +0200)
On DataHandler update or when updating a users
password via EXT:felogin, all existing
sessions are destroyed except for the current
session.

Resolves: #87298
Releases: master, 9.5, 8.7
Security-Commit: df7c0dbcf73be20e5ae9d4cf03b82c8326c9fccc
Security-Bulletin: TYPO3-CORE-SA-2019-011
Change-Id: Iff673d2ab774dde0f116c4bc9040d40374492a7a
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60704
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
typo3/sysext/core/Classes/Hooks/DestroySessionHook.php [new file with mode: 0644]
typo3/sysext/core/Classes/Session/SessionManager.php
typo3/sysext/core/Documentation/Changelog/8.7.x/Important-87298-DestroySessionsOnPasswordChange.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Functional/Session/SessionManagerTest.php [new file with mode: 0644]
typo3/sysext/core/ext_localconf.php
typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php

index f76f940..f665b23 100644 (file)
@@ -1009,6 +1009,17 @@ abstract class AbstractUserAuthentication implements LoggerAwareInterface
     }
 
     /**
+     * Regenerates the session ID and sets the cookie again.
+     *
+     * @internal
+     */
+    public function enforceNewSessionId()
+    {
+        $this->regenerateSessionId();
+        $this->setSessionCookie();
+    }
+
+    /**
      * Log out current user!
      * Removes the current session record, sets the internal ->user array to a blank string;
      * Thereby the current user (if any) is effectively logged out!
diff --git a/typo3/sysext/core/Classes/Hooks/DestroySessionHook.php b/typo3/sysext/core/Classes/Hooks/DestroySessionHook.php
new file mode 100644 (file)
index 0000000..61d8f92
--- /dev/null
@@ -0,0 +1,58 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Core\Hooks;
+
+/*
+ * 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!
+ */
+
+use TYPO3\CMS\Core\DataHandling\DataHandler;
+use TYPO3\CMS\Core\Session\SessionManager;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
+class DestroySessionHook
+{
+    /**
+     * If a fe_users' or be_users' password is updated, clear all sessions.
+     *
+     * @param string $status
+     * @param string $table
+     * @param int $id
+     * @param array $fieldArray
+     * @param DataHandler $dataHandler
+     */
+    public function processDatamap_postProcessFieldArray($status, $table, $id, $fieldArray, DataHandler $dataHandler)
+    {
+        if ($table !== 'be_users' && $table !== 'fe_users') {
+            return;
+        }
+        if ($status !== 'update') {
+            return;
+        }
+        if (!isset($fieldArray['password']) || (string)$fieldArray['password'] === '') {
+            return;
+        }
+
+        $sessionManager = GeneralUtility::makeInstance(SessionManager::class);
+        if ($table === 'be_users') {
+            // Destroy BE user sessions for backend user
+            $backend = $sessionManager->getSessionBackend('BE');
+            $sessionManager->invalidateAllSessionsByUserId($backend, (int)$id, $GLOBALS['BE_USER']);
+        }
+        if ($table === 'fe_users') {
+            // Destroy any FE user sessions for the given user
+            $backend = $sessionManager->getSessionBackend('FE');
+            $sessionManager->invalidateAllSessionsByUserId($backend, (int)$id);
+        }
+    }
+}
index 6610129..c9a4c77 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Core\Session;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication;
 use TYPO3\CMS\Core\Session\Backend\SessionBackendInterface;
 use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -63,6 +64,32 @@ class SessionManager implements SingletonInterface
     }
 
     /**
+     * Removes all sessions for a specific user ID
+     *
+     * @param SessionBackendInterface $backend see constants
+     * @param int $userId
+     * @param AbstractUserAuthentication $userAuthentication
+     */
+    public function invalidateAllSessionsByUserId(SessionBackendInterface $backend, int $userId, AbstractUserAuthentication $userAuthentication = null)
+    {
+        $sessionToRenew = '';
+        // Prevent destroying the session of the current user session, but renew session id
+        if ($userAuthentication !== null && (int)$userAuthentication->user['uid'] === $userId) {
+            $sessionToRenew = $userAuthentication->getSessionId();
+        }
+
+        foreach ($backend->getAll() as $session) {
+            if ($userAuthentication !== null && $session['ses_id'] === $sessionToRenew) {
+                $userAuthentication->enforceNewSessionId();
+                continue;
+            }
+            if ((int)$session['ses_userid'] === $userId) {
+                $backend->remove($session['ses_id']);
+            }
+        }
+    }
+
+    /**
      * Creates a session backend from configuration
      *
      * @param string $identifier the identifier
diff --git a/typo3/sysext/core/Documentation/Changelog/8.7.x/Important-87298-DestroySessionsOnPasswordChange.rst b/typo3/sysext/core/Documentation/Changelog/8.7.x/Important-87298-DestroySessionsOnPasswordChange.rst
new file mode 100644 (file)
index 0000000..32ef333
--- /dev/null
@@ -0,0 +1,48 @@
+.. include:: ../../Includes.txt
+
+==================================================================
+Important: #87298 - [SECURITY] Destroy sessions on password change
+==================================================================
+
+See :issue:`87298`
+
+Description
+===========
+
+If a user - backend or frontend - changes the password, all existing sessions of that user
+must be destroyed for security reasons.
+
+In the core, we added functionality which takes care of this task with a DataHandler hook.
+Changing passwords in the backend will destroy all existing sessions of the edited user.
+
+The frontend login extension takes care of this task if the user resets a password (password recovery process).
+
+For all third party extensions which also handle password changes we added a method to
+the SessionManager class to easily integrate this important task, please check the code below:
+
+.. code-block:: php
+
+   # For any example below, we need the SessionManager
+   use \TYPO3\CMS\Core\Session\SessionManager;
+
+   # 1) Example: Destroy all backend user sessions for a backend user
+   $sessionManager = GeneralUtility::makeInstance(SessionManager::class);
+   $sessionBackend = $sessionManager->getSessionBackend('BE');
+   $sessionManager->invalidateAllSessionsByUserId($sessionBackend, (int)$id);
+
+   # 2) Example: Destroy all frontend user sessions for a frontend user
+   $sessionManager = GeneralUtility::makeInstance(SessionManager::class);
+   $sessionBackend = $sessionManager->getSessionBackend('FE');
+   $sessionManager->invalidateAllSessionsByUserId($sessionBackend, (int)$id);
+
+   # 3) Example: Destroy all backend user sessions for a backend user but keep and renew current backend user session
+   $sessionManager = GeneralUtility::makeInstance(SessionManager::class);
+   $sessionBackend = $sessionManager->getSessionBackend('BE');
+   $sessionManager->invalidateAllSessionsByUserId($sessionBackend, (int)$id, $GLOBALS['BE_USER']);
+
+   # 4) Example: Destroy all frontend user sessions for a frontend user but keep and renew current frontend user session
+   $sessionManager = GeneralUtility::makeInstance(SessionManager::class);
+   $sessionBackend = $sessionManager->getSessionBackend('FE');
+   $sessionManager->invalidateAllSessionsByUserId($sessionBackend, (int)$id, $GLOBALS['TSFE']->fe_user);
+
+.. index:: Backend, Frontend, PHP-API, ext:core
diff --git a/typo3/sysext/core/Tests/Functional/Session/SessionManagerTest.php b/typo3/sysext/core/Tests/Functional/Session/SessionManagerTest.php
new file mode 100644 (file)
index 0000000..0416405
--- /dev/null
@@ -0,0 +1,92 @@
+<?php
+declare(strict_types = 1);
+namespace TYPO3\CMS\Core\Tests\Functional\Service;
+
+/*
+ * 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!
+ */
+
+use TYPO3\CMS\Core\Session\SessionManager;
+use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
+
+class SessionManagerTest extends FunctionalTestCase
+{
+    /**
+     * @var SessionManager
+     */
+    protected $subject;
+
+    /**
+     * @var array
+     */
+    protected $testSessionRecords = [
+        [
+            'ses_id' => 'randomSessionId1',
+            'ses_userid' => 1,
+        ],
+        [
+            'ses_id' => 'randomSessionId2',
+            'ses_userid' => 1,
+        ],
+        [
+            'ses_id' => 'randomSessionId3',
+            'ses_userid' => 2,
+        ]
+    ];
+
+    /**
+     * Set configuration for DatabaseSessionBackend
+     */
+    protected function setUp()
+    {
+        parent::setUp();
+        $this->subject = new SessionManager();
+        $frontendSessionBackend = $this->subject->getSessionBackend('FE');
+        foreach ($this->testSessionRecords as $testSessionRecord) {
+            $frontendSessionBackend->set($testSessionRecord['ses_id'], $testSessionRecord);
+        }
+        $backendSessionBackend = $this->subject->getSessionBackend('BE');
+        foreach ($this->testSessionRecords as $testSessionRecord) {
+            $backendSessionBackend->set($testSessionRecord['ses_id'], $testSessionRecord);
+        }
+    }
+
+    /**
+     * @test
+     */
+    public function clearAllSessionsByUserIdDestroyAllSessionsForBackend()
+    {
+        $backendSessionBackend = $this->subject->getSessionBackend('BE');
+        $allActiveSessions = $backendSessionBackend->getAll();
+        $this->assertCount(3, $allActiveSessions);
+        $this->subject->invalidateAllSessionsByUserId($backendSessionBackend, 1);
+        $allActiveSessions = $backendSessionBackend->getAll();
+        $this->assertCount(1, $allActiveSessions);
+        $this->assertSame('randomSessionId3', $allActiveSessions[0]['ses_id']);
+        $this->assertSame(2, (int)$allActiveSessions[0]['ses_userid']);
+    }
+
+    /**
+     * @test
+     */
+    public function clearAllSessionsByUserIdDestroyAllSessionsForFrontend()
+    {
+        $frontendSessionBackend = $this->subject->getSessionBackend('FE');
+        $allActiveSessions = $frontendSessionBackend->getAll();
+        $this->assertCount(3, $allActiveSessions);
+        $this->subject->invalidateAllSessionsByUserId($frontendSessionBackend, 1);
+        $allActiveSessions = $frontendSessionBackend->getAll();
+        $this->assertCount(1, $allActiveSessions);
+        $this->assertSame('randomSessionId3', $allActiveSessions[0]['ses_id']);
+        $this->assertSame(2, (int)$allActiveSessions[0]['ses_userid']);
+    }
+}
index b4d489a..4339e0b 100644 (file)
@@ -32,6 +32,8 @@ $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['chec
 $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['processDatamapClass'][] = \TYPO3\CMS\Core\Hooks\SiteDataHandlerCacheHook::class;
 $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['processCmdmapClass'][] = \TYPO3\CMS\Core\Hooks\SiteDataHandlerCacheHook::class;
 
+$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['processDatamapClass'][] = \TYPO3\CMS\Core\Hooks\DestroySessionHook::class;
+
 $signalSlotDispatcher->connect(
     \TYPO3\CMS\Core\Resource\ResourceStorage::class,
     \TYPO3\CMS\Core\Resource\ResourceStorageInterface::SIGNAL_PostFileDelete,
index 525be79..1ec3fc4 100644 (file)
@@ -21,6 +21,7 @@ use TYPO3\CMS\Core\Crypto\Random;
 use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\Restriction\FrontendRestrictionContainer;
+use TYPO3\CMS\Core\Session\SessionManager;
 use TYPO3\CMS\Core\Site\SiteFinder;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\HttpUtility;
@@ -407,6 +408,7 @@ class FrontendLoginController extends AbstractPlugin
                                     )
                                 )
                                 ->execute();
+                            $this->invalidateUserSessions((int)$user['uid']);
 
                             $markerArray['###STATUS_MESSAGE###'] = $this->getDisplayText(
                                 'change_password_done_message',
@@ -1020,4 +1022,16 @@ class FrontendLoginController extends AbstractPlugin
         }
         return $marker;
     }
+
+    /**
+     * Invalidate all frontend user sessions by given user id
+     *
+     * @param int $userId the user UID
+     */
+    protected function invalidateUserSessions(int $userId)
+    {
+        $sessionManager = GeneralUtility::makeInstance(SessionManager::class);
+        $sessionBackend = $sessionManager->getSessionBackend('FE');
+        $sessionManager->invalidateAllSessionsByUserId($sessionBackend, $userId, $this->frontendController->fe_user);
+    }
 }