Add more polices to the protected list.

components/policy/core/common/policy_loader_win.cc

 // Copyright (c) 2014 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/policy/core/common/policy_loader_win.h"
 
 #include <windows.h>
 #include <lm.h>       // For limits.
 #include <ntdsapi.h>  // For Ds[Un]Bind
 #include <rpc.h>      // For struct GUID
@@ skipping 12 matching lines @@
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/scoped_native_library.h"
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "base/win/win_util.h"
 #include "base/win/windows_version.h"
 #include "components/json_schema/json_schema_constants.h"
 #include "components/policy/core/common/policy_bundle.h"
@@ skipping 22 matching lines @@
 // TODO(joaodasilva): remove this for M35. http://crbug.com/325349
 const char kLegacyBrowserSupportExtensionId[] =
     "heildphpnddilhkemkielfhnkaagiabh";
 
 // The web store url that is the only trusted source for extensions.
 const char kExpectedWebStoreUrl[] =
     ";https://clients2.google.com/service/update2/crx";
 // String to be prepended to each blocked entry.
 const char kBlockedExtensionPrefix[] = "[BLOCKED]";
 
+const char* kInsecurePolicies[] = {

[Joao da Silva, 2014/07/17 09:57:55]

Document

[pastarmovj, 2014/07/17 14:12:59]

Done.

+    policy::key::kMetricsReportingEnabled,
+    policy::key::kDefaultSearchProviderEnabled,

[Joao da Silva, 2014/07/17 09:57:55]

Shouldn't we include all of the search provider policies?

[pastarmovj, 2014/07/17 14:12:59]

They are gated on this one can be a useful lead if we get a problem reported if
they are present but this one isn't.

+    policy::key::kHomepageIsNewTabPage,
+    policy::key::kHomepageLocation,
+    policy::key::kRestoreOnStartup,
+    policy::key::kRestoreOnStartupURLs

[Joao da Silva, 2014/07/17 09:57:55]

policy:: isn't needed

[pastarmovj, 2014/07/17 14:12:59]

Done.

+};
+
 // The GUID of the registry settings group policy extension.
 GUID kRegistrySettingsCSEGUID = REGISTRY_EXTENSION_GUID;
 
 // The list of possible errors that can occur while collecting information about
 // the current enterprise environment.
 enum DomainCheckErrors {
   DOMAIN_CHECK_ERROR_GET_JOIN_INFO = 0,
   DOMAIN_CHECK_ERROR_DS_BIND,
   DOMAIN_CHECK_ERROR_LAST,
 };
@@ skipping 55 matching lines @@
       if (pos == std::string::npos)
         continue;
       // Only allow custom update urls in enterprise environments.
       if (!LowerCaseEqualsASCII(entry.substr(pos), kExpectedWebStoreUrl)) {
         entry = kBlockedExtensionPrefix + entry;
         invalid_policies++;
       }
 
       filtered_values->AppendString(entry);
     }
-    policy->Set(policy::key::kExtensionInstallForcelist,
-                map_entry->level, map_entry->scope,
-                filtered_values.release(),
-                map_entry->external_data_fetcher);
+    if (invalid_policies) {
+      policy->Set(policy::key::kExtensionInstallForcelist,
+                  map_entry->level, map_entry->scope,
+                  filtered_values.release(),
+                  map_entry->external_data_fetcher);
+
+      const PolicyDetails* details = policy::GetChromePolicyDetails(
+          policy::key::kExtensionInstallForcelist);
+      UMA_HISTOGRAM_SPARSE_SLOWLY("EnterpriseCheck.InvalidPolicies",
+                                  details->id);
+    }
+
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kInsecurePolicies); ++i) {

[Joao da Silva, 2014/07/17 09:57:55]

Doesn't arraysize() work here?

[pastarmovj, 2014/07/17 14:12:59]

Done.

+      if (policy->Get(kInsecurePolicies[i])) {
+        policy->Erase(kInsecurePolicies[i]);

[Joao da Silva, 2014/07/17 09:57:55]

It would be cool to have this somehow surface in about:policy, so that admins
understand why these policies are being ignored. Unfortunately we don't have an
easy way to do that now... maybe leave a TODO here and file a bug for this?

[pastarmovj, 2014/07/17 14:12:59]

Done.

+        invalid_policies++;
+        const PolicyDetails* details =
+            policy::GetChromePolicyDetails(kInsecurePolicies[i]);
+        UMA_HISTOGRAM_SPARSE_SLOWLY("EnterpriseCheck.InvalidPolicies",
+                                    details->id);
+      }
+    }

[Joao da Silva, 2014/07/17 09:57:55]

This is in the wrong block: it shouldn't be conditional on the if() in line 137,
which is about the kExtensionInstallForcelist policy.

To make this code more clear, I suggest moving the special handling of
kExtensionInstallForcelist to its own separate function and call that from here.

[pastarmovj, 2014/07/17 14:12:59]

Done.

+
     UMA_HISTOGRAM_COUNTS("EnterpriseCheck.InvalidPoliciesDetected",
                          invalid_policies);
   }
 }
 
 // A helper class encapsulating run-time-linked function calls to Wow64 APIs.
 class Wow64Functions {
  public:
   Wow64Functions()
     : kernel32_lib_(base::FilePath(L"kernel32")),
@@ skipping 482 matching lines @@
 
 void PolicyLoaderWin::OnObjectSignaled(HANDLE object) {
   DCHECK(object == user_policy_changed_event_.handle() ||
          object == machine_policy_changed_event_.handle())
       << "unexpected object signaled policy reload, obj = "
       << std::showbase << std::hex << object;
   Reload(false);
 }
 
 }  // namespace policy