Skip to content

Blocks weak passwords on sign up or password change

Nick Malcolm requested to merge block-weak-passwords into master

What does this MR do?

Resolves Prevent users from choosing weak passwords (#23610 - closed) with minimal UX work (see TODO section further down).

This MR (behind a default OFF feature flag) prevents setting a weak password at registration or on update. The criteria are:

  • Matching, case-insensitive, one of statically defined set of 4500 weak passwords (config/weak_passwords.yml)
  • Containing, case-insensitive, a static set of forbidden words (!86310 (diffs))
  • Containing, case-insensitive, elements of the following which are >= 4 characters long:
    • The user's name
    • The user's username
    • The user's email address

Some examples of invalid passwords are

  • gitlab123 because it contains gitlab
  • superrandompassword+malcolm for a user Sir Malcolm because it contains malcolm. Note that Sir would be OK in the password because it's <4 chars, and could legitimately appear in a random password.
  • superrandompassword+fortune500co for a user with email fname.lname@fortune500co.com because it contains the email domain. Note that com would also be OK in the password for the reason above.
  • password because it's on the weak list
  • qqqqqqqqqqqqqqqqqqqq because it's on the weak list

Testing the feature

Feature.enable(:block_weak_passwords)

Why block by default?

Because the validation only runs when users register or change their password there should be minimal impact to existing users. Making this configurable was discounted; we want to avoid extraneous configuration options, and this is a security feature which shouldn't be disabled anyway.

Deploying behind a feature flag is necessary though as this touches one of the most high-traffic areas: sign ups! https://docs.gitlab.com/ee/development/feature_flags/

User change password UX

No UX work done here, so it's a default red alert box (note the error message has since changed)

Screen_Shot_2022-08-10_at_11.33.09_AM

User sign up UX

No UX work done here, so it's the default:

Screen_Shot_2022-09-01_at_3.13.38_PM

Stakeholders

  • ~"group::authentication and authorization" as this relates to Authentication
  • devopsgrowth as changes to signup flows may impact conversion
  • AppSec

TODO

TODO in other issues

  • (Optional) frontend Improve UX when registering with or updating to... (#363536)
    • What advice should we provide to users?
    • Before submit via API, would be nicest, but password is the last thing they enter. Not much time between that and pressing enter or clicking "Sign up"
    • Doing it with regular form validation errors 500s. Something to do with audit events not having an author (because the user isn't persisted).

Not in this iteration

  • Allow additional words to be blocked, ideally through the admin section. This would let admins block their own organisation names, known weak passwords, etc
  • Dynamically create the blocklist based on minimum password length. (Currently weak_passwords.yml is 8+ chars, but a bunch are irrelevant if the minimum password length is set to something higher 10)
  • Add support for checking homoglyphs, e.g. gitl@b g17l4b Git|ab

Merge Request Message

Whoever merges this can set the squash commit message to:

Account takeovers are a common cybersecurity threat made much easier
when a user decides to use a weak password to protect their account.

This MR introduces a static list of known weak passwords against which
the user's chosen password can be compared. The password is also
validated against components of the user's attributes, such as their
name, email, and username.

Weak passwords are prevented only when the `block_weak_passwords`
feature flag is enabled, and only at registration or when a password
is changed. (It is not checked or enforced when the password is not
changed, such as during sign-in).

Notably, and in comparison to password complexity rules, blocking weak
passwords is a requirement of NIST SP 800-63B:

> When processing requests to establish and change memorized secrets,
> verifiers SHALL compare the prospective secrets against a list that
> contains values known to be commonly-used, expected, or compromised.
>
> - https://pages.nist.gov/800-63-3/sp800-63b.html

See also:

- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86310
- https://gitlab.com/gitlab-org/gitlab/-/issues/23610
Edited by Nick Malcolm

Merge request reports