Treat PK11_ListCertsInSlot returning nullptr as no certs on slot

PK11_ListCertsInSlot can return nullptr. One of the cases is that the
PKCS#11 token that is backing the slot has been removed.
Treat the nullptr return value at all call sites as no certs on the
slot.

Rationale: This happens e.g. during browsertest shut down.

Also, re-enable PlatformKeysServicePerUnavailableTokenBrowserTest which
was disabled due to this.

Bug: 1109635, 1059434
Test: 400 repetitions of PlatformKeysServicePerUnavailableTokenBrowserTest don't crash

Change-Id: I585bd6872ca91e9c10cc9fd49265afbaf414756e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317801Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Reviewed-by: Omar Morsi <omorsi@google.com>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795842}

chromeos/network/onc/onc_certificate_importer_impl_unittest.cc

@@ -161,6 +161,8 @@ class ONCCertificateImporterImplTest : public testing::Test {
   net::ScopedCERTCertificateList ListCertsInSlot(PK11SlotInfo* slot) {
     net::ScopedCERTCertificateList result;
     CERTCertList* cert_list = PK11_ListCertsInSlot(slot);
+    if (!cert_list)
+      return result;
     for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
          !CERT_LIST_END(node, cert_list);
          node = CERT_LIST_NEXT(node)) {

crypto/scoped_test_nss_db.cc

@@ -30,19 +30,7 @@ ScopedTestNSSDB::ScopedTestNSSDB() {
 ScopedTestNSSDB::~ScopedTestNSSDB() {
   // Remove trust from any certs in the test DB before closing it. Otherwise NSS
   // may cache verification results even after the test DB is gone.
-  if (slot_) {
-    CERTCertList* cert_list = PK11_ListCertsInSlot(slot_.get());
-    for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
-         !CERT_LIST_END(node, cert_list);
-         node = CERT_LIST_NEXT(node)) {
-      CERTCertTrust trust = {0};
-      if (CERT_ChangeCertTrust(CERT_GetDefaultCertDB(), node->cert, &trust) !=
-          SECSuccess) {
-        LOG(ERROR) << "CERT_ChangeCertTrust failed: " << PORT_GetError();
-      }
-    }
-    CERT_DestroyCertList(cert_list);
-  }
+  RemoveTrustFromAllCerts();
 
   // NSS is allowed to do IO on the current thread since dispatching
   // to a dedicated thread would still have the affect of blocking
@@ -59,4 +47,23 @@ ScopedTestNSSDB::~ScopedTestNSSDB() {
     LOG(ERROR) << "Could not delete temporary directory.";
 }
 
+void ScopedTestNSSDB::RemoveTrustFromAllCerts() {
+  if (!slot_)
+    return;
+
+  CERTCertList* cert_list = PK11_ListCertsInSlot(slot_.get());
+  if (!cert_list)
+    return;
+
+  for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
+       !CERT_LIST_END(node, cert_list); node = CERT_LIST_NEXT(node)) {
+    CERTCertTrust trust = {0};
+    if (CERT_ChangeCertTrust(CERT_GetDefaultCertDB(), node->cert, &trust) !=
+        SECSuccess) {
+      LOG(ERROR) << "CERT_ChangeCertTrust failed: " << PORT_GetError();
+    }
+  }
+  CERT_DestroyCertList(cert_list);
+}
+
 }  // namespace crypto

crypto/scoped_test_nss_db.h

@@ -24,6 +24,9 @@ class CRYPTO_EXPORT ScopedTestNSSDB {
   PK11SlotInfo* slot() const { return slot_.get(); }
 
  private:
+  // Removes trust from all certificates found in |slot_|.
+  void RemoveTrustFromAllCerts();
+
   base::ScopedTempDir temp_dir_;
   ScopedPK11Slot slot_;
 

net/cert/internal/trust_store_nss_unittest.cc

@@ -55,6 +55,8 @@ scoped_refptr<ParsedCertificate> GetASSLTrustedBuiltinRoot() {
   scoped_refptr<X509Certificate> ssl_trusted_root;
 
   CERTCertList* cert_list = PK11_ListCertsInSlot(root_certs_slot.get());
+  if (!cert_list)
+    return nullptr;
   for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
        !CERT_LIST_END(node, cert_list); node = CERT_LIST_NEXT(node)) {
     CERTCertTrust trust;

net/cert/nss_cert_database.cc

@@ -508,6 +508,14 @@ NSSCertDatabase::CertInfoList NSSCertDatabase::ListCertsInfoImpl(
     cert_list = PK11_ListCertsInSlot(slot.get());
   else
     cert_list = PK11_ListCerts(PK11CertListUnique, nullptr);
+  // PK11_ListCerts[InSlot] can return nullptr, e.g. because the PKCS#11 token
+  // that was backing the specified slot is not available anymore.
+  // Treat it as no certificates being present on the slot.
+  if (!cert_list) {
+    LOG(WARNING) << (slot ? "PK11_ListCertsInSlot" : "PK11_ListCerts")
+                 << " returned null";
+    return certs_info;
+  }
 
   CERTCertListNode* node;
   for (node = CERT_LIST_HEAD(cert_list); !CERT_LIST_END(node, cert_list);

net/cert/nss_cert_database_unittest.cc

@@ -109,6 +109,8 @@ class CertDatabaseNSSTest : public TestWithTaskEnvironment {
   ScopedCERTCertificateList ListCerts() {
     ScopedCERTCertificateList result;
     CERTCertList* cert_list = PK11_ListCertsInSlot(test_nssdb_.slot());
+    if (!cert_list)
+      return result;
     for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
          !CERT_LIST_END(node, cert_list);
          node = CERT_LIST_NEXT(node)) {

net/cert/nss_profile_filter_chromeos_unittest.cc

@@ -44,6 +44,8 @@ crypto::ScopedPK11Slot GetRootCertsSlot() {
 ScopedCERTCertificateList ListCertsInSlot(PK11SlotInfo* slot) {
   ScopedCERTCertificateList result;
   CERTCertList* cert_list = PK11_ListCertsInSlot(slot);
+  if (!cert_list)
+    return result;
   for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list);
        !CERT_LIST_END(node, cert_list);
        node = CERT_LIST_NEXT(node)) {

testing/buildbot/filters/chromeos.browser_tests.filter

-# TODO(crbug.com/1109635): Enable this.
--AllSupportedProfilesAndTokensTypes/PlatformKeysServicePerUnavailableTokenBrowserTest.GenerateRsa/0
-
 # TODO(crbug.com/979355): Enable this.
 -AutotestPrivateWithPolicyApiTest.PolicyAPITest