diff options
| -rw-r--r-- | libraries/classes/Config/ServerConfigChecks.php | 71 | ||||
| -rw-r--r-- | libraries/classes/Setup/ConfigGenerator.php | 26 | ||||
| -rw-r--r-- | phpstan-baseline.neon | 5 | ||||
| -rw-r--r-- | psalm-baseline.xml | 29 | ||||
| -rw-r--r-- | test/classes/Config/ServerConfigChecksTest.php | 74 | ||||
| -rw-r--r-- | test/classes/Setup/ConfigGeneratorTest.php | 30 |
6 files changed, 153 insertions, 82 deletions
diff --git a/libraries/classes/Config/ServerConfigChecks.php b/libraries/classes/Config/ServerConfigChecks.php index 4a8283c742..3420091e94 100644 --- a/libraries/classes/Config/ServerConfigChecks.php +++ b/libraries/classes/Config/ServerConfigChecks.php @@ -11,17 +11,17 @@ use PhpMyAdmin\Core; use PhpMyAdmin\Sanitize; use PhpMyAdmin\Setup\Index as SetupIndex; use PhpMyAdmin\Url; -use PhpMyAdmin\Util; use function __; -use function count; use function function_exists; use function htmlspecialchars; -use function implode; use function ini_get; -use function preg_match; +use function is_string; +use function mb_strlen; +use function sodium_crypto_secretbox_keygen; use function sprintf; -use function strlen; + +use const SODIUM_CRYPTO_SECRETBOX_KEYBYTES; /** * Performs various compatibility, security and consistency checks on current config @@ -247,9 +247,12 @@ class ServerConfigChecks $cookieAuthServer, $blowfishSecretSet ): array { - if ($cookieAuthServer && $blowfishSecret === null) { + if ( + $cookieAuthServer + && (! is_string($blowfishSecret) || mb_strlen($blowfishSecret, '8bit') !== SODIUM_CRYPTO_SECRETBOX_KEYBYTES) + ) { $blowfishSecretSet = true; - $this->cfg->set('blowfish_secret', Util::generateRandom(32)); + $this->cfg->set('blowfish_secret', sodium_crypto_secretbox_keygen()); } return [ @@ -345,55 +348,21 @@ class ServerConfigChecks ): void { // $cfg['blowfish_secret'] // it's required for 'cookie' authentication - if (! $cookieAuthUsed) { - return; - } - - if ($blowfishSecretSet) { - // 'cookie' auth used, blowfish_secret was generated - SetupIndex::messagesSet( - 'notice', - 'blowfish_secret_created', - Descriptions::get('blowfish_secret'), - Sanitize::sanitizeMessage(__( - 'You didn\'t have blowfish secret set and have enabled ' - . '[kbd]cookie[/kbd] authentication, so a key was automatically ' - . 'generated for you. It is used to encrypt cookies; you don\'t need to ' - . 'remember it.' - )) - ); - - return; - } - - $blowfishWarnings = []; - // check length - if (strlen($blowfishSecret) < 32) { - // too short key - $blowfishWarnings[] = __('Key is too short, it should have at least 32 characters.'); - } - - // check used characters - $hasDigits = (bool) preg_match('/\d/', $blowfishSecret); - $hasChars = (bool) preg_match('/\S/', $blowfishSecret); - $hasNonword = (bool) preg_match('/\W/', $blowfishSecret); - if (! $hasDigits || ! $hasChars || ! $hasNonword) { - $blowfishWarnings[] = Sanitize::sanitizeMessage( - __( - 'Key should contain letters, numbers [em]and[/em] special characters.' - ) - ); - } - - if (empty($blowfishWarnings)) { + if (! $cookieAuthUsed || ! $blowfishSecretSet) { return; } + // 'cookie' auth used, blowfish_secret was generated SetupIndex::messagesSet( - 'error', - 'blowfish_warnings' . count($blowfishWarnings), + 'notice', + 'blowfish_secret_created', Descriptions::get('blowfish_secret'), - implode('<br>', $blowfishWarnings) + Sanitize::sanitizeMessage(__( + 'You didn\'t have blowfish secret set and have enabled ' + . '[kbd]cookie[/kbd] authentication, so a key was automatically ' + . 'generated for you. It is used to encrypt cookies; you don\'t need to ' + . 'remember it.' + )) ); } diff --git a/libraries/classes/Setup/ConfigGenerator.php b/libraries/classes/Setup/ConfigGenerator.php index d252636efa..af5d94a637 100644 --- a/libraries/classes/Setup/ConfigGenerator.php +++ b/libraries/classes/Setup/ConfigGenerator.php @@ -15,12 +15,18 @@ use function count; use function gmdate; use function implode; use function is_array; +use function is_string; +use function mb_strlen; use function preg_replace; +use function sodium_bin2hex; +use function sodium_crypto_secretbox_keygen; +use function sprintf; use function str_contains; use function strtr; use function var_export; use const DATE_RFC1123; +use const SODIUM_CRYPTO_SECRETBOX_KEYBYTES; /** * Config file generation class @@ -94,6 +100,12 @@ class ConfigGenerator */ private static function getVarExport($var_name, $var_value, $crlf) { + if ($var_name === 'blowfish_secret') { + $secret = self::getBlowfishSecretKey($var_value); + + return sprintf('$cfg[\'blowfish_secret\'] = \sodium_hex2bin(\'%s\');%s', sodium_bin2hex($secret), $crlf); + } + if (! is_array($var_value) || empty($var_value)) { return "\$cfg['" . $var_name . "'] = " . var_export($var_value, true) . ';' . $crlf; @@ -199,4 +211,18 @@ class ConfigGenerator return $ret; } + + /** + * @param mixed $key + * + * @psalm-return non-empty-string + */ + private static function getBlowfishSecretKey($key): string + { + if (is_string($key) && mb_strlen($key, '8bit') === SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { + return $key; + } + + return sodium_crypto_secretbox_keygen(); + } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 1c03b2ea93..c3fd561717 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -7791,6 +7791,11 @@ parameters: path: libraries/classes/Setup/ConfigGenerator.php - + message: "#^Method PhpMyAdmin\\\\Setup\\\\ConfigGenerator\\:\\:getBlowfishSecretKey\\(\\) should return non\\-empty\\-string but returns string\\.$#" + count: 2 + path: libraries/classes/Setup/ConfigGenerator.php + + - message: "#^Method PhpMyAdmin\\\\Setup\\\\ConfigGenerator\\:\\:getServerPart\\(\\) has parameter \\$servers with no value type specified in iterable type array\\.$#" count: 1 path: libraries/classes/Setup/ConfigGenerator.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index fdfa268435..be2ba27d96 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -12573,6 +12573,10 @@ </UnusedFunctionCall> </file> <file src="libraries/classes/Setup/ConfigGenerator.php"> + <LessSpecificReturnStatement occurrences="2"> + <code>$key</code> + <code>sodium_crypto_secretbox_keygen()</code> + </LessSpecificReturnStatement> <MixedArgument occurrences="1"> <code>$conf['Servers']</code> </MixedArgument> @@ -12590,6 +12594,9 @@ <code>$v</code> <code>$v</code> </MixedAssignment> + <MoreSpecificReturnType occurrences="1"> + <code>non-empty-string</code> + </MoreSpecificReturnType> <PossiblyNullOperand occurrences="1"> <code>self::getServerPart($cf, $crlf, $conf['Servers'])</code> </PossiblyNullOperand> @@ -14280,40 +14287,24 @@ </MixedArrayAccess> </file> <file src="test/classes/Config/ServerConfigChecksTest.php"> - <MixedArgument occurrences="5"> - <code>$_SESSION['messages']</code> - <code>$_SESSION['messages']['error']</code> + <MixedArgument occurrences="2"> <code>$_SESSION['messages']['error']</code> <code>$_SESSION['messages']['notice']</code> - <code>$_SESSION['messages']['notice']</code> </MixedArgument> - <MixedArrayAccess occurrences="4"> - <code>$_SESSION['messages']['error']</code> + <MixedArrayAccess occurrences="2"> <code>$_SESSION['messages']['error']</code> <code>$_SESSION['messages']['notice']</code> - <code>$_SESSION['messages']['notice']</code> </MixedArrayAccess> - <MixedArrayAssignment occurrences="20"> - <code>$_SESSION[$this->sessionID]['AllowArbitraryServer']</code> + <MixedArrayAssignment occurrences="9"> <code>$_SESSION[$this->sessionID]['AllowArbitraryServer']</code> <code>$_SESSION[$this->sessionID]['BZipDump']</code> - <code>$_SESSION[$this->sessionID]['BZipDump']</code> - <code>$_SESSION[$this->sessionID]['GZipDump']</code> <code>$_SESSION[$this->sessionID]['GZipDump']</code> <code>$_SESSION[$this->sessionID]['LoginCookieStore']</code> - <code>$_SESSION[$this->sessionID]['LoginCookieStore']</code> <code>$_SESSION[$this->sessionID]['LoginCookieValidity']</code> - <code>$_SESSION[$this->sessionID]['LoginCookieValidity']</code> - <code>$_SESSION[$this->sessionID]['SaveDir']</code> <code>$_SESSION[$this->sessionID]['SaveDir']</code> <code>$_SESSION[$this->sessionID]['Servers']</code> - <code>$_SESSION[$this->sessionID]['Servers']</code> - <code>$_SESSION[$this->sessionID]['Servers']</code> - <code>$_SESSION[$this->sessionID]['TempDir']</code> <code>$_SESSION[$this->sessionID]['TempDir']</code> <code>$_SESSION[$this->sessionID]['ZipDump']</code> - <code>$_SESSION[$this->sessionID]['ZipDump']</code> - <code>$_SESSION[$this->sessionID]['blowfish_secret']</code> </MixedArrayAssignment> <MixedArrayOffset occurrences="1"> <code>$_SESSION[$this->sessionID]</code> diff --git a/test/classes/Config/ServerConfigChecksTest.php b/test/classes/Config/ServerConfigChecksTest.php index 9e64d14e60..2d26e221b5 100644 --- a/test/classes/Config/ServerConfigChecksTest.php +++ b/test/classes/Config/ServerConfigChecksTest.php @@ -11,6 +11,10 @@ use ReflectionException; use ReflectionProperty; use function array_keys; +use function mb_strlen; +use function str_repeat; + +use const SODIUM_CRYPTO_SECRETBOX_KEYBYTES; /** * @covers \PhpMyAdmin\Config\ServerConfigChecks @@ -100,8 +104,10 @@ class ServerConfigChecksTest extends AbstractTestCase ); } - public function testBlowfishCreate(): void + public function testBlowfish(): void { + $_SESSION[$this->sessionID] = []; + $_SESSION[$this->sessionID]['blowfish_secret'] = null; $_SESSION[$this->sessionID]['Servers'] = [ '1' => [ 'host' => 'localhost', @@ -110,7 +116,6 @@ class ServerConfigChecksTest extends AbstractTestCase 'AllowRoot' => false, ], ]; - $_SESSION[$this->sessionID]['AllowArbitraryServer'] = false; $_SESSION[$this->sessionID]['LoginCookieValidity'] = -1; $_SESSION[$this->sessionID]['LoginCookieStore'] = 0; @@ -123,28 +128,73 @@ class ServerConfigChecksTest extends AbstractTestCase $configChecker = new ServerConfigChecks($GLOBALS['ConfigFile']); $configChecker->performConfigChecks(); - $this->assertEquals( - ['blowfish_secret_created'], - array_keys($_SESSION['messages']['notice']) - ); - - $this->assertArrayNotHasKey('error', $_SESSION['messages']); + /** + * @var mixed $secret + * @psalm-suppress TypeDoesNotContainType + */ + $secret = $_SESSION[$this->sessionID]['blowfish_secret'] ?? ''; + $this->assertIsString($secret); + $this->assertSame(SODIUM_CRYPTO_SECRETBOX_KEYBYTES, mb_strlen($secret, '8bit')); + $messages = $_SESSION['messages'] ?? null; + $this->assertIsArray($messages); + $this->assertArrayHasKey('notice', $messages); + $this->assertIsArray($messages['notice']); + $this->assertArrayHasKey('blowfish_secret_created', $messages['notice']); + $this->assertArrayNotHasKey('error', $messages); } - public function testBlowfish(): void + public function testBlowfishWithInvalidSecret(): void { - $_SESSION[$this->sessionID]['blowfish_secret'] = 'sec'; - + $_SESSION[$this->sessionID] = []; + $_SESSION[$this->sessionID]['blowfish_secret'] = str_repeat('a', SODIUM_CRYPTO_SECRETBOX_KEYBYTES + 1); $_SESSION[$this->sessionID]['Servers'] = [ '1' => [ 'host' => 'localhost', + 'ssl' => true, 'auth_type' => 'cookie', + 'AllowRoot' => false, ], ]; $configChecker = new ServerConfigChecks($GLOBALS['ConfigFile']); $configChecker->performConfigChecks(); - $this->assertArrayHasKey('blowfish_warnings2', $_SESSION['messages']['error']); + /** + * @var mixed $secret + * @psalm-suppress TypeDoesNotContainType + */ + $secret = $_SESSION[$this->sessionID]['blowfish_secret'] ?? ''; + $this->assertIsString($secret); + $this->assertSame(SODIUM_CRYPTO_SECRETBOX_KEYBYTES, mb_strlen($secret, '8bit')); + $messages = $_SESSION['messages'] ?? null; + $this->assertIsArray($messages); + $this->assertArrayHasKey('notice', $messages); + $this->assertIsArray($messages['notice']); + $this->assertArrayHasKey('blowfish_secret_created', $messages['notice']); + $this->assertArrayNotHasKey('error', $messages); + } + + public function testBlowfishWithValidSecret(): void + { + $_SESSION[$this->sessionID] = []; + $_SESSION[$this->sessionID]['blowfish_secret'] = str_repeat('a', SODIUM_CRYPTO_SECRETBOX_KEYBYTES); + $_SESSION[$this->sessionID]['Servers'] = ['1' => ['host' => 'localhost', 'auth_type' => 'cookie']]; + + $configChecker = new ServerConfigChecks($GLOBALS['ConfigFile']); + $configChecker->performConfigChecks(); + + /** + * @var mixed $secret + * @psalm-suppress TypeDoesNotContainType + */ + $secret = $_SESSION[$this->sessionID]['blowfish_secret'] ?? ''; + $this->assertIsString($secret); + $this->assertSame(SODIUM_CRYPTO_SECRETBOX_KEYBYTES, mb_strlen($secret, '8bit')); + $messages = $_SESSION['messages'] ?? null; + $this->assertIsArray($messages); + $this->assertArrayHasKey('notice', $messages); + $this->assertIsArray($messages['notice']); + $this->assertArrayNotHasKey('blowfish_secret_created', $messages['notice']); + $this->assertArrayNotHasKey('error', $messages); } } diff --git a/test/classes/Setup/ConfigGeneratorTest.php b/test/classes/Setup/ConfigGeneratorTest.php index 7aaaa10ec0..985feb404d 100644 --- a/test/classes/Setup/ConfigGeneratorTest.php +++ b/test/classes/Setup/ConfigGeneratorTest.php @@ -10,6 +10,13 @@ use PhpMyAdmin\Tests\AbstractTestCase; use PhpMyAdmin\Version; use ReflectionClass; +use function explode; +use function hex2bin; +use function mb_strlen; +use function str_repeat; + +use const SODIUM_CRYPTO_SECRETBOX_KEYBYTES; + /** * @covers \PhpMyAdmin\Setup\ConfigGenerator */ @@ -115,6 +122,29 @@ class ConfigGeneratorTest extends AbstractTestCase ); } + public function testGetVarExportForBlowfishSecret(): void + { + $reflection = new ReflectionClass(ConfigGenerator::class); + $method = $reflection->getMethod('getVarExport'); + $method->setAccessible(true); + + $this->assertEquals( + '$cfg[\'blowfish_secret\'] = \sodium_hex2bin(\'' + . '6161616161616161616161616161616161616161616161616161616161616161\');' . "\n", + $method->invoke(null, 'blowfish_secret', str_repeat('a', SODIUM_CRYPTO_SECRETBOX_KEYBYTES), "\n") + ); + + /** @var string $actual */ + $actual = $method->invoke(null, 'blowfish_secret', 'invalid secret', "\n"); + $this->assertStringStartsWith('$cfg[\'blowfish_secret\'] = \sodium_hex2bin(\'', $actual); + $this->assertStringEndsWith('\');' . "\n", $actual); + $pieces = explode('\'', $actual); + $this->assertCount(5, $pieces); + $binaryString = hex2bin($pieces[3]); + $this->assertIsString($binaryString); + $this->assertSame(SODIUM_CRYPTO_SECRETBOX_KEYBYTES, mb_strlen($binaryString, '8bit')); + } + /** * Test for ConfigGenerator::isZeroBasedArray */ |