Remove usage of singleton software_slot_ in nss on ChromeOS

crypto/nss_util.cc

 // Copyright (c) 2012 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 "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
 
 #include <nss.h>
 #include <pk11pub.h>
 #include <plarena.h>
@@ skipping 63 matching lines @@
     scoped_ptr<char[]> error_text(new char[PR_GetErrorTextLength() + 1]);
     PRInt32 copied = PR_GetErrorText(error_text.get());
     result = std::string(error_text.get(), copied);
   } else {
     result = base::StringPrintf("NSS error code: %d", PR_GetError());
   }
   return result;
 }
 
 #if defined(USE_NSS)
+#if !defined(OS_CHROMEOS)
 base::FilePath GetDefaultConfigDirectory() {
   base::FilePath dir;
   PathService::Get(base::DIR_HOME, &dir);
   if (dir.empty()) {
     LOG(ERROR) << "Failed to get home directory.";
     return dir;
   }
   dir = dir.AppendASCII(".pki").AppendASCII("nssdb");
   if (!base::CreateDirectory(dir)) {
     LOG(ERROR) << "Failed to create " << dir.value() << " directory.";
     dir.clear();
   }
   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
 // used by the local Google Accounts server mock we use when testing our login
 // code). On Chrome OS non-test images (where the read-only directory doesn't
 // exist), return an empty path.
 base::FilePath GetInitialConfigDirectory() {
 #if defined(OS_CHROMEOS)
   base::FilePath database_dir = base::FilePath(kReadOnlyCertDB);
   if (!base::PathExists(database_dir))
@@ skipping 100 matching lines @@
   base::debug::Alias(&nss_error);
   base::debug::Alias(&os_error);
   LOG(ERROR) << "Error initializing NSS without a persistent database: "
              << GetNSSErrorMessage();
   LOG(FATAL) << "nss_error=" << nss_error << ", os_error=" << os_error;
 }
 
 #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();
     }
   }
 
   ScopedPK11Slot GetPublicSlot() {
     return ScopedPK11Slot(
         public_slot_ ? PK11_ReferenceSlot(public_slot_.get()) : NULL);
   }
@@ skipping 16 matching lines @@
     for (SlotReadyCallbackList::iterator i = callback_list.begin();
          i != callback_list.end();
          ++i) {
       (*i).Run(ScopedPK11Slot(PK11_ReferenceSlot(private_slot_.get())));
     }
   }
 
  private:
   ScopedPK11Slot public_slot_;
   ScopedPK11Slot private_slot_;
