[!!!][TASK] Drop handling of M$ prefixed passwords 87/59487/10
authorChristian Kuhn <lolli@schwarzbu.ch>
Fri, 18 Jan 2019 14:04:34 +0000 (15:04 +0100)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Fri, 18 Jan 2019 20:09:12 +0000 (21:09 +0100)
Passwords prefixed with M$ in fe_user or be_user table
are passwords that have been previously stored as simple md5
and have been converted to salted md5 using the saltedpasswords
converter task. That task has been removed in v9 already and
affected only users who did not log in for ages (multiple
core versions), since otherwise their password would have
been upgraded already.
The patch now finally drops the handling of M$ prefixed
passwords. If there are still users like that, they have
to have their password reset by and editor or admin user.

Resolves: #87489
Releases: master
Change-Id: I2d98476e91dd40c60a1084f9b05cf4ce7b9c5829
Reviewed-on: https://review.typo3.org/59487
Tested-by: TYPO3com <noreply@typo3.com>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/core/Classes/Authentication/AuthenticationService.php
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/core/Documentation/Changelog/master/Breaking-87193-DeprecatedFunctionalityRemoved.rst
typo3/sysext/core/Tests/Unit/Authentication/AuthenticationServiceTest.php
typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php
typo3/sysext/install/Classes/Authentication/AuthenticationService.php

index 56c35ad..45e4f24 100644 (file)
@@ -16,7 +16,6 @@ namespace TYPO3\CMS\Core\Authentication;
 
 use TYPO3\CMS\Core\Crypto\PasswordHashing\InvalidPasswordHashException;
 use TYPO3\CMS\Core\Crypto\PasswordHashing\PasswordHashFactory;
-use TYPO3\CMS\Core\Crypto\PasswordHashing\PasswordHashInterface;
 use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction;
@@ -113,8 +112,6 @@ class AuthenticationService extends AbstractAuthenticationService
         $configuredDomainLock = $user['lockToDomain'];
         $userDatabaseTable = $this->db_user['table'];
 
-        $isSaltedPassword = false;
-        $isValidPassword = false;
         $isReHashNeeded = false;
         $isDomainLockMet = false;
 
@@ -125,67 +122,6 @@ class AuthenticationService extends AbstractAuthenticationService
         try {
             $hashInstance = $saltFactory->get($passwordHashInDatabase, TYPO3_MODE);
         } catch (InvalidPasswordHashException $invalidPasswordHashException) {
-            // This can be refactored if the 'else' part below is gone in TYPO3 v10.0: Log and return 100 here
-            $hashInstance = null;
-        }
-        // An instance of the currently configured salted password mechanism
-        // Don't catch InvalidPasswordHashException here: Only install tool should handle those configuration failures
-        $defaultHashInstance = $saltFactory->getDefaultHashInstance(TYPO3_MODE);
-
-        if ($hashInstance instanceof PasswordHashInterface) {
-            // We found a hash class that can handle this type of hash
-            $isSaltedPassword = true;
-            $isValidPassword = $hashInstance->checkPassword($submittedPassword, $passwordHashInDatabase);
-            if ($isValidPassword) {
-                if ($hashInstance->isHashUpdateNeeded($passwordHashInDatabase)
-                    || $defaultHashInstance != $hashInstance
-                ) {
-                    // Lax object comparison intended: Rehash if old and new salt objects are not
-                    // instances of the same class.
-                    $isReHashNeeded = true;
-                }
-                if (empty($configuredDomainLock)) {
-                    // No domain restriction set for user in db. This is ok.
-                    $isDomainLockMet = true;
-                } elseif (!strcasecmp($configuredDomainLock, $queriedDomain)) {
-                    // Domain restriction set and it matches given host. Ok.
-                    $isDomainLockMet = true;
-                }
-            }
-        } elseif (substr($user['password'], 0, 2) === 'M$') {
-            // @todo @deprecated: The entire else should be removed in TYPO3 v10.0 as dedicated breaking patch
-            // If the stored db password starts with M$, it may be a md5 password that has been
-            // upgraded to a salted md5 using the old salted passwords scheduler task.
-            // See if a salt instance is returned if we cut off the M, so Md5PasswordHash kicks in
-            try {
-                $hashInstance = $saltFactory->get(substr($passwordHashInDatabase, 1), TYPO3_MODE);
-                $isSaltedPassword = true;
-                $isValidPassword = $hashInstance->checkPassword(md5($submittedPassword), substr($passwordHashInDatabase, 1));
-                if ($isValidPassword) {
-                    // Upgrade this password to a sane mechanism now
-                    $isReHashNeeded = true;
-                    if (empty($configuredDomainLock)) {
-                        // No domain restriction set for user in db. This is ok.
-                        $isDomainLockMet = true;
-                    } elseif (!strcasecmp($configuredDomainLock, $queriedDomain)) {
-                        // Domain restriction set and it matches given host. Ok.
-                        $isDomainLockMet = true;
-                    }
-                }
-            } catch (InvalidPasswordHashException $e) {
-                // Still no instance found: $isSaltedPasswords is NOT set to true, logging and return done below
-            }
-        } else {
-            // @todo: Simplify if elseif part is gone
-            // Still no valid hash instance could be found. Probably the stored hash used a mechanism
-            // that is not available on current system. We throw the previous exception again to be
-            // handled on a higher level.
-            if ($invalidPasswordHashException !== null) {
-                throw $invalidPasswordHashException;
-            }
-        }
-
-        if (!$isSaltedPassword) {
             // Could not find a responsible hash algorithm for given password. This is unusual since other
             // authentication services would usually be called before this one with higher priority. We thus log
             // the failed login but still return '100' to proceed with other services that may follow.
@@ -197,6 +133,29 @@ class AuthenticationService extends AbstractAuthenticationService
             return 100;
         }
 
