Lack of interest vB4: More secure handling of legacy password schemes

[Steffen]

The password hashing used by vB4 isn't very secure by today's standards. XenForo automatically upgrades the passwords of imported users when they login for the first time which is great. But that leaves the question of what to do with those users who have left your community and will never login again. Depending on how old your forum is that could be thousands of users. It would be nice if we could upgrade their password security, too.

Now, we obviously don't know their plaintext password and therefore cannot use XenForo's native Core12 password scheme. But we can implement a two-step process that uses the stored vB4 password hash as the input of a secure password hashing function. This sounds way more complicated than it is so I'll let code speak.

diff --git a/src/XF/Authentication/vBulletinPlus.php b/src/XF/Authentication/vBulletinPlus.php
new file mode 100644
index 000000000..c6bf46052
--- /dev/null
+++ b/src/XF/Authentication/vBulletinPlus.php
@@ -0,0 +1,32 @@
+<?php
+
+namespace XF\Authentication;
+
+class vBulletinPlus extends Core12
+{
+    public function generate($password)
+    {
+        throw new \LogicException('Cannot generate authentication for this type.');
+    }
+
+    public function authenticate($userId, $password)
+    {
+        if (!is_string($password) || $password === '' || empty($this->data))
+        {
+            return false;
+        }
+
+        $intermediateHash = md5(md5($password) . $this->data['salt']);
+        return parent::authenticate($userId, $intermediateHash);
+    }
+
+    public function isUpgradable()
+    {
+        return true;
+    }
+
+    public function getAuthenticationName()
+    {
+        return 'XF:vBulletinPlus';
+    }
+}
diff --git a/src/addons/XFI/Import/Importer/vBulletin.php b/src/addons/XFI/Import/Importer/vBulletin.php
index 994cedf18..e0ec2f649 100644
--- a/src/addons/XFI/Import/Importer/vBulletin.php
+++ b/src/addons/XFI/Import/Importer/vBulletin.php
@@ -1438,8 +1438,8 @@ class vBulletin extends AbstractForumImporter
      */
     protected function setUserAuthData(\XF\Import\Data\User $import, array $user)
     {
-        $import->setPasswordData('XF:vBulletin', [
-            'hash' => $user['password'],
+        $import->setPasswordData('XF:vBulletinPlus', [
+            'hash' => password_hash($user['password'], PASSWORD_BCRYPT),
             'salt' => $user['salt']
         ]);

A downside is that it slows down the importer because password_hash by design is a slow function. So maybe it would be better to upgrade passwords later on. But I think the concept is sound, the code works fine for me and should improve the security of imported vB4 passwords a lot.

What do you think?

 

[Steffen]

Just found out that this approach is called "hash chaining". Specifically, chaining MD5 and bcrypt is considered to be fine because MD5 preserves enough entropy: https://security.stackexchange.com/...assword-db-off-md5-by-chaining-md5-and-bcrypt

(So I think a better name for this password scheme would be "vBulletinChain".)

 

[Kier]

I must confess, I'm not sure what the intention is here? We change the auth scheme of imported users once they log in for the first time simply because it's more convenient, but not necessarily because the XF hashing scheme is inherently better than the imported one.

What does further hashing attempt to prevent - stolen hashes being converted into plaintext passwords, or brute-force logins?

 

[Steffen]

Correct, the intention is to prevent stolen hashes being converted into plaintext passwords.

 

[Steffen]

But probably this is a feature suggestion and not a bug report. Maybe move it to the appropriate forum.