Only import certificates with Web trust from ONC if the user is managed and matches the enterprise domain of the device.

BUG=chromium-os:33879

Review URL: https://chromiumcodereview.appspot.com/10868076

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@155390 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 48 insertions, 22 deletions

diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
index 1fdaeda..dbea8c9 100644
--- a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
+++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
@@ -62,7 +62,6 @@
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
-#include "chrome/browser/policy/network_configuration_updater.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -419,12 +418,15 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() {
   // -- This used to be in ChromeBrowserMainParts::PreMainMessageLoopRun()
   // -- just after CreateProfile().
 
+  policy::BrowserPolicyConnector* connector =
+      g_browser_process->browser_policy_connector();
+
   if (parsed_command_line().HasSwitch(switches::kLoginUser) &&
       !parsed_command_line().HasSwitch(switches::kLoginPassword)) {
     // Pass the TokenService pointer to the policy connector so user policy can
     // grab a token and register with the policy server.
     // TODO(mnissler): Remove once OAuth is the only authentication mechanism.
-    g_browser_process->browser_policy_connector()->SetUserPolicyTokenService(
+    connector->SetUserPolicyTokenService(
         TokenServiceFactory::GetForProfile(profile()));
 
     // Make sure we flip every profile to not share proxies if the user hasn't
@@ -435,10 +437,9 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() {
       profile()->GetPrefs()->SetBoolean(prefs::kUseSharedProxies, false);
   }
 
-  network_config_updater_.reset(
-      new policy::NetworkConfigurationUpdater(
-          g_browser_process->policy_service(),
-          chromeos::CrosLibrary::Get()->GetNetworkLibrary()));
+  // Make sure the NetworkConfigurationUpdater is ready so that it pushes ONC
+  // configuration before login.
+  connector->GetNetworkConfigurationUpdater();
 
   // Make sure that wallpaper boot transition and other delays in OOBE
   // are disabled for tests by default.
@@ -561,9 +562,7 @@ void ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() {
   power_button_observer_.reset();
   screen_dimming_observer_.reset();
 
-  // Delete NetworkConfigurationUpdater and ContactManager while
-  // |g_browser_process| is still alive.
-  network_config_updater_.reset();
+  // Delete ContactManager while |g_browser_process| is still alive.
   contact_manager_.reset();
 
   ChromeBrowserMainPartsLinux::PostMainMessageLoopRun();
diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.h b/chrome/browser/chromeos/chrome_browser_main_chromeos.h
index cb0ddd1..29972a4 100644
--- a/chrome/browser/chromeos/chrome_browser_main_chromeos.h
+++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.h
@@ -27,10 +27,6 @@ namespace contacts {
 class ContactManager;
 }  // namespace contacts
 
-namespace policy {
-class NetworkConfigurationUpdater;
-}  // namespace policy
-
 class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux {
  public:
   explicit ChromeBrowserMainPartsChromeos(
@@ -67,7 +63,6 @@ class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux {
   scoped_ptr<chromeos::UserActivityNotifier> user_activity_notifier_;
   scoped_ptr<chromeos::VideoActivityNotifier> video_activity_notifier_;
   scoped_ptr<chromeos::ScreenDimmingObserver> screen_dimming_observer_;
-  scoped_ptr<policy::NetworkConfigurationUpdater> network_config_updater_;
   scoped_refptr<chromeos::MediaDeviceNotifications> media_device_notifications_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainPartsChromeos);
diff --git a/chrome/browser/chromeos/cros/mock_network_library.h b/chrome/browser/chromeos/cros/mock_network_library.h
index f140757..c6fedcb 100644
--- a/chrome/browser/chromeos/cros/mock_network_library.h
+++ b/chrome/browser/chromeos/cros/mock_network_library.h
@@ -189,9 +189,10 @@ class MockNetworkLibrary : public NetworkLibrary {
                                      const std::string&,
                                      int));
   MOCK_METHOD0(SwitchToPreferredNetwork, void(void));
-  MOCK_METHOD4(LoadOncNetworks, bool(const std::string&,
+  MOCK_METHOD5(LoadOncNetworks, bool(const std::string&,
                                      const std::string&,
                                      NetworkUIData::ONCSource,
+                                     bool,
                                      std::string*));
   MOCK_METHOD2(SetActiveNetwork, bool(ConnectionType, const std::string&));
 };
diff --git a/chrome/browser/chromeos/cros/network_library.h b/chrome/browser/chromeos/cros/network_library.h
index c8f2acdc..40dbc86 100644
--- a/chrome/browser/chromeos/cros/network_library.h
+++ b/chrome/browser/chromeos/cros/network_library.h
@@ -1693,6 +1693,7 @@ class NetworkLibrary {
   virtual bool LoadOncNetworks(const std::string& onc_blob,
                                const std::string& passcode,
                                NetworkUIData::ONCSource source,
+                               bool allow_web_trust_from_policy,
                                std::string* error) = 0;
 
   // This sets the active network for the network type. Note: priority order
diff --git a/chrome/browser/chromeos/cros/network_library_impl_base.cc b/chrome/browser/chromeos/cros/network_library_impl_base.cc
index 4df7501..4b59bfe 100644
--- a/chrome/browser/chromeos/cros/network_library_impl_base.cc
+++ b/chrome/browser/chromeos/cros/network_library_impl_base.cc
@@ -1152,8 +1152,10 @@ void NetworkLibraryImplBase::SwitchToPreferredNetwork() {
 bool NetworkLibraryImplBase::LoadOncNetworks(const std::string& onc_blob,
                                              const std::string& passphrase,
                                              NetworkUIData::ONCSource source,
+                                             bool allow_web_trust_from_policy,
                                              std::string* error) {
   OncNetworkParser parser(onc_blob, passphrase, source);
+  parser.set_allow_web_trust_from_policy(allow_web_trust_from_policy);
 
   if (!parser.parse_error().empty()) {
     if (error)
@@ -1163,7 +1165,8 @@ bool NetworkLibraryImplBase::LoadOncNetworks(const std::string& onc_blob,
 
   for (int i = 0; i < parser.GetCertificatesSize(); i++) {
     // Insert each of the available certs into the certificate DB.
-    if (parser.ParseCertificate(i).get() == NULL) {
+    if (parser.ParseCertificate(i).get() == NULL &&
+        !parser.parse_error().empty()) {
       DLOG(WARNING) << "Cannot parse certificate in ONC file";
       if (error)
         *error = parser.parse_error();
diff --git a/chrome/browser/chromeos/cros/network_library_impl_base.h b/chrome/browser/chromeos/cros/network_library_impl_base.h
index f7a3c4d..29bebfd 100644
--- a/chrome/browser/chromeos/cros/network_library_impl_base.h
+++ b/chrome/browser/chromeos/cros/network_library_impl_base.h
@@ -241,6 +241,7 @@ class NetworkLibraryImplBase : public NetworkLibrary {
   virtual bool LoadOncNetworks(const std::string& onc_blob,
                                const std::string& passphrase,
                                NetworkUIData::ONCSource source,
+                               bool allow_web_trust_from_policy,
                                std::string* error) OVERRIDE;
   virtual bool SetActiveNetwork(ConnectionType type,
                                 const std::string& service_path) OVERRIDE;
diff --git a/chrome/browser/chromeos/cros/onc_network_parser.cc b/chrome/browser/chromeos/cros/onc_network_parser.cc
index 8814d0c..648bdbd 100644
--- a/chrome/browser/chromeos/cros/onc_network_parser.cc
+++ b/chrome/browser/chromeos/cros/onc_network_parser.cc
@@ -282,6 +282,7 @@ OncNetworkParser::OncNetworkParser(const std::string& onc_blob,
                                    NetworkUIData::ONCSource onc_source)
     : NetworkParser(get_onc_mapper()),
       onc_source_(onc_source),
+      allow_web_trust_from_policy_(false),
       network_configs_(NULL),
       certificates_(NULL) {
   VLOG(2) << __func__ << ": OncNetworkParser called on " << onc_blob;
@@ -822,7 +823,13 @@ OncNetworkParser::ParseServerOrCaCertificate(
     const std::string& cert_type,
     const std::string& guid,
     base::DictionaryValue* certificate) {
-  net::CertDatabase cert_database;
+  // Device policy can't import certificates.
+  if (onc_source_ == NetworkUIData::ONC_SOURCE_DEVICE_POLICY) {
+    LOG(WARNING) << "Refusing to import certificate from device policy";
+    // This isn't a parsing error, so just return NULL here.
+    return NULL;
+  }
+
   bool web_trust = false;
   base::ListValue* trust_list = NULL;
   if (certificate->GetList("Trust", &trust_list)) {
@@ -850,6 +857,14 @@ OncNetworkParser::ParseServerOrCaCertificate(
     }
   }
 
+  // Web trust is only granted to certificates imported for a managed user
+  // on a managed device.
+  if (onc_source_ == NetworkUIData::ONC_SOURCE_USER_POLICY &&
+      web_trust && !allow_web_trust_from_policy_) {
+    LOG(WARNING) << "Web trust not granted for certificate: " << guid;
+    web_trust = false;
+  }
+
   std::string x509_data;
   if (!certificate->GetString("X509", &x509_data) || x509_data.empty()) {
     LOG(WARNING) << "ONC File: certificate missing appropriate "
@@ -915,6 +930,7 @@ OncNetworkParser::ParseServerOrCaCertificate(
   // TODO(mnissler, gspencer): We should probably switch to a mode where we
   // keep our own database for mapping GUIDs to certs in order to enable several
   // GUIDs to map to the same cert. See http://crosbug.com/26073.
+  net::CertDatabase cert_database;
   if (x509_cert->os_cert_handle()->isperm) {
     if (!cert_database.DeleteCertAndKey(x509_cert.get())) {
       parse_error_ = l10n_util::GetStringUTF8(
diff --git a/chrome/browser/chromeos/cros/onc_network_parser.h b/chrome/browser/chromeos/cros/onc_network_parser.h
index af8fd28..c819035 100644
--- a/chrome/browser/chromeos/cros/onc_network_parser.h
+++ b/chrome/browser/chromeos/cros/onc_network_parser.h
@@ -60,6 +60,12 @@ class OncNetworkParser : public NetworkParser {
   virtual ~OncNetworkParser();
   static const EnumMapper<PropertyIndex>* property_mapper();
 
+  // Certificates pushed from a policy source with Web trust are only imported
+  // with ParseCertificate() if this permission is granted.
+  void set_allow_web_trust_from_policy(bool allow) {
+    allow_web_trust_from_policy_ = allow;
+  }
+
   // Returns the number of networks in the "NetworkConfigs" list.
   int GetNetworkConfigsSize() const;
 
@@ -226,6 +232,10 @@ class OncNetworkParser : public NetworkParser {
   // Where the ONC blob comes from.
   NetworkUIData::ONCSource onc_source_;
 
+  // Whether certificates with Web trust should be stored when pushed from a
+  // policy source.
+  bool allow_web_trust_from_policy_;
+
   scoped_ptr<base::DictionaryValue> root_dict_;
   base::ListValue* network_configs_;
   base::ListValue* certificates_;
diff --git a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
index 2cddcda..c589c8e 100644
--- a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
@@ -134,7 +134,7 @@ class ExistingUserControllerTest : public CrosInProcessBrowserTest {
     mock_network_library_ = cros_mock_->mock_network_library();
     EXPECT_CALL(*mock_network_library_, AddUserActionObserver(_))
         .Times(AnyNumber());
-    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _))
+    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _, _))
         .WillRepeatedly(Return(true));
 
     MockSessionManagerClient* mock_session_manager_client =
diff --git a/chrome/browser/chromeos/login/login_browsertest.cc b/chrome/browser/chromeos/login/login_browsertest.cc
index 0e1503d..25f1b36 100644
--- a/chrome/browser/chromeos/login/login_browsertest.cc
+++ b/chrome/browser/chromeos/login/login_browsertest.cc
@@ -40,7 +40,7 @@ class LoginTestBase : public CrosInProcessBrowserTest {
         .WillRepeatedly(Return(false));
     EXPECT_CALL(*mock_network_library_, AddUserActionObserver(_))
         .Times(AnyNumber());
-    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _))
+    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _, _))
         .WillRepeatedly(Return(true));
   }
 
diff --git a/chrome/browser/chromeos/login/network_screen_browsertest.cc b/chrome/browser/chromeos/login/network_screen_browsertest.cc
index a47f332..638af2d 100644
--- a/chrome/browser/chromeos/login/network_screen_browsertest.cc
+++ b/chrome/browser/chromeos/login/network_screen_browsertest.cc
@@ -69,7 +69,7 @@ class NetworkScreenTest : public WizardInProcessBrowserTest {
         .Times(AnyNumber());
     EXPECT_CALL(*mock_network_library_, FindEthernetDevice())
         .Times(AnyNumber());
-    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _))
+    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _, _))
         .WillRepeatedly(Return(true));
 
     cros_mock_->SetStatusAreaMocksExpectations();
diff --git a/chrome/browser/chromeos/login/screen_locker_browsertest.cc b/chrome/browser/chromeos/login/screen_locker_browsertest.cc
index 9fb5f1e..981e31e 100644
--- a/chrome/browser/chromeos/login/screen_locker_browsertest.cc
+++ b/chrome/browser/chromeos/login/screen_locker_browsertest.cc
@@ -128,7 +128,7 @@ class ScreenLockerTest : public CrosInProcessBrowserTest {
         cros_mock_->mock_network_library();
     EXPECT_CALL(*mock_network_library, AddUserActionObserver(_))
         .Times(AnyNumber());
-    EXPECT_CALL(*mock_network_library, LoadOncNetworks(_, _, _, _))
+    EXPECT_CALL(*mock_network_library, LoadOncNetworks(_, _, _, _, _))
         .WillRepeatedly(Return(true));
   }
 
diff --git a/chrome/browser/chromeos/login/update_screen_browsertest.cc b/chrome/browser/chromeos/login/update_screen_browsertest.cc
index 58bda64..6442f1c 100644
--- a/chrome/browser/chromeos/login/update_screen_browsertest.cc
+++ b/chrome/browser/chromeos/login/update_screen_browsertest.cc
@@ -83,7 +83,7 @@ class UpdateScreenTest : public WizardInProcessBrowserTest {
         .Times(AnyNumber());
     EXPECT_CALL(*mock_network_library_, FindEthernetDevice())
         .Times(AnyNumber());
-    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _))
+    EXPECT_CALL(*mock_network_library_, LoadOncNetworks(_, _, _, _, _))
         .WillRepeatedly(Return(true));
   }