Split ONC certificate parsing from certificate import

Move parsing of the 'Certificates' section of ONC to the new class
OncParsedCertificates. CertificateImporter uses this as its input.
This is a preparation for not importing policy-provided server and
authority certificates into the NSS database anymore, but only making
them available as temporary NSS certificates.

      chromeos_unittests --gtest_filter=*CertificateImporter* &&
      chromeos_unittests --gtest_filter=OncParsedCertificatesTest*

Bug: 787602
Test: unit_tests --gtest_filter=*NetworkConfigurationUpdater* &&
Change-Id: Id42f512c9b2287eb582ef3f6ade6b31f32eaf024
Reviewed-on: https://chromium-review.googlesource.com/898963Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534719}

chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc

@@ -22,6 +22,7 @@
 #include "chromeos/network/fake_network_device_handler.h"
 #include "chromeos/network/mock_managed_network_configuration_handler.h"
 #include "chromeos/network/onc/onc_certificate_importer.h"
+#include "chromeos/network/onc/onc_parsed_certificates.h"
 #include "chromeos/network/onc/onc_test_utils.h"
 #include "chromeos/network/onc/onc_utils.h"
 #include "components/onc/onc_constants.h"
@@ -121,8 +122,10 @@ class FakeCertificateImporter : public chromeos::onc::CertificateImporter {
     onc_trusted_certificates_ = std::move(onc_trusted_certificates);
   }
 
