remove insecure rng providers and remove polyfill for hash_equals (#122)

* 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

* remove the isSecure property of the test rng class

* remove pointless test rng class

we were testing a test class, which didn't make a lot of sense.

* Revert "remove pointless test rng class"

This reverts commit f6da6be.

* Reapply "remove pointless test rng class"

This reverts commit 06220d4.

* assing rng provider to class attribute

this also aligns with other providers

* remove polyfill for hash_equals

README.md

@@ -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:
 

docs/optional-configuration.md

@@ -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
 

lib/Providers/Rng/CSRNGProvider.php

@@ -13,12 +13,4 @@ public function getRandomBytes(int $bytecount): string
     {
         return random_bytes($bytecount);    // PHP7+
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function isCryptographicallySecure(): bool
-    {
-        return true;
-    }
 }

lib/Providers/Rng/IRNGProvider.php

@@ -7,6 +7,4 @@
 interface IRNGProvider
 {
     public function getRandomBytes(int $bytecount): string;
-
-    public function isCryptographicallySecure(): bool;
 }

lib/TwoFactorAuth.php

@@ -4,12 +4,12 @@
 
 namespace RobThree\Auth;
 
+use function hash_equals;
+
 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 +51,11 @@ public function __construct(
     /**
      * 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
@@ -98,7 +95,7 @@ public function verifyCode(string $secret, string $code, int $discrepancy = 1, ?
         for ($i = -$discrepancy; $i <= $discrepancy; $i++) {
             $ts = $timestamp + ($i * $this->period);
             $slice = $this->getTimeSlice($ts);
-            $timeslice = $this->codeEquals($this->getCode($secret, $ts), $code) ? $slice : $timeslice;
+            $timeslice = hash_equals($this->getCode($secret, $ts), $code) ? $slice : $timeslice;
         }
 
         return $timeslice > 0;
@@ -174,19 +171,7 @@ public function getQrCodeProvider(): IQRCodeProvider
      */
     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
@@ -195,27 +180,6 @@ public function getTimeProvider(): ITimeProvider
         return $this->timeprovider ??= new LocalMachineTimeProvider();
     }
 
-    /**
-     * Timing-attack safe comparison of 2 codes (see http://blog.ircmaxell.com/2014/11/its-all-about-time.html)
-     */
-    private function codeEquals(string $safe, string $user): bool
-    {
-        if (function_exists('hash_equals')) {
-            return hash_equals($safe, $user);
-        }
-        // In general, it's not possible to prevent length leaks. So it's OK to leak the length. The important part is that
-        // we don't leak information about the difference of the two strings.
-        if (strlen($safe) === strlen($user)) {
-            $result = 0;
-            $strlen = strlen($safe);
-            for ($i = 0; $i < $strlen; $i++) {
-                $result |= (ord($safe[$i]) ^ ord($user[$i]));
-            }
-            return $result === 0;
-        }
-        return false;
-    }
-
     private function getTime(?int $time = null): int
     {
         return $time ?? $this->getTimeProvider()->getTime();

tests/Providers/Rng/CSRNGProviderTest.php

@@ -11,19 +11,11 @@ class CSRNGProviderTest extends TestCase
 {
     use NeedsRngLengths;
 
-    /**
-     * @requires function random_bytes
-     */
     public function testCSRNGProvidersReturnExpectedNumberOfBytes(): void
     {
-        if (function_exists('random_bytes')) {
-            $rng = new CSRNGProvider();
-            foreach ($this->rngTestLengths as $l) {
-                $this->assertSame($l, strlen($rng->getRandomBytes($l)));
-            }
-            $this->assertTrue($rng->isCryptographicallySecure());
-        } else {
-            $this->expectNotToPerformAssertions();
+        $rng = new CSRNGProvider();
+        foreach ($this->rngTestLengths as $l) {
+            $this->assertSame($l, strlen($rng->getRandomBytes($l)));
         }
     }
 }

tests/Providers/Rng/IRNGProviderTest.php

@@ -7,46 +7,12 @@
 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());
-    }
-
-    public function testCreateSecretGeneratesDesiredAmountOfEntropy(): void
-    {
-        $rng = new TestRNGProvider(true);
-
-        $tfa = new TwoFactorAuth('Test', 6, 30, Algorithm::Sha1, null, $rng);
-        $this->assertSame('A', $tfa->createSecret(5));
-        $this->assertSame('AB', $tfa->createSecret(6));
-        $this->assertSame('ABCDEFGHIJKLMNOPQRSTUVWXYZ', $tfa->createSecret(128));
-        $this->assertSame('ABCDEFGHIJKLMNOPQRSTUVWXYZ234567', $tfa->createSecret(160));
-        $this->assertSame('ABCDEFGHIJKLMNOPQRSTUVWXYZ234567ABCDEFGHIJKLMNOPQRSTUVWXYZ234567', $tfa->createSecret(320));
-        $this->assertSame('ABCDEFGHIJKLMNOPQRSTUVWXYZ234567ABCDEFGHIJKLMNOPQRSTUVWXYZ234567A', $tfa->createSecret(321));
+        $tfa = new TwoFactorAuth('Test', 6, 30, Algorithm::Sha1, null, null);
+        $this->assertIsString($tfa->createSecret());
     }
 }