froxlor / Froxlor

The server administration software for your needs - The official Froxlor development Git repository
http://www.froxlor.org
GNU General Public License v2.0
1.62k stars 455 forks source link

Check for Argon2 support before using constant PASSWORD_ARGON2X #1228

Closed z-e-r-0-t closed 7 months ago

z-e-r-0-t commented 7 months ago

Description

Adding a new mail account on a system without PHP Argon2X support an exception will be triggered.

e1

-> Check if Argon2X is supported in PHP (==constant is defined) before using it.

Type of change

How Has This Been Tested?

Adding a mail account on a system without PHP Argon2X support was possible after patch

d00p commented 7 months ago

The API in EmailAccounts.add() uses the setting system.passwordcryptfunc to determine the hash to be used. This setting, when opening the security settings, gets all available password-hashes (see https://github.com/Froxlor/Froxlor/blob/v2.1/actions/admin/settings/210.security.php#L53) , which uses Crypt::getAvailablePasswordHashes() (see https://github.com/Froxlor/Froxlor/blob/v2.1/lib/Froxlor/System/Crypt.php#L97) which only adds available hashes to the select for the setting

So, most likely a hash-algorithm is/was set which became unavailable.

d00p commented 7 months ago

Here's a more "systemwide"-safe solution:

diff --git a/lib/Froxlor/Settings.php b/lib/Froxlor/Settings.php
index dad81814..056ed8ed 100644
--- a/lib/Froxlor/Settings.php
+++ b/lib/Froxlor/Settings.php
@@ -26,6 +26,7 @@
 namespace Froxlor;

 use Froxlor\Database\Database;
+use Froxlor\System\Crypt;
 use PDO;
 use PDOStatement;

@@ -159,7 +160,17 @@ class Settings
                $result = null;
                if (isset(self::$data[$sstr[0]][$sstr[1]])) {
                        $result = self::$data[$sstr[0]][$sstr[1]];
+
+                       // if password-crypt-func is requested, check whether it's (still) available
+                       if ($sstr[0] == 'system' && $sstr[1] == 'passwordcryptfunc') {
+                               $available_hashes = Crypt::getAvailablePasswordHashes();
+                               if (!array_key_exists($result, $available_hashes)) {
+                                       $result = PASSWORD_DEFAULT;
+                                       self::Set('system.passwordcryptfunc', $result);
+                               }
+                       }
                }
+
                return $result;
        }
z-e-r-0-t commented 7 months ago

system.passwordcryptfunc is set to PASSWORD_BCRYPT/2y when the error occurs.

The problem is that the PHP version used doesn't support Argon2. If Argon2 is unsupported the constants PASSWORD_ARGON2I and PASSWORD_ARGON2ID aren't defined in PHP -> exception is triggered e.g at https://github.com/Froxlor/Froxlor/blob/dbf83c6f24b4dfdc6749eddc1b4c7e03878b5ae5/lib/Froxlor/Api/Commands/EmailAccounts.php#L160

d00p commented 7 months ago

Yes, you're right. Totally missed that part - your fix seems exactly what is needed