-  bool is_primary_user_;
 
   typedef std::vector<base::Callback<void(ScopedPK11Slot)> >
       SlotReadyCallbackList;
   SlotReadyCallbackList tpm_ready_callback_list_;
 };
 #endif  // defined(OS_CHROMEOS)
 
 class NSSInitSingleton {
  public:
 #if defined(OS_CHROMEOS)
   // Used with PostTaskAndReply to pass handles to worker thread and back.
   struct TPMModuleAndSlot {
     explicit TPMModuleAndSlot(SECMODModule* init_chaps_module)
         : chaps_module(init_chaps_module), tpm_slot(NULL) {}
     SECMODModule* chaps_module;
     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
     // to a dedicated thread would still have the affect of blocking
     // the current thread, due to NSS's internal locking requirements
     base::ThreadRestrictions::ScopedAllowIO allow_io;
 
     base::FilePath nssdb_path = path.AppendASCII(".pki").AppendASCII("nssdb");
     if (!base::CreateDirectory(nssdb_path)) {
       LOG(ERROR) << "Failed to create " << nssdb_path.value() << " directory.";
@@ skipping 145 matching lines @@
 
     PK11SlotInfo* slot = SECMOD_LookupSlot(chaps_module->moduleID, slot_id);
     if (!slot)
       LOG(ERROR) << "TPM slot " << slot_id << " not found.";
     return slot;
   }
 
   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()) {
       // This user already exists in our mapping.
       DVLOG(2) << username_hash << " already initialized.";
       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;
   }
 
   void InitializeTPMForChromeOSUser(const std::string& username_hash,
                                     CK_SLOT_ID slot_id) {
     DCHECK(thread_checker_.CalledOnValidThread());
     DCHECK(chromeos_user_map_.find(username_hash) != chromeos_user_map_.end());
 
     if (!chaps_module_)
       return;
@@ skipping 113 matching lines @@
     if (!test_slot_)
       return;
     SECStatus status = SECMOD_CloseUserDB(test_slot_);
     if (status != SECSuccess)
       PLOG(ERROR) << "SECMOD_CloseUserDB failed: " << PORT_GetError();
     PK11_FreeSlot(test_slot_);
     test_slot_ = NULL;
     ignore_result(g_test_nss_db_dir.Get().Delete());
   }
 
   PK11SlotInfo* GetPublicNSSKeySlot() {

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

afaik, on ChromeOS, this is only referenced from default NSSDatabase
implementation, which is not used anymore (as singleton NSS databse is not
used).

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

So, this is.... surprising...
The 'default' NSSCertDatabase implementation isn't used anymore on ChromeOS
(::GetInstance() is never called), but we're still compiling it into the release
binary and we have to maintain all it's dependencies which might not make any
sense in ChromeOS (like this GetPublicNSSKeySlot() ) ?!

In other words, you can't exclude this function (or at least the exposed public
function) from ChromeOS compilation.

It tried setting up a cleanup for this, but it would require some basic changes
about NSSCertDatabase. I will not continue trying that, if there is not strong
support from the owners for this...

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

yeah, I'd started looking at removing this method for ChromeOS as part of this
patch, but came to the similar conclusion as you :)

[Ryan Sleevi, 2014/06/12 18:44:04]

Sorry, I haven't looked at the ChromeOS side, but what support/concerns do you
need addressed?

[pneubeck (no reviews), 2014/06/12 18:48:29]

I think that discussion will sidetrack from the main purpose of this CL.
But I'll write a mail to explain what I meant.

     // TODO(mattm): Change to DCHECK when callers have been fixed.
     if (!thread_checker_.CalledOnValidThread()) {
       DVLOG(1) << "Called on wrong thread.\n"
                << base::debug::StackTrace().ToString();
     }
 
     if (test_slot_)
       return PK11_ReferenceSlot(test_slot_);
-    if (software_slot_)
-      return PK11_ReferenceSlot(software_slot_);
     return PK11_GetInternalKeySlot();
   }
 
   PK11SlotInfo* GetPrivateNSSKeySlot() {
     // TODO(mattm): Change to DCHECK when callers have been fixed.
     if (!thread_checker_.CalledOnValidThread()) {
       DVLOG(1) << "Called on wrong thread.\n"
                << base::debug::StackTrace().ToString();
     }
 
     if (test_slot_)
       return PK11_ReferenceSlot(test_slot_);
 
 #if defined(OS_CHROMEOS)
     if (tpm_token_enabled_for_nss_) {
       if (IsTPMTokenReady(base::Closure())) {
         return PK11_ReferenceSlot(tpm_slot_);
       } else {
         // If we were supposed to get the hardware token, but were
         // unable to, return NULL rather than fall back to sofware.
         return NULL;
       }
     }
 #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();
   }
 
 #if defined(USE_NSS)
   base::Lock* write_lock() {
     return &write_lock_;
   }
 #endif  // defined(USE_NSS)
 
   // This method is used to force NSS to be initialized without a DB.
   // Call this method before NSSInitSingleton() is constructed.
   static void ForceNoDBInit() {
     force_nodb_init_ = true;
   }
 
  private:
   friend struct base::DefaultLazyInstanceTraits<NSSInitSingleton>;
 
   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
     // other threads from accessing until the constructor is done.
     thread_checker_.DetachFromThread();
 
     DisableAESNIIfNeeded();
 
     EnsureNSPRInit();
 
