Bug #54833

Check default salting method first when determining salting method

Added by Christoph Dörfel over 8 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Authentication
Target version:
Start date:
2014-01-08
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
easy
Is Regression:
No
Sprint Focus:

Description

When SaltFactory::determineSaltingHashingMethod($saltedHash) gets called, we iterate over all available hashing methods, not prioritising the default salting method: SaltedPasswordsUtility::getDefaultSaltingHashingMethod($mode = TYPO3_MODE).
This can result in unnecessary password updates when the salting method "isValidSaltedPW($saltedHash)" returns TRUE for similar hashing implementations.

#1

Updated by Gerrit Code Review over 8 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/26692

#2

Updated by Markus Klein over 8 years ago

I don't get the problem.
isValidSaltedPW() returns TRUE we found a valid hashing algorithm, so why do we care about the default at all?

#3

Updated by Christoph Dörfel over 8 years ago

Markus Klein wrote:

I don't get the problem.
isValidSaltedPW() returns TRUE we found a valid hashing algorithm, so why do we care about the default at all?

Yes, but SaltFactory::determineSaltingHashingMethod($saltedHash) also saves the first valid hashing algorithm in SaltFactory::$instance; Most of the time, the default algorithm will also be the valid one. This saves some unnecessary instancing, but there is another point:

The fe_login extension compares the default hashing method with the one that SaltFactory::determineSaltingHashingMethod($saltedHash) stores as SaltFactory::$instance. When the salting instances aren't equal, the users password is being updated after login.

Now consider you made an extension that builds upon the BlowfishSalt (as en example). The password hash in the database will read $2y$-something for PHP >= 5.4 and $2a$ for PHP < 5.4. Then BlowfishSalt:: isValidSaltedPW() will return TRUE in PHP < 5.4 and your extension (that created the hash in the first place) is never checked. The instance in SaltFactory::$instance is now different from your hashing algorithm. This is wrong. Although you can not safely determine the correct salting algorithm for when two implementations return TRUE for isValidSaltedPW, you should at least prefer the default hashing algorithm.

edit:
Reason:
I made a salting instance that uses the password_XX() methods of PHP >= 5.5 or emulates them for PHP < 5.5. PHP 5.5 now features a constant PASSWORD_DEFAULT that references the currently strongest algorithm. This constant is designed to change over time as new and stronger algorithms are added to PHP. This means that you can't be sure that two hashing implementations won't both return TRUE.

#4

Updated by Gerrit Code Review over 8 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/26692

#5

Updated by Gerrit Code Review over 8 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/26692

#6

Updated by Gerrit Code Review over 8 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/26692

#7

Updated by Markus Klein over 8 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#8

Updated by Benni Mack almost 4 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF