Expand zxcvbn user_input blacklist

[iandunn]

The current blacklist only contains the username, but there are other known data about the current user/site that we should discourage using in passwords, because they'll lower the entropy.

I've attached a rough first pass. It needs more work, but I'd like to get some feedback.

• There's probably a better location for zxcvbn_user_input_blacklist()
• Are there performance concerns with zxcvbn_user_input_blacklist() ? There are a lot of function calls and processing, and there may be a more elegant ways to get the same results.
• Any more suggestions for additional generic words to blacklist?
• Are there any security/privacy issues, since all of the data returned by zxcvbn_user_input_blacklist() will be revealed in the page source? Probably not in the typical usage, since it's only shown on user-edit.php (and therefore is already behind a current_user_can() check). There could be issues if it were (mis)used by plugins, though.
• Any other issues?

Note that there's currently a bug in the zxcvbn implementation where ​user_input is being ignored, so this patch won't actually affect the returned score until Jon's latest patch is committed.

[nacin]

Could most of this (for the user being edited or created) simply be collected via JS?

[iandunn]

Yeah, and that'd have an extra benefit of making sure we're blacklisting the most current values for those fields, in cases where the user updates usermeta at same time as the password.

We could also get the site url and title from the DOM, but we'd miss the following items:

• Site description
• admin_email
• If the current user is editing another user, we wouldn't have the current user's:
  1. user_login,
  2. user_nicename
  3. user_email
  4. user_url
  5. first_name
  6. last_name
  7. description

So, is it worth the tradeoff? I'd personally err on the side of making the entropy score more accurate, even at the expense of a little bit of performance. Passwords are possibly the weakest link in the security chain, so educating users about what makes one strong is very important; and since changing a password is an infrequent occurrence, the performance impact won't be felt very often (if it's even noticeable). I can see the other side, though.

Or should we go with a hybrid approach? The PHP side could do the minimal amount of work to collect the data that only it can get, then pass that off to the client side. Then a JS function could add in all of the data that it has access to, and do all of the processing to clean up the array before it gets used. That way we'd get both the previous usermeta values, and the current ones.

[nacin]

I think we could do this entirely via JS, by building an array using not only the current value of the inputs, but also the original value of them, using defaultValue.

Then all we'd need to worry about is passing the current user's information to JS. No security concerns there.

home URL and site title are fine, but I would probably ignore admin_email.

Let's try to clean up a lot of the todo's here.

[nacin]

Realistically, let's work from the inside out. So, let's start with using not only the username of the user being edited, but also other information about that user, both defaultValue and current values. This should be 100% JS, I think. That'd be a good start no matter what, and it'd be committable. Anything else after that is just a bonus.

[iandunn]

25174.3.diff is an all-JS solution. I haven't tested thoroughly yet, but I think it's most of the way there. It works fine on the current user's profile, another user's profile, the install, and the unit tests.

The returned array ends up with some extra words like "profile", "installation", "admin", etc, but I don't think filtering them out is really necessarily. It would make the code a bit more complex and slower, and those words are gonna be heavily penalized by the dictionary checks anyway.

Note that ​zxcvbn's user_input check is case-sensitive, which significantly reduces the effectiveness of the blacklist. It's still a worthwhile improvement, though.

[nacin]

25174.4.diff​ introduces wp.passwordStrength to collect these two methods. Passes the tests.

[iandunn]

25174.5.diff has a few small changes:

• added unit test for userInputBlacklist()
• switched to using document.title since ​$( 'title' ).text() doesn't always work in IE
• a few minor formatting tweaks

I ran the unit tests on bunch of browsers/devices and found no problems. I also did some manual testing on various browsers without incident. So, I think it's good to commit.

[nacin]

In 25637:

Expand the zxcvbn password meter blacklist, based on user input.

props iandunn.
see #25174.

[nacin]

I had left this open for review and further testing, but things seem good.