-  void SetExpectedONCCertificates(const base::ListValue& certificates) {
-    expected_onc_certificates_.reset(certificates.DeepCopy());
+  void SetExpectedONCCertificates(
+      std::unique_ptr<chromeos::onc::OncParsedCertificates>
+          expected_onc_certificates) {
+    expected_onc_certificates_ = std::move(expected_onc_certificates);
   }
 
   void SetExpectedONCSource(::onc::ONCSource source) {
@@ -135,23 +138,29 @@ class FakeCertificateImporter : public chromeos::onc::CertificateImporter {
     return count;
   }
 
-  void ImportCertificates(const base::ListValue& certificates,
-                          ::onc::ONCSource source,
-                          const DoneCallback& done_callback) override {
+  void ImportCertificates(
+      std::unique_ptr<chromeos::onc::OncParsedCertificates> certificates,
+      ::onc::ONCSource source,
+      DoneCallback done_callback) override {
     if (expected_onc_source_ != ::onc::ONC_SOURCE_UNKNOWN)
       EXPECT_EQ(expected_onc_source_, source);
     if (expected_onc_certificates_) {
-      EXPECT_TRUE(chromeos::onc::test_utils::Equals(
-          expected_onc_certificates_.get(), &certificates));
+      EXPECT_EQ(expected_onc_certificates_->has_error(),
+                certificates->has_error());
+      EXPECT_EQ(expected_onc_certificates_->server_or_authority_certificates(),
+                certificates->server_or_authority_certificates());
+      EXPECT_EQ(expected_onc_certificates_->client_certificates(),
+                certificates->client_certificates());
     }
+
     ++call_count_;
-    done_callback.Run(true, net::x509_util::DupCERTCertificateList(
-                                onc_trusted_certificates_));
+    std::move(done_callback).Run(true, std::move(onc_trusted_certificates_));
   }
 
  private:
   ::onc::ONCSource expected_onc_source_;
-  std::unique_ptr<base::ListValue> expected_onc_certificates_;
+  std::unique_ptr<chromeos::onc::OncParsedCertificates>
+      expected_onc_certificates_;
   net::ScopedCERTCertificateList onc_trusted_certificates_;
   unsigned int call_count_;
 
@@ -173,7 +182,7 @@ const char kFakeONC[] =
     "  },"
     "  \"Certificates\": ["
     "    { \"GUID\": \"{f998f760-272b-6939-4c2beffe428697ac}\","
-    "      \"PKCS12\": \"abc\","
+    "      \"PKCS12\": \"YWJj\","
     "      \"Type\": \"Client\" }"
     "  ],"
     "  \"Type\": \"UnencryptedConfiguration\""
@@ -238,7 +247,8 @@ class NetworkConfigurationUpdaterTest : public testing::Test {
     base::ListValue* certs = NULL;
     fake_toplevel_onc->GetListWithoutPathExpansion(
         onc::toplevel_config::kCertificates, &certs);
-    AppendAll(*certs, &fake_certificates_);
+    fake_certificates_ =
+        std::make_unique<chromeos::onc::OncParsedCertificates>(*certs);
 
     certificate_importer_ = new FakeCertificateImporter;
     certificate_importer_owned_.reset(certificate_importer_);
@@ -297,7 +307,7 @@ class NetworkConfigurationUpdaterTest : public testing::Test {
 
   base::ListValue fake_network_configs_;
   base::DictionaryValue fake_global_network_config_;
-  base::ListValue fake_certificates_;
+  std::unique_ptr<chromeos::onc::OncParsedCertificates> fake_certificates_;
   StrictMock<chromeos::MockManagedNetworkConfigurationHandler>
       network_config_handler_;
   FakeNetworkDeviceHandler network_device_handler_;
@@ -515,7 +525,8 @@ TEST_F(NetworkConfigurationUpdaterTest,
   Mock::VerifyAndClearExpectations(&network_config_handler_);
   EXPECT_EQ(0u, certificate_importer_->GetAndResetImportCount());
 
-  certificate_importer_->SetExpectedONCCertificates(fake_certificates_);
+  certificate_importer_->SetExpectedONCCertificates(
+      std::move(fake_certificates_));
   certificate_importer_->SetExpectedONCSource(onc::ONC_SOURCE_USER_POLICY);
 
   ASSERT_TRUE(certificate_importer_owned_);
@@ -573,7 +584,8 @@ TEST_P(NetworkConfigurationUpdaterTestWithParam, InitialUpdates) {
                         ExpectedUsernameHash(),
                         IsEqualTo(&fake_network_configs_),
                         IsEqualTo(&fake_global_network_config_)));
-  certificate_importer_->SetExpectedONCCertificates(fake_certificates_);
+  certificate_importer_->SetExpectedONCCertificates(
+      std::move(fake_certificates_));
   certificate_importer_->SetExpectedONCSource(CurrentONCSource());
 
   CreateNetworkConfigurationUpdater();
@@ -601,7 +613,8 @@ TEST_P(NetworkConfigurationUpdaterTestWithParam,
                         IsEqualTo(&fake_network_configs_),
                         IsEqualTo(&fake_global_network_config_)));
   certificate_importer_->SetExpectedONCSource(CurrentONCSource());
-  certificate_importer_->SetExpectedONCCertificates(fake_certificates_);
+  certificate_importer_->SetExpectedONCCertificates(
+      std::move(fake_certificates_));
 
   MarkPolicyProviderInitialized();
   EXPECT_EQ(ExpectedImportCertificatesCallCount(),
@@ -624,7 +637,8 @@ TEST_P(NetworkConfigurationUpdaterTestWithParam,
                         IsEqualTo(&fake_network_configs_),
                         IsEqualTo(&fake_global_network_config_)));
   certificate_importer_->SetExpectedONCSource(CurrentONCSource());
-  certificate_importer_->SetExpectedONCCertificates(fake_certificates_);
+  certificate_importer_->SetExpectedONCCertificates(
+      std::move(fake_certificates_));
 
   CreateNetworkConfigurationUpdater();
 
@@ -650,7 +664,8 @@ TEST_P(NetworkConfigurationUpdaterTestWithParam, PolicyChange) {
                         IsEqualTo(&fake_network_configs_),
                         IsEqualTo(&fake_global_network_config_)));
   certificate_importer_->SetExpectedONCSource(CurrentONCSource());
-  certificate_importer_->SetExpectedONCCertificates(fake_certificates_);
+  certificate_importer_->SetExpectedONCCertificates(
+      std::move(fake_certificates_));
 
   PolicyMap policy;
   policy.Set(GetParam(), POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
@@ -664,7 +679,9 @@ TEST_P(NetworkConfigurationUpdaterTestWithParam, PolicyChange) {
   // Another update is expected if the policy goes away.
   EXPECT_CALL(network_config_handler_,
               SetPolicy(CurrentONCSource(), _, IsEmpty(), IsEmpty()));
-  certificate_importer_->SetExpectedONCCertificates(base::ListValue());
+  certificate_importer_->SetExpectedONCCertificates(
+      std::make_unique<chromeos::onc::OncParsedCertificates>(
+          base::ListValue()));
 
   policy.Erase(GetParam());
   UpdateProviderPolicy(policy);

chrome/browser/chromeos/policy/user_network_configuration_updater.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chromeos/network/managed_network_configuration_handler.h"
 #include "chromeos/network/onc/onc_certificate_importer_impl.h"
+#include "chromeos/network/onc/onc_parsed_certificates.h"
 #include "chromeos/network/onc/onc_utils.h"
 #include "components/policy/policy_constants.h"
 #include "components/user_manager/user.h"
@@ -109,7 +110,7 @@ void UserNetworkConfigurationUpdater::ImportCertificates(
   }
 
   certificate_importer_->ImportCertificates(
-      certificates_onc,
+      std::make_unique<chromeos::onc::OncParsedCertificates>(certificates_onc),
       onc_source_,
       base::Bind(&UserNetworkConfigurationUpdater::OnCertificatesImported,
                  base::Unretained(this)));

chrome/browser/ui/webui/net_internals/net_internals_ui.cc

@@ -84,6 +84,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon_client.h"
 #include "chromeos/network/onc/onc_certificate_importer_impl.h"
+#include "chromeos/network/onc/onc_parsed_certificates.h"
 #include "chromeos/network/onc/onc_utils.h"
 #endif
 
@@ -1020,11 +1021,10 @@ void NetInternalsMessageHandler::ImportONCFileToNSSDB(
   chromeos::onc::CertificateImporterImpl cert_importer(
       BrowserThread::GetTaskRunnerForThread(BrowserThread::IO), nssdb);
   cert_importer.ImportCertificates(
-      certificates,
+      std::make_unique<chromeos::onc::OncParsedCertificates>(certificates),
       onc_source,
       base::Bind(&NetInternalsMessageHandler::OnCertificatesImported,
-                 AsWeakPtr(),
-                 error));
+                 AsWeakPtr(), error));
 }
 
 void NetInternalsMessageHandler::OnCertificatesImported(

chromeos/BUILD.gn

@@ -419,6 +419,8 @@ component("chromeos") {
     "network/onc/onc_merger.h",
     "network/onc/onc_normalizer.cc",
     "network/onc/onc_normalizer.h",
+    "network/onc/onc_parsed_certificates.cc",
+    "network/onc/onc_parsed_certificates.h",
     "network/onc/onc_signature.cc",
     "network/onc/onc_signature.h",
     "network/onc/onc_translation_tables.cc",
@@ -717,6 +719,7 @@ test("chromeos_unittests") {
     "network/onc/onc_certificate_importer_impl_unittest.cc",
     "network/onc/onc_merger_unittest.cc",
     "network/onc/onc_normalizer_unittest.cc",
+    "network/onc/onc_parsed_certificates_unittest.cc",
     "network/onc/onc_translator_unittest.cc",
     "network/onc/onc_utils_unittest.cc",
     "network/onc/onc_validator_unittest.cc",

chromeos/network/onc/onc_certificate_importer.h

@@ -8,19 +8,16 @@
 #include "base/callback_forward.h"
 #include "base/macros.h"
 #include "chromeos/chromeos_export.h"
+#include "chromeos/network/onc/onc_parsed_certificates.h"
 #include "components/onc/onc_constants.h"
 #include "net/cert/scoped_nss_types.h"
 
-namespace base {
-class ListValue;
-}
-
 namespace chromeos {
 namespace onc {
 
 class CHROMEOS_EXPORT CertificateImporter {
  public:
-  typedef base::Callback<void(
+  typedef base::OnceCallback<void(
       bool success,
       net::ScopedCERTCertificateList onc_trusted_certificates)>
       DoneCallback;
@@ -28,7 +25,7 @@ class CHROMEOS_EXPORT CertificateImporter {
   CertificateImporter() {}
   virtual ~CertificateImporter() {}
 
-  // Import |certificates|, which must be a list of ONC Certificate objects.
+  // Import |certificates|.
   // Certificates are only imported with web trust for user imports. If the
   // "Remove" field of a certificate is enabled, then removes the certificate
   // from the store instead of importing.
@@ -37,9 +34,10 @@ class CHROMEOS_EXPORT CertificateImporter {
   // |onc_trusted_certificates| will contain the list of certificates that
   // were imported and requested the TrustBit "Web".
   // Never calls |done_callback| after this importer is destructed.
-  virtual void ImportCertificates(const base::ListValue& certificates,
-                                  ::onc::ONCSource source,
-                                  const DoneCallback& done_callback) = 0;
+  virtual void ImportCertificates(
+      std::unique_ptr<OncParsedCertificates> certificates,
+      ::onc::ONCSource source,
+      DoneCallback done_callback) = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(CertificateImporter);

chromeos/network/onc/onc_certificate_importer_impl.h

@@ -15,11 +15,10 @@
 #include "base/memory/weak_ptr.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/network/onc/onc_certificate_importer.h"
+#include "chromeos/network/onc/onc_parsed_certificates.h"
 #include "components/onc/onc_constants.h"
 
 namespace base {
-class DictionaryValue;
-class ListValue;
 class SequencedTaskRunner;
 }
 
@@ -45,47 +44,37 @@ class CHROMEOS_EXPORT CertificateImporterImpl : public CertificateImporter {
   ~CertificateImporterImpl() override;
 
   // CertificateImporter overrides
-  void ImportCertificates(const base::ListValue& certificates,
+  void ImportCertificates(std::unique_ptr<OncParsedCertificates> certificates,
                           ::onc::ONCSource source,
-                          const DoneCallback& done_callback) override;
+                          DoneCallback done_callback) override;
 
  private:
-  void RunDoneCallback(const CertificateImporter::DoneCallback& callback,
+  void RunDoneCallback(DoneCallback callback,
                        bool success,
                        net::ScopedCERTCertificateList onc_trusted_certificates);
 
   // This is the synchronous implementation of ImportCertificates. It is
   // executed on the given |io_task_runner_|.
-  static void ParseAndStoreCertificates(::onc::ONCSource source,
-                                        const DoneCallback& done_callback,
-                                        base::ListValue* certificates,
-                                        net::NSSCertDatabase* nssdb);
-
-  // Parses and stores |certificate| in the certificate store. Returns true if
-  // the operation succeeded.
-  static bool ParseAndStoreCertificate(
+  static void StoreCertificates(
       ::onc::ONCSource source,
-      bool allow_trust_imports,
-      const base::DictionaryValue& certificate,
-      net::NSSCertDatabase* nssdb,
-      net::ScopedCERTCertificateList* onc_trusted_certificates);
+      DoneCallback done_callback,
+      std::unique_ptr<OncParsedCertificates> certificates,
+      net::NSSCertDatabase* nssdb);
 
   // Imports the Server or CA certificate |certificate|. Web trust is only
   // applied if the certificate requests the TrustBits attribute "Web" and if
   // the |allow_trust_imports| permission is granted, otherwise the attribute is
   // ignored.
-  static bool ParseServerOrCaCertificate(
+  static bool StoreServerOrCaCertificate(
       ::onc::ONCSource source,
       bool allow_trust_imports,
-      const std::string& cert_type,
-      const std::string& guid,
-      const base::DictionaryValue& certificate,
+      const OncParsedCertificates::ServerOrAuthorityCertificate& certificate,
       net::NSSCertDatabase* nssdb,
       net::ScopedCERTCertificateList* onc_trusted_certificates);
 
-  static bool ParseClientCertificate(const std::string& guid,
-                                     const base::DictionaryValue& certificate,
-                                     net::NSSCertDatabase* nssdb);
+  static bool StoreClientCertificate(
+      const OncParsedCertificates::ClientCertificate& certificate,
+      net::NSSCertDatabase* nssdb);
 
   // The task runner to use for NSSCertDatabase accesses.
   scoped_refptr<base::SequencedTaskRunner> io_task_runner_;

chromeos/network/onc/onc_certificate_importer_impl_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "chromeos/network/certificate_helper.h"
+#include "chromeos/network/onc/onc_parsed_certificates.h"
 #include "chromeos/network/onc/onc_test_utils.h"
 #include "components/onc/onc_constants.h"
 #include "crypto/scoped_test_nss_db.h"
@@ -76,11 +77,10 @@ class ONCCertificateImporterImplTest : public testing::Test {
     web_trust_certificates_.clear();
     CertificateImporterImpl importer(task_runner_, test_nssdb_.get());
     importer.ImportCertificates(
-        *certificates,
+        std::make_unique<chromeos::onc::OncParsedCertificates>(*certificates),
         ::onc::ONC_SOURCE_USER_IMPORT,  // allow web trust
         base::Bind(&ONCCertificateImporterImplTest::OnImportCompleted,
-                   base::Unretained(this),
-                   expected_success));
+                   base::Unretained(this), expected_success));
 
     task_runner_->RunUntilIdle();
 

chromeos/network/onc/onc_parsed_certificates.cc

+// Copyright 2018 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 "chromeos/network/onc/onc_parsed_certificates.h"
+
+#include <string>
+#include <vector>
+
+#include "base/base64.h"
+#include "base/optional.h"
+#include "base/values.h"
+#include "chromeos/network/onc/onc_utils.h"
+#include "components/onc/onc_constants.h"
+#include "net/cert/x509_certificate.h"
+
+namespace chromeos {
+namespace onc {
+
+namespace {
+
+enum class CertificateType { kServer, kAuthority, kClient };
+
+// Returns true if the certificate described by |onc_certificate| requests web
+// trust.
+bool HasWebTrustFlag(const base::Value& onc_certificate) {
+  DCHECK(onc_certificate.is_dict());
+
+  bool web_trust_flag = false;
+  const base::Value* trust_list = onc_certificate.FindKeyOfType(
+      ::onc::certificate::kTrustBits, base::Value::Type::LIST);
+  if (!trust_list)
+    return false;
+
+  for (const base::Value& trust_entry : trust_list->GetList()) {
+    DCHECK(trust_entry.is_string());
+
+    if (trust_entry.GetString() == ::onc::certificate::kWeb) {
+      // "Web" implies that the certificate is to be trusted for SSL
+      // identification.
+      web_trust_flag = true;
+    } else {
+      // Trust bits should only increase trust and never restrict. Thus,
+      // ignoring unknown bits should be safe.
+      LOG(WARNING) << "Certificate contains unknown trust type "
+                   << trust_entry.GetString();
+    }
+  }
+
+  return web_trust_flag;
+}
+
+// Converts the ONC string certificate type into the CertificateType enum.
+// Returns |base::nullopt| if the certificate type was not understood.
+base::Optional<CertificateType> GetCertTypeAsEnum(
+    const std::string& cert_type) {
+  if (cert_type == ::onc::certificate::kServer) {
+    return CertificateType::kServer;
+  }
+
+  if (cert_type == ::onc::certificate::kAuthority) {
+    return CertificateType::kAuthority;
+  }
+
+  if (cert_type == ::onc::certificate::kClient) {
+    return CertificateType::kClient;
+  }
+
+  return base::nullopt;
+}
+
+}  // namespace
+
+OncParsedCertificates::ServerOrAuthorityCertificate::
+    ServerOrAuthorityCertificate(
+        Type type,
+        const std::string& guid,
+        const scoped_refptr<net::X509Certificate>& certificate,
+        bool web_trust_requested)
+    : type_(type),
+      guid_(guid),
+      certificate_(certificate),
+      web_trust_requested_(web_trust_requested) {}
+
+OncParsedCertificates::ServerOrAuthorityCertificate::
+    ServerOrAuthorityCertificate(const ServerOrAuthorityCertificate& other) =
+        default;
+
+OncParsedCertificates::ServerOrAuthorityCertificate&
+OncParsedCertificates::ServerOrAuthorityCertificate::operator=(
+    const ServerOrAuthorityCertificate& other) = default;
+
+OncParsedCertificates::ServerOrAuthorityCertificate::
+    ServerOrAuthorityCertificate(ServerOrAuthorityCertificate&& other) =
+        default;
+
+OncParsedCertificates::ServerOrAuthorityCertificate::
+    ~ServerOrAuthorityCertificate() = default;
+
+bool OncParsedCertificates::ServerOrAuthorityCertificate::operator==(
+    const ServerOrAuthorityCertificate& other) const {
+  if (type() != other.type())
+    return false;
+
+  if (guid() != other.guid())
+    return false;
+
+  if (!certificate()->Equals(other.certificate().get()))
+    return false;
+
+  if (web_trust_requested() != other.web_trust_requested())
+    return false;
+
+  return true;
+}
+
+bool OncParsedCertificates::ServerOrAuthorityCertificate::operator!=(
+    const ServerOrAuthorityCertificate& other) const {
+  return !(*this == other);
+}
+
+OncParsedCertificates::ClientCertificate::ClientCertificate(
+    const std::string& guid,
+    const std::string& pkcs12_data)
+    : guid_(guid), pkcs12_data_(pkcs12_data) {}
+
+OncParsedCertificates::ClientCertificate::ClientCertificate(
+    const ClientCertificate& other) = default;
+
+OncParsedCertificates::ClientCertificate&
+OncParsedCertificates::ClientCertificate::operator=(
+    const ClientCertificate& other) = default;
+
+OncParsedCertificates::ClientCertificate::ClientCertificate(
+    ClientCertificate&& other) = default;
+
+OncParsedCertificates::ClientCertificate::~ClientCertificate() = default;
+
+bool OncParsedCertificates::ClientCertificate::operator==(
+    const ClientCertificate& other) const {
+  if (guid() != other.guid())
+    return false;
+
+  if (pkcs12_data() != other.pkcs12_data())
+    return false;
+
+  return true;
+}
+
+bool OncParsedCertificates::ClientCertificate::operator!=(
+    const ClientCertificate& other) const {
+  return !(*this == other);
+}
+
+OncParsedCertificates::OncParsedCertificates()
+    : OncParsedCertificates(base::ListValue()) {}
+
+OncParsedCertificates::OncParsedCertificates(
+    const base::Value& onc_certificates) {
+  if (!onc_certificates.is_list()) {
+    LOG(WARNING) << "Value is not a list";
+    has_error_ = true;
+    return;
+  }
+
+  for (size_t i = 0; i < onc_certificates.GetList().size(); ++i) {
+    const base::Value& onc_certificate = onc_certificates.GetList().at(i);
+    DCHECK(onc_certificate.is_dict());
+
+    VLOG(2) << "Parsing certificate at index " << i << ": " << onc_certificate;
+
+    if (!ParseCertificate(onc_certificate)) {
+      has_error_ = true;
+      LOG(ERROR) << "Cannot parse certificate at index " << i;
+    } else {
+      VLOG(2) << "Successfully parsed certificate at index " << i;
+    }
+  }
+}
+
+OncParsedCertificates::~OncParsedCertificates() = default;
+
+bool OncParsedCertificates::ParseCertificate(
+    const base::Value& onc_certificate) {
+  const base::Value* guid_key = onc_certificate.FindKeyOfType(
+      ::onc::certificate::kGUID, base::Value::Type::STRING);
+  DCHECK(guid_key);
+  std::string guid = guid_key->GetString();
+
+  const base::Value* type_key = onc_certificate.FindKeyOfType(
+      ::onc::certificate::kType, base::Value::Type::STRING);
+  DCHECK(type_key);
+  base::Optional<CertificateType> type_opt =
+      GetCertTypeAsEnum(type_key->GetString());
+  if (!type_opt)
+    return false;
+
+  switch (type_opt.value()) {
+    case CertificateType::kServer:
+      return ParseServerOrCaCertificate(
+          ServerOrAuthorityCertificate::Type::kServer, guid, onc_certificate);
+    case CertificateType::kAuthority:
+      return ParseServerOrCaCertificate(
+          ServerOrAuthorityCertificate::Type::kAuthority, guid,
+          onc_certificate);
+    case CertificateType::kClient:
+      return ParseClientCertificate(guid, onc_certificate);
+    default:
+      NOTREACHED();
+      return false;
+  }
+  return false;
+}
+
+bool OncParsedCertificates::ParseServerOrCaCertificate(
+    ServerOrAuthorityCertificate::Type type,
+    const std::string& guid,
+    const base::Value& onc_certificate) {
+  bool web_trust_requested = HasWebTrustFlag(onc_certificate);
+  const base::Value* x509_data_key = onc_certificate.FindKeyOfType(
+      ::onc::certificate::kX509, base::Value::Type::STRING);
+  if (!x509_data_key || x509_data_key->GetString().empty()) {
+    LOG(ERROR) << "Certificate missing " << ::onc::certificate::kX509
+               << " certificate data.";
+    return false;
+  }
+
+  std::string certificate_der_data = DecodePEM(x509_data_key->GetString());
+  if (certificate_der_data.empty()) {
+    LOG(ERROR) << "Unable to create certificate from PEM encoding.";
+    return false;
+  }
+
+  scoped_refptr<net::X509Certificate> certificate =
+      net::X509Certificate::CreateFromBytes(certificate_der_data.data(),
+                                            certificate_der_data.length());
+  if (!certificate) {
+    LOG(ERROR) << "Unable to create certificate from PEM encoding.";
+    return false;
+  }
+
+  server_or_authority_certificates_.push_back(ServerOrAuthorityCertificate(
+      type, guid, certificate, web_trust_requested));
+  return true;
+}
+
+bool OncParsedCertificates::ParseClientCertificate(
+    const std::string& guid,
+    const base::Value& onc_certificate) {
+  const base::Value* base64_pkcs12_data_key = onc_certificate.FindKeyOfType(
+      ::onc::certificate::kPKCS12, base::Value::Type::STRING);
+  if (!base64_pkcs12_data_key || base64_pkcs12_data_key->GetString().empty()) {
+    LOG(ERROR) << "PKCS12 data is missing for client certificate.";
+    return false;
+  }
+
+  std::string pkcs12_data;
+  if (!base::Base64Decode(base64_pkcs12_data_key->GetString(), &pkcs12_data)) {
+    LOG(ERROR) << "Unable to base64 decode PKCS#12 data: \""
+               << base64_pkcs12_data_key->GetString() << "\".";
+    return false;
+  }
+
+  client_certificates_.push_back(ClientCertificate(guid, pkcs12_data));
+  return true;
+}
+
+}  // namespace onc
+}  // namespace chromeos

chromeos/network/onc/onc_parsed_certificates.h

+// Copyright 2018 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.
+
+#ifndef CHROMEOS_NETWORK_ONC_ONC_PARSED_CERTIFICATES_H_
+#define CHROMEOS_NETWORK_ONC_ONC_PARSED_CERTIFICATES_H_
+
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "chromeos/chromeos_export.h"
+
+namespace base {
+class Value;
+}  // namespace base
+
+namespace net {
+class X509Certificate;
+}
+
+namespace chromeos {
+namespace onc {
+
+// Represents certificates parsed from the ONC Certificates section.
+class CHROMEOS_EXPORT OncParsedCertificates {
+ public:
+  // A Server or Authority certificate parsed from ONC. The payload is
+  // represented as a net::X509Certificate.
+  class ServerOrAuthorityCertificate {
+   public:
+    enum class Type { kServer, kAuthority };
+
+    ServerOrAuthorityCertificate(
+        Type type,
+        const std::string& guid,
+        const scoped_refptr<net::X509Certificate>& certificate,
+        bool web_trust_requested);
+
+    ServerOrAuthorityCertificate(const ServerOrAuthorityCertificate& other);
+    ServerOrAuthorityCertificate& operator=(
+        const ServerOrAuthorityCertificate& other);
+    ServerOrAuthorityCertificate(ServerOrAuthorityCertificate&& other);
+    ~ServerOrAuthorityCertificate();
+
+    bool operator==(const ServerOrAuthorityCertificate& other) const;
+    bool operator!=(const ServerOrAuthorityCertificate& other) const;
+
+    Type type() const { return type_; }
+
+    const std::string& guid() const { return guid_; }
+
+    const scoped_refptr<net::X509Certificate>& certificate() const {
+      return certificate_;
+    }
+    // Returns true if the certificate definition in ONC had the "Web" TrustBit.
+    bool web_trust_requested() const { return web_trust_requested_; }
+
+   private:
+    Type type_;
+    std::string guid_;
+    scoped_refptr<net::X509Certificate> certificate_;
+    bool web_trust_requested_;
+  };
+
+  // A Client certificate parsed from ONC. The payload is the PKCS12 payload
+  // (base64 decoded).
+  class ClientCertificate {
+   public:
+    ClientCertificate(const std::string& guid, const std::string& pkcs12_data);
+
+    ClientCertificate(const ClientCertificate& other);
+    ClientCertificate& operator=(const ClientCertificate& other);
+    ClientCertificate(ClientCertificate&& other);
+    ~ClientCertificate();
+
+    bool operator==(const ClientCertificate& other) const;
+    bool operator!=(const ClientCertificate& other) const;
+
+    const std::string& guid() const { return guid_; }
+
+    // The PKCS12 payload of the certificate. Note: Contrary to the PKCS12 field
+    // in ONC, this data is not base64-encoded.
+    const std::string& pkcs12_data() const { return pkcs12_data_; }
+
+   private:
+    std::string guid_;
+    std::string pkcs12_data_;
+  };
+
+  // Constructs an empty OncParsedCertificates. It will have no error, and the
+  // certificate lists will be empty.
+  OncParsedCertificates();
+
+  // Parses |onc_certificates|. This must be a Value of type LIST, corresponding
+  // to the Certificates part of the ONC specification.
+  explicit OncParsedCertificates(const base::Value& onc_certificates);
+  ~OncParsedCertificates();
+
+  // Returns all certificates that were successfully parsed and had the type
+  // Server or Authoriy.
+  const std::vector<ServerOrAuthorityCertificate>&
+  server_or_authority_certificates() const {
+    return server_or_authority_certificates_;
+  }
+
+  // Returns all certificates that were successfully parsed and had the type
+  // Client.
+  const std::vector<ClientCertificate>& client_certificates() const {
+    return client_certificates_;
+  }
+
+  // Returns true if any parsing error occured. Note that certificates which had
+  // no parsing errors will still be available through
+  // server_or_authority_certificates() and client_certificates().
+  bool has_error() const { return has_error_; }
+
+ private:
+  bool ParseCertificate(const base::Value& onc_certificate);
+  bool ParseServerOrCaCertificate(ServerOrAuthorityCertificate::Type type,
+                                  const std::string& guid,
+                                  const base::Value& onc_certificate);
+  bool ParseClientCertificate(const std::string& guid,
+                              const base::Value& onc_certificate);
+
+  std::vector<ServerOrAuthorityCertificate> server_or_authority_certificates_;
+  std::vector<ClientCertificate> client_certificates_;
+  bool has_error_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(OncParsedCertificates);
+};
+
+}  // namespace onc
+}  // namespace chromeos
+
+#endif  // CHROMEOS_NETWORK_ONC_ONC_PARSED_CERTIFICATES_H_

chromeos/network/onc/onc_utils.cc

@@ -386,8 +386,6 @@ std::unique_ptr<base::DictionaryValue> MaskCredentialsInOncObject(
   return OncMaskValues::Mask(signature, onc_object, mask);
 }
 
-namespace {
-
 std::string DecodePEM(const std::string& pem_encoded) {
   // The PEM block header used for DER certificates
   const char kCertificateHeader[] = "CERTIFICATE";
@@ -415,6 +413,8 @@ std::string DecodePEM(const std::string& pem_encoded) {
   return decoded;
 }
 
+namespace {
+
 CertPEMsByGUIDMap GetServerAndCACertsByGUID(
     const base::ListValue& certificates) {
   CertPEMsByGUIDMap certs_by_guid;

chromeos/network/onc/onc_utils.h

@@ -125,6 +125,10 @@ CHROMEOS_EXPORT bool ParseAndValidateOncForImport(
     base::DictionaryValue* global_network_config,
     base::ListValue* certificates);
 
+// Parse the given PEM encoded certificate |pem_encoded| and return the
+// contained DER encoding. Returns an empty string on failure.
+std::string DecodePEM(const std::string& pem_encoded);
+
 // Parse the given PEM encoded certificate |pem_encoded| and create a
 // CERTCertificate from it.
 CHROMEOS_EXPORT net::ScopedCERTCertificate DecodePEMCertificate(