chore: Expand SecureRandomTest unit test scenarios

New test coverage for:

- charset validation failures (duplicates, too short, non-ASCII).
- default charset (CHAR_BASE64_RFC4648).
- randomness/unique output.
- minimum-sized valid charsets and large printable ASCII charset.
- a caller-provided, valid (non-predefined) custom charset.

And adjusts the length ranges tests:

- Added some smaller / more frequently used ranges.
- Added an "oddball" range to possibly catch weird stuff.
- Dropped excessive 64K test which does not need to run within our CI during standard PR runs (if deemed desirable maybe add it to an occasional stress tester, but I don't think it's necessary for now)

Signed-off-by: Josh <josh.t.richards@gmail.com>

lib/private/Security/SecureRandom.php

@@ -2,44 +2,53 @@
 
 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
  */
+
 namespace OC\Security;
 
 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
-	 */
+
+	/** @inheritdoc */
 	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--;
 		}

lib/public/Security/ISecureRandom.php

@@ -9,18 +9,23 @@ declare(strict_types=1);
 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 <code>generate($length, $characters)</code>
 	 * @since 8.0.0
 	 */
 	public const CHAR_UPPER = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
@@ -46,22 +51,39 @@ interface ISecureRandom {
 	public const CHAR_ALPHANUMERIC = self::CHAR_UPPER . self::CHAR_LOWER . self::CHAR_DIGITS;
 
 	/**
-	 * Characters that can be used for <code>generate($length, $characters)</code>, 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 8.0.0
 	 */
-	public function generate(int $length,
-		string $characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'): string;
+	public function generate(
+		int $length,
+		string $characters = ISecureRandom::CHAR_BASE64_RFC4648,
+	): string;
 }

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],
 		];
 	}
 
@@ -81,4 +81,56 @@ class SecureRandomTest extends \Test\TestCase {
 		$generator = $this->rng;
 		$generator->generate($length);
 	}
+
+	public static function invalidCharProviders(): array {
+		return [
+			'invalid_too_short' => ['abc'],
+			'invalid_duplicates' => ['aabcd'],
+			'invalid_non_ascii' => ["abcd\xf0"],
+		];
+	}
+
+	/**
+	 * @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\+\/]+$/', $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]+$/', $randomString);
+		$this->assertEquals(500, strlen($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('/^[@#$!]+$/', $randomString);
+	}
 }