Support functions usage without NSSCertDB instance

Some NSS Certificate Database functions that return information about
certificates are expensive and should not be called on the UI thread.
This CL is part of the effort of normalizing the way of gathering this
certificates information to make sure it is gathered on a worker thread.
To call these functions on a worker thread, they should not depend on
NSSCertDatabase instances, so if the instances goes out of scope, the
code does not run into undefined behaviour.

In this CL some NSS Certificate Database functions that does not depend
on database instances are converted to static to be able to use them
safely inside worker threads.

1- net_unittests --gtest_filter=NSSCertDatabase*
2- unittests --gtest_filter=CertificateManagerModel*

Bug: chromium:1043083, chromium:1012430
Test: 
Change-Id: I63a1157c5d92b44f8d5252e9475104a1adfa8376
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2054090Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Omar Morsi <omorsi@google.com>
Cr-Commit-Position: refs/heads/master@{#743023}

chrome/browser/certificate_manager_model.cc

@@ -232,13 +232,17 @@ class CertsSourcePlatformNSS : public CertificateManagerModel::CertsSource {
     cert_infos.reserve(certs.size());
     for (auto& cert : certs) {
       net::CertType type = x509_certificate_model::GetType(cert.get());
-      bool can_be_deleted = !cert_db_->IsReadOnly(cert.get());
-      bool untrusted = cert_db_->IsUntrusted(cert.get());
-      bool hardware_backed = cert_db_->IsHardwareBacked(cert.get());
-      bool web_trust_anchor = cert_db_->IsWebTrustAnchor(cert.get());
+      bool can_be_deleted = !net::NSSCertDatabase::IsReadOnly(cert.get());
+      bool untrusted = net::NSSCertDatabase::IsUntrusted(cert.get());
+      bool hardware_backed = net::NSSCertDatabase::IsHardwareBacked(cert.get());
+      bool web_trust_anchor =
+          net::NSSCertDatabase::IsWebTrustAnchor(cert.get());
       bool device_wide = false;
 #if defined(OS_CHROMEOS)
-      device_wide = cert_db_->IsCertificateOnSystemSlot(cert.get());
+
+      device_wide = net::NSSCertDatabase::IsCertificateOnSlot(
+          /*cert=*/cert.get(),
+          /*slot=*/cert_db_->GetSystemSlot().get());
 #endif
       base::string16 name = GetName(cert.get(), hardware_backed);
       cert_infos.push_back(std::make_unique<CertificateManagerModel::CertInfo>(

net/cert/nss_cert_database.cc

@@ -108,13 +108,13 @@ crypto::ScopedPK11Slot NSSCertDatabase::GetSystemSlot() const {
   return crypto::ScopedPK11Slot();
 }
 
-bool NSSCertDatabase::IsCertificateOnSystemSlot(CERTCertificate* cert) const {
-  crypto::ScopedPK11Slot system_slot = GetSystemSlot();
-  if (!system_slot)
+// static
+bool NSSCertDatabase::IsCertificateOnSlot(CERTCertificate* cert,
+                                          PK11SlotInfo* slot) {
+  if (!slot)
     return false;
 
-  return PK11_FindCertInSlot(system_slot.get(), cert, nullptr) !=
-         CK_INVALID_HANDLE;
+  return PK11_FindCertInSlot(slot, cert, nullptr) != CK_INVALID_HANDLE;
 }
 #endif
 
@@ -299,7 +299,37 @@ NSSCertDatabase::TrustBits NSSCertDatabase::GetCertTrust(
   }
 }
 
-bool NSSCertDatabase::IsUntrusted(const CERTCertificate* cert) const {
+bool NSSCertDatabase::SetCertTrust(CERTCertificate* cert,
+                                   CertType type,
+                                   TrustBits trust_bits) {
+  bool success = psm::SetCertTrust(cert, type, trust_bits);
+  if (success)
+    NotifyObserversCertDBChanged();
+
+  return success;
+}
+
+bool NSSCertDatabase::DeleteCertAndKey(CERTCertificate* cert) {
+  if (!DeleteCertAndKeyImpl(cert))
+    return false;
+  NotifyObserversCertDBChanged();
+  return true;
+}
+
+void NSSCertDatabase::DeleteCertAndKeyAsync(ScopedCERTCertificate cert,
+                                            DeleteCertCallback callback) {
+  base::PostTaskAndReplyWithResult(
+      FROM_HERE,
+      {base::ThreadPool(), base::MayBlock(),
+       base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
+      base::BindOnce(&NSSCertDatabase::DeleteCertAndKeyImplScoped,
+                     std::move(cert)),
+      base::BindOnce(&NSSCertDatabase::NotifyCertRemovalAndCallBack,
+                     weak_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+// static
+bool NSSCertDatabase::IsUntrusted(const CERTCertificate* cert) {
   CERTCertTrust nsstrust;
   SECStatus rv = CERT_GetCertTrust(cert, &nsstrust);
   if (rv != SECSuccess) {
@@ -351,7 +381,8 @@ bool NSSCertDatabase::IsUntrusted(const CERTCertificate* cert) const {
   return false;
 }
 
-bool NSSCertDatabase::IsWebTrustAnchor(const CERTCertificate* cert) const {
+// static
+bool NSSCertDatabase::IsWebTrustAnchor(const CERTCertificate* cert) {
   CERTCertTrust nsstrust;
   SECStatus rv = CERT_GetCertTrust(cert, &nsstrust);
   if (rv != SECSuccess) {
@@ -371,41 +402,14 @@ bool NSSCertDatabase::IsWebTrustAnchor(const CERTCertificate* cert) const {
   return false;
 }
 
-bool NSSCertDatabase::SetCertTrust(CERTCertificate* cert,
-                                   CertType type,
-                                   TrustBits trust_bits) {
-  bool success = psm::SetCertTrust(cert, type, trust_bits);
-  if (success)
-    NotifyObserversCertDBChanged();
-
-  return success;
-}
-
-bool NSSCertDatabase::DeleteCertAndKey(CERTCertificate* cert) {
-  if (!DeleteCertAndKeyImpl(cert))
-    return false;
-  NotifyObserversCertDBChanged();
-  return true;
-}
-
-void NSSCertDatabase::DeleteCertAndKeyAsync(ScopedCERTCertificate cert,
-                                            DeleteCertCallback callback) {
-  base::PostTaskAndReplyWithResult(
-      FROM_HERE,
-      {base::ThreadPool(), base::MayBlock(),
-       base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
-      base::BindOnce(&NSSCertDatabase::DeleteCertAndKeyImplScoped,
-                     std::move(cert)),
-      base::BindOnce(&NSSCertDatabase::NotifyCertRemovalAndCallBack,
-                     weak_factory_.GetWeakPtr(), std::move(callback)));
-}
-
-bool NSSCertDatabase::IsReadOnly(const CERTCertificate* cert) const {
+// static
+bool NSSCertDatabase::IsReadOnly(const CERTCertificate* cert) {
   PK11SlotInfo* slot = cert->slot;
   return slot && PK11_IsReadOnly(slot);
 }
 
-bool NSSCertDatabase::IsHardwareBacked(const CERTCertificate* cert) const {
+// static
+bool NSSCertDatabase::IsHardwareBacked(const CERTCertificate* cert) {
   PK11SlotInfo* slot = cert->slot;
   return slot && PK11_IsHW(slot);
 }

net/cert/nss_cert_database.h

@@ -127,9 +127,8 @@ class NET_EXPORT NSSCertDatabase {
   // See https://crbug.com/399554 .
   virtual crypto::ScopedPK11Slot GetSystemSlot() const;
 
-  // Check whether the certificate is stored on the system slot (i.e. is a
-  // device certificate).
-  bool IsCertificateOnSystemSlot(CERTCertificate* cert) const;
+  // Checks whether |cert| is stored on |slot|.
+  static bool IsCertificateOnSlot(CERTCertificate* cert, PK11SlotInfo* slot);
 #endif
 
   // Get the default slot for public key data.
@@ -204,15 +203,6 @@ class NET_EXPORT NSSCertDatabase {
   // Get trust bits for certificate.
   TrustBits GetCertTrust(const CERTCertificate* cert, CertType type) const;
 
-  // IsUntrusted returns true if |cert| is specifically untrusted. These
-  // certificates are stored in the database for the specific purpose of
-  // rejecting them.
-  bool IsUntrusted(const CERTCertificate* cert) const;
-
-  // IsWebTrustAnchor returns true if |cert| is explicitly trusted for web
-  // navigations according to the trust bits stored in the database.
-  bool IsWebTrustAnchor(const CERTCertificate* cert) const;
-
   // Set trust values for certificate.
   // Returns true on success or false on failure.
   bool SetCertTrust(CERTCertificate* cert, CertType type, TrustBits trust_bits);
@@ -228,11 +218,20 @@ class NET_EXPORT NSSCertDatabase {
   void DeleteCertAndKeyAsync(ScopedCERTCertificate cert,
                              DeleteCertCallback callback);
 
+  // IsUntrusted returns true if |cert| is specifically untrusted. These
+  // certificates are stored in the database for the specific purpose of
+  // rejecting them.
+  static bool IsUntrusted(const CERTCertificate* cert);
+
+  // IsWebTrustAnchor returns true if |cert| is explicitly trusted for web
+  // navigations according to the trust bits stored in the database.
+  static bool IsWebTrustAnchor(const CERTCertificate* cert);
+
   // Check whether cert is stored in a readonly slot.
-  bool IsReadOnly(const CERTCertificate* cert) const;
+  static bool IsReadOnly(const CERTCertificate* cert);
 
   // Check whether cert is stored in a hardware slot.
-  bool IsHardwareBacked(const CERTCertificate* cert) const;
+  static bool IsHardwareBacked(const CERTCertificate* cert);
 
  protected:
   // Certificate listing implementation used by |ListCerts*|. Static so it may