WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#21737 closed task (blessed) (fixed)

Users should have to jump through hoops to set passwords of their choosing, and we should guard better against weak passwords

Reported by: markjaquith Owned by: westi
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch
Focuses: Cc:
PR Number:

Description

People are terrible at choosing secure, unique, complex, unguessable passwords. Unless someone is using a password storage system, the chances are good that the passwords they're choosing are really weak.

We can mitigate this problem.

  1. Let's make the default to always be that WordPress picks a password for you. When installing WordPress, or when creating a new user account, or when changing your password on your profile. The default should be that we generate a secure password for the user. They can remember it, write it down (not ideal, but generally more secure than choosing a weak password), or copy and use it once, check the "remember me" box, and not worry about it until their cookie expires on that computer.
  1. If they do opt to manually create a password, we need to do better than our current password strength meter. And the lowest level should actually nag them with an AYS before they proceed. I suggest the following, to start, which would trigger the lowest level, and cause them to have to dismiss a warning (or check a checkbox... UI TBD) before continuing:
  • compare the strtolower'd version of their password to strtolower'd versions of all their info (username, first/last name, part of e-mail address before the @, etc).
  • any password that is shorter than 8 characters
  • a blacklist of popular passwords (these lists are available... even grabbing the top 100 would give use good coverage)
  • 3 or more consecutive digits ("123456" and company are very popular)
  • anything that looks like a date

Attachments (7)

lss.apple.password.indicator.png (70.6 KB) - added by convissor 7 years ago.
apple id's password indicator is a checklist
21737.zxcvbn-rot13.diff (1.2 KB) - added by duck_ 6 years ago.
21737.zxcvbn.diff (685.8 KB) - added by duck_ 6 years ago.
21737.zxcvbn-rot13.2.diff (1.5 KB) - added by duck_ 6 years ago.
21737.zxcvbn.2.diff (685.9 KB) - added by duck_ 6 years ago.
21737.zxcvbn-rot13.3.diff (2.0 KB) - added by duck_ 6 years ago.
21737.zxcvbn-user_inputs.diff (735.2 KB) - added by duck_ 6 years ago.

Change History (53)

#1 @ericlewis
7 years ago

Related:
#8296 (Auto generate password for new user)
and #18399 (Password Strength Meter should usually mark passwords that contain password as weak)

#2 @DrewAPicture
7 years ago

  • Cc xoodrew@… added

#3 @bananastalktome
7 years ago

  • Cc bananastalktome@… added

#4 @convissor
7 years ago

  • Cc danielc@… added

These points, and more, are covered by my Login Security Solution plugin. The plugin has full UTF-8 support and unit tests. Feel free to use the code or concepts therein for this project. Let me know if you need help.

@convissor
7 years ago

apple id's password indicator is a checklist

#5 @convissor
7 years ago

The image of the Apple ID password strength indicator, attached above, is a good direction to head in. It provides a list of the requirements and tells you when each one is met.

#6 @convissor
7 years ago

Some more points about the approach taken in the Login Security Solution...

It permits passwords to be in any alphabet supported by UTF-8. It distinguishes between letters (and the case thereof), numbers and punctuation in all of the scripts (alphabets). The plugin's requirement for upper and lower case letters in a password is skipped for alphabets that only have one case. If mbstring is not installed, the plugin requires the password contain only ASCII characters.

The password validator also examines for too many sequential characters (so "abcde12345" would be rejected).

It has checks to block matches of the user's information, the blog's information.

#7 @toscho
7 years ago

  • Cc info@… added

#8 @westi
7 years ago

  • Owner set to westi
  • Status changed from new to accepted

Along with a few other people I'm actively working on this ticket to have a proposed solution ready for 3.6 discussions.

#9 @dllh
7 years ago

  • Cc daryl@… added

#10 @nacin
7 years ago

#22575 was marked as a duplicate.

#11 @manishkrag
7 years ago

  • Cc manishkrag@… added

#12 @convissor
7 years ago

Westi: password related unit tests for my plugin are in tests/PasswordValidationTest.php.

#13 @DeanMarkTaylor
7 years ago

  • Cc DeanMarkTaylor added

#14 @cbraddoss
7 years ago

  • Cc cbraddoss added

#15 @iandunn
7 years ago

  • Cc ian_dunn@… added

#16 @markjaquith
7 years ago

Bonus stuff to think about:

  • simple dictionary lookup
  • repeating characters (even if not numeric)
  • database of first names
  • repeating short sequences (123123123)

#17 @iandunn
7 years ago

I remember reading somewhere that security researchers have come up with four authoritative algorithms for calculating the entropy of a password. Maybe it would be a good idea to start with one of those (and possibly add to it), rather than building one from scratch? I'm having trouble finding info on it again, though :(

