remove insecure rng providers

and remove the openssl provider. We now rely exclusively on
random_bytes(), as there are no reasons not to. Fix #121
This commit is contained in:
Nicolas CARPi
2024-04-15 23:28:45 +02:00
parent ab93dd41ce
commit d1792f07d1
12 changed files with 7 additions and 207 deletions
-1
View File
@@ -19,7 +19,6 @@ You can make use of the included [Endroid](https://robthree.github.io/TwoFactorA
* Requires PHP version >=8.1
* [cURL](http://php.net/manual/en/book.curl.php) when using the provided `QRServerProvider` (default), `ImageChartsQRCodeProvider` or `QRicketProvider` but you can also provide your own QR-code provider.
* [random_bytes()](http://php.net/manual/en/function.random-bytes.php), [OpenSSL](http://php.net/manual/en/book.openssl.php) or [Hash](http://php.net/manual/en/book.hash.php) depending on which built-in RNG you use (TwoFactorAuth will try to 'autodetect' and use the best available); however: feel free to provide your own (CS)RNG.
Optionally, you may need:
+1 -9
View File
@@ -21,15 +21,7 @@ Argument | Default value | Use
### RNG providers
This library also comes with some [Random Number Generator (RNG)](https://en.wikipedia.org/wiki/Random_number_generation) providers. The RNG provider generates a number of random bytes and returns these bytes as a string. These values are then used to create the secret. By default (no RNG provider specified) TwoFactorAuth will try to determine the best available RNG provider to use in this order.
1. [CSRNGProvider](https://github.com/RobThree/TwoFactorAuth/blob/master/lib/Providers/Rng/CSRNGProvider.php) for PHP7+
2. [OpenSSLRNGProvider](https://github.com/RobThree/TwoFactorAuth/blob/master/lib/Providers/Rng/OpenSSLRNGProvider.php) where openssl is available
3. [HashRNGProvider](https://github.com/RobThree/TwoFactorAuth/blob/master/lib/Providers/Rng/HashRNGProvider.php) **non-cryptographically secure** fallback
Each of these RNG providers have some constructor arguments that allow you to tweak some of the settings to use when creating the random bytes.
You can also implement your own by implementing the [`IRNGProvider` interface](https://github.com/RobThree/TwoFactorAuth/blob/master/lib/Providers/Rng/IRNGProvider.php).
Should you feel the need to use a CSPRNG different than `random_bytes()`, you can use the `rngprovider` argument of the constructor to provide an object implementing the [`IRNGProvider`](https://github.com/RobThree/TwoFactorAuth/blob/master/lib/Providers/Rng/IRNGProvider.php) interface.
### Time providers
-8
View File
@@ -13,12 +13,4 @@ class CSRNGProvider implements IRNGProvider
{
return random_bytes($bytecount); // PHP7+
}
/**
* {@inheritdoc}
*/
public function isCryptographicallySecure(): bool
{
return true;
}
}
-40
View File
@@ -1,40 +0,0 @@
<?php
declare(strict_types=1);
namespace RobThree\Auth\Providers\Rng;
use function in_array;
class HashRNGProvider implements IRNGProvider
{
public function __construct(private readonly string $algorithm = 'sha256')
{
$algos = array_values(hash_algos());
if (!in_array($this->algorithm, $algos, true)) {
throw new RNGException('Unsupported algorithm specified');
}
}
/**
* {@inheritdoc}
*/
public function getRandomBytes(int $bytecount): string
{
$result = '';
$hash = mt_rand();
for ($i = 0; $i < $bytecount; $i++) {
$hash = hash($this->algorithm, $hash . mt_rand(), true);
$result .= $hash[mt_rand(0, strlen($hash) - 1)];
}
return $result;
}
/**
* {@inheritdoc}
*/
public function isCryptographicallySecure(): bool
{
return false;
}
}
-2
View File
@@ -7,6 +7,4 @@ namespace RobThree\Auth\Providers\Rng;
interface IRNGProvider
{
public function getRandomBytes(int $bytecount): string;
public function isCryptographicallySecure(): bool;
}
-29
View File
@@ -1,29 +0,0 @@
<?php
declare(strict_types=1);
namespace RobThree\Auth\Providers\Rng;
class OpenSSLRNGProvider implements IRNGProvider
{
public function __construct(private readonly bool $requirestrong = true)
{
}
/**
* {@inheritdoc}
*/
public function getRandomBytes(int $bytecount): string
{
// will throw an Exception on failure
return openssl_random_pseudo_bytes($bytecount, $crypto_strong);
}
/**
* {@inheritdoc}
*/
public function isCryptographicallySecure(): bool
{
return $this->requirestrong;
}
}
+2 -19
View File
@@ -7,9 +7,7 @@ namespace RobThree\Auth;
use RobThree\Auth\Providers\Qr\IQRCodeProvider;
use RobThree\Auth\Providers\Qr\QRServerProvider;
use RobThree\Auth\Providers\Rng\CSRNGProvider;
use RobThree\Auth\Providers\Rng\HashRNGProvider;
use RobThree\Auth\Providers\Rng\IRNGProvider;
use RobThree\Auth\Providers\Rng\OpenSSLRNGProvider;
use RobThree\Auth\Providers\Time\HttpTimeProvider;
use RobThree\Auth\Providers\Time\ITimeProvider;
use RobThree\Auth\Providers\Time\LocalMachineTimeProvider;
@@ -51,14 +49,11 @@ class TwoFactorAuth
/**
* Create a new secret
*/
public function createSecret(int $bits = 80, bool $requirecryptosecure = true): string
public function createSecret(int $bits = 80): string
{
$secret = '';
$bytes = (int)ceil($bits / 5); // We use 5 bits of each byte (since we have a 32-character 'alphabet' / BASE32)
$rngprovider = $this->getRngProvider();
if ($requirecryptosecure && !$rngprovider->isCryptographicallySecure()) {
throw new TwoFactorAuthException('RNG provider is not cryptographically secure');
}
$rnd = $rngprovider->getRandomBytes($bytes);
for ($i = 0; $i < $bytes; $i++) {
$secret .= self::$_base32[ord($rnd[$i]) & 31]; //Mask out left 3 bits for 0-31 values
@@ -174,19 +169,7 @@ class TwoFactorAuth
*/
public function getRngProvider(): IRNGProvider
{
if ($this->rngprovider !== null) {
return $this->rngprovider;
}
if (function_exists('random_bytes')) {
return $this->rngprovider = new CSRNGProvider();
}
if (function_exists('openssl_random_pseudo_bytes')) {
return $this->rngprovider = new OpenSSLRNGProvider();
}
if (function_exists('hash')) {
return $this->rngprovider = new HashRNGProvider();
}
throw new TwoFactorAuthException('Unable to find a suited RNGProvider');
return $this->rngprovider ?? new CSRNGProvider();
}
public function getTimeProvider(): ITimeProvider
@@ -21,7 +21,6 @@ class CSRNGProviderTest extends TestCase
foreach ($this->rngTestLengths as $l) {
$this->assertSame($l, strlen($rng->getRandomBytes($l)));
}
$this->assertTrue($rng->isCryptographicallySecure());
} else {
$this->expectNotToPerformAssertions();
}
@@ -1,26 +0,0 @@
<?php
declare(strict_types=1);
namespace Tests\Providers\Rng;
use PHPUnit\Framework\TestCase;
use RobThree\Auth\Providers\Rng\HashRNGProvider;
class HashRNGProviderTest extends TestCase
{
use NeedsRngLengths;
/**
* @return void
*/
public function testHashRNGProvidersReturnExpectedNumberOfBytes()
{
$rng = new HashRNGProvider();
foreach ($this->rngTestLengths as $l) {
$this->assertSame($l, strlen($rng->getRandomBytes($l)));
}
$this->assertFalse($rng->isCryptographicallySecure());
}
}
+4 -25
View File
@@ -7,39 +7,18 @@ namespace Tests\Providers\Rng;
use PHPUnit\Framework\TestCase;
use RobThree\Auth\Algorithm;
use RobThree\Auth\TwoFactorAuth;
use RobThree\Auth\TwoFactorAuthException;
class IRNGProviderTest extends TestCase
{
public function testCreateSecretThrowsOnInsecureRNGProvider(): void
public function testCreateSecret(): void
{
$rng = new TestRNGProvider();
$tfa = new TwoFactorAuth('Test', 6, 30, Algorithm::Sha1, null, $rng);
$this->expectException(TwoFactorAuthException::class);
$tfa->createSecret();
}
public function testCreateSecretOverrideSecureDoesNotThrowOnInsecureRNG(): void
{
$rng = new TestRNGProvider();
$tfa = new TwoFactorAuth('Test', 6, 30, Algorithm::Sha1, null, $rng);
$this->assertSame('ABCDEFGHIJKLMNOP', $tfa->createSecret(80, false));
}
public function testCreateSecretDoesNotThrowOnSecureRNGProvider(): void
{
$rng = new TestRNGProvider(true);
$tfa = new TwoFactorAuth('Test', 6, 30, Algorithm::Sha1, null, $rng);
$this->assertSame('ABCDEFGHIJKLMNOP', $tfa->createSecret());
$tfa = new TwoFactorAuth('Test', 6, 30, Algorithm::Sha1, null, null);
$this->assertIsString($tfa->createSecret());
}
public function testCreateSecretGeneratesDesiredAmountOfEntropy(): void
{
$rng = new TestRNGProvider(true);
$rng = new TestRNGProvider();
$tfa = new TwoFactorAuth('Test', 6, 30, Algorithm::Sha1, null, $rng);
$this->assertSame('A', $tfa->createSecret(5));
@@ -1,39 +0,0 @@
<?php
declare(strict_types=1);
namespace Tests\Providers\Rng;
use PHPUnit\Framework\TestCase;
use RobThree\Auth\Providers\Rng\OpenSSLRNGProvider;
class OpenSSLRNGProviderTest extends TestCase
{
use NeedsRngLengths;
/**
* @return void
*/
public function testStrongOpenSSLRNGProvidersReturnExpectedNumberOfBytes()
{
$rng = new OpenSSLRNGProvider(true);
foreach ($this->rngTestLengths as $l) {
$this->assertSame($l, strlen($rng->getRandomBytes($l)));
}
$this->assertTrue($rng->isCryptographicallySecure());
}
/**
* @return void
*/
public function testNonStrongOpenSSLRNGProvidersReturnExpectedNumberOfBytes()
{
$rng = new OpenSSLRNGProvider(false);
foreach ($this->rngTestLengths as $l) {
$this->assertSame($l, strlen($rng->getRandomBytes($l)));
}
$this->assertFalse($rng->isCryptographicallySecure());
}
}
-8
View File
@@ -25,12 +25,4 @@ class TestRNGProvider implements IRNGProvider
return $result;
}
/**
* {@inheritdoc}
*/
public function isCryptographicallySecure(): bool
{
return $this->isSecure;
}
}