Project

General

Profile

Bug #93421

Updated by Markus Klein almost 4 years ago

h1. In short: 

 If some session data are stored for a new anonymous user (no `fe_sessions` record exists) within the application layer and `\TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData()` is called within the application layer, then the persisted `fe_sessions` record can never be reconstituted in further requests. 
 This is because the `FrontendUserAuthenticator` middleware wants to update the already persisted (through the application layer) user session record. 
 Since the update procedure does not handle the `ses_iplock` property, this property will be updated as an empty string. 
 A further request will try to reconstitute the persisted session data and compares the `ses_iplock` value from the persisted record (empty string) with the configured `[FE][lockIP]` value (which will be internal transformed to '[DISABLED]' in case of '0'). 
 Since the comparison of `'' === '[DISABLED]'` never matches, a new anonymous session will be created and the previous persisted session data leaves as garbage. 

 h1. In detail: 

 The "ipLock":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSession.php#L46 property from the UserSession model is some kind of imutable and has the default value `''`. 
 This property is always set from the "outside" ("example 1":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSession.php#L232 , "example 2":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSessionManager.php#L168 , "example 3":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSessionManager.php#L195). 
 If an anonymous session will be persisted through "UserSessionManager::fixateAnonymousSession()":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSessionManager.php#L163 , the session data to persist will be enriched with `ses_iplock` with a value of `[DISABLED]` or some parts of an ip address. 
 If the setting `[FE][lockIP]` is set to 0 ('Do not lock Backend User sessions to their IP address at all'), then the persisted `fe_sessions` record contains the string '[DISABLED]' as the value for the column `ses_iplock`. 
 If an already persisted anonymous session will be updated through "UserSessionManager::updateSession":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSessionManager.php#L265 , then the `UserSession` model will be converted to an array and this array data will be persisted. 
 Since the UserSession model was created by "UserSession::createNonFixated()":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSession.php#L247 , the property "ipLock" has always an empty string as value within the model data. 
 Therefore, the update procedure overwrite the value '[DISABLED]' (written by an previous call of 'UserSessionManager::fixateAnonymousSession()') with an empty string. 
 A further request will compare the `ses_iplock` value from the persisted record (now '' instead of '[DISABLED]') with the configured value ('[DISABLED]') within "UserSessionManager::getSessionFromSessionId()":https://github.com/TYPO3/TYPO3.CMS/blob/bb971f78a198f69cd915ae1d0921de0d2e839055/typo3/sysext/core/Classes/Session/UserSessionManager.php#L310. 
 Since the comparison of `'' === '[DISABLED]'` never matches, a new anonymous session will be created and the previous persisted session data leaves as garbage. 

 Let's take a look into some examples. 

 h2. This is a working example: 

 We assume that the `fe_sessions` table is empty or no fe cookie exists and the setting `[FE][lockIP]` is set to 0 (default configuration). 

 We start with the middleware layer in incoming direction which creates the user session. 

 <pre> 
 \TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63 
     \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261 
         \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102 
             \TYPO3\CMS\Core\Session\UserSessionManager->createAnonymousSession() Line 113 
 </pre> 

 The user session model is created by `UserSession::createNonFixated()` -> the property "ipLock" has an empty string as value within the model data. 

 Now we are within the application layer, let's say an extbase controller and we do something like this 

 <pre> 
 $this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1'); 
 </pre> 

 The data will be set to the `UserSession` model (but the model is still not persisted to the `fe_sessions` table). 

 Now we leave the application layer and enter the middleware layer again in outgoing direction. 

 <pre> 
 \TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 78 
     \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData() Line 339 
         \TYPO3\CMS\Core\Session\UserSessionManager->fixateAnonymousSession() Line 168 
 </pre> 

 The `FrontendUserAuthentication::storeSessionData()` procedure determines that the session data was never persisted before and call `UserSessionManager::fixateAnonymousSession()` to persist them. 
 `UserSessionManager::fixateAnonymousSession()` transform the `UserSession` model to an array, add the `ses_iplock` value and persist them. 

 <pre> 
         $sessionRecord = $session->toArray(); 
         $sessionRecord['ses_iplock'] = $sessionIpLock; 
         ... 
         $this->sessionBackend->set($session->getIdentifier(), $sessionRecord); 
 </pre> 

 The `ses_iplock` column (table `fe_sessions`) contains the string '[DISABLED]'. 

 Now we perform an new request. 

 <pre> 
 \TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63 
     \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261 
         \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102 
             \TYPO3\CMS\Core\Session\UserSessionManager->getSessionFromSessionId() 
 </pre> 

 `UserSessionManager::getSessionFromSessionId()` is able to reconstitute the user session from the persistence layer, because of the `$this->ipLocker->validateRemoteAddressAgainstSessionIpLock()` check succeed. 


 h2. This is a *not* not working example: 

 We assume that the `fe_sessions` table is empty or no fe cookie exists and the setting `[FE][lockIP]` is set to 0 (default configuration). 

 We start with the middleware layer in incoming direction which creates the user session. 

 <pre> 
 \TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63 
     \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261 
         \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102 
             \TYPO3\CMS\Core\Session\UserSessionManager->createAnonymousSession() Line 113 
 </pre> 

 The user session model is created by `UserSession::createNonFixated()` -> the property "ipLock" has an empty string as value within the model data. 

 Now we are within the application layer, let's say an extbase controller and we do something like this 

 <pre> 
 $this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1'); 
 $this->getFrontendUser()->setAndSaveSessionData('foo_2', 'bar_2'); 
 </pre> 

 The data will be set to the `UserSession` model. 
 The call to `FrontendUserAuthentication::setAndSaveSessionData()` results in the following calls 

 <pre> 
     \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData() Line 339 
         \TYPO3\CMS\Core\Session\UserSessionManager->fixateAnonymousSession() Line 168 
 </pre> 

 `UserSessionManager::fixateAnonymousSession()` transform the `UserSession` model to an array, add the `ses_iplock` value and persist them. 

 <pre> 
         $sessionRecord = $session->toArray(); 
         $sessionRecord['ses_iplock'] = $sessionIpLock; 
         ... 
         $this->sessionBackend->set($session->getIdentifier(), $sessionRecord); 
 </pre> 

 So far everything is going as expected 

 Now we leave the application layer and enter the middleware layer again in outgoing direction. 

 <pre> 
 \TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 78 
     \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->storeSessionData() Line 404 
         \TYPO3\CMS\Core\Session\UserSessionManager->updateSession() Line 267 
 </pre> 

 The `FrontendUserAuthentication::storeSessionData()` procedure determines that the session data was already persisted and call `UserSessionManager::updateSession()` to update them. 
 We remember that the user session model was created by `UserSession::createNonFixated()` -> the property "ipLock" has an empty string as value within the model data. 
 `UserSessionManager::updateSession()` converts the `UserSession` model to an array and persist this data, without doing anything with the 'ipLock' property. 
 Since `ipLock` is an empty string within the model an empty string will be persisted for the `ses_iplock` column (table `fe_sessions`). 

 Now we perform an new request. 

 <pre> 
 \TYPO3\CMS\Frontend\Middleware\FrontendUserAuthenticator->process() Line 63 
     \TYPO3\CMS\Frontend\Authentication\FrontendUserAuthentication->start() Line 261 
         \TYPO3\CMS\Core\Session\UserSessionManager->createFromGlobalCookieOrAnonymous() Line 102 
             \TYPO3\CMS\Core\Session\UserSessionManager->getSessionFromSessionId() 
 </pre> 

 `UserSessionManager::getSessionFromSessionId()` is not able to reconstitute the user session from the persistence layer, because of the `$this->ipLocker->validateRemoteAddressAgainstSessionIpLock()` check will fail (comparison of `'' === '[DISABLED]'`). 
 Therefore a new anonymous session will be created and the previous persisted session data leaves as garbage. 

 Since the usage of the `FlashMessagesViewHelper` or calls of `FrontendFormProtection::persistSessionToken()` results in `FrontendUserAuthentication::setAndSaveSessionData()` calls, this wrong behaviour can also happen if a developer does not call 'FrontendUserAuthentication::setAndSaveSessionData()' by its own. 
 It is enough to call `FrontendUserAuthentication::setKey()` within the application layer (a controller or something like this) and make use of a `<f:flashMessages />` ViewHelper within an template (it doesn't matter if flash messages exists or not). 

 A quick test can be performed with an controller like this which will reduces the issue scenarios a little bit 

 <pre> 
 class SessionTestController extends ActionController 
 { 
     public function workingAction() 
     { 
         dump($this->getFrontendUser()->getKey('ses', 'foo_1')); 

         $this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1'); 
     } 

     public function notworkingAction() 
     { 
         dump($this->getFrontendUser()->getKey('ses', 'foo_1')); 

         $this->getFrontendUser()->setKey('ses', 'foo_1', 'bar_1'); 
         $this->getFrontendUser()->setAndSaveSessionData('foo_2', 'bar_2'); 
     } 

     /** 
      * @return FrontendUserAuthentication 
      */ 
     protected function getFrontendUser(): FrontendUserAuthentication 
     { 
         return $GLOBALS['TSFE']->fe_user; 
     } 
 } 
 </pre> 

 A request to `notworkingAction()` creates a new `fe_sessions` record everytime you call it and `$this->getFrontendUser()->getKey('ses', 'foo_1')` will always return null. 

 The provided patch try to avoid the wrong behavior by re-set the `ses_iplock` value from the persisted data to the data to persist. 
 Since this is done within `UserSessionManager::updateSession()` i'm not quite sure if this is the right place because this method is used by FE and BE context. 
 Maybe this must be handled only within `FrontendUserAuthentication::storeSessionData()`. 

Back