Clean Obsolete HTTP Data from the Password Store

chrome/browser/password_manager/password_manager_util.cc

+// Copyright 2017 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 "chrome/browser/password_manager/password_manager_util.h"
+
+#include <algorithm>
+#include <iterator>
+#include <memory>
+#include <tuple>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/stl_util.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/autofill/core/common/password_form.h"
+#include "components/keyed_service/core/service_access_type.h"
+#include "components/password_manager/core/browser/password_store.h"
+#include "components/password_manager/core/browser/password_store_consumer.h"
+#include "components/password_manager/core/browser/statistics_table.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/prefs/pref_service.h"
+#include "content/public/browser/browser_thread.h"
+#include "net/http/transport_security_state.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "url/gurl.h"
+
+using autofill::PasswordForm;
+using password_manager::InteractionsStats;
+using password_manager::PasswordStore;
+
+namespace password_manager_util {
+
+namespace {
+
+// Utility function that checks whether HSTS is enabled for a given host.
+// This requires to be passed a URLRequestContextGetter and needs to be called
+// on the IO thread.
+bool IsHSTSActiveForHostAndRequestContext(
+    const GURL& origin,
+    const scoped_refptr<net::URLRequestContextGetter>& request_context) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  if (!origin.is_valid())
+    return false;
+
+  net::TransportSecurityState* security_state =
+      request_context->GetURLRequestContext()->transport_security_state();
+
+  if (!security_state)
+    return false;
+
+  return security_state->ShouldUpgradeToSSL(origin.host());
+}
+
+// Utility function that moves all elements from a specified iterator into a new
+// vector and returns it. The moved elements are deleted from the original
+// vector.
+std::vector<std::unique_ptr<PasswordForm>> SplitFormsFrom(
+    std::vector<std::unique_ptr<PasswordForm>>::iterator from,
+    std::vector<std::unique_ptr<PasswordForm>>* forms) {
+  const auto end_forms = std::end(*forms);
+  std::vector<std::unique_ptr<PasswordForm>> result;
+  result.reserve(std::distance(from, end_forms));
+  std::move(from, end_forms, std::back_inserter(result));
+  forms->erase(from, end_forms);
+  return result;
+}
+
+void RemoveLoginIfHSTS(const scoped_refptr<PasswordStore>& store,
+                       const PasswordForm& form,
+                       bool is_hsts) {
+  if (is_hsts)
+    store->RemoveLogin(form);
+}
+
+void RemoveSiteStatsIfHSTS(const scoped_refptr<PasswordStore>& store,
+                           const InteractionsStats& stats,
+                           bool is_hsts) {
+  if (is_hsts)
+    store->RemoveSiteStats(stats.origin_domain);
+}
+
+// This class removes obsolete HTTP data from a password store. HTTP data is
+// obsolete, if the corresponding host migrated to HTTPS and has HSTS enabled.
+class ObsoleteHttpCleaner : public password_manager::PasswordStoreConsumer {
+ public:
+  // Constructing a ObsoleteHttpCleaner will result in issuing the clean up
+  // tasks already.
+  explicit ObsoleteHttpCleaner(Profile* profile);
+  ~ObsoleteHttpCleaner() override;
+
+  // PasswordStoreConsumer:
+  // This will be called for both autofillable logins as well as blacklisted
+  // logins. Blacklisted logins are removed iff the scheme is HTTP and HSTS is
+  // enabled for the host.
+  // Autofillable logins are removed iff the scheme is HTTP and there exists
+  // another HTTPS login with active HSTS that has the same host as well as the
+  // same username and password.
+  void OnGetPasswordStoreResults(
+      std::vector<std::unique_ptr<PasswordForm>> results) override;
+
+  // This will remove all stats for HTTP sites for which HSTS is active.
+  void OnGetSiteStatistics(std::vector<InteractionsStats> stats) override;
+
+  Profile* profile() { return profile_; }
+
+  // Returns the PasswordStore associated with |profile_|.
+  PasswordStore* GetPasswordStore() const;
+
+ private:
+  Profile* profile_;

[vabr (Chromium), 2017/03/24 07:02:41]

The Profile and the content tools for threading are the reason why this code
cannot be in the component core instead. The vast majority of the could could
be, though. And should be, in case we want to have this ultimately in iOS.

The Profile is only used at a single place: to obtain the PasswordStore. If you
build the code so that it gets the store injected (as a value or as a function
returning it on demand), you can keep most of it in the component core, and only
add a thin wrapper here in //chrome/browser/password_manager.

For thread-related functions from //content there are the *TaskRunner
abstractions: you could build your code in the component core with an injected
TaskRunner, and plug the content-provided one from the wrapper in
//chrome/browser/password_manager.

I spent enough time componentising various parts of Chrome (such as
ProfileSyncService) to feel strongly about not creating code outside of the
component core unless desperately needed. :) I'm happy to talk next week if you
have questions about the above suggestion.

[vasilii, 2017/03/24 15:14:30]

I think that what you've suggested is doable. PostHSTSQueryForHostAndProfile and
DelayCleanObsoleteHttpDataForProfile will need URLRequestContextGetter. However,
the callers know it.

Regarding iOS. It's not clear to me if it's possible to get HSTS state from
WKWebView (or some cache). I guess that we need to make sure that this code
doesn't run and probably doesn't compile on iOS. Currently it's the case except
the new pref.

[vabr (Chromium), 2017/03/27 05:53:59]

I agree that the migrator code should not compile under iOS if we cannot
currently use ti there. But instead of achieving it through having the code
under //chrome, I would still prefer to have it in the component core and
exclude it in the BUILD.gn file (similarly to what we do for
login_database_posix.cc). The advantage of having it in the component core is
that no unnecessary new dependencies will creep in over time, and that it will
be ready for future use on iOS or other potential non-//chrome platforms should
there be an opportunity for that.

+
+  DISALLOW_COPY_AND_ASSIGN(ObsoleteHttpCleaner);
+};
+
+ObsoleteHttpCleaner::ObsoleteHttpCleaner(Profile* profile) : profile_(profile) {
+  DCHECK(profile_);
+  GetPasswordStore()->GetBlacklistLogins(this);
+  GetPasswordStore()->GetAutofillableLogins(this);
+  GetPasswordStore()->GetAllSiteStats(this);
+}
+
+ObsoleteHttpCleaner::~ObsoleteHttpCleaner() = default;
+
+void ObsoleteHttpCleaner::OnGetPasswordStoreResults(
+    std::vector<std::unique_ptr<PasswordForm>> results) {
+  // Non HTTP or HTTPS credentials are ignored.
+  base::EraseIf(results, [](const std::unique_ptr<PasswordForm>& form) {
+    return !form->origin.SchemeIsHTTPOrHTTPS();
+  });
+
+  // Move HTTPS forms into their own container.
+  auto https_forms = SplitFormsFrom(
+      std::partition(std::begin(results), std::end(results),
+                     [](const std::unique_ptr<PasswordForm>& form) {
+                       return form->origin.SchemeIs(url::kHttpScheme);
+                     }),
+      &results);
+
+  // Move blacklisted HTTP forms into their own container.
+  const auto blacklisted_http_forms = SplitFormsFrom(
+      std::partition(std::begin(results), std::end(results),
+                     [](const std::unique_ptr<PasswordForm>& form) {
+                       return !form->blacklisted_by_user;
+                     }),
+      &results);
+
+  // Remove blacklisted HTTP forms from the password store when HSTS is active
+  // for the given host.
+  for (const auto& form : blacklisted_http_forms) {
+    PostHSTSQueryForHostAndProfile(
+        form->origin, profile(),
+        base::Bind(&RemoveLoginIfHSTS, make_scoped_refptr(GetPasswordStore()),
+                   *form));
+  }
+
+  // Return early if there are no non-blacklisted HTTP forms.
+  if (results.empty())
+    return;
+
+  // Sort HTTPS forms according to custom comparison function. Consider two
+  // forms equivalent if they have the same host, as well as the same username
+  // and password.
+  const auto form_cmp = [](const std::unique_ptr<PasswordForm>& lhs,
+                           const std::unique_ptr<PasswordForm>& rhs) {
+    return std::forward_as_tuple(lhs->origin.host_piece(), lhs->username_value,
+                                 lhs->password_value) <
+           std::forward_as_tuple(rhs->origin.host_piece(), rhs->username_value,
+                                 rhs->password_value);
+  };
+
+  std::sort(std::begin(https_forms), std::end(https_forms), form_cmp);
+
+  // Iterate through HTTP forms and remove them from the password store if there
+  // exists an equivalent HTTPS form that has HSTS enabled.
+  for (const auto& form : results) {
+    if (std::binary_search(std::begin(https_forms), std::end(https_forms), form,
+                           form_cmp)) {
+      PostHSTSQueryForHostAndProfile(
+          form->origin, profile(),
+          base::Bind(&RemoveLoginIfHSTS, make_scoped_refptr(GetPasswordStore()),
+                     *form));
+    }
+  }
+}
+
+void ObsoleteHttpCleaner::OnGetSiteStatistics(
+    std::vector<InteractionsStats> stats) {
+  for (const auto& stat : stats) {
+    if (stat.origin_domain.SchemeIs(url::kHttpScheme)) {
+      PostHSTSQueryForHostAndProfile(
+          stat.origin_domain, profile(),
+          base::Bind(&RemoveSiteStatsIfHSTS,
+                     make_scoped_refptr(GetPasswordStore()), stat));
+    }
+  }
+}
+
+PasswordStore* ObsoleteHttpCleaner::GetPasswordStore() const {
+  return PasswordStoreFactory::GetForProfile(profile_,
+                                             ServiceAccessType::EXPLICIT_ACCESS)

[vasilii, 2017/03/23 17:39:26]

It's definitely implicit access here (not tied to user actions).

[jdoerrie, 2017/03/24 14:08:31]

Done.

+      .get();
+}
+
+void WaitUntilCleaningIsDone(std::unique_ptr<ObsoleteHttpCleaner> cleaner) {
+  // Given the async nature of the cleaning tasks it is non-trivial to determine
+  // when they are all done. In this method we make use of the fact that as long
+  // the cleaning is not completed, weak pointers to the cleaner object exist
+  // (the scheduled, but not yet executed tasks hold them). If the cleaning is
+  // not done yet, this method schedules a task on the password store, which
+  // will be behind the cleaning tasks in the task queue. When the scheduled
+  // task gets executed, this method is called again. Now it is guaranteed that
+  // the initial scheduled cleaning tasks will have been executed, but it might
+  // be the case that they wait for the result of other scheduled tasks (e.g. on
+  // Windows there could be async calls to GetIE7Login). In this case another
+  // round trip of tasks will be scheduled. Finally, when no weak ptrs remain,
+  // this method sets a boolean preference flag and returns.
+  if (cleaner->HasWeakPtrs()) {
+    const auto post_to_ui_thread =
+        [](std::unique_ptr<ObsoleteHttpCleaner> cleaner) {
+          content::BrowserThread::PostTask(
+              content::BrowserThread::UI, FROM_HERE,
+              base::Bind(&WaitUntilCleaningIsDone,
+                         base::Passed(std::move(cleaner))));
+        };
+
+    cleaner->GetPasswordStore()->ScheduleTask(
+        base::Bind(post_to_ui_thread, base::Passed(std::move(cleaner))));
+    return;
+  }
+
+  cleaner->profile()->GetPrefs()->SetBoolean(
+      password_manager::prefs::kWasObsoleteHttpDataCleaned, true);
+}
+
+void CleanObsoleteHttpDataForProfile(Profile* profile) {
+  if (!profile->GetPrefs()->GetBoolean(
+          password_manager::prefs::kWasObsoleteHttpDataCleaned)) {
+    WaitUntilCleaningIsDone(base::MakeUnique<ObsoleteHttpCleaner>(profile));
+  }
+}
+
+}  // namespace
+
+void PostHSTSQueryForHostAndProfile(const GURL& origin,
+                                    Profile* profile,
+                                    const HSTSCallback& callback) {
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&IsHSTSActiveForHostAndRequestContext, origin,
+                 make_scoped_refptr(profile->GetRequestContext())),
+      callback);
+}
+
+void DelayCleanObsoleteHttpDataForProfile(Profile* profile,
+                                          int delay_in_seconds) {
+  content::BrowserThread::PostDelayedTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&CleanObsoleteHttpDataForProfile, profile),
+      base::TimeDelta::FromSeconds(delay_in_seconds));
+}
+
+}  // namespace password_manager_util