Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Would be a good check to compare a users username to the password making sure they don't equal each other
Steps to reproduce
NA
Proposed resolution
Compare username to the password to see if they equal
Remaining tasks
Review
User interface changes
NA
API changes
NA
Data model changes
NA
Original post
We could easily include a few weak password tests like:
1. Commonly used password lists for the top 100, or 1,000 most commonly used passwords.
2. Whether it's the same as their username :(
Comment | File | Size | Author |
---|---|---|---|
#30 | 830970-30.patch | 3.9 KB | smustgrave |
#15 | security_review-name_password_check-830970-14-8.patch | 3.59 KB | tlyngej |
#10 | security_review-name-passwords-830970-10.patch | 3.73 KB | dsnopek |
Issue fork security_review-830970
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
gregglesIdeally you could combine this with the list of "trusted roles" to find users with bad passwords and advanced permissions.
Comment #2
gregglesHere's a query to do that:
It gives you results like:
So, if it's a count of zero you know that the user doesn't have one of the advanced roles.
Comment #3
coltraneAttached patch implements greggles' check for users with certain roles whose passwords are their username. It needs work because it needs more code comments, documentation, and expanded integration with forthcoming rainbow table check.
Comment #4
gregglesThis all looks awesome to me.
One other thought I had is that theoretically it's possible for UID to not have any roles, still have full permissions of course, and have a weak password. I think uid 1 should be special cased in the processing here.
I also fixed a variable typo and a typo in the message for IP's with lots of failed logins.
Comment #5
jleinenbach CreditAttribution: jleinenbach commentedAs an idea, bad passwords could be tested by checking against a cracked hashes list, downloadable e. g. here:
http://opencrack.hashkiller.com/
Comment #6
greggles@jleinbach, I generally like that idea, but we want to keep the security_review module download relatively small. My current thinking is to use a really small list of popular passwords and provide a way for sites to use an alternate set of passwords for a more complete check.
This particular issue has kind of become about checking username against username-as-password. I think we should change the UI so it is really about just that and create a new issue about how to do checking of commonly used passwords: #897960: test password strength against an extensible list of passwords that defaults to really common ones.
Comment #7
coltraneSmall reroll, I only added some comments.
Comment #8
coltraneUpdate for new check help and modified UI to only talk about usernames as passwords.
Comment #9
coltraneCommitted http://drupalcode.org/project/security_review.git/commit/0b90323 for 6.x-1.x
Port to Drupal 7 will require hashing in PHP but it's still a valuable check.
Comment #10
dsnopekAttached is a patch that ports this check to Drupal 7! A bunch of the D6 code was present in the D7 branch, so rather than adding it new, it modifies what's there.
Comment #12
dsnopekCommitted patch to 7.x-1.x! Needs to be ported to Drupal 8.
Comment #13
tlyngej CreditAttribution: tlyngej at Annertech commentedI have ported this patch to version 8.1.
Basically just added a new Check extending class (
Drupal\security_review\Checks\NamePasswords
) and amended thesecurity_review_security_review_checks()
function insecurity_review.module
.Comment #15
tlyngej CreditAttribution: tlyngej at Annertech commentedAnd, hey, why not submit a patch with content!
Comment #16
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #17
tlyngej CreditAttribution: tlyngej at Annertech commentedNew patch, where the displayed list of users are properly escaped.
Comment #18
tlyngej CreditAttribution: tlyngej at Annertech commentedComment #19
vuilRe-roll is needed.
Comment #20
visabhishek CreditAttribution: visabhishek as a volunteer and for Melity commentedJust re-rolled the patch security_review-name_password_check-830970-17-8.patch
Comment #21
smustgrave CreditAttribution: smustgrave commentedAs much as I like this idea is that out of the realm for this module? Shouldn't that fall under https://www.drupal.org/project/password_policy
Will let one of the maintainers to decide but not sure if it should be added.
Comment #22
gregglescoltrane, dsnopek and I are all maintainers who did work on this to bring it to the module.
I think password_policy should keep a site safe for new password changes, but this module gives a sense of whether a site has a problem regardless of when the problem was introduced.
If password_policy has a feature like this I would definitely be willing to not add this to Drupal 8.
I also don't see it as a top priority. If you aren't motivated by the idea you don't have to work on it :)
Comment #23
smustgrave CreditAttribution: smustgrave commentedSince the work has already been done don't see any harm. Just needs to be updated.
getUsername() has been deprecated and removed.
Comment #24
asad_ahmed CreditAttribution: asad_ahmed at OpenSense Labs commentedMade changes as per #23, replaced deprecated getUsername() with getDisplayname(). Thanks
Comment #25
smustgrave CreditAttribution: smustgrave commentedPatch #24 removes code from #20 for the module file.
Also when viewing the details this error is thrown Warning: foreach() argument must be of type array|object, string given in template_preprocess_item_list() (line 1147 of core/includes/theme.inc).
Comment #26
smustgrave CreditAttribution: smustgrave commentedReverted back to #20. Fixed the getUsername issue and error reported in #25
Comment #27
smustgrave CreditAttribution: smustgrave commentedComment #28
smustgrave CreditAttribution: smustgrave commentedReroll.
Comment #30
smustgrave CreditAttribution: smustgrave commentedComment #31
smustgrave CreditAttribution: smustgrave commentedComment #32
smustgrave CreditAttribution: smustgrave commentedComment #34
ednark CreditAttribution: ednark at Mobomo commentedI have tested this and it functions as expected without errors.
Tested on 10.1.x-dev with a standard install
module drupal/security_review:2.0.x-dev@dev
branch 830970-test-password-strength
patch applied https://www.drupal.org/files/issues/2023-01-13/830970-30.patch
Test procedure:
Create new User
Set Username to TEST and password to TEST
rerun security_review tests
see failure for "Username same as password"
goto details and see the TEST user listed as the problem
Comment #37
smustgrave CreditAttribution: smustgrave commentedComment #38
vuil