Here are some potentially helpful things I did find:

We could also borrow some existing code from something like KeePass. Their's is in KeePassLib/Cryptography/QualityEstimation.cs

After calculating the entropy, we could then run some additional checks and knock off points for things that the entropy algorithm won't take into account, like commonly used passwords, the user's name, the site's domain name, etc.

I'd suggest that a password need at least 72 bits for it to be considered "strong".

#18 follow-up: @jenmylo
7 years ago

Password requirements on the internet is one of the worst user experiences anywhere. This comment serves to voice my strong objection to WordPress requiring any kind of specific password (including banned words). Showing a message explaining why a password choice is insecure and could lead to bad outcomes later with an AYS would be useful, but deciding what someone can do on their own with their own login information site is not good.

#19 in reply to: ↑ 18 @bananastalktome
7 years ago

Replying to jenmylo:

Password requirements on the internet is one of the worst user experiences anywhere. This comment serves to voice my strong objection to WordPress requiring any kind of specific password (including banned words). Showing a message explaining why a password choice is insecure and could lead to bad outcomes later with an AYS would be useful, but deciding what someone can do on their own with their own login information site is not good.

+1000 in agreement :)

#20 @Ipstenu
7 years ago

  • Cc ipstenu@… added

#21 @ethitter
7 years ago

  • Cc erick@… added

#22 @bradparbs
7 years ago

  • Cc brad@… added

#23 @sbruner
7 years ago

In addition to the solutions listed here, we might want to simply change the term "password" to "passphrase". Users usually view passphrases as longer and more complex than passwords.

#24 @clwill
7 years ago

  • Cc chris@… added

I third the agreement with jenmylo's comment. Requirements are a very bad user experience. I am not signed up with my state DMV simply because their password requirements were so onerous.

I also want to add in concern for a widespread belief that passphrase complexity is the answer to security against brute force attacks. It just becomes an arms races. Please read this article on the long term future of the password:

http://www.wired.com/gadgetlab/2012/11/ff-mat-honan-password-hacker/

I think WP and Automattic should approach the issue of broad scale secure account attacks with the same concern and resources they did against broad scale spamming. Harness the power of millions of WP sites to identify and block the offenders. This, like spam, is a case where the sheer power of numbers can help, and only Automattic can leverage that power.

#25 follow-up: @iandunn
7 years ago

When detecting a weak password, maybe it would be helpful to show a 'Learn more about Password Security' link next to the password strength meter or AYS. The link could display a modal containing a 3-5 minute video explaining the basics of choosing a good password, and recommending some best practices.

