Chrome Settings Hardening

chrome/browser/prefs/chrome_pref_service_factory.cc

 // Copyright (c) 2012 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/prefs/chrome_pref_service_factory.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/default_pref_store.h"
 #include "base/prefs/json_pref_store.h"
 #include "base/prefs/pref_notifier_impl.h"
 #include "base/prefs/pref_registry.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/pref_value_store.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/prefs/command_line_pref_store.h"
 #include "chrome/browser/prefs/pref_hash_filter.h"
 #include "chrome/browser/prefs/pref_hash_store.h"
 #include "chrome/browser/prefs/pref_model_associator.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/prefs/pref_service_syncable_factory.h"
 #include "chrome/browser/ui/profile_error_dialog.h"
+#include "chrome/common/pref_names.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "components/policy/core/browser/configuration_policy_pref_store.h"
 #include "components/policy/core/common/policy_types.h"
 #endif
 
 #if defined(ENABLE_MANAGED_USERS)
 #include "chrome/browser/managed_mode/supervised_user_pref_store.h"
 #endif
 
 using content::BrowserContext;
 using content::BrowserThread;
 
 namespace {
 
+// These preferences must be kept in sync with the TrackedPreference enum in
+// tools/metrics/histograms/histograms.xml. To add a new preference, append it
+// to the array and add a corresponding value to the histogram enum. Each
+// tracked preference must be given a unique reporting ID.
+const PrefHashFilter::TrackedPreference kTrackedPrefs[] = {
+  { 0, prefs::kShowHomeButton, true },
+  { 1, prefs::kHomePageIsNewTabPage, true },
+  { 2, prefs::kHomePage, true },
+  { 3, prefs::kRestoreOnStartup, true },
+  { 4, prefs::kURLsToRestoreOnStartup, true },
+  { 5, prefs::kExtensionsPref, false },
+  { 6, prefs::kGoogleServicesLastUsername, true },
+  { 7, prefs::kSearchProviderOverrides, true },
+  { 8, prefs::kDefaultSearchProviderSearchURL, true },
+  { 9, prefs::kDefaultSearchProviderKeyword, true },
+  { 10, prefs::kDefaultSearchProviderName, true },
+#if !defined(OS_ANDROID)
+  { 11, prefs::kPinnedTabs, true },
+#endif
+  { 12, prefs::kExtensionKnownDisabled, true },
+  { 13, prefs::kProfileResetPromptMemento, true },
+};
+
+// The count of tracked preferences IDs across all platforms.
+const size_t kTrackedPrefsReportingIDsCount = 14;

[gab, 2014/01/02 18:40:22]

@erikwright pointed out that this might not be necessary.

I added it so that the same count is reported on every platform (e.g., if the
last kTrackedPref is ifdef'ed out on some platforms).

Is this required that the count reported via the histogram macros be the same
across all platforms (or even across multiple calls to the same histogram -- see
pref_hash_filter.cc for the histograms themselves)?

[Mark P, 2014/01/02 19:32:46]

erikwright@ is correct that this is not strictly necessary.  As long as your
emissions to your UMA enum histogram are in the valid range (i.e., the max
number you emit on a particular platform is less than the number of buckets you
pass to the UMA_HISTOGRAM_ENUMERATION macro), then you should be fine.  This
appears to be the case.

*However*, I'd prefer you keep the code as-is, using the same number of buckets
on every platform.  The dashboard code uses conflicting buckets for error
detection.  Most of your clients will emit to buckets such as [1, 2), [12, 13),
etc.  You'll never normally hit the overflow [14, infinity) bucket.  But suppose
one of your clients on a platform that uses a smaller maximum bucket has a bit
error or corrupted memory or something and emits to the overflow bucket.  Now
the dashboard will see a [12, infinity) count along with a set of normal [12,
13) counts.  I'm not sure how smart it'll be to handle this case, and even if it
does handle it well now, it might cause issues with our plans for better
removing bad data in the future.  In other words, I'd feel more comfortable if
the histogram used the same bucketing on all platforms.

+COMPILE_ASSERT(kTrackedPrefsReportingIDsCount >= arraysize(kTrackedPrefs),
+               need_to_increment_ids_count);
+
+PrefHashFilter::EnforcementLevel GetSettingsEnforcementLevel() {
+  static const char kSettingsEnforcementExperiment[] = "SettingsEnforcement";
+  struct {
+    PrefHashFilter::EnforcementLevel level;

[erikwright (departed), 2014/01/03 19:08:13]

nit: I would reverse this since, intuitively, a map maps from the first field to
the second (key-value, not "value-key").

Also, consider using an actual map like:

std::map<string, level> m;
m.insert("a", A);
m.insert("b", B);

if (trial) {
  map::iterator it = m.find(group_name);
  if (it != m.end())
    return *it;
}
return default;

I'm not sure if it will be more or less clear. Slightly less efficient (NlogN?)
but N is small and this is only done once per profile I think, so code
simplicity should dominate.

[gab, 2014/01/03 19:26:09]

Reversed the mapping (i.e., name first).

I still prefer the current form though. Building a map in local scope just to
perform a single search in it seems counter-intuitive to me.

+    const char* level_name;
+  } static const kEnforcementLevelMap[] = {
+    {
+      PrefHashFilter::NO_ENFORCEMENT,
+      "no_enforcement"
+    },
+    {
+      PrefHashFilter::ENFORCE,
+      "enforce"
+    },
+    {
+      PrefHashFilter::ENFORCE_NO_SEEDING,
+      "enforce_no_seeding"
+    },
+    {
+      PrefHashFilter::ENFORCE_NO_SEEDING_NO_MIGRATION,
+      "enforce_no_seeding_no_migration"
+    },
+  };
+
+  base::FieldTrial* trial =
+      base::FieldTrialList::Find(kSettingsEnforcementExperiment);
+  if (trial) {
+    const std::string& group_name = trial->group_name();
+    for (size_t i = 0; i < arraysize(kEnforcementLevelMap); ++i) {
+      if (kEnforcementLevelMap[i].level_name == group_name)
+        return kEnforcementLevelMap[i].level;
+    }
+  }
+  // TODO(gab): Switch default to ENFORCE_ALL when field trial config is up.
+  return PrefHashFilter::NO_ENFORCEMENT;
+}
+
 // Shows notifications which correspond to PersistentPrefStore's reading errors.
 void HandleReadError(PersistentPrefStore::PrefReadError error) {
   // Sample the histogram also for the successful case in order to get a
   // baseline on the success rate in addition to the error distribution.
   UMA_HISTOGRAM_ENUMERATION("PrefService.ReadError", error,
                             PersistentPrefStore::PREF_READ_ERROR_MAX_ENUM);
 
   if (error != PersistentPrefStore::PREF_READ_ERROR_NONE) {
 #if !defined(OS_CHROMEOS)
     // Failing to load prefs on startup is a bad thing(TM). See bug 38352 for
@@ skipping 62 matching lines @@
   }
 #endif
 
   factory->set_async(async);
   factory->set_extension_prefs(extension_prefs);
   factory->set_command_line_prefs(
       make_scoped_refptr(
           new CommandLinePrefStore(CommandLine::ForCurrentProcess())));
   factory->set_read_error_callback(base::Bind(&HandleReadError));
   scoped_ptr<PrefFilter> pref_filter;
-  if (pref_hash_store)
-    pref_filter.reset(new PrefHashFilter(pref_hash_store.Pass()));
+  if (pref_hash_store) {
+    pref_filter.reset(new PrefHashFilter(pref_hash_store.Pass(),
+                                         kTrackedPrefs,
+                                         arraysize(kTrackedPrefs),
+                                         kTrackedPrefsReportingIDsCount,
+                                         GetSettingsEnforcementLevel()));
+  }
   factory->set_user_prefs(
       new JsonPrefStore(
           pref_filename,
           pref_io_task_runner,
           pref_filter.Pass()));
 }
 
 }  // namespace
 
 namespace chrome_prefs {
@@ skipping 32 matching lines @@
                  pref_io_task_runner,
                  policy_service,
                  managed_user_settings,
                  pref_hash_store.Pass(),
                  extension_prefs,
                  async);
   return factory.CreateSyncable(pref_registry.get());
 }
 
 }  // namespace chrome_prefs