+        // An instance of the currently configured salted password mechanism
+        // Don't catch InvalidPasswordHashException here: Only install tool should handle those configuration failures
+        $defaultHashInstance = $saltFactory->getDefaultHashInstance(TYPO3_MODE);
+
+        // We found a hash class that can handle this type of hash
+        $isValidPassword = $hashInstance->checkPassword($submittedPassword, $passwordHashInDatabase);
+        if ($isValidPassword) {
+            if ($hashInstance->isHashUpdateNeeded($passwordHashInDatabase)
+                || $defaultHashInstance != $hashInstance
+            ) {
+                // Lax object comparison intended: Rehash if old and new salt objects are not
+                // instances of the same class.
+                $isReHashNeeded = true;
+            }
+            if (empty($configuredDomainLock)) {
+                // No domain restriction set for user in db. This is ok.
+                $isDomainLockMet = true;
+            } elseif (!strcasecmp($configuredDomainLock, $queriedDomain)) {
+                // Domain restriction set and it matches given host. Ok.
+                $isDomainLockMet = true;
+            }
+        }
+
         if (!$isValidPassword) {
             // Failed login attempt - wrong password
             $this->writeLogMessage(TYPO3_MODE . ' Authentication failed - wrong password for username \'%s\'', $submittedUsername);
index b409539..6e55910 100644 (file)
@@ -2607,18 +2607,10 @@ class DataHandler implements LoggerAwareInterface
                     // The strategy is to see if a salt instance can be created from the incoming value. If so,
                     // no new password was submitted and we keep the value. If no salting instance can be created,
                     // incoming value must be a new plain text value that needs to be hashed.
-                    $hashMethod = substr($value, 0, 2);
-                    // The old scheduler task turned existing non-salted passwords into salted hashes by taking the simple md5
-                    // and using that as 'password' and make a salted md5 from given hash. Those where then prefixed with 'M'.
-                    // PasswordHashFactory->get($value) only recognizes these salts if we cut off the M again.
-                    // @todo @deprecated: $isDeprecatedSaltedHash should be removed in TYPO3 v10.0 as dedicated breaking patch, similar
-                    // @todo to authUser() of AuthenticationService::class
-                    $isDeprecatedSaltedHash = $hashMethod === 'M$';
-                    $tempValue = $isDeprecatedSaltedHash ? substr($value, 1) : $value;
                     $hashFactory = GeneralUtility::makeInstance(PasswordHashFactory::class);
                     $mode = $table === 'fe_users' ? 'FE' : 'BE';
                     try {
-                        $hashFactory->get($tempValue, $mode);
+                        $hashFactory->get($value, $mode);
                     } catch (InvalidPasswordHashException $e) {
                         // We got no salted password instance, incoming value must be a new plaintext password
                         // Get an instance of the current configured salted password strategy and hash the value
index 146e347..2fce9e5 100644 (file)
@@ -1347,6 +1347,9 @@ The following features have been removed:
 * These variables are no longer declared in :file:`ext_tables.php` and :file:`ext_localconf.php` files: :php:`$_EXTKEY`, :php:`$_EXTCONF`,
   :php:`T3_SERVICES`, :php:`T3_VAR`, :php:`TYPO3_CONF_VARS`, :php:`TBE_MODULES`, :php:`TBE_MODULES_EXT`, :php:`TCA`,
   :php:`PAGES_TYPES`, :php:`TBE_STYLES`
+* Frontend, Backend and standalone install tool users who did not log in for multiple core versions and still use a :php:`M$`
+  prefixed password can not log in anymore. Auto converting those user passwords during first login has been dropped, those
+  users need their password being manually recovered or reset.
 
 
 The following database tables have been removed:
index c12fdbf..8071414 100644 (file)
@@ -17,7 +17,6 @@ namespace TYPO3\CMS\Core\Tests\Unit\Authentication;
 
 use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication;
 use TYPO3\CMS\Core\Authentication\AuthenticationService;
-use TYPO3\CMS\Core\Crypto\PasswordHashing\InvalidPasswordHashException;
 use TYPO3\CMS\Core\Log\Logger;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
@@ -139,9 +138,7 @@ class AuthenticationServiceTest extends UnitTestCase
             'password' => 'aPlainTextPassword',
             'lockToDomain' => ''
         ];
-        $this->expectException(InvalidPasswordHashException::class);
-        $this->expectExceptionCode(1533818591);
-        $subject->authUser($dbUser);
+        $this->assertEquals(100, $subject->authUser($dbUser));
     }
 
     /**
index bf5872c..609d337 100644 (file)
@@ -210,18 +210,6 @@ class DataHandlerTest extends UnitTestCase
     /**
      * @test
      */
