Use per-user nssdb in onc certificate importer

chrome/browser/chromeos/policy/network_configuration_updater.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/chromeos/policy/network_configuration_updater.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/values.h"
@@ skipping 19 matching lines @@
   if (domain != POLICY_DOMAIN_CHROME)
     return;
 
   if (policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME)) {
     VLOG(1) << LogHeader() << " initialized.";
     policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, this);
     ApplyPolicy();
   }
 }
 
+void NetworkConfigurationUpdater::SetCertDatabase(
+    net::NSSCertDatabase* cert_database) {
+  DCHECK(!cert_database_);
+  cert_database_ = cert_database;
+
+  // If the policy service is ready, the policy has to be reapplied to ensure
+  // certificates are imported.
+  if (policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME))
+    ApplyPolicy();
+}
+
 NetworkConfigurationUpdater::NetworkConfigurationUpdater(
     onc::ONCSource onc_source,
     std::string policy_key,
     scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer,
     PolicyService* policy_service,
     chromeos::ManagedNetworkConfigurationHandler* network_config_handler)
     : onc_source_(onc_source),
       network_config_handler_(network_config_handler),
       certificate_importer_(certificate_importer.Pass()),
       policy_key_(policy_key),
       policy_change_registrar_(policy_service,
                                PolicyNamespace(POLICY_DOMAIN_CHROME,
                                                std::string())),
-      policy_service_(policy_service) {
+      policy_service_(policy_service),
+      cert_database_(NULL) {
 }
 
 void NetworkConfigurationUpdater::Init() {
   policy_change_registrar_.Observe(
       policy_key_,
       base::Bind(&NetworkConfigurationUpdater::OnPolicyChanged,
                  base::Unretained(this)));
 
   if (policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME)) {
     VLOG(1) << LogHeader() << " is already initialized.";
     ApplyPolicy();
   } else {
     policy_service_->AddObserver(POLICY_DOMAIN_CHROME, this);
   }
 }
 
 void NetworkConfigurationUpdater::OnPolicyChanged(
     const base::Value* previous,
     const base::Value* current) {
   VLOG(1) << LogHeader() << " changed.";
   ApplyPolicy();
 }
 
 void NetworkConfigurationUpdater::ApplyPolicy() {

[tbarzic, 2014/02/04 23:44:56]

We could also bail out here if database_ is not set (and if the
NetworkConfigurationUpdater supports certificate importing), but I'm concerned
that it may take a while until NSS db is initialized (e.g. TPMTokenLoader logic
has a retry logic that allows delays up to 5 min).

[pneubeck (no reviews), 2014/02/05 11:03:27]

In case of Device*Updater, we cannot wait for cert_database_.

Sorry for the late insight, but I think we get the most advantages by doing the
asynchronous call GetNSSCertDatabaseForProfile independent of LoginUtils in the
User*UpdaterFactory.
The Factory should then construct the CertificateImporter(nss_database) and pass
the finished Importer to the User*Updater.
The User*Updater would cache the certificates policy until the importer is set.
The CertificateImporter can be removed from the base class and the
Device*Updater.
That requires only one additional pending_certificate_policy_ member in the
User*Updater, the nss_database dependency doesn't have to be passed around and
network updates wouldn't be delayed or the policy be reapplied.
The unit test will be simpler, because the CertificateImporter doesn't have to
be mocked before it's passed to the Updater.


Does that sound reasonable?

[tbarzic, 2014/02/06 01:19:42]

yes it does. I had a similar idea, but I didn't go that way because the approach
wouldn't work well with device policy.. but apparently, I don't have to worry
about that anymore. Also, there is a problem that User*Updater factory is called
before
GetNSSCertDatabaseForProfile can be used, but I think I can work around this.

   const PolicyMap& policies = policy_service_->GetPolicies(
       PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()));
   const base::Value* policy_value = policies.GetValue(policy_key_);
 
   std::string onc_blob;
   if (!policy_value)
     VLOG(2) << LogHeader() << " is not set.";
   else if (!policy_value->GetAsString(&onc_blob))
     LOG(ERROR) << LogHeader() << " is not a string value.";
 
   base::ListValue network_configs;
   base::DictionaryValue global_network_config;
   base::ListValue certificates;
   chromeos::onc::ParseAndValidateOncForImport(onc_blob,
                                               onc_source_,
                                               "" /* no passphrase */,
                                               &network_configs,
                                               &global_network_config,
                                               &certificates);
 
-  ImportCertificates(certificates);
+  if (cert_database_)
+    ImportCertificates(certificates, cert_database_);
   ApplyNetworkPolicy(&network_configs, &global_network_config);
 }
 
 std::string NetworkConfigurationUpdater::LogHeader() const {
   return chromeos::onc::GetSourceAsString(onc_source_);
 }
 
 }  // namespace policy