Enable client certificate patterns in device ONC policy

chromeos/cert_loader.cc

Index: chromeos/cert_loader.cc
diff --git a/chromeos/cert_loader.cc b/chromeos/cert_loader.cc
index 4e4131bad953b82db4e1cc39368f2b6c38b2a9ed..b30818b2a6a0f5c550121bc89547dc835e9c85b7 100644
--- a/chromeos/cert_loader.cc
+++ b/chromeos/cert_loader.cc
@@ -160,6 +160,18 @@ void CertLoader::UpdateCertificates(
   // Ignore any existing certificates.
   cert_list_ = std::move(cert_list);
 
+  // Extract certificates which are in the system token into the
+  // system_cert_list_ sublist.
+  system_cert_list_.clear();
+  for (net::CertificateList::const_iterator it = cert_list_->begin();

[emaxx, 2017/04/20 20:10:39]

nit: Use range-based for loop?

[pmarko, 2017/04/24 14:49:55]

Done.

+       it != cert_list_->end(); ++it) {
+    const scoped_refptr<net::X509Certificate> cert = *it;
+
+    if (IsCertificateInSystemToken(cert.get())) {

[emaxx, 2017/04/20 20:10:39]

Isn't it bad to call this function on the UI thread? I mean, considering NSS as
a black box, it may easily happen that listing all slots for a dozen of certs
may work noticeably long. Or there's some evidence that performing this is fine
on the UI thread?

[pmarko, 2017/04/24 14:49:55]

Done. Offloaded to slow task runner, see new member function CertLoader::
CertificatesLoaded and anonymous namespace::FilterSystemTokenCertificates.

+      system_cert_list_.push_back(cert);
+    }
+  }
+
   bool initial_load = !certificates_loaded_;
   certificates_loaded_ = true;
   NotifyCertificatesLoaded(initial_load);
@@ -179,4 +191,25 @@ void CertLoader::OnCertDBChanged() {
   LoadCertificates();
 }
 
+bool CertLoader::IsCertificateInSystemToken(const net::X509Certificate* cert) {
+  if (!database_->GetSystemSlot())
+    return false;
+
+  PK11SlotInfo* system_slot = database_->GetSystemSlot().get();
+
+  crypto::ScopedPK11SlotList slots_for_cert(
+      PK11_GetAllSlotsForCert(cert->os_cert_handle(), NULL));
+  if (!slots_for_cert)
+    return false;
+
+  for (PK11SlotListElement* slot_element =
+           PK11_GetFirstSafe(slots_for_cert.get());
+       slot_element; slot_element = PK11_GetNextSafe(slots_for_cert.get(),
+                                                     slot_element, PR_FALSE)) {
+    if (slot_element->slot == system_slot)
+      return true;

[emaxx, 2017/04/20 20:10:39]

I didn't look into the NSS sources to check, but can't a memory leak happen
there? Or ScopedPK11SlotList will take care of that? (hopefully without
double-freeing stuff due to PK11_GetNextSafe() calls)

[pmarko, 2017/04/24 14:49:55]

You are right! I forgot and repeated bug 329104.
Done.
(This is now in the anonymous namespace on the top of this file)

+  }
+  return false;
+}
+
 }  // namespace chromeos