tobias/friendica
1
0
Fork 0
Code Pull requests Activity

Merge pull request #11810 from MrPetovan/task/11794-password-length-limit

Browse source 
Add password length limit if using the Blowfish hashing algorithm
This commit is contained in:
Philipp 2022-08-02 17:52:26 +02:00 committed by GitHub
commit 4c52318c4e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 520 additions and 278 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/Core/Session.php
View file

@ -44,6 +44,11 @@ class Session
return DI::session()->get($name, $defaults);
}
public static function pop($name, $defaults = null)
{
return DI::session()->pop($name, $defaults);
}
public static function set($name, $value)
{
DI::session()->set($name, $value);

10
src/Core/Session/Capability/IHandleSessions.php
View file

@ -54,6 +54,16 @@ interface IHandleSessions
*/
public function get(string $name, $defaults = null);
/**
* Retrieves a value from the provided key if it exists and removes it from session
*
* @param string $name
* @param mixed $defaults
*
* @return mixed
*/
public function pop(string $name, $defaults = null);
/**
* Sets a single session variable.
* Overrides value of existing key.

14
src/Core/Session/Type/AbstractSession.php
View file

@ -52,6 +52,20 @@ class AbstractSession implements IHandleSessions
return $_SESSION[$name] ?? $defaults;
}
/**
* {@inheritDoc}
*/
public function pop(string $name, $defaults = null)
{
$value = $defaults;
if ($this->exists($name)) {
$value = $this->get($name);
$this->remove($name);
}
return $value;
}
/**
* {@inheritDoc}
*/

29
src/Model/User.php
View file

@ -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 (:)'));
}

98
src/Module/Security/Login.php
View file

