ChristianRiesen/otp

Use random_int() instead of openssl with failover to mt_rand

mattattui opened this issue · 2 comments

People who know their stuff much better than I do are counselling against the use of openssl for random number generation, and mt_rand is woefully inadequate. PHP 7 introduces two new functions for best-practice random number generation: random_bytes() and random_int(), and (just like when password_hash() was introduced) someone's kindly created a polyfill to provide these functions on existing versions of PHP: paragonie/random_compat

What about adding a composer requirement (or maybe a suggest) for paragonie/random_compat, then replacing the getRand() private method with a call to random_int(0,31) in https://github.com/ChristianRiesen/otp/blob/master/src/Otp/GoogleAuthenticator.php#L160?

Thanks for the suggestion. In this use case, it's really not that much of a problem :) It's a simple helper class, but not the definitive guide to everything. Additionally, it's a very small string this is used for, so not that much of a "non random" problem as with other things where you might use functions of this sort.

I do support best practices though and will add a check to see if random_int exists and use that instead as the default function, including the suggestion of the polyfill. With PHP 7 it should then work just out of the box, without adding any dependencies that isn't needed for those being up to date.

Sounds good to me 👍