Prevent ONC-pushed certificates from being used with multiprofiles.

chrome/browser/chromeos/login/multi_profile_user_controller.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/login/multi_profile_user_controller.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_change_registrar.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/chromeos/login/multi_profile_user_controller_delegate.h"
+#include "chrome/browser/chromeos/login/user.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_service_factory.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 
 namespace chromeos {
 
 namespace {
 
 std::string SanitizeBehaviorValue(const std::string& value) {
@@ skipping 48 matching lines @@
 
   // Always allow if there is no primary user or user being checked is the
   // primary user.
   if (primary_user_email.empty() || primary_user_email == user_email)
     return true;
 
   // Owner is not allowed to be secondary user.
   if (user_manager->GetOwnerEmail() == user_email)
     return false;
 
+  // Don't allow profiles potentially tainted by data fetched with policy-pushed
+  // certificates to join a multiprofile session.
+  if (policy::PolicyCertServiceFactory::UsedPolicyCertificates(user_email))
+    return false;
+
+  // Don't allow any secondary profiles if the primary profile is potentially
+  // tainted.
+  const User* primary_user = user_manager->GetPrimaryUser();
+  Profile* profile =
+      primary_user ? user_manager->GetProfileByUser(primary_user) : NULL;
+  policy::PolicyCertService* service =
+      profile ? policy::PolicyCertServiceFactory::GetForProfile(profile) : NULL;
+  if (service && (service->UsedPolicyCertificates() ||
+                  service->has_policy_certificates())) {

[pneubeck (no reviews), 2013/12/18 13:21:30]

another nit:

if the service does not exist, for whatever reasons, then we could still check
the tainted state in local_state (i.e. UsedPolicyCertificates).
Is it worth it, to directly call the static function of
PolicyCertServiceFactory?

your test wouldn't depend on the PolicyCertService anymore also.

[Joao da Silva, 2013/12/19 09:13:24]

local_state doesn't have the has_trust_anchors_ signal: it's possible that a
profile isn't tainted yet, but has the certs in memory; in that case we don't
want to add any other profiles either.

[pneubeck (no reviews), 2013/12/19 09:29:32]

yes, that's clear.
I meant, instead of lines 91 - 99:

const User* primary_user = user_manager->GetPrimaryUser();

// Don't allow any secondary profiles if the primary profile is
// tainted.
if
(policy::PolicyCertServiceFactory::UsedPolicyCertificates(primary_user->email()))
  return false;

// Don't allow any secondary profiles also if the primary profile is not tainted
yet, but might be potentially tainted.
Profile* profile =
  primary_user ? user_manager->GetProfileByUser(primary_user) : NULL;
policy::PolicyCertService* service =
  profile ? policy::PolicyCertServiceFactory::GetForProfile(profile) : NULL;
if (service && service->has_policy_certificates()))
  return false

[Joao da Silva, 2013/12/19 10:37:26]

I see what you meant now, thanks. Done, and added another test to cover that
code path too.

+    return false;
+  }
+
   // No user is allowed if the primary user policy forbids it.
   const std::string primary_user_behavior = GetCachedValue(primary_user_email);
   if (primary_user_behavior == kBehaviorNotAllowed)
     return false;
 
   // The user must have 'unrestricted' policy to be a secondary user.
   const std::string behavior = GetCachedValue(user_email);
   return behavior == kBehaviorUnrestricted;
 }
 
 void MultiProfileUserController::StartObserving(Profile* user_profile) {
   // Profile name could be empty during tests.
   if (user_profile->GetProfileName().empty())
     return;
 
   scoped_ptr<PrefChangeRegistrar> registrar(new PrefChangeRegistrar);
   registrar->Init(user_profile->GetPrefs());
   registrar->Add(
       prefs::kMultiProfileUserBehavior,
       base::Bind(&MultiProfileUserController::OnUserPrefChanged,
                  base::Unretained(this),
                  user_profile));
   pref_watchers_.push_back(registrar.release());
 
   OnUserPrefChanged(user_profile);
 }
 
-void MultiProfileUserController::RemoveCachedValue(
+void MultiProfileUserController::RemoveCachedValues(
     const std::string& user_email) {
   DictionaryPrefUpdate update(local_state_,
                               prefs::kCachedMultiProfileUserBehavior);
   update->RemoveWithoutPathExpansion(user_email, NULL);
+  policy::PolicyCertServiceFactory::ClearUsedPolicyCertificates(user_email);
 }
 
 std::string MultiProfileUserController::GetCachedValue(
     const std::string& user_email) const {
   const DictionaryValue* dict =
       local_state_->GetDictionary(prefs::kCachedMultiProfileUserBehavior);
   std::string value;
   if (dict && dict->GetStringWithoutPathExpansion(user_email, &value))
     return SanitizeBehaviorValue(value);
 
@@ skipping 27 matching lines @@
 
   PrefService* prefs = user_profile->GetPrefs();
   const std::string behavior =
       prefs->GetString(prefs::kMultiProfileUserBehavior);
   SetCachedValue(user_email, behavior);
 
   CheckSessionUsers();
 }
 
 }  // namespace chromeos