Prevent users from selecting excessively bad passwords based on their username or email address

Summary:
Ref T13216. We occasionally receive HackerOne reports concerned that you can select your username as a password. I suspect very few users actually do this and that this is mostly a compliance/checklist sort of issue, not a real security issue.

On this install, we have about 41,000 user accounts. Of these, 100 have their username as a password (account or VCS). A substantial subset of these are either explicitly intentional ("demo", "bugmenot") or obvious test accounts ("test" in name, or name is a nonsensical string of gibberish, or looks like "tryphab" or similar) or just a bunch of numbers (?), or clearly a "researcher" doing this on purpose (e.g., name includes "pentest" or "xss" or similar).

So I'm not sure real users are actually very inclined to do this, and we can't really ever stop them from picking awful passwords anyway. But we //can// stop researchers from reporting that this is an issue.

Don't allow users to select passwords which contain words in a blocklist: their username, real name, email addresses, or the install's domain name. These words also aren't allowed to contain the password (that is, neither your password nor your username may be a substring of the other one). We also do a little normalization to try to split apart email addresses, domains, and real names, so I can't have "evan1234" as my password.

Test Plan:
  - Added unit tests and made them pass.
  - Tried to set my password to a bunch of variations of my username / email / domain name / real name / etc, got rejected.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13216

Differential Revision: https://secure.phabricator.com/D19776

src/applications/auth/__tests__/PhabricatorAuthPasswordTestCase.php

@@ -99,6 +99,82 @@ final class PhabricatorAuthPasswordTestCase extends PhabricatorTestCase {
     $this->assertTrue($account_engine->isUniquePassword($password2));
   }
 
