From a6521ad583658eb081623bd2406791b8f12bc0ba Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 14 Dec 2014 01:13:43 +0100 Subject: [PATCH] Greatly improve quality of automatically generated passwords Previously the amount of characters for each type was deterministic, as the character types were chosen in a round robin fashion, instead of randomly choosing from the entire character set. This lead to about 47 bit of entropy with the default length of 8 characters. Additionally str_shuffle does not use a SRNG, which may have reduced the entropy even further. The new algorithm, choosing fairly from the whole range of alphanumeric characters with a default length of 12 characters provides about 71 bits of entropy. --- .../acp/form/MasterPasswordInitForm.class.php | 8 ++--- .../files/lib/form/NewPasswordForm.class.php | 2 +- .../worker/SendNewPasswordWorker.class.php | 2 +- .../files/lib/util/PasswordUtil.class.php | 29 +++++++++---------- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/wcfsetup/install/files/lib/acp/form/MasterPasswordInitForm.class.php b/wcfsetup/install/files/lib/acp/form/MasterPasswordInitForm.class.php index 93f105b30f..7f287c16be 100755 --- a/wcfsetup/install/files/lib/acp/form/MasterPasswordInitForm.class.php +++ b/wcfsetup/install/files/lib/acp/form/MasterPasswordInitForm.class.php @@ -61,7 +61,7 @@ class MasterPasswordInitForm extends MasterPasswordForm { } // check password security - if (mb_strlen($this->masterPassword) < 8) { + if (mb_strlen($this->masterPassword) < 12) { throw new UserInputException('masterPassword', 'notSecure'); } // digits @@ -76,10 +76,6 @@ class MasterPasswordInitForm extends MasterPasswordForm { if (!Regex::compile('[A-Z]')->match($this->masterPassword)) { throw new UserInputException('masterPassword', 'notSecure'); } - // special characters - if (!Regex::compile('[^0-9a-zA-Z]')->match($this->masterPassword)) { - throw new UserInputException('masterPassword', 'notSecure'); - } // password equals username if ($this->masterPassword == WCF::getUser()->username) { @@ -121,7 +117,7 @@ define('MASTER_PASSWORD', '".PasswordUtil::getDoubleSaltedHash($this->masterPass WCF::getTPL()->assign(array( 'confirmMasterPassword' => $this->confirmMasterPassword, - 'exampleMasterPassword' => PasswordUtil::getRandomPassword(12), + 'exampleMasterPassword' => PasswordUtil::getRandomPassword(16), 'relativeWcfDir' => RELATIVE_WCF_DIR )); } diff --git a/wcfsetup/install/files/lib/form/NewPasswordForm.class.php b/wcfsetup/install/files/lib/form/NewPasswordForm.class.php index 6b4e9f6908..e4297d09e6 100644 --- a/wcfsetup/install/files/lib/form/NewPasswordForm.class.php +++ b/wcfsetup/install/files/lib/form/NewPasswordForm.class.php @@ -100,7 +100,7 @@ class NewPasswordForm extends AbstractForm { parent::save(); // generate new password - $this->newPassword = PasswordUtil::getRandomPassword((REGISTER_PASSWORD_MIN_LENGTH > 9 ? REGISTER_PASSWORD_MIN_LENGTH : 9)); + $this->newPassword = PasswordUtil::getRandomPassword((REGISTER_PASSWORD_MIN_LENGTH > 12 ? REGISTER_PASSWORD_MIN_LENGTH : 12)); // update user $this->objectAction = new UserAction(array($this->user), 'update', array( diff --git a/wcfsetup/install/files/lib/system/worker/SendNewPasswordWorker.class.php b/wcfsetup/install/files/lib/system/worker/SendNewPasswordWorker.class.php index fb054e1125..6f9a713050 100644 --- a/wcfsetup/install/files/lib/system/worker/SendNewPasswordWorker.class.php +++ b/wcfsetup/install/files/lib/system/worker/SendNewPasswordWorker.class.php @@ -79,7 +79,7 @@ class SendNewPasswordWorker extends AbstractWorker { * @param \wcf\data\user\UserEditor $userEditor */ protected function sendNewPassword(UserEditor $userEditor) { - $newPassword = PasswordUtil::getRandomPassword(); + $newPassword = PasswordUtil::getRandomPassword((REGISTER_PASSWORD_MIN_LENGTH > 12 ? REGISTER_PASSWORD_MIN_LENGTH : 12)); $userAction = new UserAction(array($userEditor), 'update', array( 'data' => array( diff --git a/wcfsetup/install/files/lib/util/PasswordUtil.class.php b/wcfsetup/install/files/lib/util/PasswordUtil.class.php index ffc88674bc..34d9e1bc91 100644 --- a/wcfsetup/install/files/lib/util/PasswordUtil.class.php +++ b/wcfsetup/install/files/lib/util/PasswordUtil.class.php @@ -14,6 +14,12 @@ use wcf\system\Regex; * @category Community Framework */ final class PasswordUtil { + /** + * list of possible characters in generated passwords + * @var string + */ + const PASSWORD_CHARSET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + /** * concated list of valid blowfish salt characters * @var string @@ -203,27 +209,20 @@ final class PasswordUtil { } /** - * Generates a random user password with the given character length. + * Generates a random alphanumeric user password with the given character length. * * @param integer $length * @return string */ - public static function getRandomPassword($length = 8) { - $availableCharacters = array( - 'abcdefghijklmnopqrstuvwxyz', - 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', - '0123456789', - '+#-.,;:?!' - ); - + public static function getRandomPassword($length = 12) { + $charset = self::PASSWORD_CHARSET; $password = ''; - $type = 0; - for ($i = 0; $i < $length; $i++) { - $type = ($i % 4 == 0) ? 0 : ($type + 1); - $password .= substr($availableCharacters[$type], self::secureRandomNumber(0, strlen($availableCharacters[$type]) - 1), 1); + + for ($i = 0, $maxIndex = (strlen($charset) - 1); $i < $length; $i++) { + $password .= $charset[self::secureRandomNumber(0, $maxIndex)]; } - - return str_shuffle($password); + + return $password; } /** -- 2.20.1