From 49394aedeb96b35303eac061563c754ab23a9b96 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 1 Aug 2022 11:42:10 -0400 Subject: [PATCH] Add password length limit if using the Blowfish hashing algorithm - Add new page to reset a password that would be too long - Add support for pattern parameter in field_password --- src/Model/User.php | 29 ++++- src/Module/Security/PasswordTooLong.php | 103 ++++++++++++++++++ src/Module/Settings/Account.php | 5 +- src/Security/Authentication.php | 8 +- static/routes.config.php | 4 + view/templates/field_password.tpl | 2 +- view/templates/security/password_too_long.tpl | 22 ++++ view/theme/frio/templates/field_password.tpl | 2 +- 8 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 src/Module/Security/PasswordTooLong.php create mode 100644 view/templates/security/password_too_long.tpl diff --git a/src/Model/User.php b/src/Model/User.php index 1e002aa3f8..574830603e 100644 --- a/src/Model/User.php +++ b/src/Model/User.php @@ -735,6 +735,29 @@ class User return password_hash($password, PASSWORD_DEFAULT); } + /** + * Allowed characters are a-z, A-Z, 0-9 and special characters except white spaces, accentuated letters and colon (:). + * + * Password length is limited to 72 characters if the current default password hashing algorithm is Blowfish. + * From the manual: "Using the PASSWORD_BCRYPT as the algorithm, will result in the password parameter being + * truncated to a maximum length of 72 bytes." + * + * @see https://www.php.net/manual/en/function.password-hash.php#refsect1-function.password-hash-parameters + * + * @param string|null $delimiter Whether the regular expression is meant to be wrapper in delimiter characters + * @return string + */ + public static function getPasswordRegExp(string $delimiter = null): string + { + $allowed_characters = '!"#$%&\'()*+,-./;<=>?@[\]^_`{|}~'; + + if ($delimiter) { + $allowed_characters = preg_quote($allowed_characters, $delimiter); + } + + return '^[a-zA-Z0-9' . $allowed_characters . ']' . (PASSWORD_DEFAULT !== PASSWORD_BCRYPT ? '{1,72}' : '+') . '$'; + } + /** * Updates a user row with a new plaintext password * @@ -755,9 +778,11 @@ class User throw new Exception(DI::l10n()->t('The new password has been exposed in a public data dump, please choose another.')); } - $allowed_characters = '!"#$%&\'()*+,-./;<=>?@[\]^_`{|}~'; + if (PASSWORD_DEFAULT === PASSWORD_BCRYPT && strlen($password) > 72) { + throw new Exception(DI::l10n()->t('The password length is limited to 72 characters.')); + } - if (!preg_match('/^[a-z0-9' . preg_quote($allowed_characters, '/') . ']+$/i', $password)) { + if (!preg_match('/' . self::getPasswordRegExp('/') . '/', $password)) { throw new Exception(DI::l10n()->t('The password can\'t contain accentuated letters, white spaces or colons (:)')); } diff --git a/src/Module/Security/PasswordTooLong.php b/src/Module/Security/PasswordTooLong.php new file mode 100644 index 0000000000..9de4a345bb --- /dev/null +++ b/src/Module/Security/PasswordTooLong.php @@ -0,0 +1,103 @@ +. + * + */ + +namespace Friendica\Module\Security; + +use Friendica\App; +use Friendica\Core\L10n; +use Friendica\Core\Renderer; +use Friendica\Database\DBA; +use Friendica\Model\User; +use Friendica\Module\Response; +use Friendica\Navigation\SystemMessages; +use Friendica\Util\Profiler; +use Psr\Log\LoggerInterface; + +class PasswordTooLong extends \Friendica\BaseModule +{ + /** @var SystemMessages */ + private $sysmsg; + + public function __construct(SystemMessages $sysmsg, L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, array $server, array $parameters = []) + { + parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters); + + $this->sysmsg = $sysmsg; + } + + protected function post(array $request = []) + { + $newpass = $request['password']; + $confirm = $request['password_confirm']; + + try { + if ($newpass != $confirm) { + throw new \Exception($this->l10n->t('Passwords do not match.')); + } + + // check if the old password was supplied correctly before changing it to the new value + User::getIdFromPasswordAuthentication(local_user(), $request['password_current']); + + if (strlen($request['password_current']) <= 72) { + throw new \Exception($this->l10n->t('Password does not need changing.')); + } + + $result = User::updatePassword(local_user(), $newpass); + if (!DBA::isResult($result)) { + throw new \Exception($this->l10n->t('Password update failed. Please try again.')); + } + + $this->sysmsg->addInfo($this->l10n->t('Password changed.')); + + $this->baseUrl->redirect($request['return_url'] ?? ''); + } catch (\Exception $e) { + $this->sysmsg->addNotice($e->getMessage()); + $this->sysmsg->addNotice($this->l10n->t('Password unchanged.')); + } + } + + protected function content(array $request = []): string + { + // Nothing to do here + if (PASSWORD_DEFAULT !== PASSWORD_BCRYPT) { + $this->baseUrl->redirect(); + } + + $tpl = Renderer::getMarkupTemplate('security/password_too_long.tpl'); + $o = Renderer::replaceMacros($tpl, [ + '$l10n' => [ + 'ptitle' => $this->l10n->t('Password Too Long'), + 'desc' => $this->l10n->t('Since version 2022.09, we\'ve realized that any password longer than 72 characters is truncated during hashing. To prevent any confusion about this behavior, please update your password to be fewer or equal to 72 characters.'), + 'submit' => $this->l10n->t('Update Password'), + ], + + '$baseurl' => $this->baseUrl->get(true), + '$form_security_token' => self::getFormSecurityToken('security/password_too_long'), + '$return_url' => $request['return_url'] ?? '', + + '$password_current' => ['password_current', $this->l10n->t('Current Password:'), '', $this->l10n->t('Your current password to confirm the changes'), 'required', 'autocomplete="off"'], + '$password' => ['password', $this->l10n->t('New Password:'), '', $this->l10n->t('Allowed characters are a-z, A-Z, 0-9 and special characters except white spaces, accentuated letters and colon (:).') . ' ' . $this->l10n->t('Password length is limited to 72 characters.'), 'required', 'autocomplete="off"', User::getPasswordRegExp()], + '$password_confirm' => ['password_confirm', $this->l10n->t('Confirm:'), '', '', 'required', 'autocomplete="off"'], + ]); + + return $o; + } +} diff --git a/src/Module/Settings/Account.php b/src/Module/Settings/Account.php index 33a5feff49..d20827e6ab 100644 --- a/src/Module/Settings/Account.php +++ b/src/Module/Settings/Account.php @@ -551,6 +551,9 @@ class Account extends BaseSettings $notify_type = DI::pConfig()->get(local_user(), 'system', 'notify_type'); + $passwordRules = DI::l10n()->t('Allowed characters are a-z, A-Z, 0-9 and special characters except white spaces, accentuated letters and colon (:).') + . (PASSWORD_DEFAULT === PASSWORD_BCRYPT ? ' ' . DI::l10n()->t('Password length is limited to 72 characters.') : ''); + $tpl = Renderer::getMarkupTemplate('settings/account.tpl'); $o = Renderer::replaceMacros($tpl, [ '$ptitle' => DI::l10n()->t('Account Settings'), @@ -563,7 +566,7 @@ class Account extends BaseSettings '$open' => $this->parameters['open'] ?? 'password', '$h_pass' => DI::l10n()->t('Password Settings'), - '$password1' => ['password', DI::l10n()->t('New Password:'), '', DI::l10n()->t('Allowed characters are a-z, A-Z, 0-9 and special characters except white spaces, accentuated letters and colon (:).'), false, 'autocomplete="off"'], + '$password1' => ['password', DI::l10n()->t('New Password:'), '', $passwordRules, false, 'autocomplete="off"', User::getPasswordRegExp()], '$password2' => ['confirm', DI::l10n()->t('Confirm:'), '', DI::l10n()->t('Leave password fields blank unless changing'), false, 'autocomplete="off"'], '$password3' => ['opassword', DI::l10n()->t('Current Password:'), '', DI::l10n()->t('Your current password to confirm the changes'), false, 'autocomplete="off"'], '$password4' => ['mpassword', DI::l10n()->t('Password:'), '', DI::l10n()->t('Your current password to confirm the changes of the email address'), false, 'autocomplete="off"'], diff --git a/src/Security/Authentication.php b/src/Security/Authentication.php index d1e64534c6..42dc023404 100644 --- a/src/Security/Authentication.php +++ b/src/Security/Authentication.php @@ -291,8 +291,14 @@ class Authentication $this->dba->update('user', ['openid' => $openid_identity, 'openidserver' => $openid_server], ['uid' => $record['uid']]); } - $this->setForUser($a, $record, true, true); + /** + * @see User::getPasswordRegExp() + */ + if (PASSWORD_DEFAULT === PASSWORD_BCRYPT && strlen($password) > 72) { + $return_path = '/security/password_too_long?' . http_build_query(['return_path' => $return_path]); + } + $this->setForUser($a, $record, true, true); $this->baseUrl->redirect($return_path); } diff --git a/static/routes.config.php b/static/routes.config.php index ce41c23d3a..f9935a53f4 100644 --- a/static/routes.config.php +++ b/static/routes.config.php @@ -549,6 +549,10 @@ return [ '/{type:users}/{guid}' => [Module\Diaspora\Receive::class, [ R::POST]], ], + '/security' => [ + '/password_too_long' => [Module\Security\PasswordTooLong::class, [R::GET, R::POST]], + ], + '/settings' => [ '[/]' => [Module\Settings\Account::class, [R::GET, R::POST]], '/account' => [ diff --git a/view/templates/field_password.tpl b/view/templates/field_password.tpl index 6295880621..07241fb11b 100644 --- a/view/templates/field_password.tpl +++ b/view/templates/field_password.tpl @@ -1,7 +1,7 @@
- + {{if $field.3}} {{$field.3 nofilter}} {{/if}} diff --git a/view/templates/security/password_too_long.tpl b/view/templates/security/password_too_long.tpl new file mode 100644 index 0000000000..81e698597d --- /dev/null +++ b/view/templates/security/password_too_long.tpl @@ -0,0 +1,22 @@ +
+

{{$l10n.ptitle}}

+ +
+
{{$l10n.desc}}
+
+
+ +
+
+ + + {{include file="field_password.tpl" field=$password_current}} + {{include file="field_password.tpl" field=$password}} + {{include file="field_password.tpl" field=$password_confirm}} + +
+ +
+
+
+
diff --git a/view/theme/frio/templates/field_password.tpl b/view/theme/frio/templates/field_password.tpl index df29b2fda2..25a7d0c4ce 100644 --- a/view/theme/frio/templates/field_password.tpl +++ b/view/theme/frio/templates/field_password.tpl @@ -1,7 +1,7 @@
- + {{if $field.3}} {{$field.3 nofilter}} {{/if}}