[TASK] Mitigate argon2i hash issues

* Let the "stored hash uses not supported mechanism" bubble up.
  Instead of just a "login failed", an error is raised hinting
  that something is broken.
* Improve exception message #1533818591: If an upgrade or new
  installation has been performed on a system that does support
  argon2i, users are upgraded to this mechanism. If the instance
  is later deployed to a server that does not support argon2i, the
  hash comparison will fail.
* Improve exception message #1533822084: This one is usually only
  raised if a core upgrade from v8 to v9 has just been performed on
  an instance that does not support argon2i, and a backend login is
  executed before the install tool silent configuration upgrader
  configured the system properly.
* Wiki pages with more details:
  https://wiki.typo3.org/Exception/CMS/1533818591
  https://wiki.typo3.org/Exception/CMS/1533822084

Resolves: #86392
Releases: master
Change-Id: I51e4ee9a198b9b92feec43c37a8b6b9b41c1b6f9
Reviewed-on: https://review.typo3.org/58402


Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>

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

@@ -121,9 +121,10 @@ class AuthenticationService extends AbstractAuthenticationService
         $saltFactory = GeneralUtility::makeInstance(PasswordHashFactory::class);
 
         // Get a hashed password instance for the hash stored in db of this user
+        $invalidPasswordHashException = null;
         try {
             $hashInstance = $saltFactory->get($passwordHashInDatabase, TYPO3_MODE);
-        } catch (InvalidPasswordHashException $e) {
+        } catch (InvalidPasswordHashException $invalidPasswordHashException) {
             // This can be refactored if the 'else' part below is gone in v10: Log and return 100 here
             $hashInstance = null;
         }
@@ -151,30 +152,36 @@ class AuthenticationService extends AbstractAuthenticationService
                     $isDomainLockMet = true;
                 }
             }
-        } else {
+        } elseif (substr($user['password'], 0, 2) === 'M$') {
             // @todo @deprecated: The entire else should be removed in v10.0 as dedicated breaking patch
-            if (substr($user['password'], 0, 2) === 'M$') {
-                // 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;
-                        }
+            // 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
                 }
+            } 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;
             }
         }
 

typo3/sysext/core/Classes/Crypto/PasswordHashing/PasswordHashFactory.php

@@ -76,7 +76,11 @@ class PasswordHashFactory
             }
         }
         // Do not add the hash to the exception to prevent information disclosure
-        throw new InvalidPasswordHashException('No implementation found that handles given hash.', 1533818591);
+        throw new InvalidPasswordHashException(
+            'No implementation found to handle given hash. This happens if the stored hash uses a'
+            . ' mechanism not supported by current server. Follow the wiki link to fix this issue.',
+            1533818591
+        );
     }
 
     /**
@@ -123,7 +127,9 @@ class PasswordHashFactory
         }
         if (!$hashInstance->isAvailable()) {
             throw new InvalidPasswordHashException(
-                'Configured default hash method ' . $defaultHashClassName . ' is not available, missing php requirement?',
+                'Configured default hash method ' . $defaultHashClassName . ' is not available. If'
+                . ' the instance has just been upgraded, please log in to the standalone install tool'
+                . ' at typo3/install.php to fix this. Follow the wiki link for more details.',
                 1533822084
             );
         }

typo3/sysext/core/Tests/Unit/Authentication/AuthenticationServiceTest.php

@@ -17,6 +17,7 @@ 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;
 
@@ -116,7 +117,7 @@ class AuthenticationServiceTest extends UnitTestCase
     /**
      * @test
      */
-    public function authUserReturns100IfPasswordInDbIsNotASaltedPassword(): void
+    public function authUserThrowsExceptionIfPasswordInDbDoesNotResolveToAValidHash(): void
     {
         $subject = new AuthenticationService();
         $pObjProphecy = $this->prophesize(AbstractUserAuthentication::class);
@@ -138,7 +139,9 @@ class AuthenticationServiceTest extends UnitTestCase
             'password' => 'aPlainTextPassword',
             'lockToDomain' => ''
         ];
-        $this->assertSame(100, $subject->authUser($dbUser));
+        $this->expectException(InvalidPasswordHashException::class);
+        $this->expectExceptionCode(1533818591);
+        $subject->authUser($dbUser);
     }
 
     /**

typo3/sysext/install/Classes/Authentication/AuthenticationService.php

@@ -55,7 +55,7 @@ class AuthenticationService
             try {
                 $hashInstance = $hashFactory->get($installToolPassword, 'BE');
                 $validPassword = $hashInstance->checkPassword($password, $installToolPassword);
-            } catch (InvalidPasswordHashException $e) {
+            } 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
@@ -68,6 +68,12 @@ class AuthenticationService
                         $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;
                 }
             }
         }