Resolve certificate references in ONC by PEM.

chrome/browser/chromeos/policy/network_configuration_updater_impl_cros.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_impl_cros.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 147 matching lines @@
                    << chromeos::onc::GetSourceAsString(onc_source)
                    << " is not a string value.";
     }
   }
 
   base::ListValue network_configs;
   base::ListValue certificates;
   ParseAndValidateOncForImport(
       onc_blob, onc_source, "", &network_configs, &certificates);
 
-  network_library_->LoadOncNetworks(network_configs, onc_source);
-
+  chromeos::CertificateHandler::CertsByGUID imported_server_and_ca_certs;
   scoped_ptr<net::CertificateList> web_trust_certs(new net::CertificateList);
   certificate_handler_->ImportCertificates(
-      certificates, onc_source, web_trust_certs.get());
+      certificates, onc_source, web_trust_certs.get(),
+      &imported_server_and_ca_certs);
+
+  if (!chromeos::onc::ResolveServerCertRefsInNetworks(
+          imported_server_and_ca_certs, &network_configs)) {
+    LOG(ERROR) << "Some certificate references in the ONC policy for source "
+               << chromeos::onc::GetSourceAsString(onc_source)
+               << " could not be resolved.";
+  }
+
+  net::CertificateList imported_server_and_ca_certs_list;
+  for (chromeos::CertificateHandler::CertsByGUID::iterator it =
+           imported_server_and_ca_certs.begin();
+       it != imported_server_and_ca_certs.end(); ++it) {
+    imported_server_and_ca_certs_list.push_back(it->second);
+  }
+  network_library_->LoadOncNetworks(
+      network_configs,
+      onc_source,
+      base::Bind(&chromeos::onc::GetPEMEncodedCertFromFingerprint,
+                 imported_server_and_ca_certs_list));

[stevenjb, 2013/06/21 22:47:13]

This feels awkward. Do we really need to abstract this, or can we just pass a
imported_server_and_ca_certs_list through and have LocalTranslator call
GetPEMEncodedCertFromFingerprint directly?

[pneubeck (no reviews), 2013/06/24 08:12:31]

This way it's easier to test the Translator and LoadOncNetworks. The
dependencies are also simpler.
Note, that X509Certificate is not an interface and does not provide a way for
Mocks/Stubs, which could have fake fingerprints.

[stevenjb, 2013/06/27 17:23:31]

I looked at this more closely, and I still don't like it much. 

* We call LoadOncNetworks() from two non-test places; both bind this to
GetPEMEncodedCertFromFingerprint().
* NetworkLibraryStubTest calls LoadOncNetworks() with a test callback, but it
uses the Stub implementation, which could override LoadOncNetworks() (or add a
private GetPEMEncodedCertFromFingerprint() method to NetworkLibrary that has the
different implementations in the stub vs. cros impl).

onc::GetPEMEncodedCertFromFingerprint seems like an implementation detail that
the caller shouldn't need to know about. I would much rather see that detail
provided by the cros / stub implementation.

I guess I don't care too much because I am hoping to move LoadOncNetworks() out
of NetworkLibrary very soon, but generally speaking I do not think the added
code complexity is justified for the sake of a unit test. Stub implementations
or test setters seem like better options.

 
   if (onc_source == chromeos::onc::ONC_SOURCE_USER_POLICY)
     SetTrustAnchors(web_trust_certs.Pass());
 }
 
 }  // namespace policy