Admin user unable to login after installation - salt issue

Bug #676461 reported by Andrew Nicols
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Critical
Andrew Nicols

Bug Description

During installation, the final step is typically to set the admin password. At present on master this isn't happening and the admin user is unable to login.
Looks like this is caused by a change in e9151dd85cabeb527b7a3dc6d03699e9bb949634 to auth/internal/lib.php to the salt system.

When the admin user is created, it's not created using an Auth object, but by inserting directly into the database. The password is set in plaintext and without salt. The installation then seesm to log in the admin user with that plaintext password.
The salt change in the above commit removes the ability for users to log in using a plaintext password if there's no salt.

I can see two ways forward:
* revert the salt change; or
* salt the admin password.

I've provided a fix for the latter in the attached file on the assumption that the salt was changed for a reason that I'm not privy to, but I haven't applied it to master in case the preferred option is to revert the salt change.

Rather than using auth/lib.php:encrypt_password() (which just returns sha1($salt . $password)), I've just done the sha1. This is to avoid having to include auth/lib.php just for one function. Obviously, this would break if salting methodology changed in the future.

Revision history for this message
Andrew Nicols (dobedobedoh) wrote :
Revision history for this message
Richard Mansfield (richard-mansfield) wrote :

Hi Andrew,

Your patch looks like the right thing to do; please commit it. It's a bit embarrassing we didn't find that problem in testing!

We actually decided that those salt patches didn't need to be private, but forgot to change the bug to public, but I'll do that now:

https://bugs.launchpad.net/mahara/+bug/662424

visibility: private → public
Changed in mahara:
status: New → Confirmed
assignee: nobody → Andrew Nicols (dobedobedoh)
status: Confirmed → Fix Committed
Revision history for this message
Andrew Nicols (dobedobedoh) wrote :

Hi Richard,

Cheers - that sheds a bit of light on things! I've pushed my fix to master.

Andrew

Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.