Page MenuHomePhabricator

Enlarge Popular Password File to 100,000 entries and enforce the new minimum in the config
Closed, ResolvedPublic

Description

https://nakedsecurity.sophos.com/2016/08/18/nists-new-password-rules-what-you-need-to-know/ suggests to use a list of 100,000 popular passwords to blacklist...

MW has 10,000. We should improve on this

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 414602 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/vendor@master] Add wikimedia/password-blacklist 0.1.1

https://gerrit.wikimedia.org/r/414602

Change 414603 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] [WIP] Add PasswordPolicy to check the password isn't in larger blacklist

https://gerrit.wikimedia.org/r/414603

Tgr awarded a token.Feb 28 2018, 6:40 AM
aezell added a subscriber: aezell.Oct 29 2018, 5:07 PM
Restricted Application added a subscriber: MGChecker. ยท View Herald TranscriptOct 31 2018, 4:48 PM
TBolliger moved this task from Untriaged to Backlog on the Anti-Harassment board.Oct 31 2018, 4:48 PM

Change 414602 merged by jenkins-bot:
[mediawiki/vendor@master] Add wikimedia/password-blacklist 0.1.3

https://gerrit.wikimedia.org/r/414602

Change 414603 merged by jenkins-bot:
[mediawiki/core@master] Add PasswordPolicy to check the password isn't in the large blacklist

https://gerrit.wikimedia.org/r/414603

Does the patch above just increase the amount of passwords in the library, or does it also enforce 100,000 on account creation?

Does the patch above just increase the amount of passwords in the library, or does it also enforce 100,000 on account creation?

Mostly the former. Depending on config, it should do the latter

Not sure we want to do it default in MW yet (maybe we do?) but should just need one line or mw-config for prod

OK, thank you Reedy!

This appears to be live on production for admins only: https://en.wikipedia.org/wiki/Special:PasswordPolicies