In my opinion, one of the best practices should be using a password manager. They make it relatively easy and convenient to use strong passwords like cT,Mo&aFv;Ubn3t<S`6$WY{r:ek?g9Jx5w)'8@CP. They can be too complicated for beginners, but I think they're acceptable for the average user, especially when they integrate with the browser. The video could contain a step-by-step example of creating a password in a password manager, setting it in WordPress, and then logging in with it.

#26 in reply to: ↑ 25 ; follow-up: @bananastalktome
7 years ago

I think a relevant XKCD is in order: http://xkcd.com/936/

#27 in reply to: ↑ 26 ; follow-up: @clwill
7 years ago

Replying to bananastalktome:

I think a relevant XKCD is in order: http://xkcd.com/936/

The lesson to be learned from that is if you reduce if from 1000guesses/sec to 10guesses/day the timeframe turns into millennia, regardless of password strength.

#28 in reply to: ↑ 27 @iandunn
7 years ago

Replying to clwill:

The lesson to be learned from that is if you reduce if from 1000guesses/sec to 10guesses/day the timeframe turns into millennia, regardless of password strength.

Are you proposing that Core place a hard limit on the number of times per day a user can fail a login attempt? The problem with doing that is that it can easily be abused to lock the user out of their account. All an attacker has to do is login with an incorrect password 10x per day, and the legitimate user can never log in.

Even if you do it based on IP rather than the user account, modern botnets will coordinate their attack to vary the IP with each attempt, and can have massive IP pools.

#29 @clwill
7 years ago

My point is that equation has two sides to it. Both sides should be addressed. There are many approaches, but if you only were to drop it to 10guesses/sec you cut the problem by two orders of magnitude.

Focusing only on one side of the equation, and the one that WP has the least control over (user behavior) is myopic.

#30 @desrosj
6 years ago

  • Cc desrosj@… added

#31 @nacin
6 years ago

  • Milestone changed from Awaiting Review to 3.7
  • Type changed from feature request to task (blessed)

Setting this to 3.7 as it is related to our password strengthening. We may not do exactly this, but I'd like to keep it tracked.

#32 @nacin
6 years ago

Talking to desrosj at WordCamp Providence. The thinking is that Dropbox's zxcvbn library is actually pretty cool. It's heavy at 600+ kb, but that's only for pages where we need a password strength meter, and we could probably even just load the script async once they focus on a password field.

Otto42 has a plugin: http://wordpress.org/plugins/zxcvbn/. It's a really simple implementation (it just replaces our existing password-strength-meter script and adds a dependency of zxcvbn.js). I think we should strongly consider it for 3.7.

@duck_
6 years ago

#33 @azaozz
6 years ago

Wondering if the async loading bits for zxcvbn.min.js should be part of the existing password-strength-meter.js (and the src will be in pwsL10n). No reason to be a separate JS file. Of course zxcvbn.min.js would still be listed in script_loader making it available to load from PHP.

Last edited 6 years ago by azaozz (previous) (diff)

#34 @nacin
6 years ago

Things get concatenated anyway in the admin, so I have little objection to keeping them separate. The benefit of keeping zxcvbn-async.js separate is that, as it is an external library, others can use it. And the library is realistically big enough to require async uploading for all usage.

If we want everyone to use our password-strength-meter.js file, then we should move it out of wp-admin. That said, it seems pretty specific to *our* meter, which may not be the only use case of zxcvbn. It would also be nice to just keep the separation from our code and their code — though the async script is indeed tiny, and also modified by us for the script location. I don't have a strong preference either way.

#35 @nacin
6 years ago

Looking a bit further, upstream zxcvbn-async.js is hard-coded to pull the file from Dropbox. Obviously we don't want that, so we modified it. At the same time, we deliberately don't register zxcvbn.min.js, because zxcvbn-async.js should be used instead. That is good! But if we combine zxcvbn-async.js with our password meter file, the end result is likely increased direct references by plugins to zxcvbn.min.js.

We could almost treat zxcvbn-async.js as a WP file. Maybe even call it wp-zxcvbn.js or wp-password-strength.js. And then merge password-strength-meter.js into that, rather than the other way around.

#36 @azaozz
6 years ago

At the same time, we deliberately don't register zxcvbn.min.js, because zxcvbn-async.js should be used instead.

Agreed, that's a good thing to do.

We could almost treat zxcvbn-async.js as a WP file. Maybe even call it wp-zxcvbn.js or wp-password-strength.js.

Yep, my thoughts exactly. zxcvbn-async.js is a simple async loading of another JS file on window.onload. We can have the remainder of passwordStrength(password1, username, password2) in the same file.

On the other hand if we want to experiment with loading zxcvbn.js "on demand", perhaps after a password field is focused, we will need password-strength-meter.js to handle that. In this case zxcvbn-async.js should be in script_loader so the library can be loaded properly but is not going to be used in core.

Loading zxcvbn.js on demand seems pretty straightforward. It fires zxcvbn_load_hook callback when ready. This can be used to remove a spinner (we will need one) and start checking user input.

Last edited 6 years ago by azaozz (previous) (diff)

@duck_
6 years ago

#37 @buffler
6 years ago

  • Cc jeremy.buller@… added

#38 @DrewAPicture
6 years ago

#18399 was marked as a duplicate.

#39 @nacin
6 years ago

In 25156:

Add Dropbox's zxcvbn library for realistic password strength estimation.

Upstream: https://github.com/lowe/zxcvbn. MIT License.
Modified for WordPress with a rot13 of the included word dictionaries, for PG-ness.

props duck_.
see #21737.

#40 @nacin
6 years ago

In 25157:

Use Dropbox's zxcvbn library for our password meter.

The library was added in [25156].

props duck_.
see #21737.

#41 @iandunn
6 years ago

It looks like the user_input parameter is being ignored.

If I run console.log( zxcvbn( 'iandunn', [ 'iandunn' ] ) ); on the official demo site at https://dl.dropboxusercontent.com/u/209/zxcvbn/test/index.html, it returns an entropy of 0 because the password appears in user_input, but if I run that against trunk, it returns 14.

I mentioned it to Jon on IRC and he thought that user_input needs to pass through rot_13().

#42 @ericlewis
6 years ago

  • Cc eric.andrew.lewis@… added

#43 @ericlewis
6 years ago

  • Cc eric.andrew.lewis@… removed

#44 @duck_
6 years ago

In 25159:

zxcvbn: Apply ROT13 when building the dynamic user_inputs dictionary.

The modified matcher assumes that the dictionaries are ROT13 encoded.
All of the static dictionaries were, but user_inputs wasn't. See #21737.

#45 @DrewAPicture
6 years ago

  • Keywords has-patch added

What else is left for this task?

#46 @nacin
6 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

zxcvbn is a great start here. It definitely doesn't do everything this ticket originally proposed, but I think we should open new more targeted tickets for 3.8 and beyond.

Note: See TracTickets for help on using tickets.