Posted by smoovb on January 13, 2012 at 9:02pm
4 followers
Jump to:
| Project: | Password policy |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (duplicate) |
Issue Summary
I work in the government sector and we had originally written our own module to handle the departments password policy until we came across this module. It allows us to enforce all the constraints necessary for user password compliance.
Where it falls short is in the use of MD5 for hashes. Our policy also contains a development constraint that no new projects shall use MD5. To meet this requirement I have made changes to the password_policy module to add SHA256 support. A password salt was also added due to concerns of the users entire password history being available in the web page. The changes were worked into the dev version and the patch is added below.
Bill
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| password_policy-sha256salt-1.patch | 12.72 KB | Ignored: Check issue status. | None | None |
Comments
#1
I have exactly the same concern. It seemed a bit pointless to install http://drupal.org/project/phpass and then have this module bring everything back down to the md5 level.
#2
I don't think we should be integrating an external library to do this. Instead shouldn't we just check for the phpass module and then use their user_hash_password() function if it exists? That seems much simpler. In other words I don't think we should be deciding on our own to put our password hashes into one format or another, we should be mimicking whatever is used for the main {users} table. This should satisfy any security requirements and be a straightforward implementation.
#3
More in-depth discussion here - #598424: Secure Password Hashes (phpass) and Password module integration
#4
erikwebb,
For some it might make sense to check what password hash is being used by Drupal in the {users} table and then match that in the Password Policy module, but for others that are using an external source to store the passwords, like an LDAP server which is what we do, it doesn't. Which is why leaving it as an option selection in this module is important for us and perhaps others.
As for using an external JavaScript library to check previous password hashes against the current one being set by the user on the client side, the Password Policy module already does this for the MD5 hash type. The MD5 library in constraints/scripts is from http://www.webtoolkit.info/ and the SHA256 library we've included in this patch is from the same source.
Obviously there is a security risk in loading the previous password hashes into the HTML for comparison as discussed in #598424: Secure Password Hashes (phpass) and Password module integration, but that risk is minimized a bit by this patch as a result of using SHA256 plus a salt. Replacing this functionality with AJAX calls is an awesome idea and would be more secure but it looks like that part of the discussion has stalled.
Discussions in #598424: Secure Password Hashes (phpass) and Password module integration about how best to include more secure hashing methods and password history comparison have gone in so many directions and are now stalled it doesn't seem likely to ever be finished. Password Policy module co-maintainer deekayen even stated he would not support the use of phpass in connection with Password Policy.
Please re-consider including this patch in the current 6.x-1.x branch of the module so people can take advantage of the additional security it offers today. Perhaps the ideas discussed in #598424: Secure Password Hashes (phpass) and Password module integration could be added in a new 6.x-2.x branch?
Thanks.
Disclaimer: I work with smoovb and am obviously biased.
#5
erik makes a good point, using the password handling for whatever other module you're using to modify the hashes in the user table for Password Policy makes a lot of sense.
that said, the wisdom behind the default behaviour of this module using md5 hashes is questionable.
i think that you're both right, we should check for the existence of the Secure Password Hashes module and use that, but if it doesn't exist we should use something stronger than md5 as a fallback.
#6
I think we have to rely on another module's implementation. If LDAP passwords are being used instead, but you also have some local user accounts in Drupal, then I'm not sure what kinds of strange issues will arise from mixing password hashes.
I agree completely in the need and that assuming md5 hashes is poor, but we need to make sure we aren't introducing any behavior that's unique to this module.
#7
#598424: Secure Password Hashes (phpass) and Password module integration