We (Security or AHT, I don't care who) needs to update the minimum for Users from 100 to 100,000 in the config. AHT starts our next sprint in 2 days, if I don't hear back we'll take this on.

TBolliger renamed this task from Enlarge Popular Password File to 100,000 entries to Enlarge Popular Password File to 100,000 entries and enforce the new minimum in the config.Dec 3 2018, 6:47 PM
Reedy added a comment.Dec 3 2018, 7:02 PM

This appears to be live on production for admins only: https://en.wikipedia.org/wiki/Special:PasswordPolicies

No it's not, it's there for bots, interface admins, 'crats

Condensed password policies only showing this policy and their state:

	'policies' => [
		'bureaucrat' => [
			'PasswordNotInLargeBlacklist' => true,
		],
		'sysop' => [
			'PasswordNotInLargeBlacklist' => true,
		],
		'interface-admin' => [
			'PasswordNotInLargeBlacklist' => true,
		],
		'bot' => [
			'PasswordNotInLargeBlacklist' => true,
		],
		'default' => [
			'PasswordNotInLargeBlacklist' => false,
		],
	],

Need to decide if we want to make this true by default for all users in MediaWiki. I wasn't overly sure, so in DefaultSettings.php I didn't enable it for all users

It's either a 1 line change in DefaultSettings.php or wmf-config

I'm fairly confident the plan is for all new passwords to be outside 100,000. This is based on this permissioned Google Doc authored by @JBennett. It was last edited Nov. 1 so other decisions/discussions may supercede this.

Ping @JBennett! ๐Ÿ›Ž

Reedy added a comment.Dec 4 2018, 8:30 AM

I'm fairly confident the plan is for all new passwords to be outside 100,000. This is based on this permissioned Google Doc authored by @JBennett. It was last edited Nov. 1 so other decisions/discussions may supercede this.
Ping @JBennett! ๐Ÿ›Ž

Well, WMF != MW. But if the intention is to make these changes for all users and help "harden" MW core at the same time, I'm fine with making that change in MW core too

Change 477487 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Prevent all users from having a password in the blacklist

https://gerrit.wikimedia.org/r/477487

aezell added a comment.Dec 4 2018, 3:58 PM

Is there community input required to +2 @Reedy's patch for MW core? If so, we should start that now. I guess that would happen on Meta?

Even if the patch above is delayed, we should make the change in wmf-config as soon as possible.

I'm fairly confident the plan is for all new passwords to be outside 100,000. This is based on this permissioned Google Doc authored by @JBennett. It was last edited Nov. 1 so other decisions/discussions may supercede this.
Ping @JBennett! ๐Ÿ›Ž

Well, WMF != MW. But if the intention is to make these changes for all users and help "harden" MW core at the same time, I'm fine with making that change in MW core too

The goal of the passwd changes were really targeting WMF but I agree MW would benefit. If we can do both w/out delaying WMF changes then that sounds great.

Is there community input required to +2 @Reedy's patch for MW core? If so, we should start that now. I guess that would happen on Meta?
Even if the patch above is delayed, we should make the change in wmf-config as soon as possible.

Aren't major changes to MW the responsibility of TechCom? I wouldn't think this is major enough to go through TechCom as it can be overridden on a local install level, correct?

Wikimedia Community discussions for this change are being handled in T205931

@aezell: (For general understanding, this list might not be complete:)
There are RfC's, and TechCom-RFC s for architecture which is TechCom. For broader community consultation (more than a User-notice heads-up announcement in Tech News about some changes to take place) see non-public office:Community Relations for specialist support.
And mentioning for completeness only: For config changes per wiki (which require per-community consensus) see mw:Requesting_wiki_configuration_changes.

Thanks @Aklapper. I'm still learning about the layers of communication that exist around here.

Reedy added a comment.Dec 5 2018, 5:24 AM

Making MW have sane defaults and follow best practices doesn't/shouldn't really need much approval. It's trivially easy to disable it people so desire

I do note that changing it in MW core will mean it is default enabled on WMF wikis when it gets deployed unless we make a mw-config patch to disable it until "support" (or notification has occured) is in place for rolling it out to everyone. We didn't have this support/notification (AFAIK) for admins et al, but that's a smaller group

For the general public, it's definitely going to be a more noticeable change

John and @CKoerner_WMF are working on public facing communication, aiming for Dec. 13.

Yep! I'll have a message going out Thursday, Dec 6. More info: T205931#4795569

Change 479574 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[operations/mediawiki-config@master] Require that passwords are not in the most common 100k list for all users

https://gerrit.wikimedia.org/r/479574

Tgr added a comment.Mar 14 2019, 9:12 PM

Do we have a timeline for this? PasswordCannotBePopular has been deprecated in core and our using of it is spamming the logs. If we don't plan to do this soon, we should probably revert or temporarily disable the deprecation.

The public password policy states that passwords must not be from the top 100k passwords, and Chris already informed a lot of large channels about this change: T205931

I think we're good to merge in the coming weeks. I would suggest you wait until T211621 and T211622 are merged and deployed, just to be safe.

@Tgr The original patch that's included in this task has a comment from you which points to this patch: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/477487

The two tasks that @TBolliger points too are a bug fix for and an improvement on the original work we did to enforce longer passwords. None of that work, at least to my eye, really touches this popular password blacklist.

I think the work for popular passwords and password length, while near each other in the code, likely don't touch much of the same code. Could we make a patch that removes our usage of the deprecated method in favor of the new one that is separate from both of these concerns?

Tgr added a comment.Mar 15 2019, 9:27 PM

I think the work for popular passwords and password length, while near each other in the code, likely don't touch much of the same code.

There are three changes:

  1. Increase password length to 8. This is a trivial config change.
  2. Replace the old, smaller password blacklist with the new, larger one. This is a trivial config change (plus a lot of work to provide the new list, which is what most of this task was about, but that has concluded several months ago).
  3. Exempt non-privileged users from getting login warnings due to the new changes. This is what most of the recent work was about.

1 depends on 3, since we want to minimize impact on existing users. 2 probably also does for the same reason, although I don't think there was any discussion on that. 1 and 2 are unrelated, except doing them at the same time might be nicer in terms of messaging / user confusion (but not a big deal either way).

Could we make a patch that removes our usage of the deprecated method in favor of the new one that is separate from both of these concerns?

The current patch already is separate as far as I can see, although we might want to change it to have suggestChangeOnLogin => true for the default policy group (and false for the others), per above.

although we might want to change it to have suggestChangeOnLogin => true for the default policy group (and false for the others), per above.

Except 477487 is a core patch... Let's have a new task for core requirements to reduce confusion: T218449: Determine new password requirements for MediaWiki core

T211550: Password length check should count unicode characters is another (minor) thing that we should fix at some point and it will make some valid passwords invalid, so we might as well do it in the same batch.

Change 479574 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[operations/mediawiki-config@master] Require that passwords are not in the most common 100k list for all users

https://gerrit.wikimedia.org/r/479574

Reedy added a comment.EditedAug 27 2019, 5:07 PM

Do we need anyone to sign off on this.. Community Tech? or AHT? Or can we (I, James, or someone else) just deploy James' patch? Ping @Niharika :)

This helps fix T231360 and means we can deprecate it properly in core (to be removed at a later date)

Change 479574 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[operations/mediawiki-config@master] Require that passwords are not in the most common 100k list for all users
https://gerrit.wikimedia.org/r/479574

Then we need to do some comms support, get it in tech news (ala User-notice) and let people know it's gonna happen

@Reedy AHT were working on this on behalf of the Security team. They'd be better to talk to about this than us as they understood the implications of making this change when they asked us to do it.

Reedy added a comment.Aug 27 2019, 6:01 PM

@Reedy AHT were working on this on behalf of the Security team. They'd be better to talk to about this than us as they understood the implications of making this change when they asked us to do it.

Sooo...

I'm fairly confident the plan is for all new passwords to be outside 100,000. This is based on this permissioned Google Doc authored by @JBennett. It was last edited Nov. 1 so other decisions/discussions may supercede this.
Ping @JBennett! ๐Ÿ›Ž

^ Looking back at that document, it was the intention to roll this out for all "regular" users too....

So IMHO, this really just needs the User-notice, chose a point in time to deploy and then we can go ahead with it?

Yeah. Let's pick next week and slap in Tech/News now?

Reedy added a comment.Aug 27 2019, 6:14 PM

Yeah. Let's pick next week and slap in Tech/News now?

I think that works for me, if we can get it out?

Reedy added a comment.Aug 27 2019, 6:43 PM

Yeah. Let's pick next week and slap in Tech/News now?

I think that works for me, if we can get it out?

Looks like if we get it out/written up for Friday, we should be all good?

It's tagged with User-notice and in the right column before Wednesday, as per the docs

Reedy added a comment.Aug 27 2019, 6:51 PM

Looks alright to me. Do we need to be any more specific? Giving numbers? Or to the reference list of words? (The latter seems overkill)

The most important bit is the link to the policy (which says we already enforce "not in the top 100k", ahem).

Change 479574 merged by jenkins-bot:
[operations/mediawiki-config@master] Require that passwords are not in the most common 100k list for all users

https://gerrit.wikimedia.org/r/479574

Change 477487 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Prevent all users from having a password in the blacklist

https://gerrit.wikimedia.org/r/477487

Reedy added a comment.Sep 5 2019, 11:07 PM

Change 477487 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Prevent all users from having a password in the blacklist
https://gerrit.wikimedia.org/r/477487

We should decide what to do about this one (ie should we enable it in mw core)... Especially with T232024: Branch REL1_34 for MediaWiki and deployed extensions coming up soon. It's trivially disable-able

I suspect the only thing that patch is some RELEASE-NOTES to go with it

Mentioned in SAL (#wikimedia-operations) [2019-09-05T23:09:13Z] <jforrester@deploy1001> Synchronized wmf-config/CommonSettings.php: T151425 Require that passwords are not in the most common 100k list for all users (duration: 00m 48s)

Jdforrester-WMF closed this task as Resolved.Sep 5 2019, 11:10 PM
Jdforrester-WMF claimed this task.

Done in prod.

Change 477487 merged by jenkins-bot:
[mediawiki/core@master] Prevent all users from having a password in the blacklist

https://gerrit.wikimedia.org/r/477487