Remove usage of singleton software_slot_ in nss on ChromeOS

crypto/nss_util.cc

Index: crypto/nss_util.cc
diff --git a/crypto/nss_util.cc b/crypto/nss_util.cc
index 5958ad972daf74db958b9c50b88badfef4169b5b..2495df4cd52f0e236ba59f04e883e44ba1e476db 100644
--- a/crypto/nss_util.cc
+++ b/crypto/nss_util.cc
@@ -81,6 +81,7 @@ std::string GetNSSErrorMessage() {
 }
 
 #if defined(USE_NSS)
+#if !defined(OS_CHROMEOS)
 base::FilePath GetDefaultConfigDirectory() {
   base::FilePath dir;
   PathService::Get(base::DIR_HOME, &dir);
@@ -96,6 +97,7 @@ base::FilePath GetDefaultConfigDirectory() {
   DVLOG(2) << "DefaultConfigDirectory: " << dir.value();
   return dir;
 }
+#endif  // !defined(IS_CHROMEOS)
 
 // On non-Chrome OS platforms, return the default config directory. On Chrome OS
 // test images, return a read-only directory with fake root CA certs (which are
@@ -216,11 +218,10 @@ void CrashOnNSSInitFailure() {
 #if defined(OS_CHROMEOS)
 class ChromeOSUserData {
  public:
-  ChromeOSUserData(ScopedPK11Slot public_slot, bool is_primary_user)
-      : public_slot_(public_slot.Pass()),
-        is_primary_user_(is_primary_user) {}
+  ChromeOSUserData(ScopedPK11Slot public_slot)
+      : public_slot_(public_slot.Pass()) {}
   ~ChromeOSUserData() {
-    if (public_slot_ && !is_primary_user_) {
+    if (public_slot_) {
       SECStatus status = SECMOD_CloseUserDB(public_slot_.get());
       if (status != SECSuccess)
         PLOG(ERROR) << "SECMOD_CloseUserDB failed: " << PORT_GetError();
@@ -257,7 +258,6 @@ class ChromeOSUserData {
  private:
   ScopedPK11Slot public_slot_;
   ScopedPK11Slot private_slot_;
-  bool is_primary_user_;
 
   typedef std::vector<base::Callback<void(ScopedPK11Slot)> >
       SlotReadyCallbackList;
@@ -276,24 +276,6 @@ class NSSInitSingleton {
     PK11SlotInfo* tpm_slot;
   };
 
-  void OpenPersistentNSSDB() {
-    DCHECK(thread_checker_.CalledOnValidThread());
-
-    if (!chromeos_user_logged_in_) {
-      // GetDefaultConfigDirectory causes us to do blocking IO on UI thread.
-      // Temporarily allow it until we fix http://crbug.com/70119
-      base::ThreadRestrictions::ScopedAllowIO allow_io;
-      chromeos_user_logged_in_ = true;
-
-      // This creates another DB slot in NSS that is read/write, unlike
-      // the fake root CA cert DB and the "default" crypto key
-      // provider, which are still read-only (because we initialized
-      // NSS before we had a cryptohome mounted).
-      software_slot_ = OpenUserDB(GetDefaultConfigDirectory(),
-                                  kNSSDatabaseName);
-    }
-  }
-
   PK11SlotInfo* OpenPersistentNSSDBForPath(const base::FilePath& path) {
     DCHECK(thread_checker_.CalledOnValidThread());
     // NSS is allowed to do IO on the current thread since dispatching
@@ -459,7 +441,6 @@ class NSSInitSingleton {
   bool InitializeNSSForChromeOSUser(
       const std::string& email,
       const std::string& username_hash,
-      bool is_primary_user,
       const base::FilePath& path) {
     DCHECK(thread_checker_.CalledOnValidThread());
     if (chromeos_user_map_.find(username_hash) != chromeos_user_map_.end()) {
@@ -468,15 +449,15 @@ class NSSInitSingleton {
       return false;
     }
     ScopedPK11Slot public_slot;
-    if (is_primary_user) {
-      DVLOG(2) << "Primary user, using GetPublicNSSKeySlot()";
-      public_slot.reset(GetPublicNSSKeySlot());
+    if (test_slot_) {
+      DVLOG(2) << "Using the test slot for the user's software slot.";

[tbarzic, 2014/06/12 01:41:37]

so, the main functional change here is that the public slot gets opened when not
running on cros (i.e. on Linux ChromeOS; though this has been the case for
secondary users for a while) and in Guest mode.
(though, this is not reached for users that do not have username hash, like in
retail mode).
(OpenPersistentNSSDB used to be called only when TPM was enabled)

What I'm not certain about is public account user (whether username hash gets
set for these, and if so whether public slot should be disabled).
pneubeck: do you know anything about this?

[pneubeck (no reviews), 2014/06/12 15:14:54]

In all types of modes/sessions the username_hash should be set to a usable
value. What makes you believe that it isn't the case for retail mode?

In public sessions and guest mode, the user should not be able to persist
anything beyond logout.
(The device wide token that Darren is working on, however should allow use to
get certificates _into_ a public session; but again likely not the other way
round)
Public session can be controlled by policy, whereas the guest mode should behave
identical on any device no matter what was configured on the device or who is
using/owning that device.
Apart from that these sessions should be as similar to regular sessions. I.e.
temporary installation of certs/keys should work as usual.

One other thing to consider are ephemeral sessions, in which case any user
account will be removed after usage (the home directory is mounted in a tmpfs).


To be honest, I don't have the full picture and am not sure what your change
implies. If you want to, we could VC and you could explain me some more
background.

[tbarzic, 2014/06/12 18:38:12]

For users other than guest and public account, it doesn't change much.
For guest/public account user, an r/w user db will be opened in the users home
dir, and it would enable importing certificates (though, not binding them to
hardware as TPM is not loaded in these cases).

My main concerns with public account user was whether allowing temporary
certificate installation makes sense, and if it does making sure that
certificates is not persistent. Though, looking at
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...,
cryptohome for public account is mounted as ephemeral, so it's home dir should
be discarded on log out (and with it the user DB opened here).

[tbarzic, 2014/06/12 18:41:49]

btw. we used to open user db for guest users, but http://crrev.com/215720
started bypassing OpenPersistentNSSDb step for guest users.
+stevenjb, in case there was a reason I'm not aware of to do this (I'll nee an
OWNER approval for chromeos/ regardless)

+      public_slot.reset(PK11_ReferenceSlot(test_slot_));
     } else {
       DVLOG(2) << "Opening NSS DB " << path.value();
       public_slot.reset(OpenPersistentNSSDBForPath(path));
     }
     chromeos_user_map_[username_hash] =
-        new ChromeOSUserData(public_slot.Pass(), is_primary_user);
+        new ChromeOSUserData(public_slot.Pass());
     return true;
   }
 
@@ -619,8 +600,6 @@ class NSSInitSingleton {
 
     if (test_slot_)
       return PK11_ReferenceSlot(test_slot_);
-    if (software_slot_)
-      return PK11_ReferenceSlot(software_slot_);
     return PK11_GetInternalKeySlot();
   }
 
@@ -645,10 +624,6 @@ class NSSInitSingleton {
       }
     }
 #endif
-    // If we weren't supposed to enable the TPM for NSS, then return
-    // the software slot.
-    if (software_slot_)
-      return PK11_ReferenceSlot(software_slot_);
     return PK11_GetInternalKeySlot();
   }
 
@@ -671,11 +646,9 @@ class NSSInitSingleton {
       : tpm_token_enabled_for_nss_(false),
         initializing_tpm_token_(false),
         chaps_module_(NULL),
-        software_slot_(NULL),
         test_slot_(NULL),
         tpm_slot_(NULL),
-        root_(NULL),
-        chromeos_user_logged_in_(false) {
+        root_(NULL) {
     base::TimeTicks start_time = base::TimeTicks::Now();
 
     // It's safe to construct on any thread, since LazyInstance will prevent any
@@ -795,11 +768,6 @@ class NSSInitSingleton {
       PK11_FreeSlot(tpm_slot_);
       tpm_slot_ = NULL;
     }
-    if (software_slot_) {
-      SECMOD_CloseUserDB(software_slot_);
-      PK11_FreeSlot(software_slot_);
-      software_slot_ = NULL;
-    }
     CloseTestNSSDB();
     if (root_) {
       SECMOD_UnloadUserModule(root_);
@@ -902,11 +870,9 @@ class NSSInitSingleton {
   typedef std::vector<base::Closure> TPMReadyCallbackList;
   TPMReadyCallbackList tpm_ready_callback_list_;
   SECMODModule* chaps_module_;
-  PK11SlotInfo* software_slot_;
   PK11SlotInfo* test_slot_;
   PK11SlotInfo* tpm_slot_;
   SECMODModule* root_;
-  bool chromeos_user_logged_in_;
 #if defined(OS_CHROMEOS)
   typedef std::map<std::string, ChromeOSUserData*> ChromeOSUserMap;
   ChromeOSUserMap chromeos_user_map_;
@@ -1070,10 +1036,6 @@ AutoSECMODListReadLock::~AutoSECMODListReadLock() {
 #endif  // defined(USE_NSS)
 
 #if defined(OS_CHROMEOS)
-void OpenPersistentNSSDB() {
-  g_nss_singleton.Get().OpenPersistentNSSDB();
-}
-
 void EnableTPMTokenForNSS() {
   g_nss_singleton.Get().EnableTPMTokenForNSS();
 }
@@ -1099,7 +1061,6 @@ ScopedTestNSSChromeOSUser::ScopedTestNSSChromeOSUser(
   constructed_successfully_ =
       InitializeNSSForChromeOSUser(username_hash,
                                    username_hash,
-                                   false /* is_primary_user */,
                                    temp_dir_.path());
 }
 
@@ -1115,10 +1076,9 @@ void ScopedTestNSSChromeOSUser::FinishInit() {
 bool InitializeNSSForChromeOSUser(
     const std::string& email,
     const std::string& username_hash,
-    bool is_primary_user,
     const base::FilePath& path) {
   return g_nss_singleton.Get().InitializeNSSForChromeOSUser(
-      email, username_hash, is_primary_user, path);
+      email, username_hash, path);
 }
 void InitializeTPMForChromeOSUser(
     const std::string& username_hash,