Enable client certificate patterns in device ONC policy

chromeos/cert_loader.cc

 // Copyright (c) 2013 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/cert_loader.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 142 matching lines @@
 
 void CertLoader::UpdateCertificates(
     std::unique_ptr<net::CertificateList> cert_list) {
   CHECK(thread_checker_.CalledOnValidThread());
   DCHECK(certificates_update_running_);
   VLOG(1) << "UpdateCertificates: " << cert_list->size();
 
   // 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);
 
   certificates_update_running_ = false;
   if (certificates_update_required_)
     LoadCertificates();
 }
 
 void CertLoader::NotifyCertificatesLoaded(bool initial_load) {
   for (auto& observer : observers_)
     observer.OnCertificatesLoaded(*cert_list_, initial_load);
 }
 
 void CertLoader::OnCertDBChanged() {
   VLOG(1) << "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