Feature #73050
closed
Add a CSPRNG to TYPO3
100%
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().
- remove all the
GeneralUtility::generateRandomBytesXYZmethods, 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\Randomto produce crypto-save random bytes, integers and hex strings - Add a check to
\TYPO3\CMS\Install\SystemEnvironment\Checkthat 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.
Updated by Georg Ringer about 8 years ago
Thanks for creating this issue.
As PHP7 will be required for version 8, there would be no need for the polyfill.
Updated by Christian Futterlieb about 8 years ago
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?
Updated by Gerrit Code Review about 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/46507
Updated by Christian Futterlieb about 8 years ago
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?
Updated by Gerrit Code Review about 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/46507
Updated by Gerrit Code Review about 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/46507
Updated by Helmut Hummel about 8 years ago
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?
Updated by Christian Futterlieb about 8 years ago
Helmut Hummel wrote:
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 inSeriously: What exactly is wrong with the current Random methods in 7.6?
GeneralUtility::generateRandomBytes(), imo:
- preferrably uses
openssl_random_pseudo_bytes()(which can fail to produce secure data):- without checking 2nd parameter
- without checking for PHP versions, that contain this bug: https://bugs.php.net/bug.php?id=70014
- 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
Updated by Christian Futterlieb about 8 years ago
- 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.
Updated by Gerrit Code Review about 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/46507
Updated by Gerrit Code Review about 8 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/46507
Updated by Gerrit Code Review about 8 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/46507
Updated by Christian Futterlieb about 8 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 5bc1aed17b811a3f58bb50f37dbb1cf903593d4f.
Updated by