@@ skipping 99 matching lines @@
   // prevent non-joinable threads from using NSS after it's already been shut
   // down.
   ~NSSInitSingleton() {
 #if defined(OS_CHROMEOS)
     STLDeleteValues(&chromeos_user_map_);
 #endif
     if (tpm_slot_) {
       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_);
       SECMOD_DestroyModule(root_);
       root_ = NULL;
     }
     if (chaps_module_) {
       SECMOD_UnloadUserModule(chaps_module_);
       SECMOD_DestroyModule(chaps_module_);
       chaps_module_ = NULL;
@@ skipping 82 matching lines @@
   }
 
   // If this is set to true NSS is forced to be initialized without a DB.
   static bool force_nodb_init_;
 
   bool tpm_token_enabled_for_nss_;
   bool initializing_tpm_token_;
   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_;
 #endif
 #if defined(USE_NSS)
   // TODO(davidben): When https://bugzilla.mozilla.org/show_bug.cgi?id=564011
   // is fixed, we will no longer need the lock.
   base::Lock write_lock_;
 #endif  // defined(USE_NSS)
 
@@ skipping 143 matching lines @@
     SECMOD_GetReadLock(lock_);
   }
 
 AutoSECMODListReadLock::~AutoSECMODListReadLock() {
   SECMOD_ReleaseReadLock(lock_);
 }
 
 #endif  // defined(USE_NSS)
 
 #if defined(OS_CHROMEOS)
-void OpenPersistentNSSDB() {
-  g_nss_singleton.Get().OpenPersistentNSSDB();
-}
-
 void EnableTPMTokenForNSS() {
   g_nss_singleton.Get().EnableTPMTokenForNSS();
 }
 
 bool IsTPMTokenEnabledForNSS() {
   return g_nss_singleton.Get().IsTPMTokenEnabledForNSS();
 }
 
 bool IsTPMTokenReady(const base::Closure& callback) {
   return g_nss_singleton.Get().IsTPMTokenReady(callback);
 }
 
 void InitializeTPMToken(int token_slot_id,
                         const base::Callback<void(bool)>& callback) {
   g_nss_singleton.Get().InitializeTPMToken(token_slot_id, callback);
 }
 
 ScopedTestNSSChromeOSUser::ScopedTestNSSChromeOSUser(
     const std::string& username_hash)
     : username_hash_(username_hash), constructed_successfully_(false) {
   if (!temp_dir_.CreateUniqueTempDir())
     return;
   constructed_successfully_ =
       InitializeNSSForChromeOSUser(username_hash,
                                    username_hash,
-                                   false /* is_primary_user */,
                                    temp_dir_.path());
 }
 
 ScopedTestNSSChromeOSUser::~ScopedTestNSSChromeOSUser() {
   if (constructed_successfully_)
     g_nss_singleton.Get().CloseTestChromeOSUser(username_hash_);
 }
 
 void ScopedTestNSSChromeOSUser::FinishInit() {
   InitializePrivateSoftwareSlotForChromeOSUser(username_hash_);
 }
 
 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,
     CK_SLOT_ID slot_id) {
   g_nss_singleton.Get().InitializeTPMForChromeOSUser(username_hash, slot_id);
 }
 void InitializePrivateSoftwareSlotForChromeOSUser(
     const std::string& username_hash) {
   g_nss_singleton.Get().InitializePrivateSoftwareSlotForChromeOSUser(
       username_hash);
@@ skipping 20 matching lines @@
 
 PK11SlotInfo* GetPublicNSSKeySlot() {
   return g_nss_singleton.Get().GetPublicNSSKeySlot();
 }
 
 PK11SlotInfo* GetPrivateNSSKeySlot() {
   return g_nss_singleton.Get().GetPrivateNSSKeySlot();
 }
 
 }  // namespace crypto