Project

General

Profile

Actions

Bug #98123

closed

Use best/strongest settings available in PasswordHashing

Added by Rasmus Sallling over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Security
Start date:
2022-08-11
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
12
PHP Version:
7.4
Tags:
Complexity:
medium
Is Regression:
Sprint Focus:

Description

I recently had to do a dive into the TYPO3 password hashing code (TYPO3\CMS\Core\Crypto\PasswordHashing), because of some related performance issues.
At that time, I noticed what I consider a dangerous check mechanism on the argon2 cost parameters.

PHP added global constants that set the default settings for argon2 generation costs, PASSWORD_ARGON2_DEFAULT_MEMORY_COST, PASSWORD_ARGON2_DEFAULT_TIME_COST and PASSWORD_ARGON2_DEFAULT_THREADS.

TYPO3 has, arguably sanely, decided to up these default to improve hash quality. (I would personally consider it saner to let PHP decide and increase this as CPUs get faster, but it's not unreasonable).
As part of this TYPO3 has decided that it will allow the user to change this value, but the user must NEVER set it lower than the PHP settings and so checks those and throws an exception if it fails.
TYPO3, however, does NOT check whether the PHP defaults are higher than its own settings, before setting them.

The result is that if PHPs defaults ever get higher than TYPO3s, for instance if PHP decides to increase the "memory_cost", the entire hashing library will start returning InvalidArgumentException for all calls. Given that these settings are likely to increase as CPUs get faster, this seems like a dangerous timebomb to leave lying around.

I would suggest one from this list:
1: If the TYPO3 default becomes lower than the PHP default, just use the PHP default.
2: TYPO3 just use the PHP defaults and leave the decision to increase complexity to the PHP developers/users.
3: TYPO3 should log a warning/error with an intimidating message, instead of an exception.

Actions #1

Updated by Oliver Hader over 1 year ago

  • Assignee deleted (Oliver Hader)

Thanks for creating this issue.

I'd opt for using the best option/value/setting available - if PHP value is stronger use it, if TYPO3 value is stronger use it. So that's somewhat option "1".

Basically this affects all TYPO3 versions since v9 when this functionality had been rewritten.

Actions #2

Updated by Oliver Hader over 1 year ago

  • Status changed from New to Accepted
Actions #3

Updated by Oliver Hader over 1 year ago

  • Subject changed from Dangerous timebomb in TYPO3 PasswordHashing code to Use best/strongest settings available in PasswordHashing
Actions #4

Updated by Christian Kuhn over 1 year ago

  • Status changed from Accepted to Closed

Hey. I hope it's ok to close here since I think this is not an actual problem:

PHP will most likely not change defaults in minor or at least patch level versions. So "current" code is "safe". In case PHP raises defaults beyond our extended settings, we will most likely see this when looking at major/minor PHP releases and will adapt accordingly.

As such, I think it's ok to close here for now with "no action required".

Please feel free to re-open in case you think this is still an active issue.

Actions

Also available in: Atom PDF