diff --git a/lib/private/Security/SecureRandom.php b/lib/private/Security/SecureRandom.php index efaa74a9e1a4b..905616b6826fd 100644 --- a/lib/private/Security/SecureRandom.php +++ b/lib/private/Security/SecureRandom.php @@ -2,7 +2,7 @@ declare(strict_types=1); /** - * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-FileCopyrightText: 2016-2025 Nextcloud GmbH and Nextcloud contributors * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ @@ -12,36 +12,42 @@ use OCP\Security\ISecureRandom; /** - * Class SecureRandom provides a wrapper around the random_int function to generate - * secure random strings. For PHP 7 the native CSPRNG is used, older versions do - * use a fallback. + * Secure random string generator recommended for tokens, passwords, secrets, and similar security use cases. * - * Usage: - * \OC::$server->get(ISecureRandom::class)->generate(10); - * @package OC\Security + * @see \OCP\Security\ISecureRandom */ class SecureRandom implements ISecureRandom { - /** - * Generate a secure random string of specified length. - * @param int $length The length of the generated string - * @param string $characters An optional list of characters to use if no character list is - * specified all valid base64 characters are used. - * @throws \LengthException if an invalid length is requested - */ #[\Override] public function generate( int $length, - string $characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/', + string $characters = ISecureRandom::CHAR_BASE64_RFC4648, ): string { + if ($length <= 0) { - throw new \LengthException('Invalid length specified: ' . $length . ' must be bigger than 0'); + throw new \LengthException( + 'Invalid length specified: ' . $length . ' must be greater than 0' + ); + } + + if ( + // Check for ASCII-only (no multibyte characters) + !mb_check_encoding($characters, 'ASCII') + // Check for uniqueness: number of unique bytes must equal original length + || strlen(count_chars($characters, 3)) !== strlen($characters) + // Check minimum length + || strlen($characters) < 4 + ) { + throw new \InvalidArgumentException( + 'Character set must be ASCII-only, unique, and at least four characters long.' + ); } + // Build string by selecting random characters from $characters and appending $maxCharIndex = \strlen($characters) - 1; $randomString = ''; - while ($length > 0) { $randomNumber = \random_int(0, $maxCharIndex); + // Safe: $characters is guaranteed ASCII; indexed access is byte-correct. $randomString .= $characters[$randomNumber]; $length--; } diff --git a/lib/public/Security/ISecureRandom.php b/lib/public/Security/ISecureRandom.php index 5d47bdc7a1bf8..57ed59c1f5b30 100644 --- a/lib/public/Security/ISecureRandom.php +++ b/lib/public/Security/ISecureRandom.php @@ -10,18 +10,23 @@ namespace OCP\Security; /** - * Class SecureRandom provides a wrapper around the random_int function to generate - * secure random strings. For PHP 7 the native CSPRNG is used, older versions do - * use a fallback. + * Secure random string generator for tokens, passwords, secrets, and similar security use cases. * - * Usage: - * \OCP\Server::get(ISecureRandom::class)->generate(10); + * A wrapper around PHP's random_int(), utilizing the native CSPRNG. + * @link https://www.php.net/manual/en/function.random-int.php * + * By default, uses the RFC 4648 Base64 alphabet for random string generation, and allows + * custom character sets if desired. + * + * Example usage: + * - Typical (if ISecureRandom $random is provided by DI): + * `$secret = $this->random->generate(48);` + * - Non-DI: + * `$secret = \OCP\Server::get(\OCP\Security\ISecureRandom::class)->generate(48);` * @since 8.0.0 */ interface ISecureRandom { /** - * Flags for characters that can be used for generate($length, $characters) * @since 8.0.0 */ public const CHAR_UPPER = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; @@ -47,22 +52,40 @@ interface ISecureRandom { public const CHAR_ALPHANUMERIC = self::CHAR_UPPER . self::CHAR_LOWER . self::CHAR_DIGITS; /** - * Characters that can be used for generate($length, $characters), to - * generate human-readable random strings. Lower- and upper-case characters and digits - * are included. Characters which are ambiguous are excluded, such as I, l, and 1 and so on. - * + * Lowercase, uppercase characters, and digits. Ambiguous characters are excluded (e.g., I, l, and 1). * @since 23.0.0 */ public const CHAR_HUMAN_READABLE = 'abcdefgijkmnopqrstwxyzABCDEFGHJKLMNPQRSTWXYZ23456789'; /** - * Generate a random string of specified length. - * @param int $length The length of the generated string - * @param string $characters An optional list of characters to use if no character list is - * specified all valid base64 characters are used. - * @return string + * Standard Base64 alphabet per RFC4648. + * @link https://datatracker.ietf.org/doc/html/rfc4648#section-4 + * @since 33.0.0 + */ + public const CHAR_BASE64_RFC4648 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'; + + /** + * Generate a secure random string of the specified length. + * + * Security notes: + * - For most secure applications (tokens, passwords, CSRF values), an ample and diverse + * character set, such as the default CHAR_BASE64_RFC4648, is typically a good choice. + * - Overly small (<4), non-unique, or multibyte character sets weaken security and are not permitted. + * + * @param int $length Number of characters (must be > 0). + * @param string $characters Optional list of unique, single-byte (ASCII) characters + * to use. Defaults to the CHAR_BASE64_RFC4648 alphabet. A custom set should contain at least 4 + * characters, and must not contain duplicates or multibyte (non-ASCII) characters. It is strongly + * recommended to use predefined constants from ISecureRandom, which all meet the requirements. + * @return string The randomly generated string. + * @throws \LengthException If $length <= 0. + * @throws \InvalidArgumentException if $characters contains non-ASCII characters, duplicates, + * or fewer than 4 unique characters. + * @since 35.0.0 $characters has to be >4 chars long, non-ASCII characters are rejected, not contain duplicates. * @since 8.0.0 */ - public function generate(int $length, - string $characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'): string; + public function generate( + int $length, + string $characters = ISecureRandom::CHAR_BASE64_RFC4648, + ): string; } diff --git a/tests/lib/Security/SecureRandomTest.php b/tests/lib/Security/SecureRandomTest.php index 13804a55f6c41..ef2d4b4d1bbb7 100644 --- a/tests/lib/Security/SecureRandomTest.php +++ b/tests/lib/Security/SecureRandomTest.php @@ -16,11 +16,11 @@ class SecureRandomTest extends \Test\TestCase { public static function stringGenerationProvider(): array { return [ [1, 1], + [16, 16], + [31, 31], + [64, 64], [128, 128], - [256, 256], [1024, 1024], - [2048, 2048], - [64000, 64000], ]; } @@ -82,4 +82,53 @@ public function testInvalidLengths($length): void { $generator = $this->rng; $generator->generate($length); } + + public static function invalidCharProviders(): array { + return [ + 'invalid_too_short' => ['abc'], + 'invalid_duplicates' => ['aabcd'], + 'invalid_non_ascii' => ["abcd\xf0"], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('invalidCharProviders')] + public function testInvalidCharacterSets(string $invalidCharset): void { + $this->expectException(\InvalidArgumentException::class); + $this->rng->generate(10, $invalidCharset); + } + + public function testDefaultCharsetBase64Characters(): void { + $randomString = $this->rng->generate(100); + $this->assertMatchesRegularExpression('/^[A-Za-z0-9\+\/]{100}$/', $randomString); + } + + public function testAllOutputsAreUnique(): void { + // While collisions are technically possible, extremely unlikely for these sizes + $first = $this->rng->generate(1000); + $second = $this->rng->generate(1000); + $this->assertNotEquals($first, $second, "Random output should not be repeated."); + } + + public function testMinimumValidCharset(): void { + $charset = 'abcd'; + $randomString = $this->rng->generate(500, $charset); + $this->assertMatchesRegularExpression('/^[abcd]{500}$/', $randomString); + } + + public function testLargeCustomCharset(): void { + $charset = ''; + for ($i = 32; $i <= 126; $i++) { // all printable ASCII + $charset .= chr($i); + } + $randomString = $this->rng->generate(200, $charset); + foreach (str_split($randomString) as $char) { + $this->assertStringContainsString($char, $charset); + } + } + + public function testUserProvidedValidCharset(): void { + $charset = '@#$!'; + $randomString = $this->rng->generate(64, $charset); + $this->assertMatchesRegularExpression('/^[@#$!]{64}$/', $randomString); + } }