diff --git a/ChangeLog b/ChangeLog index 46def253e4..d4bc5e966e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -34,6 +34,7 @@ phpMyAdmin - ChangeLog - issue Fix PHP warning on GIS visualization when there is only one GIS column - issue #17728 Some select HTML tags will now have the correct UI style - issue #17734 PHP deprecations will only be shown when in a development environment +- issue #17369 Fix server error when blowfish_secret is not exactly 32 bytes long 5.2.0 (2022-05-10) - issue #16521 Upgrade Bootstrap to version 5 diff --git a/config.sample.inc.php b/config.sample.inc.php index 34b6a9dabf..2fca0b5bc4 100644 --- a/config.sample.inc.php +++ b/config.sample.inc.php @@ -10,8 +10,8 @@ declare(strict_types=1); /** - * This is needed for cookie based authentication to encrypt password in - * cookie. Needs to be 32 chars long. + * This is needed for cookie based authentication to encrypt the cookie. + * Needs to be a 32-bytes long string of random bytes. See FAQ 2.10. */ $cfg['blowfish_secret'] = ''; /* YOU MUST FILL IN THIS FOR COOKIE AUTH! */ diff --git a/doc/config.rst b/doc/config.rst index 5594823827..20366d140d 100644 --- a/doc/config.rst +++ b/doc/config.rst @@ -1888,6 +1888,8 @@ Generic settings A secret key used to encrypt/decrypt the URL query string. Should be 32 bytes long. + .. seealso:: :ref:`faq2_10` + Cookie authentication options ----------------------------- @@ -1896,13 +1898,33 @@ Cookie authentication options :type: string :default: ``''`` - The "cookie" auth\_type uses AES algorithm to encrypt the password. If you - are using the "cookie" auth\_type, enter here a random passphrase of your - choice. It will be used internally by the AES algorithm: you won’t be - prompted for this passphrase. + The "cookie" auth\_type uses the :term:`Sodium` extension to encrypt the cookies (see :term:`Cookie`). If you are + using the "cookie" auth\_type, enter here a generated string of random bytes to be used as an encryption key. It + will be used internally by the :term:`Sodium` extension: you won't be prompted for this encryption key. - The secret should be 32 characters long. Using shorter will lead to weaker security - of encrypted cookies, using longer will cause no harm. + Since a binary string is usually not printable, it can be converted into a hexadecimal representation (using a + function like `sodium_bin2hex `_) and then used in the configuration file. For + example: + + .. code-block:: php + + // The string is a hexadecimal representation of a 32-bytes long string of random bytes. + $cfg['blowfish_secret'] = sodium_hex2bin('f16ce59f45714194371b48fe362072dc3b019da7861558cd4ad29e4d6fb13851'); + + Using a binary string is recommended. However, if all 32 bytes of the string are visible + characters, then a function like `sodium_bin2hex `_ is not required. For + example: + + .. code-block:: php + + // A string of 32 characters. + $cfg['blowfish_secret'] = 'JOFw435365IScA&Q!cDugr!lSfuAz*OW'; + + .. warning:: + + The encryption key must be 32 bytes long. If it is longer than the length of bytes, only the first 32 bytes will + be used, and if it is shorter, a new temporary key will be automatically generated for you. However, this + temporary key will only last for the duration of the session. .. note:: @@ -1910,11 +1932,21 @@ Cookie authentication options Blowfish algorithm was originally used to do the encryption. .. versionchanged:: 3.1.0 + Since version 3.1.0 phpMyAdmin can generate this on the fly, but it makes a bit weaker security as this generated secret is stored in session and furthermore it makes impossible to recall user name from cookie. + .. versionchanged:: 5.2.0 + + Since version 5.2.0, phpMyAdmin uses the + `sodium\_crypto\_secretbox `_ and + `sodium\_crypto\_secretbox\_open `_ PHP functions to encrypt + and decrypt cookies, respectively. + + .. seealso:: :ref:`faq2_10` + .. config:option:: $cfg['CookieSameSite'] :type: string @@ -3809,8 +3841,8 @@ following example shows two of them: .. code-block:: php `_ :term:`PHP` function. Since this function returns a binary string, +the returned value should be converted to printable format before being able to copy it. + +For example, the :config:option:`$cfg['blowfish_secret']` configuration directive requires a 32-bytes long string. The +following command can be used to generate a hexadecimal representation of this string. + +.. code-block:: sh + + php -r 'echo bin2hex(random_bytes(32)) . PHP_EOL;' + +The above example will output something similar to: + +.. code-block:: sh + + f16ce59f45714194371b48fe362072dc3b019da7861558cd4ad29e4d6fb13851 + +And then this hexadecimal value can be used in the configuration file. + +.. code-block:: php + + $cfg['blowfish_secret'] = sodium_hex2bin('f16ce59f45714194371b48fe362072dc3b019da7861558cd4ad29e4d6fb13851'); + +The `sodium_hex2bin `_ is used here to convert the hexadecimal value back to the +binary format. + .. _faqlimitations: Known limitations diff --git a/doc/glossary.rst b/doc/glossary.rst index 87eb074a27..8e328ae25d 100644 --- a/doc/glossary.rst +++ b/doc/glossary.rst @@ -335,6 +335,11 @@ From Wikipedia, the free encyclopedia .. seealso:: + Sodium + The Sodium PHP extension. + + .. seealso:: `PHP manual for Sodium extension `_ + Storage Engines MySQL can use several different formats for storing data on disk, these are called storage engines or table types. phpMyAdmin allows a user to diff --git a/doc/setup.rst b/doc/setup.rst index c2b7b78928..1f2563edfc 100644 --- a/doc/setup.rst +++ b/doc/setup.rst @@ -587,8 +587,8 @@ simple configuration may look like this: .. code-block:: xml+php 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('
', $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/Config/Settings.php b/libraries/classes/Config/Settings.php index c5ee9ede3d..62d344c516 100644 --- a/libraries/classes/Config/Settings.php +++ b/libraries/classes/Config/Settings.php @@ -118,10 +118,9 @@ final class Settings public $AllowThirdPartyFraming; /** - * The 'cookie' auth_type uses AES algorithm to encrypt the password. If - * at least one server configuration uses 'cookie' auth_type, enter here a - * pass phrase that will be used by AES. The maximum length seems to be 46 - * characters. + * The 'cookie' auth_type uses the Sodium extension to encrypt the cookies. If at least one server configuration + * uses 'cookie' auth_type, enter here a generated string of random bytes to be used as an encryption key. The + * encryption key must be 32 bytes long. * * @var string */ diff --git a/libraries/classes/Controllers/HomeController.php b/libraries/classes/Controllers/HomeController.php index 322ed3702e..39235fdc06 100644 --- a/libraries/classes/Controllers/HomeController.php +++ b/libraries/classes/Controllers/HomeController.php @@ -30,10 +30,7 @@ use function ini_get; use function mb_strlen; use function preg_match; use function sprintf; -use function trigger_error; -use const E_USER_NOTICE; -use const E_USER_WARNING; use const PHP_VERSION; use const SODIUM_CRYPTO_SECRETBOX_KEYBYTES; @@ -48,6 +45,12 @@ class HomeController extends AbstractController /** @var DatabaseInterface */ private $dbi; + /** + * @var array> + * @psalm-var list + */ + private $errors = []; + public function __construct( ResponseRenderer $response, Template $template, @@ -241,6 +244,7 @@ class HomeController extends AbstractController 'config_storage_message' => $configStorageMessage ?? '', 'has_theme_manager' => $cfg['ThemeManager'], 'themes' => $this->themeManager->getThemesArray(), + 'errors' => $this->errors, ]); } @@ -256,16 +260,16 @@ class HomeController extends AbstractController */ $gc_time = (int) ini_get('session.gc_maxlifetime'); if ($gc_time < $cfg['LoginCookieValidity']) { - trigger_error( - __( + $this->errors[] = [ + 'message' => __( 'Your PHP parameter [a@https://www.php.net/manual/en/session.' . 'configuration.php#ini.session.gc-maxlifetime@_blank]session.' . 'gc_maxlifetime[/a] is lower than cookie validity configured ' . 'in phpMyAdmin, because of this, your login might expire sooner ' . 'than configured in phpMyAdmin.' ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } } @@ -273,14 +277,14 @@ class HomeController extends AbstractController * Check whether LoginCookieValidity is limited by LoginCookieStore. */ if ($cfg['LoginCookieStore'] != 0 && $cfg['LoginCookieStore'] < $cfg['LoginCookieValidity']) { - trigger_error( - __( + $this->errors[] = [ + 'message' => __( 'Login cookie store is lower than cookie validity configured in ' . 'phpMyAdmin, because of this, your login will expire sooner than ' . 'configured in phpMyAdmin.' ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } /** @@ -292,39 +296,43 @@ class HomeController extends AbstractController && $cfg['Server']['controluser'] === 'pma' && $cfg['Server']['controlpass'] === 'pmapass' ) { - trigger_error( - __( + $this->errors[] = [ + 'message' => __( 'Your server is running with default values for the ' . 'controluser and password (controlpass) and is open to ' . 'intrusion; you really should fix this security weakness' . ' by changing the password for controluser \'pma\'.' ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } /** * Check if user does not have defined blowfish secret and it is being used. */ if (! empty($_SESSION['encryption_key'])) { - if (empty($cfg['blowfish_secret'])) { - trigger_error( - __( - 'The configuration file now needs a secret passphrase (blowfish_secret).' + $encryptionKeyLength = mb_strlen($cfg['blowfish_secret'], '8bit'); + if ($encryptionKeyLength < SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { + $this->errors[] = [ + 'message' => __( + 'The configuration file needs a valid key for cookie encryption.' + . ' A temporary key was automatically generated for you.' + . ' Please refer to the [doc@cfg_blowfish_secret]documentation[/doc].' ), - E_USER_WARNING - ); - } elseif (mb_strlen($cfg['blowfish_secret'], '8bit') !== SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { - trigger_error( - sprintf( + 'severity' => 'warning', + ]; + } elseif ($encryptionKeyLength > SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { + $this->errors[] = [ + 'message' => sprintf( __( - 'The secret passphrase in configuration (blowfish_secret) is not the correct length.' - . ' It should be %d bytes long.' + 'The cookie encryption key in the configuration file is longer than necessary.' + . ' It should only be %d bytes long.' + . ' Please refer to the [doc@cfg_blowfish_secret]documentation[/doc].' ), SODIUM_CRYPTO_SECRETBOX_KEYBYTES ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } } @@ -333,16 +341,16 @@ class HomeController extends AbstractController * production environment. */ if (@file_exists(ROOT_PATH . 'config')) { - trigger_error( - __( + $this->errors[] = [ + 'message' => __( 'Directory [code]config[/code], which is used by the setup script, ' . 'still exists in your phpMyAdmin directory. It is strongly ' . 'recommended to remove it once phpMyAdmin has been configured. ' . 'Otherwise the security of your server may be compromised by ' . 'unauthorized people downloading your configuration.' ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } /** @@ -353,22 +361,22 @@ class HomeController extends AbstractController && ini_get('suhosin.request.max_value_length') && ini_get('suhosin.simulation') == '0' ) { - trigger_error( - sprintf( + $this->errors[] = [ + 'message' => sprintf( __( 'Server running with Suhosin. Please refer to %sdocumentation%s for possible issues.' ), '[doc@faq1-38]', '[/doc]' ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } /* Missing template cache */ if ($this->config->getTempDir('twig') === null) { - trigger_error( - sprintf( + $this->errors[] = [ + 'message' => sprintf( __( 'The $cfg[\'TempDir\'] (%s) is not accessible. ' . 'phpMyAdmin is not able to cache templates and will ' . @@ -376,8 +384,8 @@ class HomeController extends AbstractController ), $this->config->get('TempDir') ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } $this->checkLanguageStats(); @@ -410,12 +418,12 @@ class HomeController extends AbstractController return; } - trigger_error( - 'You are using an incomplete translation, please help to make it ' - . 'better by [a@https://www.phpmyadmin.net/translate/' - . '@_blank]contributing[/a].', - E_USER_NOTICE - ); + $this->errors[] = [ + 'message' => 'You are using an incomplete translation, please help to make it ' + . 'better by [a@https://www.phpmyadmin.net/translate/' + . '@_blank]contributing[/a].', + 'severity' => 'notice', + ]; } private function checkPhpExtensionsRequirements(): void @@ -425,15 +433,15 @@ class HomeController extends AbstractController * to tell user something might be broken without it, see bug #1063149. */ if (! extension_loaded('mbstring')) { - trigger_error( - __( + $this->errors[] = [ + 'message' => __( 'The mbstring PHP extension was not found and you seem to be using' . ' a multibyte charset. Without the mbstring extension phpMyAdmin' . ' is unable to split strings correctly and it may result in' . ' unexpected results.' ), - E_USER_WARNING - ); + 'severity' => 'warning', + ]; } /** @@ -443,12 +451,13 @@ class HomeController extends AbstractController return; } - trigger_error( - __( + $this->errors[] = [ + 'message' => __( 'The curl extension was not found and allow_url_fopen is ' . 'disabled. Due to this some features such as error reporting ' . 'or version check are disabled.' - ) - ); + ), + 'severity' => 'notice', + ]; } } diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php index 33faf56b34..e083ddf19a 100644 --- a/libraries/classes/Plugins/Auth/AuthenticationCookie.php +++ b/libraries/classes/Plugins/Auth/AuthenticationCookie.php @@ -597,11 +597,21 @@ class AuthenticationCookie extends AuthenticationPlugin */ private function getEncryptionSecret(): string { + /** @var mixed $key */ $key = $GLOBALS['cfg']['blowfish_secret'] ?? null; - if (is_string($key) && mb_strlen($key, '8bit') === SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { + if (! is_string($key)) { + return $this->getSessionEncryptionSecret(); + } + + $length = mb_strlen($key, '8bit'); + if ($length === SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { return $key; } + if ($length > SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { + return mb_substr($key, 0, SODIUM_CRYPTO_SECRETBOX_KEYBYTES, '8bit'); + } + return $this->getSessionEncryptionSecret(); } @@ -610,6 +620,7 @@ class AuthenticationCookie extends AuthenticationPlugin */ private function getSessionEncryptionSecret(): string { + /** @var mixed $key */ $key = $_SESSION['encryption_key'] ?? null; if (is_string($key) && mb_strlen($key, '8bit') === SODIUM_CRYPTO_SECRETBOX_KEYBYTES) { return $key; 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/libraries/config.default.php b/libraries/config.default.php index 3ecb83faa2..2c3acd51c1 100644 --- a/libraries/config.default.php +++ b/libraries/config.default.php @@ -100,10 +100,9 @@ $cfg['TranslationWarningThreshold'] = 80; $cfg['AllowThirdPartyFraming'] = false; /** - * The 'cookie' auth_type uses AES algorithm to encrypt the password. If - * at least one server configuration uses 'cookie' auth_type, enter here a - * pass phrase that will be used by AES. The maximum length seems to be 46 - * characters. + * The 'cookie' auth_type uses the Sodium extension to encrypt the cookies. If at least one server configuration + * uses 'cookie' auth_type, enter here a generated string of random bytes to be used as an encryption key. The + * encryption key must be 32 bytes long. * * @global string $cfg['blowfish_secret'] */ diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 1c03b2ea93..c3fd561717 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -7790,6 +7790,11 @@ parameters: count: 1 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 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index fdfa268435..874757e842 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -9042,14 +9042,12 @@ $_SESSION['browser_access_time'][$key] - + $GLOBALS['pma_auth_server'] $_form_params['route'] $captchaSiteVerifyURL $captchaSiteVerifyURL $key - $key - $key $password $serverCookie $serverCookie @@ -12573,6 +12571,10 @@ + + $key + sodium_crypto_secretbox_keygen() + $conf['Servers'] @@ -12590,6 +12592,9 @@ $v $v + + non-empty-string + self::getServerPart($cf, $crlf, $conf['Servers']) @@ -14280,40 +14285,24 @@ - - $_SESSION['messages'] + $_SESSION['messages']['error'] - $_SESSION['messages']['error'] - $_SESSION['messages']['notice'] $_SESSION['messages']['notice'] - + $_SESSION['messages']['error'] - $_SESSION['messages']['error'] - $_SESSION['messages']['notice'] $_SESSION['messages']['notice'] - - $_SESSION[$this->sessionID]['AllowArbitraryServer'] + $_SESSION[$this->sessionID]['AllowArbitraryServer'] $_SESSION[$this->sessionID]['BZipDump'] - $_SESSION[$this->sessionID]['BZipDump'] - $_SESSION[$this->sessionID]['GZipDump'] $_SESSION[$this->sessionID]['GZipDump'] $_SESSION[$this->sessionID]['LoginCookieStore'] - $_SESSION[$this->sessionID]['LoginCookieStore'] - $_SESSION[$this->sessionID]['LoginCookieValidity'] $_SESSION[$this->sessionID]['LoginCookieValidity'] $_SESSION[$this->sessionID]['SaveDir'] - $_SESSION[$this->sessionID]['SaveDir'] - $_SESSION[$this->sessionID]['Servers'] - $_SESSION[$this->sessionID]['Servers'] $_SESSION[$this->sessionID]['Servers'] $_SESSION[$this->sessionID]['TempDir'] - $_SESSION[$this->sessionID]['TempDir'] $_SESSION[$this->sessionID]['ZipDump'] - $_SESSION[$this->sessionID]['ZipDump'] - $_SESSION[$this->sessionID]['blowfish_secret'] $_SESSION[$this->sessionID] diff --git a/templates/home/index.twig b/templates/home/index.twig index 72b70a242b..9e089f840c 100644 --- a/templates/home/index.twig +++ b/templates/home/index.twig @@ -9,7 +9,7 @@
{{ sync_favorite_tables|raw }}
-
+
{% if has_server %} {% if is_demo %} @@ -273,6 +273,17 @@
+ + {% for error in errors %} + + {% endfor %}
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 */