Add StartWeakCheck to InsecureCredentialsManager

This CL adds a simple version of the StartWeakCheck function to the
InsecureCredentialsManager that doesn't use the zxcvbn-cpp library yet.
Checking long passwords is implemented as it will be in the final
version of password weakness check. It also adds the GetWeakCredentials
function to the InsecureCredentialsManager.

Bug: 1119752
Change-Id: I50013c9824701fdccabda7a76644f37c90fa6227
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398622
Commit-Queue: Irina Fedorova <irfedorova@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805723}

components/password_manager/core/browser/BUILD.gn

@@ -242,6 +242,8 @@ static_library("browser") {
     "ui/post_save_compromised_helper.h",
     "ui/saved_passwords_presenter.cc",
     "ui/saved_passwords_presenter.h",
+    "ui/weak_check_utility.cc",
+    "ui/weak_check_utility.h",
     "votes_uploader.cc",
     "votes_uploader.h",
     "well_known_change_password_state.cc",
@@ -635,6 +637,7 @@ source_set("unit_tests") {
     "ui/insecure_credentials_manager_unittest.cc",
     "ui/post_save_compromised_helper_unittest.cc",
     "ui/saved_passwords_presenter_unittest.cc",
+    "ui/weak_check_utility_unittest.cc",
     "vote_uploads_test_matchers.h",
     "votes_uploader_unittest.cc",
     "well_known_change_password_state_unittest.cc",

components/password_manager/core/browser/ui/insecure_credentials_manager.cc

@@ -8,13 +8,16 @@
 #include <iterator>
 #include <set>
 
+#include "base/bind.h"
 #include "base/containers/flat_set.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/compromised_credentials_table.h"
 #include "components/password_manager/core/browser/ui/credential_utils.h"
 #include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
+#include "components/password_manager/core/browser/ui/weak_check_utility.h"
 #include "components/password_manager/core/common/password_manager_features.h"
 
 namespace password_manager {
@@ -64,12 +67,13 @@ InsecureCredentialTypeFlags ConvertCompromiseType(CompromiseType type) {
   NOTREACHED();
 }
 
-// This function takes two lists of insecure credentials and saved passwords and
-// joins them, producing a map that contains CredentialWithPassword as keys and
-// vector<autofill::PasswordForm> as values with InsecureCredentialTypeFlags as
-// values.
+// This function takes three lists of compromised credentials, weak passwords
+// and saved passwords and joins them, producing a map that contains
+// CredentialWithPassword as keys and vector<autofill::PasswordForm> as values
+// with InsecureCredentialTypeFlags as values.
 CredentialPasswordsMap JoinInsecureCredentialsWithSavedPasswords(
-    const std::vector<CompromisedCredentials>& credentials,
+    const std::vector<CompromisedCredentials>& compromised_credentials,
+    const base::flat_set<base::string16>& weak_passwords,
     SavedPasswordsPresenter::SavedPasswordsView saved_passwords) {
   CredentialPasswordsMap credentials_to_forms;
 
@@ -96,7 +100,7 @@ CredentialPasswordsMap JoinInsecureCredentialsWithSavedPasswords(
   // size of 1, however.
   std::multiset<autofill::PasswordForm, CredentialWithoutPasswordLess>
       password_forms(saved_passwords.begin(), saved_passwords.end());
-  for (const auto& credential : credentials) {
+  for (const auto& credential : compromised_credentials) {
     auto range = password_forms.equal_range(credential);
     // Make use of a set to only filter out repeated passwords, if any.
     std::for_each(
@@ -106,8 +110,7 @@ CredentialPasswordsMap JoinInsecureCredentialsWithSavedPasswords(
               credentials_to_forms[compromised_credential];
 
           // Using |= operator to save in a bit mask both Leaked and Phished.
-          credential_to_form.type =
-              credential_to_form.type |
+          credential_to_form.type |=
               ConvertCompromiseType(credential.compromise_type);
 
           // Use the latest time. Relevant when the same credential is both
@@ -122,19 +125,36 @@ CredentialPasswordsMap JoinInsecureCredentialsWithSavedPasswords(
         });
   }
 
+  for (const auto& form : saved_passwords) {
+    if (weak_passwords.contains(form.password_value)) {
+      CredentialView weak_credential(form);
+      auto& credential_to_form = credentials_to_forms[weak_credential];
+      credential_to_form.type |= InsecureCredentialTypeFlags::kWeakCredential;
+
+      // This helps not to create a copy of the |form| in case the credential
+      // has also been compromised. This is important because we don't want to
+      // delete the form twice in the RemoveCredential.
+      if (!IsCompromised(credential_to_form.type)) {
+        credential_to_form.forms.push_back(form);
+      }
+    }
+  }
+
   return credentials_to_forms;
 }
 
-std::vector<CredentialWithPassword> ExtractCompromisedCredentials(
-    const CredentialPasswordsMap& credentials_to_forms) {
+std::vector<CredentialWithPassword> ExtractInsecureCredentials(
+    const CredentialPasswordsMap& credentials_to_forms,
+    bool (*condition)(const InsecureCredentialTypeFlags&)) {
   std::vector<CredentialWithPassword> credentials;
-  credentials.reserve(credentials_to_forms.size());
   for (const auto& credential_to_forms : credentials_to_forms) {
+    if (condition(credential_to_forms.second.type)) {
       CredentialWithPassword credential(credential_to_forms.first);
       credential.insecure_type = credential_to_forms.second.type;
       credential.create_time = credential_to_forms.second.latest_time;
       credentials.push_back(std::move(credential));
     }
+  }
   return credentials;
 }
 
@@ -205,6 +225,14 @@ void InsecureCredentialsManager::Init() {
   compromised_credentials_reader_.Init();
 }
 
+void InsecureCredentialsManager::StartWeakCheck() {
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
+      base::BindOnce(&BulkWeakCheck, presenter_->GetSavedPasswords()),
+      base::BindOnce(&InsecureCredentialsManager::OnWeakCheckDone,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
 void InsecureCredentialsManager::SaveCompromisedCredential(
     const LeakCheckCredential& credential) {
   // Iterate over all currently saved credentials and mark those as compromised
@@ -265,7 +293,12 @@ bool InsecureCredentialsManager::RemoveCredential(
 
 std::vector<CredentialWithPassword>
 InsecureCredentialsManager::GetCompromisedCredentials() const {
-  return ExtractCompromisedCredentials(credentials_to_forms_);
+  return ExtractInsecureCredentials(credentials_to_forms_, &IsCompromised);
+}
+
+std::vector<CredentialWithPassword>
+InsecureCredentialsManager::GetWeakCredentials() const {
+  return ExtractInsecureCredentials(credentials_to_forms_, &IsWeak);
 }
 
 SavedPasswordsPresenter::SavedPasswordsView
@@ -285,29 +318,51 @@ void InsecureCredentialsManager::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void InsecureCredentialsManager::OnWeakCheckDone(
+    base::flat_set<base::string16> weak_passwords) {
+  weak_passwords_ = std::move(weak_passwords);
+
+  credentials_to_forms_ = JoinInsecureCredentialsWithSavedPasswords(
+      compromised_credentials_, weak_passwords_,
+      presenter_->GetSavedPasswords());
+  NotifyWeakCredentialsChanged();
+}
+
 // Re-computes the list of compromised credentials with passwords after
 // obtaining a new list of compromised credentials.
 void InsecureCredentialsManager::OnCompromisedCredentialsChanged(
     const std::vector<CompromisedCredentials>& compromised_credentials) {
   compromised_credentials_ = compromised_credentials;
-  UpdateCachedDataAndNotifyObservers(presenter_->GetSavedPasswords());
+
+  credentials_to_forms_ = JoinInsecureCredentialsWithSavedPasswords(
+      compromised_credentials_, weak_passwords_,
+      presenter_->GetSavedPasswords());
+  NotifyCompromisedCredentialsChanged();
 }
 
 // Re-computes the list of insecure credentials with passwords after obtaining a
 // new list of saved passwords.
 void InsecureCredentialsManager::OnSavedPasswordsChanged(
     SavedPasswordsPresenter::SavedPasswordsView saved_passwords) {
-  UpdateCachedDataAndNotifyObservers(saved_passwords);
+  credentials_to_forms_ = JoinInsecureCredentialsWithSavedPasswords(
+      compromised_credentials_, weak_passwords_, saved_passwords);
+  NotifyCompromisedCredentialsChanged();
+  NotifyWeakCredentialsChanged();
 }
 
-void InsecureCredentialsManager::UpdateCachedDataAndNotifyObservers(
-    SavedPasswordsPresenter::SavedPasswordsView saved_passwords) {
-  credentials_to_forms_ = JoinInsecureCredentialsWithSavedPasswords(
-      compromised_credentials_, saved_passwords);
-  std::vector<CredentialWithPassword> credentials =
-      ExtractCompromisedCredentials(credentials_to_forms_);
+void InsecureCredentialsManager::NotifyCompromisedCredentialsChanged() {
+  std::vector<CredentialWithPassword> compromised_credentials =
+      ExtractInsecureCredentials(credentials_to_forms_, &IsCompromised);
+  for (auto& observer : observers_) {
+    observer.OnCompromisedCredentialsChanged(compromised_credentials);
+  }
+}
+
+void InsecureCredentialsManager::NotifyWeakCredentialsChanged() {
+  std::vector<CredentialWithPassword> weak_credentials =
+      ExtractInsecureCredentials(credentials_to_forms_, &IsWeak);
   for (auto& observer : observers_) {
-    observer.OnCompromisedCredentialsChanged(credentials);
+    observer.OnWeakCredentialsChanged(weak_credentials);
   }
 }
 

components/password_manager/core/browser/ui/insecure_credentials_manager.h

@@ -8,6 +8,7 @@
 #include <map>
 #include <vector>
 
+#include "base/containers/flat_set.h"
 #include "base/containers/span.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/observer_list.h"
@@ -37,6 +38,13 @@ enum class InsecureCredentialTypeFlags {
   kWeakCredential = 1 << 2,
 };
 
+constexpr InsecureCredentialTypeFlags operator&(
+    InsecureCredentialTypeFlags lhs,
+    InsecureCredentialTypeFlags rhs) {
+  return static_cast<InsecureCredentialTypeFlags>(static_cast<int>(lhs) &
+                                                  static_cast<int>(rhs));
+}
+
 constexpr InsecureCredentialTypeFlags operator|(
     InsecureCredentialTypeFlags lhs,
     InsecureCredentialTypeFlags rhs) {
@@ -44,6 +52,26 @@ constexpr InsecureCredentialTypeFlags operator|(
                                                   static_cast<int>(rhs));
 }
 
+constexpr InsecureCredentialTypeFlags& operator|=(
+    InsecureCredentialTypeFlags& lhs,
+    InsecureCredentialTypeFlags rhs) {
+  lhs = lhs | rhs;
+  return lhs;
+}
+
+// Checks that |flag| contains at least one of compromised types.
+constexpr bool IsCompromised(const InsecureCredentialTypeFlags& flag) {
+  return (flag & (InsecureCredentialTypeFlags::kCredentialLeaked |
+                  InsecureCredentialTypeFlags::kCredentialPhished)) !=
+         InsecureCredentialTypeFlags::kSecure;
+}
+
+// Checks that |flag| contains weak type.
+constexpr bool IsWeak(const InsecureCredentialTypeFlags& flag) {
+  return (flag & InsecureCredentialTypeFlags::kWeakCredential) !=
+         InsecureCredentialTypeFlags::kSecure;
+}
+
 // Simple struct that augments key values of InsecureCredentials and a password.
 struct CredentialView {
   CredentialView(std::string signon_realm,
@@ -125,6 +153,10 @@ class InsecureCredentialsManager
 
   void Init();
 
+  // Computes weak credentials in a separate thread and then passes the result
+  // to OnWeakCheckDone.
+  void StartWeakCheck();
+
   // Marks all saved credentials which have same username & password as
   // compromised.
   void SaveCompromisedCredential(const LeakCheckCredential& credential);
@@ -141,6 +173,9 @@ class InsecureCredentialsManager
   // Returns a vector of currently compromised credentials.
   std::vector<CredentialWithPassword> GetCompromisedCredentials() const;
 
+  // Returns a vector of currently weak credentials.
+  std::vector<CredentialWithPassword> GetWeakCredentials() const;
+
   // Returns password forms which map to provided insecure credential.
   // In most of the cases vector will have 1 element only.
   SavedPasswordsPresenter::SavedPasswordsView GetSavedPasswordsFor(
@@ -154,6 +189,10 @@ class InsecureCredentialsManager
   using CredentialPasswordsMap =
       std::map<CredentialView, CredentialMetadata, PasswordCredentialLess>;
 
+  // Updates |weak_passwords| set and notifies observers that weak credentials
+  // were changed.
+  void OnWeakCheckDone(base::flat_set<base::string16> weak_passwords);
+
   // CompromisedCredentialsReader::Observer:
   void OnCompromisedCredentialsChanged(
       const std::vector<CompromisedCredentials>& compromised_credentials)
@@ -163,9 +202,11 @@ class InsecureCredentialsManager
   void OnSavedPasswordsChanged(
       SavedPasswordsPresenter::SavedPasswordsView passwords) override;
 
-  // Function to update |credentials_to_forms_| and notify observers.
-  void UpdateCachedDataAndNotifyObservers(
-      SavedPasswordsPresenter::SavedPasswordsView saved_passwords);
+  // Notifies observers when compromised credentials have changed.
+  void NotifyCompromisedCredentialsChanged();
+
+  // Notifies observers when weak credentials have changed.
+  void NotifyWeakCredentialsChanged();
 
   // Returns the `profile_store_` or `account_store_` if `form` is stored in the
   // profile store of the account store accordingly.
@@ -186,6 +227,9 @@ class InsecureCredentialsManager
   // Cache of the most recently obtained compromised credentials.
   std::vector<CompromisedCredentials> compromised_credentials_;
 
+  // Cache of the most recently obtained weak passwords.
+  base::flat_set<base::string16> weak_passwords_;
+
   // A map that matches CredentialView to corresponding PasswordForms, latest
   // create_type and combined insecure type.
   CredentialPasswordsMap credentials_to_forms_;
@@ -201,6 +245,8 @@ class InsecureCredentialsManager
       observed_saved_password_presenter_{this};
 
   base::ObserverList<Observer, /*check_empty=*/true> observers_;
+
+  base::WeakPtrFactory<InsecureCredentialsManager> weak_ptr_factory_{this};
 };
 
 }  // namespace password_manager

components/password_manager/core/browser/ui/weak_check_utility.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/password_manager/core/browser/ui/weak_check_utility.h"
+
+namespace password_manager {
+
+namespace {
+
+// Passwords longer than this constant should not be checked for weakness using
+// the zxcvbn-cpp library. This is because the runtime grows extremely, starting
+// at a password length of 40.
+// See https://github.com/dropbox/zxcvbn#runtime-latency
+// Needs to stay in sync with google3 constant: http://shortn/_1ufIF61G4X
+constexpr int kZxcvbnLengthCap = 40;
+
+// If the password has a score of 2 or less, this password should be marked as
+// weak. The lower the password score, the weaker it is.
+constexpr int kHighSeverityScore = 0;
+constexpr int kLowSeverityScore = 2;
+
+constexpr int kStrongPasswordScore = 4;
+
+// Very rough, extremely simplified strength check that only makes sense for
+// long passwords.
+int SimpleLongPasswordStrengthEstimate(const base::string16& password) {
+  base::flat_set<base::char16> chars;
+
+  for (auto character : password) {
+    chars.insert(character);
+    if (chars.size() > 4) {
+      return kStrongPasswordScore;
+    }
+  }
+  return kHighSeverityScore;
+}
+
+// Returns the |password| score.
+int PasswordWeakCheck(const base::string16& password) {
+  // zxcvbn's computation time explodes for long passwords, so don't use it for
+  // those.
+  if (password.size() > kZxcvbnLengthCap) {
+    return SimpleLongPasswordStrengthEstimate(password);
+  }
+  // TODO(crbug.com/1119752): Compute result by zxcvbn-cpp.
+  return kHighSeverityScore;
+}
+
+}  // namespace
+
+base::flat_set<base::string16> BulkWeakCheck(
+    SavedPasswordsPresenter::SavedPasswordsView saved_passwords) {
+  std::vector<base::string16> weak_passwords;
+
+  for (const auto& password : saved_passwords) {
+    if (PasswordWeakCheck(password.password_value) <= kLowSeverityScore) {
+      weak_passwords.push_back(password.password_value);
+    }
+  }
+
+  return base::flat_set<base::string16>(std::move(weak_passwords));
+}
+
+}  // namespace password_manager

components/password_manager/core/browser/ui/weak_check_utility.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_UI_WEAK_CHECK_UTILITY_H_
+#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_UI_WEAK_CHECK_UTILITY_H_
+
+#include "base/containers/flat_set.h"
+#include "base/strings/string16.h"
+#include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
+
+namespace password_manager {
+
+// Checks each password for weakness.
+base::flat_set<base::string16> BulkWeakCheck(
+    SavedPasswordsPresenter::SavedPasswordsView passwords);
+
+}  // namespace password_manager
+
+#endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_UI_WEAK_CHECK_UTILITY_H_

components/password_manager/core/browser/ui/weak_check_utility_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/password_manager/core/browser/ui/weak_check_utility.h"
+
+#include <vector>
+#include "base/strings/utf_string_conversions.h"
+#include "components/autofill/core/common/password_form.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace password_manager {
+
+namespace {
+
+constexpr char kUsername1[] = "alice";
+constexpr char kUsername2[] = "bob";
+
+constexpr char kWeakShortPassword[] = "123456";
+constexpr char kWeakLongPassword[] =
+    "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcda";
+constexpr char kStrongLongPassword[] =
+    "pmsFlsnoab4nsl#losb@skpfnsbkjb^klsnbs!cns";
+
+using autofill::PasswordForm;
+using ::testing::ElementsAre;
+
+PasswordForm MakeSavedPassword(base::StringPiece username,
+                               base::StringPiece password) {
+  PasswordForm form;
+  form.signon_realm = "https://example.com";
+  form.username_value = base::ASCIIToUTF16(username);
+  form.password_value = base::ASCIIToUTF16(password);
+  return form;
+}
+
+}  // namespace
+
+TEST(WeakCheckUtilityTest, WeakPasswordsNotFound) {
+  std::vector<PasswordForm> passwords = {
+      MakeSavedPassword(kUsername1, kStrongLongPassword)};
+
+  EXPECT_THAT(BulkWeakCheck(passwords), testing::IsEmpty());
+}
+
+TEST(WeakCheckUtilityTest, DetectedShortAndLongWeakPasswords) {
+  std::vector<PasswordForm> passwords = {
+      MakeSavedPassword(kUsername1, kStrongLongPassword),
+      MakeSavedPassword(kUsername1, kWeakShortPassword),
+      MakeSavedPassword(kUsername2, kWeakLongPassword)};
+
+  base::flat_set<base::string16> weak_passwords = BulkWeakCheck(passwords);
+
+  EXPECT_THAT(weak_passwords,
+              ElementsAre(base::ASCIIToUTF16(kWeakShortPassword),
+                          base::ASCIIToUTF16(kWeakLongPassword)));
+}
+
+TEST(WeakCheckUtilityTest, WeakPasswordsSetContainsNoCopies) {
+  std::vector<PasswordForm> passwords = {
+      MakeSavedPassword(kUsername1, kWeakShortPassword),
+      MakeSavedPassword(kUsername2, kWeakShortPassword)};
+
+  base::flat_set<base::string16> weak_passwords = BulkWeakCheck(passwords);
+
+  EXPECT_THAT(weak_passwords,
+              ElementsAre(base::ASCIIToUTF16(kWeakShortPassword)));
+}
+
+}  // namespace password_manager