Prevent ONC-pushed certificates from being used with multiprofiles.

chrome/browser/chromeos/policy/policy_cert_service_factory.cc

 // Copyright 2013 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/chromeos/policy/policy_cert_service_factory.h"
 
 #include "base/memory/singleton.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/pref_service.h"
+#include "base/prefs/scoped_user_pref_update.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/chromeos/policy/policy_cert_service.h"
 #include "chrome/browser/chromeos/policy/policy_cert_verifier.h"
 #include "chrome/browser/chromeos/policy/user_network_configuration_updater_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/browser_context_keyed_service/browser_context_dependency_manager.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 
 namespace policy {
@@ skipping 13 matching lines @@
   if (!service)
     return scoped_ptr<PolicyCertVerifier>();
   return service->CreatePolicyCertVerifier();
 }
 
 // static
 PolicyCertServiceFactory* PolicyCertServiceFactory::GetInstance() {
   return Singleton<PolicyCertServiceFactory>::get();
 }
 
+// static
+void PolicyCertServiceFactory::SetUsedPolicyCertificates(
+    const std::string& user_id) {
+  if (!UsedPolicyCertificates(user_id)) {

[pneubeck (no reviews), 2013/12/17 15:25:31]

if (Used...)
  return;

to remove indentation, if you like

[Joao da Silva, 2013/12/17 16:36:41]

Done.

+    ListPrefUpdate update(g_browser_process->local_state(),
+                          prefs::kUsedPolicyCertificates);

[pneubeck (no reviews), 2013/12/17 15:25:31]

is it safe to use local_state implicitly in static functions, or should we
better pass it as an argument?

[Joao da Silva, 2013/12/17 16:36:41]

That's a good point. This call is made from the service which only lives as long
as the Profile does, so this is safe.

[pneubeck (no reviews), 2013/12/17 17:57:00]

Yeah, in case of this function. The other to functions have the same "issue".

+    update->AppendString(user_id);
+  }
+}
+
+// static
+void PolicyCertServiceFactory::ClearUsedPolicyCertificates(
+    const std::string& user_id) {

[pneubeck (no reviews), 2013/12/17 15:25:31]

not sure whether we consistently use "user_id" as a synonym for "user_email".
If not, better use user_email consistently.

[Joao da Silva, 2013/12/17 16:36:41]

This comes from UserManager, which uses user_id.

+  ListPrefUpdate update(g_browser_process->local_state(),
+                        prefs::kUsedPolicyCertificates);
+  update->Remove(base::StringValue(user_id), NULL);
+}
+
+// static
+bool PolicyCertServiceFactory::UsedPolicyCertificates(
+    const std::string& user_id) {
+  base::StringValue value(user_id);
+  const base::ListValue* list =
+      g_browser_process->local_state()->GetList(prefs::kUsedPolicyCertificates);
+  return list && list->Find(value) != list->end();

[pneubeck (no reviews), 2013/12/17 15:25:31]

!list is an error so logging or DCHECK should be more correct.

[Joao da Silva, 2013/12/17 16:36:41]

Done.

+}
+
+// static
+void PolicyCertServiceFactory::RegisterPrefs(PrefRegistrySimple* local_state) {
+  local_state->RegisterListPref(prefs::kUsedPolicyCertificates);
+}
+
 PolicyCertServiceFactory::PolicyCertServiceFactory()
     : BrowserContextKeyedServiceFactory(
           "PolicyCertService",
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(UserNetworkConfigurationUpdaterFactory::GetInstance());
 }
 
 PolicyCertServiceFactory::~PolicyCertServiceFactory() {}
 
 BrowserContextKeyedService* PolicyCertServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = static_cast<Profile*>(context);
+
+  chromeos::UserManager* user_manager = chromeos::UserManager::Get();
+  chromeos::User* user =
+      user_manager->GetUserByProfile(profile->GetOriginalProfile());
+  if (!user)
+    return NULL;
+
+  // Backwards compatibility: profiles that used policy-pushed certificates used
+  // to have this condition marked in their prefs. This signal has moved to
+  // local_state though, to support checking it before the profile is loaded.
+  // Check the profile here and update the local_state, if appropriate.
+  // TODO(joaodasilva): remove this, eventually.
+  PrefService* prefs = profile->GetOriginalProfile()->GetPrefs();
+  if (prefs->GetBoolean(prefs::kUsedPolicyCertificatesOnce)) {

[pneubeck (no reviews), 2013/12/17 15:25:31]

is a UMA metric necessary, to decide when to remove this?

[Joao da Silva, 2013/12/17 16:36:41]

This is mostly used by schools, and IIRC they disable metrics reporting. We'll
have to sync with Vidya about this.

+    prefs->ClearPref(prefs::kUsedPolicyCertificatesOnce);
+    SetUsedPolicyCertificates(user->email());
+  }
+
   UserNetworkConfigurationUpdater* net_conf_updater =
       UserNetworkConfigurationUpdaterFactory::GetForProfile(profile);
   if (!net_conf_updater)
     return NULL;
 
-  // In case of usage of additional trust anchors from an incognito profile, the
-  // prefs of the original profile have to be marked.
-  return new PolicyCertService(net_conf_updater,
-                               profile->GetOriginalProfile()->GetPrefs());
+  return new PolicyCertService(net_conf_updater, user->email(), user_manager);
 }
 
 content::BrowserContext* PolicyCertServiceFactory::GetBrowserContextToUse(
     content::BrowserContext* context) const {
   return chrome::GetBrowserContextOwnInstanceInIncognito(context);
 }
 
 void PolicyCertServiceFactory::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterBooleanPref(

[pneubeck (no reviews), 2013/12/17 15:25:31]

add a comment that it's still here for backwards compatibility.

[Joao da Silva, 2013/12/17 16:36:41]

Done.

       prefs::kUsedPolicyCertificatesOnce,
       false,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 bool PolicyCertServiceFactory::ServiceIsNULLWhileTesting() const {
   return true;
 }
 
 }  // namespace policy