Generated secrets aren't cryptographically secure

[zabszk]

Hi. You used mt_rand to generate TOTP secret, but it isn't cryptographically secure generator and secret can be predicted by 3rd parties. You should use random_int, random_bytes or openssl_random_pseudo_bytes() instead.

More information on php.net: http://php.net/manual/en/function.mt-rand.php

[stolendata]

In the context of a TOTP secret (which in some sense isn't a component of cryptography) the output doesn't really have to be cryptographically secure - it suffices if it's "unpredictably pseudorandom". genSecret() is in this implementation of TOTP merely a convenience, not integral.

[zabszk]

It should be secure, because if generator is precitible, someone can get totp secret and "bypass" 2FA. Only crypto generators are unpredictible.

Look at other implemenations:

• https://github.com/PHPGangsta/GoogleAuthenticator/blob/master/PHPGangsta/GoogleAuthenticator.php
• https://github.com/RobThree/TwoFactorAuth/blob/master/lib/TwoFactorAuth.php
• https://github.com/Dolondro/google-authenticator/blob/master/src/SecretFactory.php

Everyone is using secure generators. Your implemenation is really easy to use, but is insecure. If you don't want to change it, you should make a note in project's about file about that.

[stolendata]

It's good that everyone else is very serious about password generation in their implementations. Of course I'm aware that the MT is in itself a deterministic algorithm and unsuitable for cryptographic key components, but you seem to have ignored the part about "cryptographically secure" versus "unpredictably pseudorandom" passwords. If you are up to the task, it would be interesting to see your approach to predict the latter half of any output from genSecret() when being given the first half as an outset, even if it includes a timing attack.

[zabszk]

It's possible to calculate seed of mt_rand, when we know the generated value (working tool: http://www.openwall.com/php_mt_seed/), so it's possible to predict the value. I don't test this tool, but accordint to reddit, this tool should work. Even salt for passwords should be generated using cryptographically secure generators.

[stolendata]

genSecret() doesn't use values from mt_rand() in a linear fashion.

[zabszk]

I found in RFC 6238 (TOTP definition) that it should be crypto generator.

"As indicated in the algorithm requirement section, keys SHOULD be chosen at random or using a cryptographically strong pseudorandom generator properly seeded with a random value." Source: https://tools.ietf.org/html/rfc6238 (page 5)

[zabszk]

You can use this: function secure_rand($min, $max) { return (unpack("N", openssl_random_pseudo_bytes(4)) % ($max - $min)) + $min; } Source: https://stackoverflow.com/questions/32732940/generate-a-secure-random-integer-in-range-with-php-5-6

Or you can use random_int(min, max), but that works only on PHP 7.0 and newer.