@ -21,54 +21,76 @@
namespace Friendica\Module\Security;
use Friendica\App;
use Friendica\BaseModule;
use Friendica\Core\Config\Capability\IManageConfigValues;
use Friendica\Core\Hook;
use Friendica\Core\L10n;
use Friendica\Core\Renderer;
use Friendica\Core\Session;
use Friendica\Core\Session\Capability\IHandleSessions;
use Friendica\DI;
use Friendica\Module\Register;
use Friendica\Module\Response;
use Friendica\Security\Authentication;
use Friendica\Util\Profiler;
use Psr\Log\LoggerInterface;
/**
* Login module
*/
class Login extends BaseModule
{
/** @var Authentication */
private $auth;
/** @var IManageConfigValues */
private $config;
/** @var IHandleSessions */
private $session;
public function __construct(Authentication $auth, IManageConfigValues $config, IHandleSessions $session, 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->auth = $auth;
$this->config = $config;
$this->session = $session;
}
protected function content(array $request = []): string
{
$return_path = $_REQUEST['return_path'] ?? '' ;
$return_path = $request['return_path'] ?? $this->session->pop('return_path', '') ;
if (local_user()) {
DI::baseUrl()->redirect($return_path);
} elseif (!empty($return_path)) {
Session::set('return_path', $return_path);
$this->baseUrl->redirect($return_path);
}
return self::form(Session::get('return_path'), intval(DI::config()->get('config', 'register_policy')) !== \Friendica\Module\Register::CLOSED);
return self::form($return_path, intval($this->config->get('config', 'register_policy')) !== \Friendica\Module\Register::CLOSED);
}
protected function post(array $request = [])
{
$return_path = Session::get('return_path');
Session::clear();
Session::set('return_path', $return_path);
$this->session->clear();
// OpenId Login
if (
empty($_POST['password'])
&& (!empty($_POST['openid_url'])
|| !empty($_POST['username']))
empty($request['password'])
&& (!empty($request['openid_url'])
|| !empty($request['username']))
) {
$openid_url = trim(($_POST['openid_url'] ?? '') ?: $_POST['username']);
$openid_url = trim(($request['openid_url'] ?? '') ?: $request['username']);
DI::auth()->withOpenId($openid_url, !empty($_POST['remember']));
$this->auth->withOpenId($openid_url, !empty($request['remember']));
}
if (!empty($_POST['auth-params']) && $_POST['auth-params'] === 'login') {
DI::auth()->withPassword(
if (!empty($request['auth-params']) && $request['auth-params'] === 'login') {
$this->auth->withPassword(
DI::app(),
trim($_POST['username']),
trim($_POST['password']),
!empty($_POST['remember'])
trim($request['username']),
trim($request['password']),
!empty($request['remember']),
$request['return_path'] ?? ''
);
}
}
@ -76,26 +98,23 @@ class Login extends BaseModule
/**
* Wrapper for adding a login box.
*
* @param string $return_path The path relative to the base the user should be sent
* back to after login completes
* @param bool $register If $register == true provide a registration link.
* This will most always depend on the value of config.register_policy.
* @param array $hiddens optional
* @param string|null $return_path The path relative to the base the user should be sent back to after login completes.
* @param bool $register If $register == true provide a registration link.
* This will almost always depend on the value of config.register_policy.
*
* @return string Returns the complete html for inserting into the page
*
* @throws \Friendica\Network\HTTPException\InternalServerErrorException
* @throws \Friendica\Network\HTTPException\ServiceUnavailableException
* @hooks 'login_hook' string $o
*/
public static function form($return_path = null, $register = false, $hiddens = [])
public static function form(string $return_path = null, bool $register = false): string
{
$o = '';
$noid = DI::config()->get('system', 'no_openid');
if ($noid) {
Session::remove('openid_identity');
Session::remove('openid_attributes');
DI::session()->remove('openid_identity');
DI::session()->remove('openid_attributes');
}
$reg = false;
@ -107,10 +126,6 @@ class Login extends BaseModule
];
}
if (is_null($return_path)) {
$return_path = DI::args()->getQueryString();
}
if (local_user()) {
$tpl = Renderer::getMarkupTemplate('logout.tpl');
} else {
@ -122,13 +137,12 @@ class Login extends BaseModule
);
$tpl = Renderer::getMarkupTemplate('login.tpl');
$_SESSION['return_path'] = $return_path;
}
if (!empty(Session::get('openid_identity'))) {
if (!empty(DI::session()->get('openid_identity'))) {
$openid_title = DI::l10n()->t('Your OpenID: ');
$openid_readonly = true;
$identity = Session::get('openid_identity');
$identity = DI::session()->get('openid_identity');
$username_desc = DI::l10n()->t('Please enter your username and password to add the OpenID to your existing account.');
} else {
$openid_title = DI::l10n()->t('Or login using OpenID: ');
@ -137,7 +151,7 @@ class Login extends BaseModule
$username_desc = '';
}
$o .= Renderer::replaceMacros(
$o = Renderer::replaceMacros(
$tpl,
[
'$dest_url' => DI::baseUrl()->get(true) . '/login',
@ -151,7 +165,7 @@ class Login extends BaseModule
'$openid' => !$noid,
'$lopenid' => ['openid_url', $openid_title, $identity, '', $openid_readonly],
'$hiddens' => $hiddens,
'$hiddens' => ['return_path' => $return_path ?? DI::args()->getQueryString()],
'$register' => $reg,
@ -174,14 +188,14 @@ class Login extends BaseModule
/**
* Get the URL to the register page and add OpenID parameters to it
*/
private static function getRegisterURL()
private static function getRegisterURL(): string
{
if (empty(Session::get('openid_identity'))) {
if (empty(DI::session()->get('openid_identity'))) {
return 'register';
}
$args = [];
$attr = Session::get('openid_attributes', []);
$attr = DI::session()->get('openid_attributes', []);
if (is_array($attr) && count($attr)) {
foreach ($attr as $k => $v) {
@ -218,7 +232,7 @@ class Login extends BaseModule
$args['photo'] = $photo;
}
$args['openid_url'] = trim(Session::get('openid_identity'));
$args['openid_url'] = trim(DI::session()->get('openid_identity'));
return 'register?' . http_build_query($args);
}

6
src/Module/Security/OpenID.php
View file

@ -73,9 +73,7 @@ class OpenID extends BaseModule
DI::auth()->setForUser(DI::app(), $user, true, true);
// just in case there was no return url set
// and we fell through
DI::baseUrl()->redirect();
$this->baseUrl->redirect(DI::session()->pop('return_path', ''));
}
// Successful OpenID login - but we can't match it to an existing account.
@ -84,7 +82,7 @@ class OpenID extends BaseModule
$session->set('openid_identity', $authId);
// Detect the server URL
$open_id_obj = new LightOpenID(DI::baseUrl()->getHostName());
$open_id_obj = new LightOpenID(DI::baseUrl()->getHostname());
$open_id_obj->identity = $authId;
$session->set('openid_server', $open_id_obj->discover($open_id_obj->identity));

103
src/Module/Security/PasswordTooLong.php Normal file
View file

@ -0,0 +1,103 @@
<?php
/**
* @copyright Copyright (C) 2010-2022, the Friendica project
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
*/
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;
}
}

2
src/Module/Security/TwoFactor/Recovery.php
View file

@ -73,6 +73,8 @@ class Recovery extends BaseModule
info($this->t('Remaining recovery codes: %d', RecoveryCode::countValidForUser(local_user())));
$this->auth->setForUser($this->app, User::getById($this->app->getLoggedInUserId()), true, true);
$this->baseUrl->redirect($this->session->pop('return_path', ''));
} else {
notice($this->t('Invalid code, please retry.'));
}

3
src/Module/Security/TwoFactor/Trust.php
View file

@ -102,6 +102,7 @@ class Trust extends BaseModule
try {
$this->auth->setForUser($this->app, User::getById($this->app->getLoggedInUserId()), true, true);
$this->baseUrl->redirect($this->session->pop('return_path', ''));
} catch (FoundException | TemporaryRedirectException | MovedPermanentlyException $e) {
// exception wanted!
throw $e;
@ -122,7 +123,7 @@ class Trust extends BaseModule
$trustedBrowser = $this->trustedBrowserRepository->selectOneByHash($this->cookie->get('2fa_cookie_hash'));
if (!$trustedBrowser->trusted) {
$this->auth->setForUser($this->app, User::getById($this->app->getLoggedInUserId()), true, true);
$this->baseUrl->redirect();
$this->baseUrl->redirect($this->session->pop('return_path', ''));
}
} catch (TrustedBrowserNotFoundException $exception) {
$this->logger->notice('Trusted Browser of the cookie not found.', ['cookie_hash' => $this->cookie->get('trusted'), 'uid' => $this->app->getLoggedInUserId(), 'exception' => $exception]);

5
src/Module/Settings/Account.php
View file

@ -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"'],

32
src/Security/Authentication.php
View file

@ -244,15 +244,19 @@ class Authentication
/**
* Attempts to authenticate using login/password
*
* @param App $a The Friendica Application context
* @param string $username User name
* @param string $password Clear password
* @param bool $remember Whether to set the session remember flag
* @param App $a The Friendica Application context
* @param string $username
* @param string $password Clear password
* @param bool $remember Whether to set the session remember flag
* @param string $return_path The relative path to redirect the user to after authentication
*
* @throws HttpException\InternalServerErrorException In case of Friendica internal exceptions
* @throws Exception A general Exception (like SQL Grammar exceptions)
* @throws HTTPException\ForbiddenException
* @throws HTTPException\FoundException
* @throws HTTPException\InternalServerErrorException In case of Friendica internal exceptions
* @throws HTTPException\MovedPermanentlyException
* @throws HTTPException\TemporaryRedirectException
*/
public function withPassword(App $a, string $username, string $password, bool $remember)
public function withPassword(App $a, string $username, string $password, bool $remember, string $return_path = '')
{
$record = null;
@ -287,10 +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]);
}
$return_path = $this->session->get('return_path', '');
$this->session->remove('return_path');
$this->setForUser($a, $record, true, true);
$this->baseUrl->redirect($return_path);
}
@ -382,10 +390,6 @@ class Authentication
if ($login_initial) {
Hook::callAll('logged_in', $user_record);
if (DI::args()->getModuleName() !== 'home' && $this->session->exists('return_path')) {
$this->baseUrl->redirect($this->session->get('return_path'));
}
}
}

