Enable system NSS key slot.

net/cert/nss_cert_database_chromeos.cc

 // Copyright 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 "net/cert/nss_cert_database_chromeos.h"
 
 #include <cert.h>
 #include <pk11pub.h>
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
 #include "base/task_runner.h"
 #include "net/base/crypto_module.h"
 #include "net/cert/x509_certificate.h"
 
 namespace net {
 
 NSSCertDatabaseChromeOS::NSSCertDatabaseChromeOS(
     crypto::ScopedPK11Slot public_slot,
     crypto::ScopedPK11Slot private_slot)
     : NSSCertDatabase(public_slot.Pass(), private_slot.Pass()) {
-  profile_filter_.Init(GetPublicSlot(), GetPrivateSlot());
+  // By default, don't use a system slot. Only if explicitly set by
+  // SetSystemSlot, the system slot will be used.
+  profile_filter_.Init(GetPublicSlot(),
+                       GetPrivateSlot(),
+                       crypto::ScopedPK11Slot() /* no system slot */);
 }
 
 NSSCertDatabaseChromeOS::~NSSCertDatabaseChromeOS() {}
 
+void NSSCertDatabaseChromeOS::SetSystemSlot(
+    crypto::ScopedPK11Slot system_slot) {

[mattm, 2014/07/30 22:57:34]

Seems a little iffy that initializing the system slot is asynchronous but done
after the NSSCertDatabaseChromeOS is created, if anything uses it during that
time they wouldn't get results from the system slot. Is there a reason the
system slot isn't just another parameter to the constructor?

[pneubeck (no reviews), 2014/07/31 06:31:25]

Yes, full ack.
That's why originally I tried to make the initialization of NSSCertDB explicit.
I tried to do that in user_session_manager, where we already have some cert db
code (and initialize CertTokenLoader). However, when I tried to write tests for
it, I realized that this is the totally wrong place to do it. The login flow and
user session manager are usually modified/mocked in browser tests and the
initialization didn't work as expected.

Instead, I started writing a CL to add a CertDBService, a KeyedService, which
will take care of most initialization on top of nss_util (also replacing
nss_context).
See https://codereview.chromium.org/419013003/

However, this change will not be small and not making it into 38.

Therefore, this not ideal, short-term solution of setting the system slot from
profile_io_data.
According to the current usage, it should never occur that someone access the
NSSCertDB before this setter was called (if it ever get's called).

[mattm, 2014/07/31 10:29:30]

Okay, sounds reasonable. And thinking about the initialization order more it
does seem reasonably safe.

+  system_slot_ = system_slot.Pass();
+  profile_filter_.Init(GetPublicSlot(), GetPrivateSlot(), GetSystemSlot());
+}
+
 void NSSCertDatabaseChromeOS::ListCertsSync(CertificateList* certs) {
   ListCertsImpl(profile_filter_, certs);
 }
 
 void NSSCertDatabaseChromeOS::ListCerts(
     const base::Callback<void(scoped_ptr<CertificateList> certs)>& callback) {
   scoped_ptr<CertificateList> certs(new CertificateList());
 
   // base::Pased will NULL out |certs|, so cache the underlying pointer here.
   CertificateList* raw_certs = certs.get();
   GetSlowTaskRunner()->PostTaskAndReply(
       FROM_HERE,
       base::Bind(&NSSCertDatabaseChromeOS::ListCertsImpl,
                  profile_filter_,
                  base::Unretained(raw_certs)),
       base::Bind(callback, base::Passed(&certs)));
 }
 
+crypto::ScopedPK11Slot NSSCertDatabaseChromeOS::GetSystemSlot() const {
+  if (system_slot_)
+    return crypto::ScopedPK11Slot(PK11_ReferenceSlot(system_slot_.get()));
+  return crypto::ScopedPK11Slot();
+}
+
 void NSSCertDatabaseChromeOS::ListModules(CryptoModuleList* modules,
                                           bool need_rw) const {
   NSSCertDatabase::ListModules(modules, need_rw);
 
   size_t pre_size = modules->size();
   modules->erase(
       std::remove_if(
           modules->begin(),
           modules->end(),
           NSSProfileFilterChromeOS::ModuleNotAllowedForProfilePredicate(
@@ skipping 13 matching lines @@
                    certs->begin(),
                    certs->end(),
                    NSSProfileFilterChromeOS::CertNotAllowedForProfilePredicate(
                        profile_filter)),
                certs->end());
   DVLOG(1) << "filtered " << pre_size - certs->size() << " of " << pre_size
            << " certs";
 }
 
 }  // namespace net