Test NSSCertDatabaseChromeOS in the presence of the system slot.

Depends on:
https://codereview.chromium.org/424523002/

BUG=210525

Review URL: https://codereview.chromium.org/429633004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287209 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 104 insertions, 49 deletions

diff --git a/net/cert/nss_cert_database_chromeos_unittest.cc b/net/cert/nss_cert_database_chromeos_unittest.cc
index 900edad..13396b6 100644
--- a/net/cert/nss_cert_database_chromeos_unittest.cc
+++ b/net/cert/nss_cert_database_chromeos_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/run_loop.h"
 #include "crypto/nss_util_internal.h"
 #include "crypto/scoped_test_nss_chromeos_user.h"
+#include "crypto/scoped_test_nss_db.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/cert_database.h"
 #include "net/test/cert_test_util.h"
@@ -61,6 +62,8 @@ class NSSCertDatabaseChromeOSTest : public testing::Test,
             user_1_.username_hash(),
             base::Callback<void(crypto::ScopedPK11Slot)>())));
     db_1_->SetSlowTaskRunnerForTest(base::MessageLoopProxy::current());
+    db_1_->SetSystemSlot(
+        crypto::ScopedPK11Slot(PK11_ReferenceSlot(system_db_.slot())));
     db_2_.reset(new NSSCertDatabaseChromeOS(
         crypto::GetPublicSlotForChromeOSUser(user_2_.username_hash()),
         crypto::GetPrivateSlotForChromeOSUser(
@@ -98,6 +101,7 @@ class NSSCertDatabaseChromeOSTest : public testing::Test,
 
   crypto::ScopedTestNSSChromeOSUser user_1_;
   crypto::ScopedTestNSSChromeOSUser user_2_;
+  crypto::ScopedTestNSSDB system_db_;
   scoped_ptr<NSSCertDatabaseChromeOS> db_1_;
   scoped_ptr<NSSCertDatabaseChromeOS> db_2_;
 };
@@ -276,4 +280,40 @@ TEST_F(NSSCertDatabaseChromeOSTest, NoCrashIfShutdownBeforeDoneOnWorkerPool) {
   EXPECT_LT(0U, certlist.size());
 }
 
+TEST_F(NSSCertDatabaseChromeOSTest, ListCertsReadsSystemSlot) {
+  scoped_refptr<X509Certificate> cert_1(
+      ImportClientCertAndKeyFromFile(GetTestCertsDirectory(),
+                                     "client_1.pem",
+                                     "client_1.pk8",
+                                     db_1_->GetPublicSlot().get()));
+
+  scoped_refptr<X509Certificate> cert_2(
+      ImportClientCertAndKeyFromFile(GetTestCertsDirectory(),
+                                     "client_2.pem",
+                                     "client_2.pk8",
+                                     db_1_->GetSystemSlot().get()));
+  CertificateList certs;
+  db_1_->ListCertsSync(&certs);
+  EXPECT_TRUE(IsCertInCertificateList(cert_1.get(), certs));
+  EXPECT_TRUE(IsCertInCertificateList(cert_2.get(), certs));
+}
+
+TEST_F(NSSCertDatabaseChromeOSTest, ListCertsDoesNotCrossReadSystemSlot) {
+  scoped_refptr<X509Certificate> cert_1(
+      ImportClientCertAndKeyFromFile(GetTestCertsDirectory(),
+                                     "client_1.pem",
+                                     "client_1.pk8",
+                                     db_2_->GetPublicSlot().get()));
+
+  scoped_refptr<X509Certificate> cert_2(
+      ImportClientCertAndKeyFromFile(GetTestCertsDirectory(),
+                                     "client_2.pem",
+                                     "client_2.pk8",
+                                     system_db_.slot()));
+  CertificateList certs;
+  db_2_->ListCertsSync(&certs);
+  EXPECT_TRUE(IsCertInCertificateList(cert_1.get(), certs));
+  EXPECT_FALSE(IsCertInCertificateList(cert_2.get(), certs));
+}
+
 }  // namespace net
diff --git a/net/ssl/client_cert_store_chromeos_unittest.cc b/net/ssl/client_cert_store_chromeos_unittest.cc
index 12a1b43..33601ef 100644
--- a/net/ssl/client_cert_store_chromeos_unittest.cc
+++ b/net/ssl/client_cert_store_chromeos_unittest.cc
@@ -10,13 +10,11 @@
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/run_loop.h"
-#include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
 #include "crypto/rsa_private_key.h"
 #include "crypto/scoped_test_nss_chromeos_user.h"
 #include "crypto/scoped_test_system_nss_key_slot.h"
 #include "net/base/test_data_directory.h"
-#include "net/cert/cert_type.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/client_cert_store_unittest-inl.h"
 #include "net/test/cert_test_util.h"
@@ -25,24 +23,6 @@ namespace net {
 
 namespace {
 
-bool ImportClientCertToSlot(const scoped_refptr<X509Certificate>& cert,
-                            PK11SlotInfo* slot) {
-  std::string nickname = cert->GetDefaultNickname(USER_CERT);
-  {
-    crypto::AutoNSSWriteLock lock;
-    SECStatus rv = PK11_ImportCert(slot,
-                                   cert->os_cert_handle(),
-                                   CK_INVALID_HANDLE,
-                                   nickname.c_str(),
-                                   PR_FALSE);
-    if (rv != SECSuccess) {
-      LOG(ERROR) << "Could not import cert";
-      return false;
-    }
-  }
-  return true;
-}
-
 enum ReadFromSlot {
   READ_FROM_SLOT_USER,
   READ_FROM_SLOT_SYSTEM
@@ -166,33 +146,6 @@ INSTANTIATE_TYPED_TEST_CASE_P(ChromeOS_ReadSystem,
 
 class ClientCertStoreChromeOSTest : public ::testing::Test {
  public:
-  scoped_refptr<X509Certificate> ImportCertToSlot(
-      const std::string& cert_filename,
-      const std::string& key_filename,
-      PK11SlotInfo* slot) {
-    if (!ImportSensitiveKeyFromFile(
-            GetTestCertsDirectory(), key_filename, slot)) {
-      LOG(ERROR) << "Could not import private key from file " << key_filename;
-      return NULL;
-    }
-
-    scoped_refptr<X509Certificate> cert(
-        ImportCertFromFile(GetTestCertsDirectory(), cert_filename));
-
-    if (!cert) {
-      LOG(ERROR) << "Failed to parse cert from file " << cert_filename;
-      return NULL;
-    }
-
-    if (!ImportClientCertToSlot(cert, slot))
-      return NULL;
-
-    // |cert| continues to point to the original X509Certificate before the
-    // import to |slot|. However this should not make a difference for this
-    // test.
-    return cert;
-  }
-
   scoped_refptr<X509Certificate> ImportCertForUser(
       const std::string& username_hash,
       const std::string& cert_filename,
@@ -204,7 +157,8 @@ class ClientCertStoreChromeOSTest : public ::testing::Test {
       return NULL;
     }
 
-    return ImportCertToSlot(cert_filename, key_filename, slot.get());
+    return ImportClientCertAndKeyFromFile(
+        GetTestCertsDirectory(), cert_filename, key_filename, slot.get());
   }
 
 };
@@ -347,7 +301,10 @@ TEST_F(ClientCertStoreChromeOSTest, RequestDoesCrossReadSystemDB) {
       ImportCertForUser(user1.username_hash(), "client_1.pem", "client_1.pk8"));
   ASSERT_TRUE(cert_1);
   scoped_refptr<X509Certificate> cert_2(
-      ImportCertToSlot("client_2.pem", "client_2.pk8", system_slot.slot()));
+      ImportClientCertAndKeyFromFile(GetTestCertsDirectory(),
+                                     "client_2.pem",
+                                     "client_2.pk8",
+                                     system_slot.slot()));
   ASSERT_TRUE(cert_2);
 
   scoped_refptr<SSLCertRequestInfo> request_all(new SSLCertRequestInfo());
diff --git a/net/test/cert_test_util.h b/net/test/cert_test_util.h
index 4e6ae9b..6334dd7 100644
--- a/net/test/cert_test_util.h
+++ b/net/test/cert_test_util.h
@@ -38,6 +38,15 @@ scoped_ptr<crypto::RSAPrivateKey> ImportSensitiveKeyFromFile(
     const base::FilePath& dir,
     const std::string& key_filename,
     PK11SlotInfo* slot);
+
+bool ImportClientCertToSlot(const scoped_refptr<X509Certificate>& cert,
+                            PK11SlotInfo* slot);
+
+scoped_refptr<X509Certificate> ImportClientCertAndKeyFromFile(
+    const base::FilePath& dir,
+    const std::string& cert_filename,
+    const std::string& key_filename,
+    PK11SlotInfo* slot);
 #endif
 
 // Imports all of the certificates in |cert_file|, a file in |certs_dir|, into a
diff --git a/net/test/cert_test_util_nss.cc b/net/test/cert_test_util_nss.cc
index 5ff7830..a3e06c7 100644
--- a/net/test/cert_test_util_nss.cc
+++ b/net/test/cert_test_util_nss.cc
@@ -4,10 +4,15 @@
 
 #include "net/test/cert_test_util.h"
 
+#include <pk11pub.h>
+#include <secmodt.h>
+
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/path_service.h"
+#include "crypto/nss_util.h"
 #include "crypto/rsa_private_key.h"
+#include "net/cert/cert_type.h"
 
 namespace net {
 
@@ -36,4 +41,48 @@ scoped_ptr<crypto::RSAPrivateKey> ImportSensitiveKeyFromFile(
   return private_key.Pass();
 }
 
+bool ImportClientCertToSlot(const scoped_refptr<X509Certificate>& cert,
+                            PK11SlotInfo* slot) {
+  std::string nickname = cert->GetDefaultNickname(USER_CERT);
+  {
+    crypto::AutoNSSWriteLock lock;
+    SECStatus rv = PK11_ImportCert(slot,
+                                   cert->os_cert_handle(),
+                                   CK_INVALID_HANDLE,
+                                   nickname.c_str(),
+                                   PR_FALSE);
+    if (rv != SECSuccess) {
+      LOG(ERROR) << "Could not import cert";
+      return false;
+    }
+  }
+  return true;
+}
+
+scoped_refptr<X509Certificate> ImportClientCertAndKeyFromFile(
+    const base::FilePath& dir,
+    const std::string& cert_filename,
+    const std::string& key_filename,
+    PK11SlotInfo* slot) {
+  if (!ImportSensitiveKeyFromFile(dir, key_filename, slot)) {
+    LOG(ERROR) << "Could not import private key from file " << key_filename;
+    return NULL;
+  }
+
+  scoped_refptr<X509Certificate> cert(ImportCertFromFile(dir, cert_filename));
+
+  if (!cert) {
+    LOG(ERROR) << "Failed to parse cert from file " << cert_filename;
+    return NULL;
+  }
+
+  if (!ImportClientCertToSlot(cert, slot))
+    return NULL;
+
+  // |cert| continues to point to the original X509Certificate before the
+  // import to |slot|. However this should not make a difference as NSS handles
+  // state globally.
+  return cert;
+}
+
 }  // namespace net