4
static/routes.config.php
View file

@ -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' => [

461
view/lang/C/messages.po
View file

File diff suppressed because it is too large Load diff

2
view/templates/field_password.tpl
View file

@ -1,7 +1,7 @@
<div class="field password" id="wrapper_{{$field.0}}">
<label for="id_{{$field.0}}">{{$field.1}}{{if $field.4}} <span class="required" title="{{$field.4}}">*</span>{{/if}}</label>
<input type="password" name="{{$field.0}}" id="id_{{$field.0}}" value="{{$field.2 nofilter}}"{{if $field.4}} required{{/if}}{{if $field.5 eq "autofocus"}} autofocus{{/if}} aria-describedby="{{$field.0}}_tip">
<input type="password" name="{{$field.0}}" id="id_{{$field.0}}" value="{{$field.2 nofilter}}"{{if $field.4}} required{{/if}}{{if $field.5 eq "autofocus"}} autofocus{{elseif $field.5}} {{$field.5}}{{/if}}{{if $field.6}} pattern="(($field.6}}"{{/if}} aria-describedby="{{$field.0}}_tip">
{{if $field.3}}
<span class="field_help" role="tooltip" id="{{$field.0}}_tip">{{$field.3 nofilter}}</span>
{{/if}}

22
view/templates/security/password_too_long.tpl Normal file
View file

