Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[3.0] Password improvements #8464

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
'Sources/minify',
'Sources/random_compat',
'Sources/ReCaptcha',
'Sources/ZxcvbnPhp',
'Themes',
])
// Skip all index.php files and ssi_example.php.
Expand Down
33 changes: 31 additions & 2 deletions Languages/en_US/Errors.php
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,41 @@
$txt['profile_error_bad_avatar_url_too_long'] = 'The avatar URL you specified is too long, please use a shorter URL.';
$txt['profile_error_bad_avatar_too_large'] = 'The image you are trying to use surpasses the max width/height settings, please use a smaller one.';
$txt['profile_error_bad_avatar_fail_reencode'] = 'The image you uploaded was corrupted and the attempt to recover it failed.';

$txt['profile_error_password_short'] = 'Your password must be {0, plural,
one {at least # character long}
other {at least # characters long}
}.';
$txt['profile_error_password_restricted_words'] = 'Your password must not contain your username, email address or other commonly used words.';
$txt['profile_error_password_chars'] = 'Your password must contain a mix of upper and lower case letters, as well as digits.';
$txt['profile_error_password_restricted_words'] = 'Your password must not contain your username, email address, or other commonly used words.';
$txt['profile_error_password_weak'] = 'This password is too weak.';
$txt['profile_error_password_top_10'] = 'This is a top-10 common password.';
$txt['profile_error_password_top_100'] = 'This is a top-100 common password.';
$txt['profile_error_password_very_common'] = 'This is a very common password.';
$txt['profile_error_password_similar_to_common'] = 'This is similar to a commonly used password.';
$txt['profile_error_password_single_word'] = 'A word by itself is easy to guess.';
$txt['profile_error_password_use_a_few_words'] = 'Use a few words, avoid common phrases.';
$txt['profile_error_password_simple_is_fine'] = 'No need for symbols, digits, or uppercase letters.';
$txt['profile_error_password_add_more_words'] = 'Add another word or two. Uncommon words are better.';
$txt['profile_error_password_recent_years'] = 'Recent years are easy to guess.';
$txt['profile_error_password_avoid_recent_years'] = 'Avoid recent years.';
$txt['profile_error_password_avoid_personal_years'] = 'Avoid years that are associated with you.';
$txt['profile_error_password_dates_are_easy'] = 'Dates are often easy to guess.';
$txt['profile_error_password_avoid_personal_dates_and_years'] = 'Avoid dates and years that are associated with you.';
$txt['profile_error_password_straight_rows'] = 'Straight rows of keys are easy to guess.';
$txt['profile_error_password_short_patterns'] = 'Short keyboard patterns are easy to guess.';
$txt['profile_error_password_use_longer_pattern'] = 'Use a longer keyboard pattern with more turns.';
$txt['profile_error_password_sequences'] = 'Sequences like “abc” or “6543” are easy to guess.';
$txt['profile_error_password_avoid_sequences'] = 'Avoid sequences.';
$txt['profile_error_password_reversed_words'] = 'Reversed words aren’t much harder to guess.';
$txt['profile_error_password_repeated_chars'] = 'Repeats like “aaa” are easy to guess.';
$txt['profile_error_password_repeated_strings'] = 'Repeats like “abcabcabc” are only slightly harder to guess than “abc”.';
$txt['profile_error_password_avoid_repeated'] = 'Avoid repeated words and characters.';
$txt['profile_error_password_l33t_useless'] = 'Predictable substitutions like “@” instead of “a” don’t help very much.';
$txt['profile_error_password_names'] = 'Names and surnames by themselves are easy to guess.';
$txt['profile_error_password_common_names'] = 'Common names and surnames are easy to guess.';
$txt['profile_error_password_caps_useless'] = 'Capitalization doesn’t help very much.';
$txt['profile_error_password_all_caps_useless'] = 'All uppercase is almost as easy to guess as all lowercase.';

$txt['profile_error_already_requested_group'] = 'You already have an outstanding request for this group!';
$txt['profile_error_signature_not_yet_saved'] = 'The signature has not been saved.';
$txt['profile_error_personal_text_too_long'] = 'The personal text is too long.';
Expand Down
11 changes: 6 additions & 5 deletions Languages/en_US/Help.php
Original file line number Diff line number Diff line change
Expand Up @@ -466,12 +466,13 @@
$helptxt['approveAccountDeletion'] = 'When this setting is checked, any user request to delete his own account has to be approved by an administrator';

$helptxt['send_welcomeEmail'] = 'When this setting is enabled all new members will be sent an email welcoming them to your community';
$helptxt['password_strength'] = 'This setting determines the strength required for passwords selected by your forum users. The stronger the password, the harder it should be to compromise member’s accounts.
Its possible settings are:
$helptxt['password_strength'] = 'This setting determines the strength required for passwords selected by your forum users. The stronger the password, the harder it should be to compromise member’s accounts. Passwords are evaluated using the <a href="https://github.com/bjeavons/zxcvbn-php" class="bbc_link" target="_blank" rel="noopener">zxcvbn library</a> and checked for other requirements.
<br><br>
Possible values are:
<ul class="normallist">
<li><strong>Low:</strong> The password must be at least four characters long.</li>
<li><strong>Medium:</strong> The password must be at least eight characters long, and cannot be part of a user’s name or email address.</li>
<li><strong>High:</strong> As for medium, except the password must also contain a mixture of upper and lower case letters, and at least one number.</li>
<li><strong>Low:</strong> The password must be at least six characters long, it must not contain the user’s name or email address, and it must earn a zxcvbn score of at least 2.</li>
<li><strong>Medium:</strong> As for low, except the password must be at least eight characters long and it must earn a zxcvbn score of at least 3.</li>
<li><strong>High:</strong> As for medium, except it must earn a zxcvbn score of 4.</li>
</ul>';
$helptxt['enable_password_conversion'] = 'By enabling this setting, SMF will attempt to detect passwords stored in other formats and convert them to the format SMF uses. Typically this is used for forums converted to SMF, but may have other uses as well. Disabling this prevents a user from logging in using their password after a conversion and they would need to reset their password.';

Expand Down
6 changes: 3 additions & 3 deletions Languages/en_US/ManageSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@
$txt['loadavg_disabled_conf'] = 'Load balancing support is disabled by your host configuration.';

$txt['setting_password_strength'] = 'Required strength for user passwords';
$txt['setting_password_strength_low'] = 'Low - 4 character minimum';
$txt['setting_password_strength_medium'] = 'Medium - cannot contain username';
$txt['setting_password_strength_high'] = 'High - mixture of different characters';
$txt['setting_password_strength_low'] = 'Low';
$txt['setting_password_strength_medium'] = 'Medium';
$txt['setting_password_strength_high'] = 'High';
$txt['setting_enable_password_conversion'] = 'Allow password hash conversion';

$txt['antispam_Settings'] = 'Anti-Spam Verification';
Expand Down
2 changes: 1 addition & 1 deletion Languages/en_US/Profile.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
$txt['dob_month'] = 'Month (MM)';
$txt['dob_day'] = 'Day (DD)';
$txt['dob_year'] = 'Year (YYYY)';
$txt['password_strength'] = 'For best security, you should use eight or more characters with a combination of letters, numbers, and symbols.';
$txt['password_strength'] = 'For best security, you should use eight or more characters. It is a good idea to use a phrase with multiple words.';
$txt['include_website_url'] = 'This must be included if you specify a URL below.';
$txt['complete_url'] = 'This must be a complete URL.';
$txt['sig_info'] = 'Signatures are displayed at the bottom of each post or personal message. BBCode and smileys may be used in your signature.';
Expand Down
2 changes: 1 addition & 1 deletion Sources/Actions/Activate.php
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ protected function updateEmail(): void
if (
!empty($_POST['new_email'])
&& !empty($_REQUEST['passwd'])
&& Security::hashVerifyPassword($this->member->username, $_REQUEST['passwd'], $this->member->passwd)
&& Security::hashVerifyPassword($_REQUEST['passwd'], $this->member->passwd)
&& (
$this->member->is_activated == User::NOT_ACTIVATED
|| $this->member->is_activated == User::UNVALIDATED
Expand Down
3 changes: 2 additions & 1 deletion Sources/Actions/Admin/Members.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use SMF\Logging;
use SMF\Mail;
use SMF\Menu;
use SMF\Security;
use SMF\Theme;
use SMF\Time;
use SMF\User;
Expand Down Expand Up @@ -1245,7 +1246,7 @@ public function approve(): void
// We have to do this for each member I'm afraid.
foreach ($member_info as $member) {
// Generate a random activation code.
$validation_code = User::generateValidationCode();
$validation_code = Security::generateValidationCode();

// Set these members for activation - I know this includes two id_member checks but it's safer than bodging $condition ;).
Db::$db->query(
Expand Down
9 changes: 7 additions & 2 deletions Sources/Actions/#2.php
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ public function main(): void
User::$my_id = (reset($loaded))->id;

// Bad password! Thought you could fool the database?!
if (!Security::hashVerifyPassword(User::$profiles[User::$my_id]['member_name'], Utils::htmlspecialcharsDecode($_POST['passwrd']), User::$profiles[User::$my_id]['passwd'])) {
if (!Security::hashVerifyPassword(Utils::htmlspecialcharsDecode($_POST['passwrd']), User::$profiles[User::$my_id]['passwd'])) {
// If the forum was recently upgraded, password might be encrypted
// using a different algorithm. If so, fix it. Otherwise, bail out.
if (!$this->checkPasswordFallbacks()) {
Expand Down Expand Up @@ -515,6 +515,11 @@ protected function checkPasswordFallbacks(): bool
// Maybe we were too hasty... let's try some other authentication methods.
$other_passwords = [];

// SMF 2.1 prepended the username before the password.
if (Security::hashVerifyPassword(Utils::strtolower(User::$profiles[User::$my_id]['member_name']) . Utils::htmlspecialcharsDecode($_POST['passwrd']), User::$profiles[User::$my_id]['passwd'])) {
$other_passwords[] = User::$profiles[User::$my_id]['passwd'];
}

// SMF 1.1 and 2.0 password styles.
if (strlen(User::$profiles[User::$my_id]['passwd']) == 40) {
// Maybe they are using a hash from before the password fix.
Expand Down Expand Up @@ -612,7 +617,7 @@ protected function checkPasswordFallbacks(): bool

// Whichever encryption it was using, let's make it use SMF's now ;).
if (in_array(User::$profiles[User::$my_id]['passwd'], $other_passwords)) {
User::$profiles[User::$my_id]['passwd'] = Security::hashPassword(User::$profiles[User::$my_id]['member_name'], Utils::htmlspecialcharsDecode($_POST['passwrd']));
User::$profiles[User::$my_id]['passwd'] = Security::hashPassword(Utils::htmlspecialcharsDecode($_POST['passwrd']));
User::$profiles[User::$my_id]['password_salt'] = bin2hex(random_bytes(16));

// Update the password and set up the hash.
Expand Down
7 changes: 6 additions & 1 deletion Sources/Actions/#TFA.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,12 @@ public function execute(): void

$backup = $_POST['tfa_backup'];

if (Security::hashVerifyPassword($member['member_name'], $backup, $member['tfa_backup'])) {
if (
// 3.0
Security::hashVerifyPassword($backup, $member['tfa_backup'])
// 2.1
|| Security::hashVerifyPassword(Utils::strtolower($member['member_name']) . $backup, $member['tfa_backup'])
) {
// Get rid of their current TFA settings
User::updateMemberData($member['id_member'], [
'tfa_secret' => '',
Expand Down
2 changes: 1 addition & 1 deletion Sources/Actions/Profile/Main.php
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ public function execute(): void
$good_password = in_array(true, IntegrationHook::call('integrate_verify_password', [Profile::$member->username, $password, false]), true);

// Bad password!!!
if (!$good_password && !Security::hashVerifyPassword(Profile::$member->username, $password, Profile::$member->passwd)) {
if (!$good_password && !Security::hashVerifyPassword($password, Profile::$member->passwd)) {
Profile::$member->save_errors[] = 'bad_password';
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/Actions/Profile/TFASetup.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ protected function validateAndSave(): void

if (empty(Utils::$context['password_auth_failed']) && $valid_code) {
$backup = bin2hex(random_bytes(8));
$backup_encrypted = Security::hashPassword(User::$me->username, $backup);
$backup_encrypted = Security::hashPassword($backup);

User::updateMemberData(Profile::$member->id, [
'tfa_secret' => $_SESSION['tfa_secret'],
Expand Down
20 changes: 13 additions & 7 deletions Sources/Actions/Register2.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ function (&$value, $key) {
}

if (isset($_POST['secret_answer']) && $_POST['secret_answer'] != '') {
$_POST['secret_answer'] = Security::hashPassword($_POST['user'], $_POST['secret_answer']);
$_POST['secret_answer'] = Security::hashPassword($_POST['secret_answer']);
}

// Maybe you want set the displayed name during registration
Expand Down Expand Up @@ -494,12 +494,12 @@ public static function registerMember(array &$reg_options, bool $return_errors =
$validation_code = '';

if ($reg_options['require'] == 'activation') {
$validation_code = User::generateValidationCode();
$validation_code = Security::generateValidationCode();
}

// If you haven't put in a password generate one.
if ($reg_options['interface'] == 'admin' && $reg_options['password'] == '') {
$reg_options['password'] = User::generateValidationCode();
$reg_options['password'] = Security::generatePassword();
$reg_options['password_check'] = $reg_options['password'];
}
// Does the first password match the second?
Expand All @@ -514,14 +514,20 @@ public static function registerMember(array &$reg_options, bool $return_errors =

// Now perform hard password validation as required.
if (!empty($reg_options['check_password_strength']) && $reg_options['password'] != '') {
$password_error = User::validatePassword($reg_options['password'], $reg_options['username'], [$reg_options['email']]);
$password_error = Security::validatePassword($reg_options['password'], $reg_options['username'], [$reg_options['email']]);

// Password isn't legal?
if ($password_error != null) {
$error_code = ['lang', 'profile_error_password_' . $password_error, false];
Lang::load('Errors');

if (isset(Lang::$txt['profile_error_password_' . $password_error])) {
$error_code = ['lang', 'profile_error_password_' . $password_error, false];
} else {
$error_code = ['done', $password_error, false];
}

if ($password_error == 'short') {
$error_code[] = [empty(Config::$modSettings['password_strength']) ? 4 : 8];
$error_code[] = Security::minimumPasswordLength();
}

$reg_errors[] = $error_code;
Expand Down Expand Up @@ -613,7 +619,7 @@ public static function registerMember(array &$reg_options, bool $return_errors =
$reg_options['register_vars'] = [
'member_name' => $reg_options['username'],
'email_address' => $reg_options['email'],
'passwd' => Security::hashPassword($reg_options['username'], $reg_options['password']),
'passwd' => Security::hashPassword($reg_options['password']),
'password_salt' => bin2hex(random_bytes(16)),
'posts' => 0,
'date_registered' => time(),
Expand Down
Loading