Closed xperseguers closed 2 weeks ago
I think the random password is just a dummy to have some value in that field, right? So there wouldn’t be a security risk if just some not random value is inserted, right? Do you know how other ldap extensions handle this?
While reading the issue, I had the same thought @klodeckl. As long there is no hashing algorithm defined in the string ($argon2i et. al.) the comparison with the entered value will fail anyway. So a random string would be sufficient IHMO.
This is most probably true in TYPO3 nowadays, but it wasn't the case before where password could a simple md5 or even stored as plain text.
I'd still want to investigate the various tasks, to have real actual figures. Ideally I'm still not really OK having a dumb arbitrary identical string for every LDAP user.
=> The random bytes is no problem, the PasswordHashFactory
is the actual problem, at least here with Argon2i as in this test.
@klodeckl and @mschwemer Suggested to be tested as that could be "good enough" and a correct trade-off:
index 8cf7ba4..5401f5c 100755
--- a/Classes/Domain/Repository/Typo3UserRepository.php
+++ b/Classes/Domain/Repository/Typo3UserRepository.php
@@ -464,7 +464,7 @@ class Typo3UserRepository
{
$cryptoService = GeneralUtility::makeInstance(Random::class);
$password = $cryptoService->generateRandomBytes(20);
- $hashInstance = GeneralUtility::makeInstance(PasswordHashFactory::class)->getDefaultHashInstance('BE');
+ $hashInstance = GeneralUtility::makeInstance(\TYPO3\CMS\Core\Crypto\PasswordHashing\BcryptPasswordHash::class);
return $hashInstance->getHashedPassword($password);
}
}
Of course we could argue that having an actual "non-usable" hashed/password in the database is "more secure", the problem being to ensure that non-usable string can indeed never be used.
Decently-fast LDAP-over-SSH:
__INVALID__
as "random" password for 4158 users: 1656 seconds => 27 minutes
Right now batch-importing users takes a huge amount of time. @klodeckl reported in #167 that he had the feeling it went from 30-45 min in previous version(s) of the extension to more than 90 min.
During some tests on other tickets, I figured out that the random password generation in https://github.com/xperseguers/t3ext-ig_ldap_sso_auth/blob/0bcabe51607db3c830eaa86edc4b5f3126199ee6/Classes/Domain/Repository/Typo3UserRepository.php#L457-L469 was the culprit for massively slowing down the batch import.
A quick guess is that generating (proper) random password requires much entropy from the system and quite some computation as we rely on TYPO3's internals to have the exact same (random) password format as other parts.
It is important to understand that the random password is never supposed to be used and is more a protection to prevent a (LDAP) user from being able to bypass the actual LDAP-based password check by using a "local" password.
An early explanation about the possible reason @klodeckl experienced a slow-down of factor 2 [could be] the change of generating 16-byte random passwords to 20-byte random passwords, to be in line with what the TYPO3 Core does. This was done with commit e1901026.
As a guess of mine, @klodeckl hardcoded an arbitrary password to be used in batch import (as part of https://github.com/xperseguers/t3ext-ig_ldap_sso_auth/issues/167#issuecomment-2433482541) and the impact is massive: his import duration went down to about 8 minutes!
Next Steps
/dev/urandom
and alike) during a batch import in a real-life environment (typically 1000+ users), some possible reading: https://www.redhat.com/en/blog/understanding-random-number-generators-and-their-limitations-linux