diff --git a/README.md b/README.md index 96f566d..0731499 100644 --- a/README.md +++ b/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: diff --git a/docs/optional-configuration.md b/docs/optional-configuration.md index d3e2556..e10a279 100644 --- a/docs/optional-configuration.md +++ b/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 diff --git a/lib/Providers/Rng/CSRNGProvider.php b/lib/Providers/Rng/CSRNGProvider.php index 97351f3..f078575 100644 --- a/lib/Providers/Rng/CSRNGProvider.php +++ b/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; - } } diff --git a/lib/Providers/Rng/HashRNGProvider.php b/lib/Providers/Rng/HashRNGProvider.php deleted file mode 100644 index 20241da..0000000 --- a/lib/Providers/Rng/HashRNGProvider.php +++ /dev/null @@ -1,40 +0,0 @@ -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; - } -} diff --git a/lib/Providers/Rng/IRNGProvider.php b/lib/Providers/Rng/IRNGProvider.php index ed18453..08b7fa3 100644 --- a/lib/Providers/Rng/IRNGProvider.php +++ b/lib/Providers/Rng/IRNGProvider.php @@ -7,6 +7,4 @@ interface IRNGProvider { public function getRandomBytes(int $bytecount): string; - - public function isCryptographicallySecure(): bool; } diff --git a/lib/Providers/Rng/OpenSSLRNGProvider.php b/lib/Providers/Rng/OpenSSLRNGProvider.php deleted file mode 100644 index 4063847..0000000 --- a/lib/Providers/Rng/OpenSSLRNGProvider.php +++ /dev/null @@ -1,29 +0,0 @@ -requirestrong; - } -} diff --git a/lib/TwoFactorAuth.php b/lib/TwoFactorAuth.php index f3f867e..0133448 100644 --- a/lib/TwoFactorAuth.php +++ b/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(); diff --git a/tests/Providers/Rng/CSRNGProviderTest.php b/tests/Providers/Rng/CSRNGProviderTest.php index 64f66e8..39739e9 100644 --- a/tests/Providers/Rng/CSRNGProviderTest.php +++ b/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))); } } } diff --git a/tests/Providers/Rng/HashRNGProviderTest.php b/tests/Providers/Rng/HashRNGProviderTest.php deleted file mode 100644 index 4a71930..0000000 --- a/tests/Providers/Rng/HashRNGProviderTest.php +++ /dev/null @@ -1,26 +0,0 @@ -rngTestLengths as $l) { - $this->assertSame($l, strlen($rng->getRandomBytes($l))); - } - - $this->assertFalse($rng->isCryptographicallySecure()); - } -} diff --git a/tests/Providers/Rng/IRNGProviderTest.php b/tests/Providers/Rng/IRNGProviderTest.php index 7ff40c8..fd2c742 100644 --- a/tests/Providers/Rng/IRNGProviderTest.php +++ b/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()); } } diff --git a/tests/Providers/Rng/OpenSSLRNGProviderTest.php b/tests/Providers/Rng/OpenSSLRNGProviderTest.php deleted file mode 100644 index 368c7cc..0000000 --- a/tests/Providers/Rng/OpenSSLRNGProviderTest.php +++ /dev/null @@ -1,39 +0,0 @@ -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()); - } -} diff --git a/tests/Providers/Rng/TestRNGProvider.php b/tests/Providers/Rng/TestRNGProvider.php deleted file mode 100644 index c166c66..0000000 --- a/tests/Providers/Rng/TestRNGProvider.php +++ /dev/null @@ -1,36 +0,0 @@ -isSecure; - } -}