Prefer device policy provided certs over user's

When user and device policies provide the same certificate, keep the
certificate as device-wide certificate when combining them.

Bug: chromium:1045902
Test: chromeos_unittests --gtest_filter=NetworkCertLoaderTest*
Change-Id: I8685ba03e5a333814583d158a216f8ff0a98865c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2025108Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Omar Morsi <omorsi@google.com>
Cr-Commit-Position: refs/heads/master@{#736999}

chromeos/network/network_cert_loader.cc

@@ -6,8 +6,8 @@
 
 #include <algorithm>
 #include <initializer_list>
+#include <map>
 #include <memory>
-#include <set>
 #include <utility>
 
 #include "base/bind.h"
@@ -81,11 +81,22 @@ NetworkCertLoader::NetworkCertList CombineNetworkCertLists(
     total_size += list->size();
   NetworkCertLoader::NetworkCertList result;
   result.reserve(total_size);
-  std::set<const CERTCertificate*> already_added_certs;
+
+  std::map<const CERTCertificate*, size_t> added_cert_to_position;
   for (const NetworkCertLoader::NetworkCertList* list : network_cert_lists) {
     for (const NetworkCertLoader::NetworkCert& network_cert : *list) {
-      if (already_added_certs.insert(network_cert.cert()).second)
+      auto it = added_cert_to_position.find(network_cert.cert());
+      if (it == added_cert_to_position.end()) {
+        // This certificate wasn't added before.
+        // Add it and save its position in the result list.
+        added_cert_to_position.insert({network_cert.cert(), result.size()});
         result.push_back(network_cert.Clone());
+      } else if (network_cert.is_device_wide()) {
+        // Replace the already added certificate with the device-wide one so
+        // that it can be used for shared configurations.
+        size_t position = it->second;
+        result[position] = network_cert.Clone();
+      }
     }
   }
   return result;

chromeos/network/network_cert_loader_unittest.cc

@@ -751,6 +751,44 @@ TEST_F(NetworkCertLoaderTest, UpdateOnTwoPolicyCertificateProviders) {
   ASSERT_EQ(1U, GetAndResetCertificatesLoadedEventsCount());
 }
 
+// Tests that device-wide certificates have higher priority when combining two
+// policy provided certificates such that one of them is device-wide and the
+// other is not.
+TEST_F(NetworkCertLoaderTest, PreferDeviceWideCertsWhenCombining) {
+  // Load same CA cert for device and user policy.
+  scoped_refptr<net::X509Certificate> certificate =
+      net::ImportCertFromFile(net::GetTestCertsDirectory(), "root_ca_cert.pem");
+
+  ASSERT_TRUE(certificate.get());
+
+  StartCertLoaderWithPrimaryDB();
+
+  FakePolicyCertificateProvider device_policy_certs_provider;
+  device_policy_certs_provider.SetAuthorityCertificates({certificate});
+
+  FakePolicyCertificateProvider user_policy_certs_provider;
+  user_policy_certs_provider.SetAuthorityCertificates({certificate});
+
+  cert_loader_->SetDevicePolicyCertificateProvider(
+      &device_policy_certs_provider);
+  ASSERT_EQ(1U, GetAndResetCertificatesLoadedEventsCount());
+
+  cert_loader_->SetUserPolicyCertificateProvider(&user_policy_certs_provider);
+  ASSERT_EQ(1U, GetAndResetCertificatesLoadedEventsCount());
+
+  EXPECT_FALSE(IsCertInCertificateList(certificate.get(),
+                                       false /* device_wide */,
+                                       cert_loader_->authority_certs()));
+  EXPECT_TRUE(IsCertInCertificateList(certificate.get(), true /* device_wide */,
+                                      cert_loader_->authority_certs()));
+
+  cert_loader_->SetUserPolicyCertificateProvider(nullptr);
+  EXPECT_EQ(1U, GetAndResetCertificatesLoadedEventsCount());
+
+  cert_loader_->SetDevicePolicyCertificateProvider(nullptr);
+  EXPECT_EQ(1U, GetAndResetCertificatesLoadedEventsCount());
+}
+
 TEST_F(NetworkCertLoaderTest,
        NoUpdateDueToPolicyCertificateProviderBeforeCertDbLoaded) {
   // Load a CA cert for testing.