-    public function checkValueInputEvalWithSaltedPasswordKeepsExistingHashForMd5HashedHash(): void
-    {
-        // Note the involved salted passwords are NOT mocked since the factory is static
-        $subject = new DataHandler();
-        $inputValue = 'M$1$GNu9HdMt$RwkPb28pce4nXZfnplVZY/';
-        $result = $subject->checkValue_input_Eval($inputValue, ['saltedPassword'], '', 'be_users');
-        $this->assertSame($inputValue, $result['value']);
-    }
-
-    /**
-     * @test
-     */
     public function checkValueInputEvalWithSaltedPasswordReturnsHashForSaltedPassword(): void
     {
         // Note the involved salted passwords are NOT mocked since the factory is static
index 02afadf..e0cbaae 100644 (file)
@@ -15,7 +15,6 @@ namespace TYPO3\CMS\Install\Authentication;
  * The TYPO3 project - inspiring people to share!
  */
 
-use TYPO3\CMS\Core\Configuration\ConfigurationManager;
 use TYPO3\CMS\Core\Crypto\PasswordHashing\InvalidPasswordHashException;
 use TYPO3\CMS\Core\Crypto\PasswordHashing\PasswordHashFactory;
 use TYPO3\CMS\Core\Mail\MailMessage;
@@ -53,30 +52,10 @@ class AuthenticationService
         if ($password !== null && $password !== '') {
             $installToolPassword = $GLOBALS['TYPO3_CONF_VARS']['BE']['installToolPassword'];
             $hashFactory = GeneralUtility::makeInstance(PasswordHashFactory::class);
-            try {
-                $hashInstance = $hashFactory->get($installToolPassword, 'BE');
-                $validPassword = $hashInstance->checkPassword($password, $installToolPassword);
-            } catch (InvalidPasswordHashException $invalidPasswordHashException) {
-                // Given hash in global configuration is not a valid salted password
-                if (md5($password) === $installToolPassword) {
-                    // Update configured install tool hash if it is still "MD5" and password matches
-                    // @todo: This should be removed in TYPO3 v10.0 with a dedicated breaking patch
-                    // @todo: Additionally, this code should check required hash updates and update the hash if needed
-                    $hashInstance = $hashFactory->getDefaultHashInstance('BE');
-                    $configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
-                    $configurationManager->setLocalConfigurationValueByPath(
-                        'BE/installToolPassword',
-                        $hashInstance->getHashedPassword($password)
-                    );
-                    $validPassword = true;
-                } else {
-                    // Still no valid hash instance could be found. Probably the stored hash used a mechanism
-                    // that is not available on current system. We throw the previous exception again to be
-                    // handled on a higher level. The install tool will render an according exception message
-                    // that links to the wiki.
-                    throw $invalidPasswordHashException;
-                }
-            }
+            // Throws an InvalidPasswordHashException if no hash mechanism for the stored password is found
+            $hashInstance = $hashFactory->get($installToolPassword, 'BE');
+            // @todo: This code should check required hash updates and update the hash if needed
+            $validPassword = $hashInstance->checkPassword($password, $installToolPassword);
         }
         if ($validPassword) {
             $this->sessionService->setAuthorized();