Skip to content

Commit

Permalink
[SECURITY] Do not log sensitive data in authentication process
Browse files Browse the repository at this point in the history 
When having the debug logging activated for the
authentication process, sensitive data is not being
logged anymore.

This change
* removes password from being logged
* hashes the cookie value processed for logging

Resolves: #93925
Releases: master, 11.3, 10.4, 9.5
Change-Id: I8c610a72014de571ef52b4430c43f8d149b273d9
Security-Bulletin: CORE-SA-2021-012
Security-References: CVE-2021-32767
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69990
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
  • Loading branch information
@bmack @ohader
bmack authored and ohader committed Jul 20, 2021
1 parent 6f8d69d commit 0b49501
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 11 deletions.
46 changes: 37 additions & 9 deletions typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ protected function setSessionCookie()
);
$message = $isRefreshTimeBasedCookie ? 'Updated Cookie: {session}, {domain}' : 'Set Cookie: {session}, {domain}';
$this->logger->debug($message, [
'session' => $sessionId,
'session' => sha1($sessionId),
'domain' => $cookieDomain,
]);
}
Expand Down Expand Up @@ -440,14 +440,14 @@ public function checkAuthentication()
$authInfo = $this->getAuthInfoArray();
// Get Login/Logout data submitted by a form or params
$loginData = $this->getLoginFormData();
$this->logger->debug('Login data', $loginData);
$this->logger->debug('Login data', $this->removeSensitiveLoginDataForLoggingInfo($loginData));
// Active logout (eg. with "logout" button)
if ($loginData['status'] === LoginType::LOGOUT) {
if ($this->writeStdLog) {
// $type,$action,$error,$details_nr,$details,$data,$tablename,$recuid,$recpid
$this->writelog(SystemLogType::LOGIN, SystemLogLoginAction::LOGOUT, SystemLogErrorClassification::MESSAGE, 2, 'User %s logged out', [$this->user['username']], '', 0, 0);
}
$this->logger->info('User logged out. Id: {session}', ['session' => $this->userSession->getIdentifier()]);
$this->logger->info('User logged out. Id: {session}', ['session' => sha1($this->userSession->getIdentifier())]);
$this->logoff();
}
// Determine whether we need to skip session update.
Expand Down Expand Up @@ -556,7 +556,7 @@ public function checkAuthentication()
// Use 'auth' service to authenticate the user
// If one service returns FALSE then authentication failed
// a service might return 100 which means there's no reason to stop but the user can't be authenticated by that service
$this->logger->debug('Auth user', $tempuser);
$this->logger->debug('Auth user', $this->removeSensitiveLoginDataForLoggingInfo($tempuser, true));
$subType = 'authUser' . $this->loginType;

/** @var AuthenticationService $serviceObj */
Expand Down Expand Up @@ -641,7 +641,7 @@ public function checkAuthentication()
// Mark the current login attempt as failed
if (empty($tempuserArr) && $activeLogin) {
$this->logger->debug('Login failed', [
'loginData' => $loginData
'loginData' => $this->removeSensitiveLoginDataForLoggingInfo($loginData)
]);
} elseif (!empty($tempuserArr)) {
$this->logger->debug('Login failed', [
Expand Down Expand Up @@ -861,7 +861,7 @@ public function enforceNewSessionId()
*/
public function logoff()
{
$this->logger->debug('logoff: ses_id = {session}', ['session' => $this->userSession->getIdentifier()]);
$this->logger->debug('logoff: ses_id = {session}', ['session' => sha1($this->userSession->getIdentifier())]);

$_params = [];
foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_userauth.php']['logoff_pre_processing'] ?? [] as $_funcRef) {
Expand Down Expand Up @@ -1094,7 +1094,7 @@ public function setSessionData($key, $data)
public function setAndSaveSessionData($key, $data)
{
$this->userSession->set($key, $data);
$this->logger->debug('setAndSaveSessionData: ses_id = {session}', ['session' => $this->userSession->getIdentifier()]);
$this->logger->debug('setAndSaveSessionData: ses_id = {session}', ['session' => sha1($this->userSession->getIdentifier())]);
$this->userSession = $this->userSessionManager->updateSession($this->userSession);
}

Expand Down Expand Up @@ -1138,7 +1138,7 @@ public function isActiveLogin(ServerRequestInterface $request): bool
*/
public function processLoginData($loginData)
{
$this->logger->debug('Login data before processing', $loginData);
$this->logger->debug('Login data before processing', $this->removeSensitiveLoginDataForLoggingInfo($loginData));
$subType = 'processLoginData' . $this->loginType;
$authInfo = $this->getAuthInfoArray();
$isLoginDataProcessed = false;
Expand All @@ -1156,11 +1156,39 @@ public function processLoginData($loginData)
}
if ($isLoginDataProcessed) {
$loginData = $processedLoginData;
$this->logger->debug('Processed login data', $processedLoginData);
$this->logger->debug('Processed login data', $this->removeSensitiveLoginDataForLoggingInfo($processedLoginData));
}
return $loginData;
}

/**
* Removes any sensitive data from the incoming data (either from loginData, processedLogin data
* or the user record from the DB).
*
* No type hinting is added because it might be possible that the incoming data is of any other type.
*
* @param mixed|array $data
* @param bool $isUserRecord
* @return mixed
*/
protected function removeSensitiveLoginDataForLoggingInfo($data, bool $isUserRecord = false)
{
if ($isUserRecord && is_array($data)) {
$fieldNames = ['uid', 'pid', 'tstamp', 'crdate', 'cruser_id', 'deleted', 'disabled', 'starttime', 'endtime', 'username', 'admin', 'usergroup', 'db_mountpoints', 'file_mountpoints', 'file_permissions', 'workspace_perms', 'lastlogin', 'workspace_id', 'category_perms'];
$data = array_intersect_key($data, array_combine($fieldNames, $fieldNames));
}
if (isset($data['uident'])) {
$data['uident'] = '********';
}
if (isset($data['uident_text'])) {
$data['uident_text'] = '********';
}
if (isset($data['password'])) {
$data['password'] = '********';
}
return $data;
}

/**
* Returns an info array which provides additional information for auth services
*
Expand Down
4 changes: 2 additions & 2 deletions typo3/sysext/core/Classes/Session/UserSessionManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public function createAnonymousSession(): UserSession
*/
public function createSessionFromStorage(string $sessionId): UserSession
{
$this->logger->debug('Fetch session with identifier {session}', ['session' => $sessionId]);
$this->logger->debug('Fetch session with identifier {session}', ['session' => sha1($sessionId)]);
$sessionRecord = $this->sessionBackend->get($sessionId);
return UserSession::createFromRecord($sessionId, $sessionRecord);
}
Expand Down Expand Up @@ -189,7 +189,7 @@ public function fixateAnonymousSession(UserSession $session, bool $isPermanent =
public function elevateToFixatedUserSession(UserSession $session, int $userId, bool $isPermanent = false): UserSession
{
$sessionId = $session->getIdentifier();
$this->logger->debug('Create session ses_id = {session}', ['session' => $sessionId]);
$this->logger->debug('Create session ses_id = {session}', ['session' => sha1($sessionId)]);
// Delete any session entry first
$this->sessionBackend->remove($sessionId);
// Re-create session entry
Expand Down

0 comments on commit 0b49501

Please sign in to comment.