Feature #73050
closed
Added by Christian Futterlieb about 8 years ago.
Updated over 5 years ago.
Description
I'd like to bring some crypto-related code into TYPO3 core. First topic: a CSPRNG
As always in cryptography, using a widely used/adopted/reviewed library should be the way to go. This one seems to do a good job: https://github.com/paragonie/random_compat. It is a PHP 5.x polyfill for PHP 7's random_bytes()
and random_int()
.
In the proposed change, I cover following tasks:
- remove all the
GeneralUtility::generateRandomBytesXYZ
methods, because they're covered by random_compat. Leave just the fallback method in place (and slightly improve it)
- Add a simple API in
\TYPO3\CMS\Core\Crypto\Random
to produce crypto-save random bytes, integers and hex strings
- Add a check to
\TYPO3\CMS\Install\SystemEnvironment\Check
that creates a warning, when no CSPRNG can be generated on the system (and the fallback will be used therefor). From the crypto-view it would be much better to fail instead of just warn.. please share your opinion on this!
Furthermore I'd like to come up with things like a Crypto\Hash
class to do proper hashing and verifying, a Crypto\Password
class for password-related stuff, a saltedpasswords salt and so on. I'll open new tasks for these ideas when they're ready.
Thanks for creating this issue.
As PHP7 will be required for version 8, there would be no need for the polyfill.
Great you mentioned that, I was just about pushing the stuff to gerrit ;-)
What about version 7.6? I'd like to have a good CSPRNG in there too.. So, how should I proceed? Because: in PHP7 it's just creating the Crypto\Random class and changing GeneralUtility accordingly. But with PHP<7 there comes in some additional code and the system env check. Should I open a new issue for TYPO3 7.6 though?
- Status changed from New to Under Review
Hi
I just pushed the change for master. To get the CSPRNG into 7.6 LTS, the changes would be very different to these in the proposed one:
- No deprecation
- Leaving in a (unsafe) fallback
- According tests
- Adding system check (warn when the CSPRNG is not available on the system)
- Adding the polyfill
Can these things be covered by 'backporting' the change to 7.6? Or do I have to add a separate change? Any statements from anybody, please?
Christian Futterlieb wrote:
What about version 7.6? I'd like to have a good CSPRNG in there too..
Seriously: What exactly is wrong with the current Random methods in 7.6?
Helmut Hummel wrote:
Seriously: What exactly is wrong with the current Random methods in 7.6?
Glad you asked that, nobody else did.. The key part is that it is not
crypto-save, say: unusable for cryptographic applications (like a login system). At the moment there are some problems in
GeneralUtility::generateRandomBytes()
, imo:
- preferrably uses
openssl_random_pseudo_bytes()
(which can fail to produce secure data):
- uses
mcrypt_create_iv()
(which can fail to produce secure data), see https://bugs.php.net/bug.php?id=52523, https://bugs.php.net/bug.php?id=55169
- uses a silent fallback without further noticing anybody
As a consequence of this, TYPO3 cannot be treated as save (from a cryptographic view) by default. Sure, one is able to configure his system to make TYPO3 produce secure data, but this needs knowledge of core-internals.
Therefor, my proposal is to implement:
- a 'real' CSPRNG (with
random_bytes()
for PHP>=7 and random_compat polyfill for the rest). Like this we have the advantage of code written by cryptographers and an api for everybody inside TYPO3
- the secure hashing API, see #73164
And: just to take into consideration: these are the security-related components in TYPO3:
- core (authentication, managing the sessionIds, formProtection, generating sessionTokens). Wide parts of the core depend on the security of these.
- saltedpasswords (every Salt class besides Blowfish)
- felogin (generating 'reset-password' hashes)
- install (generating the TYPO3 encryptionKey [!])
TL/DR: Many core parts of TYPO3 rely on the unpredictability of the RNG. Imo it is very important to truly fulfil this responsibility.
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
- Status changed from Resolved to Closed
Also available in: Atom
PDF