Page MenuHomePhabricator

Allow different password requirements by group
Closed, ResolvedPublic

Description

I am suggesting this primarily for Extension:SecurePasswords (which is not listed as a component). However, it could conceivably be implemented in core (at least the length part), or another extension.

Allow setting different requirements for different user groups (admin/sysop, buearuacrat, steward, etc.). This allows making the requirements for users with elevated rights more stringent than for just editors.

This was discussed in the now-closed bug 16435.


Version: master
Severity: enhancement
See Also:
T18435: New extension to enforce minimum password strength.
T27925: Increase $wgMinimalPasswordLength

Details

Reference
bz44788

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:13 AM
bzimport set Reference to bz44788.
bzimport added a subscriber: Unknown Object (MLST).

So when being granted specific privs/group membership, users will be required to change their password to match the required strength level before they can effectively use any of their new privileges?

(In reply to comment #1)

So when being granted specific privs/group membership, users will be required
to change their password to match the required strength level before they can
effectively use any of their new privileges?

I imagine the password requirement(s) would be effective on the next log in, not prior to the new privileges being effective.

Yeah, I think MZM's suggestion is simpler (less code to hook in everywhere).

Either:

  1. Make them change it immediately after login before doing anything.
  2. Block login entirely if it's a bad password, which would effectively require the password change before the group change (which the community/bureaucrat would need to clearly communicate). If they didn't handle this ahead of time, it would require a reset.

I'm hesitant about putting such a feature into Extension:SecurePasswords, primarily because I would never in my life advocate the deployment of this extension onto WMF wikis.

Sure the idea of better password requirements is nice, but look at the rest of the extension. It uses what is quite possibly (no offense to the author) the most unnecessarily complicated password hashing scheme I have ever seen. And I'll explain why step-by-step:

  • Using a different hash based on the user ID - all this does is require an attacker to change hashes for each user. It has the same security implication as a salt. In any realistic offline attack, changing hash algorithms would be trivial, especially since the algorithm to calculate which hash to use is public and uses non-secret information.
  • Using three different password secret keys - if the attacker gains access to the wiki configuration, they'll have access to all three keys. More keys does not make things better.
  • Compressing the password hash - The hash is mostly random, so this would have no effect at the expense of reducing performance and requiring a new PHP extension dependency.
  • Using mcrypt to encrypt the password hash - A better solution would be to just HMAC the password hash with the secret keys....but it already does that. Encrypted or unencrypted, an attacker needs access to the secret keys to even begin brute forcing a hash, so encrypting it with a third key that the attacker already has access to is useless.

And the most important lesson out of all of this is that it completely replaces the current password hashing method without any method for replacing it should a better hashing scheme come along (and it already has).

I would either make a new extension specifically for password strength or wait for Daniel's password hashing API to be merged and then for this extension to be changed to use that API.

(In reply to comment #4)

I would either make a new extension specifically for password strength or
wait
for Daniel's password hashing API to be merged and then for this extension to
be changed to use that API.

The new password system has nothing to do with the front end of passwords (except for adding a new error message to distinguish "The password you entered is invalid" from "The hash attached to this user is corrupt.

Changing the way password strength requirements work can be done without waiting for the password hashing system. The conditionals don't belong inside the classes handling storage format. There should be no conflicts implementing password restrictions in the proper places.

(In reply to comment #5)

(In reply to comment #4)

I would either make a new extension specifically for password strength or
wait
for Daniel's password hashing API to be merged and then for this extension to
be changed to use that API.

The new password system has nothing to do with the front end of passwords
(except for adding a new error message to distinguish "The password you
entered
is invalid" from "The hash attached to this user is corrupt.

Changing the way password strength requirements work can be done without
waiting for the password hashing system. The conditionals don't belong inside
the classes handling storage format. There should be no conflicts
implementing
password restrictions in the proper places.

I agree completely. That's why I was saying not to use this extension, because it mixes both front end and back end, and if deployed, would make using the new back end system impossible since it returns false on a hook.

Can this task be marked resolved?

I realize that this task is technically attached to the SecurePasswords extension, but the functionality as requested in the task description has been implemented. Is there any value in putting this logic into the SecurePasswords extension as well? If not, I think we can call this task resolved.

Tgr claimed this task.
Tgr updated the task description. (Show Details)
Tgr set Security to None.