Skip to content

Loading…

Login should ignore converter passwords by default #107

Merged
merged 2 commits into from

3 participants

@jdarwood007
Simple Machines member

By default we shouldn't try to convert non-compatible passwords. Conversions should enable this option automatically to allow proper conversion of passwords. There is no reason to attempt to fix these passwords for SMF installs that are not conversions and is just a waste of resources to do so.
But we always try to fix old SMF passwords, as there is no reliable way to determine if those options need to be enabled to fix previous bugs, without doing the password checks in the first place. I suppose we could make this a drop down to also disable SMF ones, but seems worthless.

Jeremy D added some commits
Jeremy D Merge branch 'master' of github.com:SimpleMachines/SMF2.1 f523de3
Jeremy D By default we shouldn't try to convert non-compatible passwords. Con…
…versions should enable this option automatically to allow proper conversion of passwords. There is no reason to attempt to fix these passwords for SMF installs that are not conversions and is just a waste of resources to do so.

Signed-off-by: Jeremy D <github@sleepycode.com>
01e099f
@jdarwood007 jdarwood007 was assigned
@emanuele45 emanuele45 commented on the diff
Sources/LogInOut.php
@@ -336,7 +336,8 @@ function Login2()
$other_passwords[] = sha1(strtolower($user_settings['member_name']) . un_htmlspecialchars($_POST['passwrd']));
// BurningBoard3 style of hashing.
- $other_passwords[] = sha1($user_settings['password_salt'] . sha1($user_settings['password_salt'] . sha1($_POST['passwrd'])));
+ if (!empty($modSettings['enable_password_conversion']))

that one seems not necessary. Am I wrong?

@jdarwood007 Simple Machines member

I didn't delete it, it was indented. Burning board is another software so it should follow the same exclusions. Only SMF passwords from 1.0 (md5), linux-bugged sha1 and pre-htmlchars fix should be tested.

Yups, sorry, I missed one of the IFs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@emanuele45 emanuele45 merged commit 7687ab0 into SimpleMachines:master
@norv

As far as I remember, there's no good way to detect installs that have been converted. Which means converters should be updated at least several months I'd say before this release, to give some time to users of converted boards to login at least once.

@jdarwood007
Simple Machines member

They get updated on the first login. So admins should have theirs updated at least to enable the setting if need be. As well emails should be carried over so they can just do a lost password recovery.

@jdarwood007 jdarwood007 deleted the jdarwood007:login-ignore-convter-passwords branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 29, 2012
  1. By default we shouldn't try to convert non-compatible passwords. Con…

    Jeremy D committed
    …versions should enable this option automatically to allow proper conversion of passwords. There is no reason to attempt to fix these passwords for SMF installs that are not conversions and is just a waste of resources to do so.
    
    Signed-off-by: Jeremy D <github@sleepycode.com>
View
6 .gitignore
@@ -0,0 +1,6 @@
+Settings.php
+Settings_bak.php
+db_last_error.php
+cache/data*.php
+Packages/backups/*
+Packages/temp
View
7 Sources/LogInOut.php
@@ -292,7 +292,7 @@ function Login2()
$other_passwords = array();
// None of the below cases will be used most of the time (because the salt is normally set.)
- if ($user_settings['password_salt'] == '')
+ if (!empty($modSettings['enable_password_conversion']) && $user_settings['password_salt'] == '')
{
// YaBB SE, Discus, MD5 (used a lot), SHA-1 (used some), SMF 1.0.x, IkonBoard, and none at all.
$other_passwords[] = crypt($_POST['passwrd'], substr($_POST['passwrd'], 0, 2));
@@ -318,7 +318,7 @@ function Login2()
$other_passwords[] = md5(crypt($_POST['passwrd'], 'CRYPT_MD5'));
}
// The hash should be 40 if it's SHA-1, so we're safe with more here too.
- elseif (strlen($user_settings['passwd']) == 32)
+ elseif (!empty($modSettings['enable_password_conversion']) && strlen($user_settings['passwd']) == 32)
{
// vBulletin 3 style hashing? Let's welcome them with open arms \o/.
$other_passwords[] = md5(md5($_POST['passwrd']) . $user_settings['password_salt']);
@@ -336,7 +336,8 @@ function Login2()
$other_passwords[] = sha1(strtolower($user_settings['member_name']) . un_htmlspecialchars($_POST['passwrd']));
// BurningBoard3 style of hashing.
- $other_passwords[] = sha1($user_settings['password_salt'] . sha1($user_settings['password_salt'] . sha1($_POST['passwrd'])));
+ if (!empty($modSettings['enable_password_conversion']))

that one seems not necessary. Am I wrong?

@jdarwood007 Simple Machines member

I didn't delete it, it was indented. Burning board is another software so it should follow the same exclusions. Only SMF passwords from 1.0 (md5), linux-bugged sha1 and pre-htmlchars fix should be tested.

Yups, sorry, I missed one of the IFs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $other_passwords[] = sha1($user_settings['password_salt'] . sha1($user_settings['password_salt'] . sha1($_POST['passwrd'])));
// Perhaps we converted to UTF-8 and have a valid password being hashed differently.
if ($context['character_set'] == 'utf8' && !empty($modSettings['previousCharacterSet']) && $modSettings['previousCharacterSet'] != 'utf8')
View
1 Sources/ManageSettings.php
@@ -561,6 +561,7 @@ function ModifyGeneralSecuritySettings($return_config = false)
'',
// Password strength.
array('select', 'password_strength', array($txt['setting_password_strength_low'], $txt['setting_password_strength_medium'], $txt['setting_password_strength_high'])),
+ array('check', 'enable_password_conversion'),
'',
// Reporting of personal messages?
array('check', 'enableReportPM'),
View
1 Themes/default/languages/Help.english.php
@@ -461,6 +461,7 @@
<li><strong>Medium:</strong> The password must be at least eight characters long, and can not be part of a users 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>
</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 would need to reset their password.';
$helptxt['coppaAge'] = 'The value specified in this box will determine the minimum age that new members must be to be granted immediate access to the forums.
On registration they will be prompted to confirm whether they are over this age, and if not will either have their application rejected or suspended awaiting parental approval - dependant on the type of restriction chosen.
View
1 Themes/default/languages/ManageSettings.english.php
@@ -156,6 +156,7 @@
$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_enable_password_conversion'] = 'Allow password hash conversion';
$txt['antispam_Settings'] = 'Anti-Spam Verification';
$txt['antispam_Settings_desc'] = 'This section allows you to setup verification checks to ensure the user is a human (and not a bot), and tweak how and where these apply.';
Something went wrong with that request. Please try again.