@ -0,0 +1,22 @@
<div class="generic-page-wrapper">
<h1>{{$l10n.ptitle}}</h1>
<div id="settings-nick-wrapper">
<div id="settings-nickname-desc" class="info-message">{{$l10n.desc}}</div>
</div>
<div id="settings-nick-end"></div>
<div id="settings-form">
<form class="settings-content-block" action="security/password_too_long" method="post" autocomplete="off" enctype="multipart/form-data">
<input type="hidden" name="form_security_token" value="{{$form_security_token}}">
<input type="hidden" name="return_url" value="{{$return_url}}">
{{include file="field_password.tpl" field=$password_current}}
{{include file="field_password.tpl" field=$password}}
{{include file="field_password.tpl" field=$password_confirm}}
<div class="settings-submit-wrapper">
<button type="submit" name="password-submit" class="btn btn-primary" value="{{$l10n.submit}}">{{$l10n.submit}}</button>
</div>
</form>
</div>
</div>

2
view/theme/frio/templates/field_password.tpl
View file

@ -1,7 +1,7 @@
<div id="id_{{$field.0}}_wrapper" class="form-group field input password">
<label for="id_{{$field.0}}" id="label_{{$field.0}}">{{$field.1}}{{if $field.4}} <span class="required" title="{{$field.4}}">*</span>{{/if}}</label>
<input class="form-control" name="{{$field.0}}" id="id_{{$field.0}}" type="password" value="{{$field.2 nofilter}}" {{if $field.4}} required{{/if}}{{if $field.5 eq "autofocus"}} autofocus{{elseif $field.5}} {{$field.5}}{{/if}} aria-describedby="{{$field.0}}_tip">
<input class="form-control" name="{{$field.0}}" id="id_{{$field.0}}" type="password" value="{{$field.2 nofilter}}" {{if $field.4}} required{{/if}}{{if $field.5 eq "autofocus"}} autofocus{{elseif $field.5}} {{$field.5}}{{/if}}{{if $field.6}} pattern="{{$field.6}}"{{/if}} aria-describedby="{{$field.0}}_tip">
{{if $field.3}}
<span class="help-block" id="{{$field.0}}_tip" role="tooltip">{{$field.3 nofilter}}</span>
{{/if}}