+  public function testPasswordBlocklisting() {
+    $user = $this->generateNewTestUser();
+
+    $user
+      ->setUsername('iasimov')
+      ->setRealName('Isaac Asimov')
+      ->save();
+
+    $test_type = PhabricatorAuthPassword::PASSWORD_TYPE_TEST;
+    $content_source = $this->newContentSource();
+
+    $engine = id(new PhabricatorAuthPasswordEngine())
+      ->setViewer($user)
+      ->setContentSource($content_source)
+      ->setPasswordType($test_type)
+      ->setObject($user);
+
+    $env = PhabricatorEnv::beginScopedEnv();
+    $env->overrideEnvConfig('account.minimum-password-length', 4);
+
+    $passwords = array(
+      'a23li432m9mdf' => true,
+
+      // Empty.
+      '' => false,
+
+      // Password length tests.
+      'xh3' => false,
+      'xh32' => true,
+
+      // In common password blocklist.
+      'password1' => false,
+
+      // Tests for the account identifier blocklist.
+      'isaac' => false,
+      'iasimov' => false,
+      'iasimov1' => false,
+      'asimov' => false,
+      'iSaAc' => false,
+      '32IASIMOV' => false,
+      'i-am-iasimov-this-is-my-long-strong-password' => false,
+      'iasimo' => false,
+
+      // These are okay: although they're visually similar, they aren't mutual
+      // substrings of any identifier.
+      'iasimo1' => true,
+      'isa1mov' => true,
+    );
+
+    foreach ($passwords as $password => $expect) {
+      $this->assertBlocklistedPassword($engine, $password, $expect);
+    }
+  }
+
+  private function assertBlocklistedPassword(
+    PhabricatorAuthPasswordEngine $engine,
+    $raw_password,
+    $expect_valid) {
+
+    $envelope_1 = new PhutilOpaqueEnvelope($raw_password);
+    $envelope_2 = new PhutilOpaqueEnvelope($raw_password);
+
+    $caught = null;
+    try {
+      $engine->checkNewPassword($envelope_1, $envelope_2);
+    } catch (PhabricatorAuthPasswordException $exception) {
+      $caught = $exception;
+    }
+
+    $this->assertEqual(
+      $expect_valid,
+      !($caught instanceof PhabricatorAuthPasswordException),
+      pht('Validity of password "%s".', $raw_password));
+  }
+
+
   public function testPasswordUpgrade() {
     $weak_hasher = new PhabricatorIteratedMD5PasswordHasher();
 

src/applications/auth/engine/PhabricatorAuthPasswordEngine.php

@@ -115,7 +115,9 @@ final class PhabricatorAuthPasswordEngine
     // revoked passwords or colliding passwords either, so we can skip these
     // checks.
 
-    if ($this->getObject()->getPHID()) {
+    $object = $this->getObject();
+
+    if ($object->getPHID()) {
       if ($this->isRevokedPassword($password)) {
         throw new PhabricatorAuthPasswordException(
           pht(
@@ -132,6 +134,66 @@ final class PhabricatorAuthPasswordEngine
           pht('Not Unique'));
       }
     }
+
+    // Prevent use of passwords which are similar to any object identifier.
+    // For example, if your username is "alincoln", your password may not be
+    // "alincoln", "lincoln", or "alincoln1".
+    $viewer = $this->getViewer();
+    $blocklist = $object->newPasswordBlocklist($viewer, $this);
+
+    // Smallest number of overlapping characters that we'll consider to be
+    // too similar.
+    $minimum_similarity = 4;
+
+    // Add the domain name to the blocklist.
+    $base_uri = PhabricatorEnv::getAnyBaseURI();
+    $base_uri = new PhutilURI($base_uri);
+    $blocklist[] = $base_uri->getDomain();
+
+    // Generate additional subterms by splitting the raw blocklist on
+    // characters like "@", " " (space), and "." to break up email addresses,
+    // readable names, and domain names into components.
+    $terms_map = array();
+    foreach ($blocklist as $term) {
+      $terms_map[$term] = $term;
+      foreach (preg_split('/[ @.]/', $term) as $subterm) {
+        $terms_map[$subterm] = $term;
+      }
+    }
+
+    // Skip very short terms: it's okay if your password has the substring
+    // "com" in it somewhere even if the install is on "mycompany.com".
+    foreach ($terms_map as $term => $source) {
+      if (strlen($term) < $minimum_similarity) {
+        unset($terms_map[$term]);
+      }
+    }
+
+    // Normalize terms for comparison.
+    $normal_map = array();
+    foreach ($terms_map as $term => $source) {
+      $term = phutil_utf8_strtolower($term);
+      $normal_map[$term] = $source;
+    }
+
+    // Finally, make sure that none of the terms appear in the password,
+    // and that the password does not appear in any of the terms.
+    $normal_password = phutil_utf8_strtolower($raw_password);
+    if (strlen($normal_password) >= $minimum_similarity) {
+      foreach ($normal_map as $term => $source) {
+        if (strpos($term, $normal_password) === false &&
+            strpos($normal_password, $term) === false) {
+          continue;
+        }
+
+        throw new PhabricatorAuthPasswordException(
+          pht(
+            'The password you entered is very similar to a nonsecret account '.
+            'identifier (like a username or email address). Choose a more '.
+            'distinct password.'),
+          pht('Not Distinct'));
+      }
+    }
   }
 
   public function isValidPassword(PhutilOpaqueEnvelope $envelope) {

src/applications/auth/password/PhabricatorAuthPasswordHashInterface.php

@@ -6,4 +6,22 @@ interface PhabricatorAuthPasswordHashInterface {
     PhutilOpaqueEnvelope $envelope,
     PhabricatorAuthPassword $password);
 
+  /**
+   * Return a list of strings which passwords associated with this object may
+   * not be similar to.
+   *
+   * This method allows you to prevent users from selecting their username
+   * as their password or picking other passwords which are trivially similar
+   * to an account or object identifier.
+   *
+   * @param PhabricatorUser The user selecting the password.
+   * @param PhabricatorAuthPasswordEngine The password engine updating a
+   *  password.
+   * @return list<string> Blocklist of nonsecret identifiers which the password
+   *  should not be similar to.
+   */
+  public function newPasswordBlocklist(
+    PhabricatorUser $viewer,
+    PhabricatorAuthPasswordEngine $engine);
+
 }

src/applications/people/storage/PhabricatorUser.php

@@ -1665,5 +1665,23 @@ final class PhabricatorUser
     return new PhutilOpaqueEnvelope($digest);
   }
 
+  public function newPasswordBlocklist(
+    PhabricatorUser $viewer,
+    PhabricatorAuthPasswordEngine $engine) {
+
+    $list = array();
+    $list[] = $this->getUsername();
+    $list[] = $this->getRealName();
+
+    $emails = id(new PhabricatorUserEmail())->loadAllWhere(
+      'userPHID = %s',
+      $this->getPHID());
+    foreach ($emails as $email) {
+      $list[] = $email->getAddress();
+    }
+
+    return $list;
+  }
+
 
 }